SIMD-0186: Clarify program/loader validation#311
Conversation
2501babe
force-pushed
the
sim186-loader-val
branch
2 times, most recently
from
c81fa42 to
cdab0a4
Compare
2501babe
force-pushed
the
sim186-loader-val
branch
from
cdab0a4 to
5f4dd69
Compare
| 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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. | ||
|
|
There was a problem hiding this comment.
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.
|
@jstarry @mjain-jump if i could have your approvals on this, so foundation can merge |
jstarry
left a comment
jstarry
left a comment
There was a problem hiding this comment.
Sorry just a few more small requests
| * 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. |
There was a problem hiding this comment.
nit: SIMD-0162 is activated on all clusters so we can remove this now
|
|
||
| The process is as follows; for each instruction's program ID: | ||
|
|
||
| * Verify the account exists. |
There was a problem hiding this comment.
Can we add the expected error for this check? (ProgramAccountNotFound)
| 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: |
There was a problem hiding this comment.
Same here, can you add the expected error if this check fails? (InvalidProgramForExecution)
2501babe
force-pushed
the
sim186-loader-val
branch
from
4afa17a to
52c4a8e
Compare
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
We've got approval from Firedancer and Anza, merging!
5 participants
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