-
Notifications
You must be signed in to change notification settings - Fork 4.5k
adds system instruction to upgrade legacy nonce versions (backport #25789) #25890
Conversation
…#25789) #25788 permanently disables durable transactions with legacy nonce versions which are within chain blockhash domain. This commit adds a new system instruction for a one-time idempotent upgrade of legacy nonce accounts in order to bump them out of chain blockhash domain. (cherry picked from commit b419031) # Conflicts: # runtime/src/system_instruction_processor.rs # sdk/program/src/system_instruction.rs # web3.js/src/system-program.ts
automerge label removed due to a CI failure |
6f50a58
to
908488f
Compare
automerge label removed due to a CI failure |
automerge label removed due to a CI failure |
af426df
to
d72a228
Compare
automerge label removed due to a CI failure |
d72a228
to
79d0e39
Compare
automerge label removed due to a CI failure |
SystemInstruction::UpgradeNonceAccount => { | ||
let separate_nonce_from_blockhash = invoke_context | ||
.feature_set | ||
.is_active(&feature_set::separate_nonce_from_blockhash::id()); | ||
if !separate_nonce_from_blockhash { | ||
return Err(InstructionError::InvalidInstructionData); | ||
} | ||
let nonce_account = keyed_account_at_index(keyed_accounts, first_instruction_account)?; | ||
if !system_program::check_id(&nonce_account.owner()?) { | ||
return Err(InstructionError::InvalidAccountOwner); | ||
} | ||
if !nonce_account.is_writable() { | ||
return Err(InstructionError::InvalidArgument); | ||
} | ||
let nonce_versions: nonce::state::Versions = nonce_account.state()?; | ||
match nonce_versions.upgrade() { | ||
None => Err(InstructionError::InvalidArgument), | ||
Some(nonce_versions) => nonce_account.set_state(&nonce_versions), | ||
} | ||
} |
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.
@Lichtso @jackcmay Backporting #25789 to v1.9 was not straightforward given the KeyedAccount
changes.
Would you please double check if this code is precisely equivalent to master:
https://github.com/solana-labs/solana/blob/98176fad7/runtime/src/system_instruction_processor.rs#L486-L507
e.g. that failure conditions and resulting kinds of error has not changed.
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.
lgtm
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.
There is only one instruction account so at least there can be no borrow collision by account aliasing. The only tricky part here is the index of keyed_account_at_index(keyed_accounts, first_instruction_account)?
vs instruction_context.try_borrow_instruction_account(transaction_context, 0)?
and that is correct.
This is an automatic backport of pull request #25789 done by Mergify.
Cherry-pick of b419031 has failed:
To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally
Mergify commands and options
More conditions and actions can be found in the documentation.
You can also trigger Mergify actions by commenting on this pull request:
@Mergifyio refresh
will re-evaluate the rules@Mergifyio rebase
will rebase this PR on its base branch@Mergifyio update
will merge the base branch into this PR@Mergifyio backport <destination>
will backport this PR on<destination>
branchAdditionally, on Mergify dashboard you can:
Finally, you can contact us on https://mergify.com