From 9c7a7fcde47ef794d14040b7a4abd0047b98fbac Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Sun, 5 Jun 2022 16:09:25 -0400 Subject: [PATCH] feat(nonce): adds system instruction to upgrade legacy nonce versions https://github.com/solana-labs/solana/pull/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. --- cli/src/cli.rs | 10 ++ cli/src/nonce.rs | 68 ++++++- docs/src/developing/clients/javascript-api.md | 1 + runtime/src/system_instruction_processor.rs | 166 +++++++++++++++++- sdk/program/src/nonce/state/mod.rs | 53 ++++++ sdk/program/src/system_instruction.rs | 20 ++- transaction-status/src/parse_system.rs | 9 + web3.js/src/system-program.ts | 10 +- 8 files changed, 331 insertions(+), 6 deletions(-) diff --git a/cli/src/cli.rs b/cli/src/cli.rs index 1f0f2ce1a7d458..e29f087538e38c 100644 --- a/cli/src/cli.rs +++ b/cli/src/cli.rs @@ -157,6 +157,10 @@ pub enum CliCommand { destination_account_pubkey: Pubkey, lamports: u64, }, + UpgradeNonceAccount { + nonce_account: Pubkey, + memo: Option, + }, // Program Deployment Deploy { program_location: String, @@ -633,6 +637,7 @@ pub fn parse_command( ("withdraw-from-nonce-account", Some(matches)) => { parse_withdraw_from_nonce_account(matches, default_signer, wallet_manager) } + ("upgrade-nonce-account", Some(matches)) => parse_upgrade_nonce_account(matches), // Program Deployment ("deploy", Some(matches)) => { let (address_signer, _address) = signer_of(matches, "address_signer", wallet_manager)?; @@ -1019,6 +1024,11 @@ pub fn process_command(config: &CliConfig) -> ProcessResult { destination_account_pubkey, *lamports, ), + // Upgrade nonce account out of blockhash domain. + CliCommand::UpgradeNonceAccount { + nonce_account, + memo, + } => process_upgrade_nonce_account(&rpc_client, config, *nonce_account, memo.as_ref()), // Program Deployment diff --git a/cli/src/nonce.rs b/cli/src/nonce.rs index d94468c770f72e..8ac115c49169cf 100644 --- a/cli/src/nonce.rs +++ b/cli/src/nonce.rs @@ -13,7 +13,7 @@ use { solana_clap_utils::{ input_parsers::*, input_validators::*, - keypair::{DefaultSigner, SignerIndex}, + keypair::{CliSigners, DefaultSigner, SignerIndex}, memo::{memo_arg, MEMO_ARG}, nonce::*, }, @@ -30,8 +30,8 @@ use { pubkey::Pubkey, system_instruction::{ advance_nonce_account, authorize_nonce_account, create_nonce_account, - create_nonce_account_with_seed, instruction_to_nonce_error, withdraw_nonce_account, - NonceError, SystemError, + create_nonce_account_with_seed, instruction_to_nonce_error, upgrade_nonce_account, + withdraw_nonce_account, NonceError, SystemError, }, system_program, transaction::{Transaction, TransactionError}, @@ -173,6 +173,19 @@ impl NonceSubCommands for App<'_, '_> { .arg(nonce_authority_arg()) .arg(memo_arg()), ) + .subcommand( + SubCommand::with_name("upgrade-nonce-account") + .about("One-time idempotent upgrade of legacy nonce versions \ + in order to bump them out of chain blockhash domain.") + .arg( + pubkey!(Arg::with_name("nonce_account_pubkey") + .index(1) + .value_name("NONCE_ACCOUNT_ADDRESS") + .required(true), + "Nonce account to upgrade. "), + ) + .arg(memo_arg()), + ) } } @@ -325,6 +338,21 @@ pub fn parse_withdraw_from_nonce_account( }) } +pub(crate) fn parse_upgrade_nonce_account( + matches: &ArgMatches<'_>, +) -> Result { + let nonce_account = pubkey_of(matches, "nonce_account_pubkey").unwrap(); + let memo = matches.value_of(MEMO_ARG.name).map(String::from); + Ok(CliCommandInfo { + command: CliCommand::UpgradeNonceAccount { + nonce_account, + memo, + }, + // TODO: need signers. + signers: CliSigners::default(), + }) +} + /// Check if a nonce account is initialized with the given authority and hash pub fn check_nonce_account( nonce_account: &Account, @@ -656,6 +684,40 @@ pub fn process_withdraw_from_nonce_account( } } +pub(crate) fn process_upgrade_nonce_account( + rpc_client: &RpcClient, + config: &CliConfig, + nonce_account: Pubkey, + memo: Option<&String>, +) -> ProcessResult { + let latest_blockhash = rpc_client.get_latest_blockhash()?; + let ixs = vec![upgrade_nonce_account(nonce_account)].with_memo(memo); + // TODO what signers? + let message = Message::new(&ixs, Some(&config.signers[0].pubkey())); + let mut tx = Transaction::new_unsigned(message); + tx.try_sign(&config.signers, latest_blockhash)?; + check_account_for_fee_with_commitment( + rpc_client, + &config.signers[0].pubkey(), + &tx.message, + config.commitment, + )?; + let merge_errors = + get_feature_is_active(rpc_client, &merge_nonce_error_into_system_error::id())?; + let result = rpc_client.send_and_confirm_transaction_with_spinner(&tx); + if merge_errors { + log_instruction_custom_error::(result, config) + } else { + log_instruction_custom_error_ex::(result, config, |ix_error| { + if let InstructionError::Custom(_) = ix_error { + instruction_to_nonce_error(ix_error, merge_errors) + } else { + None + } + }) + } +} + #[cfg(test)] mod tests { use { diff --git a/docs/src/developing/clients/javascript-api.md b/docs/src/developing/clients/javascript-api.md index 2563891404c161..66b05b5a6696fa 100644 --- a/docs/src/developing/clients/javascript-api.md +++ b/docs/src/developing/clients/javascript-api.md @@ -238,6 +238,7 @@ pub enum SystemInstruction { /** 9 **/AllocateWithSeed {/**/}, /** 10 **/AssignWithSeed {/**/}, /** 11 **/TransferWithSeed {/**/}, + /** 12 **/UpgradeNonceAccount, } ``` diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index d8991ce38cb0ad..aa8575a6345704 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -483,6 +483,25 @@ pub fn process_instruction( instruction_context.try_borrow_instruction_account(transaction_context, 0)?; authorize_nonce_account(&mut me, &nonce_authority, &signers, invoke_context) } + 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); + } + instruction_context.check_number_of_instruction_accounts(1)?; + let mut nonce_account = + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + if nonce_account.get_owner() != &system_program::id() { + return Err(InstructionError::InvalidAccountOwner); + } + let nonce_versions: nonce::state::Versions = nonce_account.get_state()?; + match nonce_versions.upgrade() { + None => Err(InstructionError::InvalidArgument), + Some(nonce_versions) => nonce_account.set_state(&nonce_versions), + } + } SystemInstruction::Allocate { space } => { instruction_context.check_number_of_instruction_accounts(1)?; let mut account = instruction_context @@ -568,11 +587,18 @@ mod tests { use solana_sdk::{ account::{self, Account, AccountSharedData, ReadableAccount}, client::SyncClient, + fee_calculator::FeeCalculator, genesis_config::create_genesis_config, hash::{hash, Hash}, instruction::{AccountMeta, Instruction, InstructionError}, message::Message, - nonce, nonce_account, recent_blockhashes_account, + nonce::{ + self, + state::{ + Data as NonceData, DurableNonce, State as NonceState, Versions as NonceVersions, + }, + }, + nonce_account, recent_blockhashes_account, signature::{Keypair, Signer}, system_instruction, system_program, sysvar::{self, recent_blockhashes::IterItem, rent::Rent}, @@ -2202,4 +2228,142 @@ mod tests { }, ); } + + #[test] + fn test_nonce_account_upgrade_check_owner() { + let nonce_address = Pubkey::new_unique(); + let versions = NonceVersions::Legacy(Box::new(NonceState::Uninitialized)); + let nonce_account = AccountSharedData::new_data( + 1_000_000, // lamports + &versions, // state + &Pubkey::new_unique(), // owner + ) + .unwrap(); + let accounts = process_instruction( + &serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(), + vec![(nonce_address, nonce_account.clone())], + vec![AccountMeta { + pubkey: nonce_address, + is_signer: false, + is_writable: false, + }], + Err(InstructionError::InvalidAccountOwner), + super::process_instruction, + ); + assert_eq!(accounts.len(), 1); + assert_eq!(accounts[0], nonce_account); + } + + #[test] + fn test_nonce_account_upgrade() { + let nonce_address = Pubkey::new_unique(); + let versions = NonceVersions::Legacy(Box::new(NonceState::Uninitialized)); + let nonce_account = AccountSharedData::new_data( + 1_000_000, // lamports + &versions, // state + &system_program::id(), // owner + ) + .unwrap(); + assert_eq!( + nonce_account.deserialize_data::().unwrap(), + NonceVersions::Legacy(Box::new(NonceState::Uninitialized)) + ); + let mut accounts = process_instruction( + &serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(), + vec![(nonce_address, nonce_account)], + vec![AccountMeta { + pubkey: nonce_address, + is_signer: false, + is_writable: false, + }], + Ok(()), + super::process_instruction, + ); + assert_eq!(accounts.len(), 1); + let nonce_account = accounts.remove(0); + let upgraded_nonce_account = NonceVersions::Current(Box::new(NonceState::Uninitialized)); + assert_eq!( + nonce_account.deserialize_data::().unwrap(), + upgraded_nonce_account + ); + let accounts = process_instruction( + &serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(), + vec![(nonce_address, nonce_account)], + vec![AccountMeta { + pubkey: nonce_address, + is_signer: false, + is_writable: false, + }], + Err(InstructionError::InvalidArgument), + super::process_instruction, + ); + assert_eq!(accounts.len(), 1); + assert_eq!( + accounts[0].deserialize_data::().unwrap(), + upgraded_nonce_account + ); + let blockhash = Hash::from([171; 32]); + let durable_nonce = + DurableNonce::from_blockhash(&blockhash, /*separate_domains:*/ false); + let data = NonceData { + authority: Pubkey::new_unique(), + durable_nonce, + fee_calculator: FeeCalculator { + lamports_per_signature: 2718, + }, + }; + let versions = NonceVersions::Legacy(Box::new(NonceState::Initialized(data.clone()))); + let nonce_account = AccountSharedData::new_data( + 1_000_000, // lamports + &versions, // state + &system_program::id(), // owner + ) + .unwrap(); + assert_eq!( + nonce_account.deserialize_data::().unwrap(), + NonceVersions::Legacy(Box::new(NonceState::Initialized(data.clone()))) + ); + let mut accounts = process_instruction( + &serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(), + vec![(nonce_address, nonce_account)], + vec![AccountMeta { + pubkey: nonce_address, + is_signer: false, + is_writable: false, + }], + Ok(()), + super::process_instruction, + ); + assert_eq!(accounts.len(), 1); + let nonce_account = accounts.remove(0); + let durable_nonce = + DurableNonce::from_blockhash(&blockhash, /*separate_domains:*/ true); + assert_ne!(data.durable_nonce, durable_nonce); + let data = NonceData { + durable_nonce, + ..data + }; + let upgraded_nonce_account = + NonceVersions::Current(Box::new(NonceState::Initialized(data))); + assert_eq!( + nonce_account.deserialize_data::().unwrap(), + upgraded_nonce_account + ); + let accounts = process_instruction( + &serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(), + vec![(nonce_address, nonce_account)], + vec![AccountMeta { + pubkey: nonce_address, + is_signer: false, + is_writable: false, + }], + Err(InstructionError::InvalidArgument), + super::process_instruction, + ); + assert_eq!(accounts.len(), 1); + assert_eq!( + accounts[0].deserialize_data::().unwrap(), + upgraded_nonce_account + ); + } } diff --git a/sdk/program/src/nonce/state/mod.rs b/sdk/program/src/nonce/state/mod.rs index eadcc33aa9dcc0..9297410752e7f4 100644 --- a/sdk/program/src/nonce/state/mod.rs +++ b/sdk/program/src/nonce/state/mod.rs @@ -57,6 +57,25 @@ impl Versions { State::Initialized(ref data) => hash == &data.blockhash(), } } + + // Upgrades legacy nonces out of chain blockhash domains. + pub fn upgrade(self) -> Option { + match self { + Self::Legacy(mut state) => { + match *state { + State::Uninitialized => (), + State::Initialized(ref mut data) => { + data.durable_nonce = DurableNonce::from_blockhash( + &data.blockhash(), + true, // separate_domains + ); + } + } + Some(Self::Current(state)) + } + Self::Current(_) => None, + } + } } impl From for State { @@ -123,4 +142,38 @@ mod tests { assert!(versions.verify_recent_blockhash(durable_nonce.as_hash(), separate_domains)); } } + + #[test] + fn test_nonce_versions_upgrade() { + // Uninitialized + let versions = Versions::Legacy(Box::new(State::Uninitialized)); + let versions = versions.upgrade().unwrap(); + assert_eq!(versions, Versions::Current(Box::new(State::Uninitialized))); + assert_eq!(versions.upgrade(), None); + // Initialized + let blockhash = Hash::from([171; 32]); + let durable_nonce = + DurableNonce::from_blockhash(&blockhash, /*separate_domains:*/ false); + let data = Data { + authority: Pubkey::new_unique(), + durable_nonce, + fee_calculator: FeeCalculator { + lamports_per_signature: 2718, + }, + }; + let versions = Versions::Legacy(Box::new(State::Initialized(data.clone()))); + let durable_nonce = + DurableNonce::from_blockhash(&blockhash, /*separate_domains:*/ true); + assert_ne!(data.durable_nonce, durable_nonce); + let data = Data { + durable_nonce, + ..data + }; + let versions = versions.upgrade().unwrap(); + assert_eq!( + versions, + Versions::Current(Box::new(State::Initialized(data))) + ); + assert_eq!(versions.upgrade(), None); + } } diff --git a/sdk/program/src/system_instruction.rs b/sdk/program/src/system_instruction.rs index d62333a1b17066..4cdb980cb3ca68 100644 --- a/sdk/program/src/system_instruction.rs +++ b/sdk/program/src/system_instruction.rs @@ -142,7 +142,7 @@ pub fn instruction_to_nonce_error( /// maximum permitted size of data: 10 MB pub const MAX_PERMITTED_DATA_LENGTH: u64 = 10 * 1024 * 1024; -#[frozen_abi(digest = "2xnDcizcPKKR7b624FeuuPd1zj5bmnkmVsBWgoKPTh4w")] +#[frozen_abi(digest = "5e22s2kFu9Do77hdcCyxyhuKHD8ThAB6Q6dNaLTCjL5M")] #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, AbiExample, AbiEnumVisitor)] pub enum SystemInstruction { /// Create a new account @@ -307,6 +307,13 @@ pub enum SystemInstruction { /// Owner to use to derive the funding account address from_owner: Pubkey, }, + + /// One-time idempotent upgrade of legacy nonce versions in order to bump + /// them out of chain blockhash domain. + /// + /// # Account references + /// 0. `[WRITE]` Nonce account + UpgradeNonceAccount, } pub fn create_account( @@ -940,6 +947,17 @@ pub fn authorize_nonce_account( ) } +/// One-time idempotent upgrade of legacy nonce versions in order to bump +/// them out of chain blockhash domain. +pub fn upgrade_nonce_account(nonce_pubkey: Pubkey) -> Instruction { + let account_metas = vec![AccountMeta::new(nonce_pubkey, /*is_signer:*/ false)]; + Instruction::new_with_bincode( + system_program::id(), + &SystemInstruction::UpgradeNonceAccount, + account_metas, + ) +} + #[cfg(test)] mod tests { use { diff --git a/transaction-status/src/parse_system.rs b/transaction-status/src/parse_system.rs index ab6953f8e5a876..12bdaedfec863b 100644 --- a/transaction-status/src/parse_system.rs +++ b/transaction-status/src/parse_system.rs @@ -133,6 +133,15 @@ pub fn parse_system( }), }) } + SystemInstruction::UpgradeNonceAccount => { + check_num_system_accounts(&instruction.accounts, 1)?; + Ok(ParsedInstructionEnum { + instruction_type: "upgradeNonce".to_string(), + info: json!({ + "nonceAccount": account_keys[instruction.accounts[0] as usize].to_string(), + }), + }) + } SystemInstruction::Allocate { space } => { check_num_system_accounts(&instruction.accounts, 1)?; Ok(ParsedInstructionEnum { diff --git a/web3.js/src/system-program.ts b/web3.js/src/system-program.ts index 7680a0cdd02319..89e0b4c82b0909 100644 --- a/web3.js/src/system-program.ts +++ b/web3.js/src/system-program.ts @@ -566,7 +566,8 @@ export type SystemInstructionType = | 'InitializeNonceAccount' | 'Transfer' | 'TransferWithSeed' - | 'WithdrawNonceAccount'; + | 'WithdrawNonceAccount' + | 'UpgradeNonceAccount'; type SystemInstructionInputData = { AdvanceNonceAccount: IInstructionInputData; @@ -616,6 +617,7 @@ type SystemInstructionInputData = { WithdrawNonceAccount: IInstructionInputData & { lamports: number; }; + UpgradeNonceAccount: IInstructionInputData; }; /** @@ -724,6 +726,12 @@ export const SYSTEM_INSTRUCTION_LAYOUTS = Object.freeze<{ ], ), }, + UpgradeNonceAccount: { + index: 12, + layout: BufferLayout.struct< + SystemInstructionInputData['UpgradeNonceAccount'] + >([BufferLayout.u32('instruction')]), + }, }); /**