-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue: invoke_transfer_checked
failed with out of memory for 2 transfers with hook
#7004
Comments
@makarychev Thanks for flagging. You're correct that the vector allocation when creating the CPI syscall is going to eat up memory, but it should depend on the size of the extra accounts, not the quantity. If your hook program requires only small accounts - like system accounts - then it can include many more than 6. However, if you require a handful of program data accounts (for example), then this will definitely eat up a lot of heap, since the vector will contain Out of curiosity, what are the 6 accounts you're using for the hook? Are they huge? Regardless, we should probably benchmark this and document the constraint somewhere. I doubt there is much we can do about the upper bound of vector size, though. |
thanks @buffalojoec for prompt response
|
The accounts
where first 5 accounts are default for transfer hook execute instruction
but all Looking through sdk code I have found that sdk allocate each iteration Vector |
@makarychev I ran a little test of my own to see if I could replicate. I was able to get to fifteen accounts, 14 of which are the size of an SPL Mint (85 bytes). Check out the test over here. In the meantime, I'm going to self-assign this and probably roll some more testing/benchmarking in the near future for extra meta limits. |
As a side note, we could probably remove some heap allocations by using an array of 32 AccountInfos on the stack and switching it to a vec only if it goes over that size. Looks like 32 AccountInfos takes up 1536 bytes:
|
Or we could adopt an allocator that recovers heap space used after every call to |
I would like to highlight that we strugle with memory limitation when I have experimented a bit with
It saves 10KB on heap in example |
Problem:
It is possible to brake external programs which use token extension with transfer hook if it requires more than 6 additional extra account meta.
sdk function
invoke_transfer_checked
use some part of data allocation on heap withVector
by pushing new elements into it dynamically while resolving extra account meta data.So I suppose that during the preparation of accounts for extra accounts and pushing into Vectors it consumes a huge portion of heap memory.
Vectors creation:
Pushing into vectors:
The simple transfer hook program which fails with the
out of memory
due to intensive allocation insideinvoke_transfer_checked
https://github.com/makarychev/solana-transfer-extensions/Failing integration test: https://github.com/makarychev/solana-transfer-extensions/actions/runs/9844698072/job/27178656558#step:9:74
Program log: Error: memory allocation failed, out of memory
Instruction code: https://github.com/makarychev/solana-transfer-extensions/blob/main/programs/transfer-extensions/src/instructions/multi_transfers.rs#L40
Transfer Hook code: https://github.com/makarychev/solana-transfer-extensions/blob/main/programs/transfer-hook/src/instructions/handler.rs#L41
Questions:
How can we send 2 transfers from 1 external program instruction if transfer hook has 8 extra account? Does transfer with transfer hook execution have constraint on extra accounts count?
Could
invoke_transfer_checked
be optimized or in such scenario or it is required to fill extra accounts manually (not byinvoke_transfer_checked
)?Probably it is possible to execute transfer with transfer hook more efficient?
The text was updated successfully, but these errors were encountered: