Skip to content

Conversation

@Reinis-FRP
Copy link
Contributor

@Reinis-FRP Reinis-FRP commented Sep 24, 2024

Increases max refund count by using Address Lookup Table (ALT). This is capped at 19 refunds as higher count causes out of memory panic when looping through refund transfers. To overcome this limit we would need to split individual refund claims in separate transaction. And if we split this, then we don't really need to use ALT.

Edit: limit is now increased to 21 by dropping expensive debug msg of input data. Now the bottleneck is calldata size, but that could be resolved by passing it from a buffer account.

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Copy link
Contributor

@md0x md0x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very nice, and without even having to touch the Rust code! I wonder what the limit would be if we got rid of CPI events and thus passed one fewer account.

@Reinis-FRP
Copy link
Contributor Author

This is very nice, and without even having to touch the Rust code! I wonder what the limit would be if we got rid of CPI events and thus passed one fewer account.

Turns out the out of memory panic was caused by debug msg!("input: {:?}", input); within to_keccak_hash which is quite memory expensive due to parsing raw bytes as array in strings. I'm now pushing update here that allows reaching 21 refunds, and now the message size limit is hit due to refund accounts also being present in calldata. In a follow-up I plan to check how to work around this passing calldata from buffer account.

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
@Reinis-FRP Reinis-FRP merged commit 1ca8a4a into master Sep 25, 2024
9 checks passed
@Reinis-FRP Reinis-FRP deleted the reinis-frp/alt-in-refunds branch September 25, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants