Skip to content
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

Loader-v3 programdata account size is not counted towards limit if executed through CPI #2274

Open
tao-stones opened this issue Jul 24, 2024 · 7 comments
Assignees

Comments

@tao-stones
Copy link

Problem

@Lichtso repots:

we are not accounting for loader-v3 programs executed through CPI here:
https://github.com/anza-xyz/agave/blob/177f722410eada6b44cd3f65d992d9a846f21106/svm/src/account_loader.rs#L280
Since it was added (in [#29743](https://github.com/solana-labs/solana/pull/29743)) it was changed once to account for loader-v3 programdata accounts, but only in top-level instructions ([#30703](https://github.com/solana-labs/solana/pull/30703)).

Only in top-level instructions we use program.account_size from the program cache which correctly includes the loader-v3 programdata account length. The else branch however does use account.data().len() which is the proxy account length.

Add a test to demonstrate it.

Proposed Solution

@Lichtso Lichtso changed the title Loader-v3 programdata account size is not counted towards to limit if executed through CPI Loader-v3 programdata account size is not counted towards limit if executed through CPI Jul 24, 2024
@jstarry
Copy link

jstarry commented Aug 13, 2024

I'd like to take on this task by refactoring the existing code and likely adding a feature gate to make this behavior consistent with other loaders.

@jstarry jstarry self-assigned this Aug 13, 2024
@tao-stones
Copy link
Author

I'd like to take on this task by refactoring the existing code and likely adding a feature gate to make this behavior consistent with other loaders.

@Lichtso do you have something in pipeline might potentially conflict with it?

@Lichtso
Copy link

Lichtso commented Aug 13, 2024

Yes, but @jstarry and I already coordinated.

@jstarry
Copy link

jstarry commented Nov 12, 2024

FYI this will be addressed in @2501babe's loaded account data size SIMD here: solana-foundation/solana-improvement-documents#186

@ceciEstErmat
Copy link

I was looking at how exactly are CPI program executed and it's a bit confusing.
The executor seem to require the program to be in the cache - but nowhere in the CPI code is this one added into it.
How is this program data executed really ?

https://github.com/anza-xyz/agave/blob/master/programs/bpf_loader/src/lib.rs#L457

@Lichtso
Copy link

Lichtso commented Dec 15, 2024

The executor seem to require the program to be in the cache - but nowhere in the CPI code is this one added into it.

It happens earlier in transaction loading as one can only CPI programs which are present in the transactions account keys:

let program_cache_for_tx_batch = self.replenish_program_cache(

@ceciEstErmat
Copy link

Ah right I for some reason assumed it was loading only the actual program form top level ix - but it is the load account that does that :D
Thx !

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

No branches or pull requests

4 participants