-
Notifications
You must be signed in to change notification settings - Fork 301
SIMD-0242: Static Nonce Account Only #242
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| --- | ||
| simd: '0242' | ||
| title: Static Nonce Account Only | ||
| authors: | ||
| - Andrew Fitzgerald (Anza) | ||
| category: Standard | ||
| type: Core | ||
| status: Review | ||
| created: 2025-02-03 | ||
| feature: | ||
| --- | ||
|
|
||
| ## Summary | ||
|
|
||
| The advance nonce instruction in a transaction can currently specify any | ||
| account in the transaction as the account to advance. | ||
| This proposal would restrict the advance nonce instruction to only be able to | ||
| advance a statically included account. | ||
|
|
||
| ## Motivation | ||
|
|
||
| - There is a separate proposal to relax the constraints on transaction account | ||
| resolution. That proposal would allow transactions with invalid lookups or | ||
| duplicate accounts to be included in a block, pay fees, but not processed. | ||
| - However, that relaxation cannot happen without specifying the interaction | ||
| with nonce accounts. Rather than complicating the protocol and specifying the | ||
| interactions between lookups and nonces, this proposal aims to simplify by | ||
| restricting nonce accounts to statically declared accounts. | ||
|
|
||
| ## New Terminology | ||
|
|
||
| No new terminology is introduced. | ||
| Clarification of existing terminology is provided: | ||
|
|
||
| - nonce transaction | ||
| - A transaction that includes `SystemInstruction::AdvanceNonceAccount` as its | ||
| first instruction. | ||
| - nonce account | ||
| - The account specified in the first account of the | ||
| `SystemInstruction::AdvanceNonceAccount` instruction in a nonce | ||
| transaction. | ||
| - statically included account | ||
| - An account that has its' `Pubkey` directly included in the transaction | ||
| message. | ||
|
|
||
| ## Detailed Design | ||
|
|
||
| - Nonce transactions MUST have a nonce account index that is less the number of | ||
| statically included accounts in the transaction. | ||
| - Leader nodes MUST drop nonce transactions that do not meet this requirement | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious - why is this check not in the SVM transaction sanitization, instead of the block builder? This might make sense from an API perspective, and so that users are charged for submitting these invalid transactions.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it could be done in SVM, really anywhere before we get to actually publishing a block.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @topointon-jump, as discussed elsewhere, agave will implement feature-gated checks in the runtime to check the new constraint and fail the transaction. This detail is implementation specific, which iirc we should avoid in proposals, so making a comment here for your records. Agave's runtime will mark these transactions with a specific error, which should enable frankendancer to use it w/ minimal changes. |
||
| without including them in a block. | ||
| - Nonce transactions that do not meet this requirement are invalid and MUST | ||
| result in the entire block being rejected by the validators. | ||
|
|
||
| ## Alternatives Considered | ||
|
|
||
| - Do nothing | ||
| - This means we cannot relax account resolution constraints | ||
| - Delete nonces entirely | ||
| - preferred by some, but is a separate proposal and a more thought-out | ||
| transition plan is necessary. | ||
|
|
||
| ## Impact | ||
|
|
||
| - Clients who currently send nonce-transactions with looked up nonce accounts | ||
| will need to change their behavior. | ||
|
|
||
| ## Security Considerations | ||
|
|
||
| - Requires a feature-gate to be enabled to avoid forking the network. | ||
|
|
||
| ## Backwards Compatibility | ||
|
|
||
| - Some previously valid transactions may not be valid under the new | ||
| restrictions. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need the recent blockhashes sysvar to be static 💀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And we should explicitly state what happens if you append a bunch of lookup accounts to the advance nonce ix 💀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3328140
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait actually we don't need this change. We would never process the actual advance nonce ix for a tx with invalid lookups, we advance the nonce in the runtime. So only the nonce needs to be static. Let's revert 3328140
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did we drop the in-program verify+advance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we never actually need the recent blockhashes sysvar to advance the nonce; but if the transaction would otherwise have succeeded, not having that account as an input would make it fail on the nonce ix processing?
That seems extremely odd, but probably not relevant to this SIMD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, not having the recent blockhashes sysvar as an account input would make it fail the nonce ix processing in the SVM. But if nonce ix processing fails in the SVM, the runtime takes over and manually advances the nonce since the runtime obviously has access to the latest blockhash and therefore knows what the next durable nonce should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
signers are already required to be statically specified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, signers are already required to be static.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still need to revert 3328140