Skip to content

SIMD-0186: Clarify program/loader validation #311

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

2501babe
Copy link
Member

the text is straightforward but where this lives and how its added is a bit tricky. we made these changes in the 186 agave impl, but they mirror changes in the 162 agave impl, and neither simd actually describes them. they are highly desirable changes because they drastically simplify the code and there is essentially no difference between them aside from whether the errors happen during execution or during loading

tbh we dont want to rename and rewrite the whole 186 text to expand the scope of the simd to fully describe account loading, so the simplest solutions are to either add it as an independent section (as done here) or make a new short "hey this is how program and loader validation works btw" simd. i think adding the section here is better because these changes are associated with the 186 code (although they reflect changes made in the 162 code) and because it is less likely for someone to miss them, because a new simd would be a random document tied to nothing

these changes were made in 186 because it was a fortuitous place to be able to delete complex code and replace it with simple code. the main concern is just that other validator clients replicate the changes

@2501babe 2501babe self-assigned this Jun 26, 2025
@2501babe 2501babe force-pushed the sim186-loader-val branch 2 times, most recently from c81fa42 to cdab0a4 Compare June 26, 2025 21:35
Comment on lines 121 to 122
their program owners, are validated. This process is unrelated to the rest of
this SIMD, but is not defined elsewhere, so we define it here.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fair to say that it is related to this SIMD because now that we don't need to load the owner account to track its size, we are able to make this simplification.

Copy link
Member Author

Choose a reason for hiding this comment

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

ill reword it, im trying to adhere to the philosophy that the simd document is primary and the solana validators are concrete implementations of the simd. its unrelated in the sense that this simd is exclusively about how sizes are determined so the validation is sort of a tangent

`NativeLoader1111111111111111111111111111111` itself. This special case has been
removed, and `NativeLoader1111111111111111111111111111111` behaves like any
other account.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should also say that previously the program id's owner was validated by checking if it was equal to the native loader or, if not, was loaded and validated by checking that it was executable and that the owner's owner == native loader.

@2501babe
Copy link
Member Author

@jstarry @mjain-jump if i could have your approvals on this, so foundation can merge

Copy link
Contributor

@jstarry jstarry left a comment

Choose a reason for hiding this comment

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

Sorry just a few more small requests

Comment on lines 128 to 131
* If [SIMD-0162](
https://github.com/solana-foundation/solana-improvement-documents/pull/162)
has not been activated, verify the program account is marked executable. When
SIMD-0162 is active, skip this step.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: SIMD-0162 is activated on all clusters so we can remove this now

Copy link
Member Author

Choose a reason for hiding this comment

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


The process is as follows; for each instruction's program ID:

* Verify the account exists.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add the expected error for this check? (ProgramAccountNotFound)

Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/solana-foundation/solana-improvement-documents/pull/162)
has not been activated, verify the program account is marked executable. When
SIMD-0162 is active, skip this step.
* Verify the program account's owner is one of:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, can you add the expected error if this check fails? (InvalidProgramForExecution)

Copy link
Member Author

Choose a reason for hiding this comment

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

@2501babe 2501babe force-pushed the sim186-loader-val branch from 4afa17a to 52c4a8e Compare July 29, 2025 14:44
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.

2 participants