From 4e4885f7f9d331c664010e6f79cff2cb315a50a7 Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Thu, 9 Jun 2022 15:28:37 +0000 Subject: [PATCH] permanently disables durable nonces with chain blockhash domain (#25788) https://github.com/solana-labs/solana/pull/25744 separated durable nonce and blockhash domains, which will stop double execution going forward. However it is possible that a durable transaction has *already* been executed once as a normal transaction and it is now a valid durable transaction. #25744 cannot stop such transactions to be re-executed until the nonce accounts are advanced. This commit adds a new nonce version indicating that the nonce is moved out of the blockhash domain, and permanently disables durable transactions for legacy nonces which are in the blockhash domain. --- account-decoder/src/parse_account_data.rs | 5 +- account-decoder/src/parse_nonce.rs | 10 +- cli/src/nonce.rs | 56 ++-- client/src/blockhash_query.rs | 5 +- client/src/nonce_utils.rs | 5 +- rpc/src/rpc.rs | 13 +- rpc/src/transaction_status_service.rs | 6 +- runtime/src/accounts.rs | 152 ++++++---- runtime/src/bank.rs | 102 ++++--- runtime/src/nonce_keyed_account.rs | 260 ++++++++---------- runtime/src/system_instruction_processor.rs | 66 +++-- sdk/program/src/nonce/state/current.rs | 10 + sdk/program/src/nonce/state/mod.rs | 167 ++++++++++- sdk/src/nonce_account.rs | 164 +++++++++-- .../src/send_transaction_service.rs | 32 ++- 15 files changed, 709 insertions(+), 344 deletions(-) diff --git a/account-decoder/src/parse_account_data.rs b/account-decoder/src/parse_account_data.rs index 6a9cd0b6c2b1f8..af1a58f1cadef9 100644 --- a/account-decoder/src/parse_account_data.rs +++ b/account-decoder/src/parse_account_data.rs @@ -145,7 +145,10 @@ mod test { assert_eq!(parsed.program, "vote".to_string()); assert_eq!(parsed.space, VoteState::size_of() as u64); - let nonce_data = Versions::new_current(State::Initialized(Data::default())); + let nonce_data = Versions::new( + State::Initialized(Data::default()), + true, // separate_domains + ); let nonce_account_data = bincode::serialize(&nonce_data).unwrap(); let parsed = parse_account_data( &account_pubkey, diff --git a/account-decoder/src/parse_nonce.rs b/account-decoder/src/parse_nonce.rs index b4b6ea044a8364..d2dedc818a2d02 100644 --- a/account-decoder/src/parse_nonce.rs +++ b/account-decoder/src/parse_nonce.rs @@ -7,10 +7,9 @@ use { }; pub fn parse_nonce(data: &[u8]) -> Result { - let nonce_state: Versions = bincode::deserialize(data) + let nonce_versions: Versions = bincode::deserialize(data) .map_err(|_| ParseAccountError::from(InstructionError::InvalidAccountData))?; - let nonce_state = nonce_state.convert_to_current(); - match nonce_state { + match nonce_versions.state() { // This prevents parsing an allocated System-owned account with empty data of any non-zero // length as `uninitialized` nonce. An empty account of the wrong length can never be // initialized as a nonce account, and an empty account of the correct length may not be an @@ -58,7 +57,10 @@ mod test { #[test] fn test_parse_nonce() { - let nonce_data = Versions::new_current(State::Initialized(Data::default())); + let nonce_data = Versions::new( + State::Initialized(Data::default()), + true, // separate_domains + ); let nonce_account_data = bincode::serialize(&nonce_data).unwrap(); assert_eq!( parse_nonce(&nonce_account_data).unwrap(), diff --git a/cli/src/nonce.rs b/cli/src/nonce.rs index fdf253943331d8..d94468c770f72e 100644 --- a/cli/src/nonce.rs +++ b/cli/src/nonce.rs @@ -933,11 +933,10 @@ mod tests { DurableNonce::from_blockhash(&Hash::default(), /*separate_domains:*/ true); let blockhash = *durable_nonce.as_hash(); let nonce_pubkey = solana_sdk::pubkey::new_rand(); - let data = Versions::new_current(State::Initialized(nonce::state::Data::new( - nonce_pubkey, - durable_nonce, - 0, - ))); + let data = Versions::new( + State::Initialized(nonce::state::Data::new(nonce_pubkey, durable_nonce, 0)), + true, // separate_domains + ); let valid = Account::new_data(1, &data, &system_program::ID); assert!(check_nonce_account(&valid.unwrap(), &nonce_pubkey, &blockhash).is_ok()); @@ -957,11 +956,14 @@ mod tests { let invalid_durable_nonce = DurableNonce::from_blockhash(&hash(b"invalid"), /*separate_domains:*/ true); - let data = Versions::new_current(State::Initialized(nonce::state::Data::new( - nonce_pubkey, - invalid_durable_nonce, - 0, - ))); + let data = Versions::new( + State::Initialized(nonce::state::Data::new( + nonce_pubkey, + invalid_durable_nonce, + 0, + )), + true, // separate_domains + ); let invalid_hash = Account::new_data(1, &data, &system_program::ID).unwrap(); if let CliError::InvalidNonce(err) = check_nonce_account(&invalid_hash, &nonce_pubkey, &blockhash).unwrap_err() @@ -976,11 +978,14 @@ mod tests { } let new_nonce_authority = solana_sdk::pubkey::new_rand(); - let data = Versions::new_current(State::Initialized(nonce::state::Data::new( - new_nonce_authority, - durable_nonce, - 0, - ))); + let data = Versions::new( + State::Initialized(nonce::state::Data::new( + new_nonce_authority, + durable_nonce, + 0, + )), + true, // separate_domains + ); let invalid_authority = Account::new_data(1, &data, &system_program::ID); if let CliError::InvalidNonce(err) = check_nonce_account(&invalid_authority.unwrap(), &nonce_pubkey, &blockhash).unwrap_err() @@ -994,7 +999,7 @@ mod tests { ); } - let data = Versions::new_current(State::Uninitialized); + let data = Versions::new(State::Uninitialized, /*separate_domains:*/ true); let invalid_state = Account::new_data(1, &data, &system_program::ID); if let CliError::InvalidNonce(err) = check_nonce_account(&invalid_state.unwrap(), &nonce_pubkey, &blockhash).unwrap_err() @@ -1005,7 +1010,8 @@ mod tests { #[test] fn test_account_identity_ok() { - let nonce_account = nonce_account::create_account(1).into_inner(); + let nonce_account = + nonce_account::create_account(1, /*separate_domains:*/ true).into_inner(); assert_eq!(account_identity_ok(&nonce_account), Ok(())); let system_account = Account::new(1, 0, &system_program::id()); @@ -1024,14 +1030,18 @@ mod tests { #[test] fn test_state_from_account() { - let mut nonce_account = nonce_account::create_account(1).into_inner(); + let mut nonce_account = + nonce_account::create_account(1, /*separate_domains:*/ true).into_inner(); assert_eq!(state_from_account(&nonce_account), Ok(State::Uninitialized)); let durable_nonce = DurableNonce::from_blockhash(&Hash::new(&[42u8; 32]), /*separate_domains:*/ true); let data = nonce::state::Data::new(Pubkey::new(&[1u8; 32]), durable_nonce, 42); nonce_account - .set_state(&Versions::new_current(State::Initialized(data.clone()))) + .set_state(&Versions::new( + State::Initialized(data.clone()), + true, // separate_domains + )) .unwrap(); assert_eq!( state_from_account(&nonce_account), @@ -1047,7 +1057,8 @@ mod tests { #[test] fn test_data_from_helpers() { - let mut nonce_account = nonce_account::create_account(1).into_inner(); + let mut nonce_account = + nonce_account::create_account(1, /*separate_domains:*/ true).into_inner(); let state = state_from_account(&nonce_account).unwrap(); assert_eq!( data_from_state(&state), @@ -1062,7 +1073,10 @@ mod tests { DurableNonce::from_blockhash(&Hash::new(&[42u8; 32]), /*separate_domains:*/ true); let data = nonce::state::Data::new(Pubkey::new(&[1u8; 32]), durable_nonce, 42); nonce_account - .set_state(&Versions::new_current(State::Initialized(data.clone()))) + .set_state(&Versions::new( + State::Initialized(data.clone()), + true, // separate_domains + )) .unwrap(); let state = state_from_account(&nonce_account).unwrap(); assert_eq!(data_from_state(&state), Ok(&data)); diff --git a/client/src/blockhash_query.rs b/client/src/blockhash_query.rs index 798900de3f1827..fc4330655d24e3 100644 --- a/client/src/blockhash_query.rs +++ b/client/src/blockhash_query.rs @@ -421,7 +421,10 @@ mod tests { }; let nonce_account = Account::new_data_with_space( 42, - &nonce::state::Versions::new_current(nonce::State::Initialized(data)), + &nonce::state::Versions::new( + nonce::State::Initialized(data), + true, // separate_domains + ), nonce::State::size(), &system_program::id(), ) diff --git a/client/src/nonce_utils.rs b/client/src/nonce_utils.rs index cbc1121a48453b..33048d1aa6339b 100644 --- a/client/src/nonce_utils.rs +++ b/client/src/nonce_utils.rs @@ -129,9 +129,8 @@ pub fn state_from_account>( account: &T, ) -> Result { account_identity_ok(account)?; - StateMut::::state(account) - .map_err(|_| Error::InvalidAccountData) - .map(|v| v.convert_to_current()) + let versions = StateMut::::state(account).map_err(|_| Error::InvalidAccountData)?; + Ok(State::from(versions)) } /// Deserialize the state data of a durable transaction nonce account. diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 83f6c215df3fcc..4d83b6961319f2 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -5409,11 +5409,10 @@ pub mod tests { let authority = Pubkey::new_unique(); let account = AccountSharedData::new_data( 42, - &nonce::state::Versions::new_current(nonce::State::new_initialized( - &authority, - DurableNonce::default(), - 1000, - )), + &nonce::state::Versions::new( + nonce::State::new_initialized(&authority, DurableNonce::default(), 1000), + true, // separate_domains + ), &system_program::id(), ) .unwrap(); @@ -5444,8 +5443,8 @@ pub mod tests { system_program::id().to_string(), {"filters": [{ "memcmp": { - "offset": 0, - "bytes": bs58::encode(vec![1, 0, 0, 0]).into_string(), + "offset": 4, + "bytes": bs58::encode(vec![0, 0, 0, 0]).into_string(), }, }]}, ])), diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 7de26d667b50d4..a17ce226d84ac8 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -323,13 +323,15 @@ pub(crate) mod tests { let expected_transaction = transaction.clone(); let pubkey = Pubkey::new_unique(); - let mut nonce_account = nonce_account::create_account(1).into_inner(); + let mut nonce_account = + nonce_account::create_account(1, /*separate_domains:*/ true).into_inner(); let durable_nonce = DurableNonce::from_blockhash(&Hash::new(&[42u8; 32]), /*separate_domains:*/ true); let data = nonce::state::Data::new(Pubkey::new(&[1u8; 32]), durable_nonce, 42); nonce_account - .set_state(&nonce::state::Versions::new_current( + .set_state(&nonce::state::Versions::new( nonce::State::Initialized(data), + true, // separate_domains )) .unwrap(); diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index bc0a063b065f46..0cb4c6a9c508a1 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1143,7 +1143,7 @@ impl Accounts { res: &'a [TransactionExecutionResult], loaded: &'a mut [TransactionLoadResult], rent_collector: &RentCollector, - durable_nonce: &DurableNonce, + durable_nonce: &(DurableNonce, /*separate_domains:*/ bool), lamports_per_signature: u64, leave_nonce_on_success: bool, ) { @@ -1178,7 +1178,7 @@ impl Accounts { execution_results: &'a [TransactionExecutionResult], load_results: &'a mut [TransactionLoadResult], rent_collector: &RentCollector, - durable_nonce: &DurableNonce, + durable_nonce: &(DurableNonce, /*separate_domains:*/ bool), lamports_per_signature: u64, leave_nonce_on_success: bool, ) -> Vec<(&'a Pubkey, &'a AccountSharedData)> { @@ -1265,7 +1265,7 @@ fn prepare_if_nonce_account<'a>( execution_result: &Result<()>, is_fee_payer: bool, maybe_nonce: Option<(&'a NonceFull, bool)>, - durable_nonce: &DurableNonce, + &(durable_nonce, separate_domains): &(DurableNonce, bool), lamports_per_signature: u64, ) -> bool { if let Some((nonce, rollback)) = maybe_nonce { @@ -1284,17 +1284,15 @@ fn prepare_if_nonce_account<'a>( // // Since we know we are dealing with a valid nonce account, // unwrap is safe here - let state = StateMut::::state(nonce.account()) - .unwrap() - .convert_to_current(); - if let NonceState::Initialized(ref data) = state { - account - .set_state(&NonceVersions::new_current(NonceState::new_initialized( - &data.authority, - *durable_nonce, - lamports_per_signature, - ))) - .unwrap(); + let nonce_versions = StateMut::::state(nonce.account()).unwrap(); + if let NonceState::Initialized(ref data) = nonce_versions.state() { + let nonce_state = NonceState::new_initialized( + &data.authority, + durable_nonce, + lamports_per_signature, + ); + let nonce_versions = NonceVersions::new(nonce_state, separate_domains); + account.set_state(&nonce_versions).unwrap(); } true } else { @@ -1347,6 +1345,7 @@ mod tests { bank::{DurableNonceFee, TransactionExecutionDetails}, rent_collector::RentCollector, }, + assert_matches::assert_matches, solana_address_lookup_table_program::state::LookupTableMeta, solana_sdk::{ account::{AccountSharedData, WritableAccount}, @@ -1660,7 +1659,10 @@ mod tests { nonce.pubkey(), AccountSharedData::new_data( min_balance * 2, - &NonceVersions::new_current(NonceState::Initialized(nonce::state::Data::default())), + &NonceVersions::new( + NonceState::Initialized(nonce::state::Data::default()), + true, // separate_domains + ), &system_program::id(), ) .unwrap(), @@ -2963,7 +2965,7 @@ mod tests { &execution_results, loaded.as_mut_slice(), &rent_collector, - &DurableNonce::default(), + &(DurableNonce::default(), /*separate_domains:*/ true), 0, true, // leave_nonce_on_success ); @@ -3068,20 +3070,24 @@ mod tests { Pubkey, AccountSharedData, AccountSharedData, - DurableNonce, + (DurableNonce, /*separate_domains:*/ bool), u64, Option, ) { - let data = - NonceVersions::new_current(NonceState::Initialized(nonce::state::Data::default())); + let data = NonceVersions::new( + NonceState::Initialized(nonce::state::Data::default()), + true, // separate_domains + ); let account = AccountSharedData::new_data(42, &data, &system_program::id()).unwrap(); let mut pre_account = account.clone(); pre_account.set_lamports(43); + let durable_nonce = + DurableNonce::from_blockhash(&Hash::new(&[1u8; 32]), /*separate_domains:*/ true); ( Pubkey::default(), pre_account, account, - DurableNonce::from_blockhash(&Hash::new(&[1u8; 32]), /*separate_domains:*/ true), + (durable_nonce, /*separate_domains:*/ true), 1234, None, ) @@ -3093,7 +3099,7 @@ mod tests { tx_result: &Result<()>, is_fee_payer: bool, maybe_nonce: Option<(&NonceFull, bool)>, - durable_nonce: &DurableNonce, + durable_nonce: &(DurableNonce, /*separate_domains:*/ bool), lamports_per_signature: u64, expect_account: &AccountSharedData, ) -> bool { @@ -3139,9 +3145,14 @@ mod tests { let mut expect_account = pre_account; expect_account - .set_state(&NonceVersions::new_current(NonceState::Initialized( - nonce::state::Data::new(Pubkey::default(), blockhash, lamports_per_signature), - ))) + .set_state(&NonceVersions::new( + NonceState::Initialized(nonce::state::Data::new( + Pubkey::default(), + blockhash.0, + lamports_per_signature, + )), + true, // separate_domains + )) .unwrap(); assert!(run_prepare_if_nonce_account_test( @@ -3225,9 +3236,14 @@ mod tests { let nonce = NonceFull::new(pre_account_address, pre_account, maybe_fee_payer_account); expect_account - .set_state(&NonceVersions::new_current(NonceState::Initialized( - nonce::state::Data::new(Pubkey::default(), blockhash, lamports_per_signature), - ))) + .set_state(&NonceVersions::new( + NonceState::Initialized(nonce::state::Data::new( + Pubkey::default(), + blockhash.0, + lamports_per_signature, + )), + true, // separate_domains + )) .unwrap(); assert!(run_prepare_if_nonce_account_test( @@ -3267,7 +3283,7 @@ mod tests { )), false, Some((&nonce, true)), - &DurableNonce::default(), + &(DurableNonce::default(), /*separate_domains:*/ true), 1, &post_fee_payer_account.clone(), )); @@ -3278,7 +3294,7 @@ mod tests { &Ok(()), true, Some((&nonce, true)), - &DurableNonce::default(), + &(DurableNonce::default(), /*separate_domains:*/ true), 1, &post_fee_payer_account.clone(), )); @@ -3292,7 +3308,7 @@ mod tests { )), true, None, - &DurableNonce::default(), + &(DurableNonce::default(), /*separate_domains:*/ true), 1, &post_fee_payer_account.clone(), )); @@ -3306,7 +3322,7 @@ mod tests { )), true, Some((&nonce, true)), - &DurableNonce::default(), + &(DurableNonce::default(), /*separate_domains:*/ true), 1, &pre_fee_payer_account, )); @@ -3323,9 +3339,14 @@ mod tests { let to_address = Pubkey::new_unique(); let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique(), /*separate_domains:*/ true); - let nonce_state = NonceVersions::new_current(NonceState::Initialized( - nonce::state::Data::new(nonce_authority.pubkey(), durable_nonce, 0), - )); + let nonce_state = NonceVersions::new( + NonceState::Initialized(nonce::state::Data::new( + nonce_authority.pubkey(), + durable_nonce, + 0, + )), + true, // separate_domains + ); let nonce_account_post = AccountSharedData::new_data(43, &nonce_state, &system_program::id()).unwrap(); let from_account_post = AccountSharedData::new(4199, 0, &Pubkey::default()); @@ -3350,9 +3371,14 @@ mod tests { let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique(), /*separate_domains:*/ true); - let nonce_state = NonceVersions::new_current(NonceState::Initialized( - nonce::state::Data::new(nonce_authority.pubkey(), durable_nonce, 0), - )); + let nonce_state = NonceVersions::new( + NonceState::Initialized(nonce::state::Data::new( + nonce_authority.pubkey(), + durable_nonce, + 0, + )), + true, // separate_domains + ); let nonce_account_pre = AccountSharedData::new_data(42, &nonce_state, &system_program::id()).unwrap(); let from_account_pre = AccountSharedData::new(4242, 0, &Pubkey::default()); @@ -3397,7 +3423,7 @@ mod tests { &execution_results, loaded.as_mut_slice(), &rent_collector, - &durable_nonce, + &(durable_nonce, /*separate_domains:*/ true), 0, true, // leave_nonce_on_success ); @@ -3421,11 +3447,14 @@ mod tests { collected_nonce_account.lamports(), nonce_account_pre.lamports(), ); - assert!(nonce_account::verify_nonce_account( - &collected_nonce_account, - durable_nonce.as_hash(), - ) - .is_some()); + assert_matches!( + nonce_account::verify_nonce_account( + &collected_nonce_account, + durable_nonce.as_hash(), + true, // separate_domins + ), + Some(_) + ); } #[test] @@ -3439,9 +3468,14 @@ mod tests { let to_address = Pubkey::new_unique(); let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique(), /*separate_domains:*/ true); - let nonce_state = NonceVersions::new_current(NonceState::Initialized( - nonce::state::Data::new(nonce_authority.pubkey(), durable_nonce, 0), - )); + let nonce_state = NonceVersions::new( + NonceState::Initialized(nonce::state::Data::new( + nonce_authority.pubkey(), + durable_nonce, + 0, + )), + true, // separate_domains + ); let nonce_account_post = AccountSharedData::new_data(43, &nonce_state, &system_program::id()).unwrap(); let from_account_post = AccountSharedData::new(4200, 0, &Pubkey::default()); @@ -3466,9 +3500,14 @@ mod tests { let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique(), /*separate_domains:*/ true); - let nonce_state = NonceVersions::new_current(NonceState::Initialized( - nonce::state::Data::new(nonce_authority.pubkey(), durable_nonce, 0), - )); + let nonce_state = NonceVersions::new( + NonceState::Initialized(nonce::state::Data::new( + nonce_authority.pubkey(), + durable_nonce, + 0, + )), + true, // separate_domains + ); let nonce_account_pre = AccountSharedData::new_data(42, &nonce_state, &system_program::id()).unwrap(); @@ -3512,7 +3551,7 @@ mod tests { &execution_results, loaded.as_mut_slice(), &rent_collector, - &durable_nonce, + &(durable_nonce, /*separate_domains:*/ true), 0, true, // leave_nonce_on_success ); @@ -3527,11 +3566,14 @@ mod tests { collected_nonce_account.lamports(), nonce_account_pre.lamports() ); - assert!(nonce_account::verify_nonce_account( - &collected_nonce_account, - durable_nonce.as_hash(), - ) - .is_some()); + assert_matches!( + nonce_account::verify_nonce_account( + &collected_nonce_account, + durable_nonce.as_hash(), + true, // separate_domins + ), + Some(_) + ); } #[test] diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 2b652dd2131209..a49a9b2688f8a3 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3677,6 +3677,11 @@ impl Bank { self.rc.accounts.accounts_db.set_shrink_paths(paths); } + pub fn separate_nonce_from_blockhash(&self) -> bool { + self.feature_set + .is_active(&feature_set::separate_nonce_from_blockhash::id()) + } + fn check_age<'a>( &self, txs: impl Iterator, @@ -3684,9 +3689,7 @@ impl Bank { max_age: usize, error_counters: &mut ErrorCounters, ) -> Vec { - let separate_nonce_from_blockhash = self - .feature_set - .is_active(&feature_set::separate_nonce_from_blockhash::id()); + let separate_nonce_from_blockhash = self.separate_nonce_from_blockhash(); let enable_durable_nonce = separate_nonce_from_blockhash && self .feature_set @@ -3768,8 +3771,11 @@ impl Bank { let nonce_address = message.get_durable_nonce(self.feature_set.is_active(&nonce_must_be_writable::id()))?; let nonce_account = self.get_account_with_fixed_root(nonce_address)?; - let nonce_data = - nonce_account::verify_nonce_account(&nonce_account, message.recent_blockhash())?; + let nonce_data = nonce_account::verify_nonce_account( + &nonce_account, + message.recent_blockhash(), + self.separate_nonce_from_blockhash(), + )?; if self .feature_set @@ -4567,10 +4573,10 @@ impl Bank { let mut write_time = Measure::start("write_time"); let durable_nonce = { - let separate_nonce_from_blockhash = self - .feature_set - .is_active(&feature_set::separate_nonce_from_blockhash::id()); - DurableNonce::from_blockhash(&last_blockhash, separate_nonce_from_blockhash) + let separate_nonce_from_blockhash = self.separate_nonce_from_blockhash(); + let durable_nonce = + DurableNonce::from_blockhash(&last_blockhash, separate_nonce_from_blockhash); + (durable_nonce, separate_nonce_from_blockhash) }; self.rc.accounts.store_cached( self.slot(), @@ -6985,9 +6991,14 @@ pub(crate) mod tests { DurableNonce::from_blockhash(&Hash::new_unique(), /*separate_domains:*/ true); let nonce_account = AccountSharedData::new_data( 43, - &nonce::state::Versions::new_current(nonce::State::Initialized( - nonce::state::Data::new(Pubkey::default(), durable_nonce, lamports_per_signature), - )), + &nonce::state::Versions::new( + nonce::State::Initialized(nonce::state::Data::new( + Pubkey::default(), + durable_nonce, + lamports_per_signature, + )), + true, // separate_domains + ), &system_program::id(), ) .unwrap(); @@ -9502,9 +9513,10 @@ pub(crate) mod tests { let nonce = Keypair::new(); let nonce_account = AccountSharedData::new_data( min_balance + 42, - &nonce::state::Versions::new_current(nonce::State::Initialized( - nonce::state::Data::default(), - )), + &nonce::state::Versions::new( + nonce::State::Initialized(nonce::state::Data::default()), + true, // separate_domains + ), &system_program::id(), ) .unwrap(); @@ -11671,14 +11683,12 @@ pub(crate) mod tests { } fn get_nonce_blockhash(bank: &Bank, nonce_pubkey: &Pubkey) -> Option { - bank.get_account(nonce_pubkey).and_then(|acc| { - let state = - StateMut::::state(&acc).map(|v| v.convert_to_current()); - match state { - Ok(nonce::State::Initialized(ref data)) => Some(data.blockhash()), - _ => None, - } - }) + let account = bank.get_account(nonce_pubkey)?; + let nonce_versions = StateMut::::state(&account); + match nonce_versions.ok()?.state() { + nonce::State::Initialized(ref data) => Some(data.blockhash()), + _ => None, + } } fn nonce_setup( @@ -11921,9 +11931,10 @@ pub(crate) mod tests { let nonce = Keypair::new(); let nonce_account = AccountSharedData::new_data( 42_424_242, - &nonce::state::Versions::new_current(nonce::State::Initialized( - nonce::state::Data::default(), - )), + &nonce::state::Versions::new( + nonce::State::Initialized(nonce::state::Data::default()), + true, // separate_domains + ), &system_program::id(), ) .unwrap(); @@ -11953,9 +11964,14 @@ pub(crate) mod tests { DurableNonce::from_blockhash(&bank.last_blockhash(), true /* separate domains */); let nonce_account = AccountSharedData::new_data( 42_424_242, - &nonce::state::Versions::new_current(nonce::State::Initialized( - nonce::state::Data::new(nonce_authority, durable_nonce, 5000), - )), + &nonce::state::Versions::new( + nonce::State::Initialized(nonce::state::Data::new( + nonce_authority, + durable_nonce, + 5000, + )), + true, // separate_domains + ), &system_program::id(), ) .unwrap(); @@ -12434,10 +12450,9 @@ pub(crate) mod tests { let (stored_nonce_hash, stored_fee_calculator) = bank .get_account(&nonce_pubkey) .and_then(|acc| { - let state = - StateMut::::state(&acc).map(|v| v.convert_to_current()); - match state { - Ok(nonce::State::Initialized(ref data)) => { + let nonce_versions = StateMut::::state(&acc); + match nonce_versions.ok()?.state() { + nonce::State::Initialized(ref data) => { Some((data.blockhash(), data.fee_calculator)) } _ => None, @@ -12471,10 +12486,9 @@ pub(crate) mod tests { let (nonce_hash, fee_calculator) = bank .get_account(&nonce_pubkey) .and_then(|acc| { - let state = - StateMut::::state(&acc).map(|v| v.convert_to_current()); - match state { - Ok(nonce::State::Initialized(ref data)) => { + let nonce_versions = StateMut::::state(&acc); + match nonce_versions.ok()?.state() { + nonce::State::Initialized(ref data) => { Some((data.blockhash(), data.fee_calculator)) } _ => None, @@ -12504,10 +12518,9 @@ pub(crate) mod tests { let (stored_nonce_hash, stored_fee_calculator) = bank .get_account(&nonce_pubkey) .and_then(|acc| { - let state = - StateMut::::state(&acc).map(|v| v.convert_to_current()); - match state { - Ok(nonce::State::Initialized(ref data)) => { + let nonce_versions = StateMut::::state(&acc); + match nonce_versions.ok()?.state() { + nonce::State::Initialized(ref data) => { Some((data.blockhash(), data.fee_calculator)) } _ => None, @@ -12541,10 +12554,9 @@ pub(crate) mod tests { let (nonce_hash, fee_calculator) = bank .get_account(&nonce_pubkey) .and_then(|acc| { - let state = - StateMut::::state(&acc).map(|v| v.convert_to_current()); - match state { - Ok(nonce::State::Initialized(ref data)) => { + let nonce_versions = StateMut::::state(&acc); + match nonce_versions.ok()?.state() { + nonce::State::Initialized(ref data) => { Some((data.blockhash(), data.fee_calculator)) } _ => None, diff --git a/runtime/src/nonce_keyed_account.rs b/runtime/src/nonce_keyed_account.rs index c73820b0612ce8..fcdca442c6afd5 100644 --- a/runtime/src/nonce_keyed_account.rs +++ b/runtime/src/nonce_keyed_account.rs @@ -16,11 +16,13 @@ use { std::collections::HashSet, }; -fn get_durable_nonce(invoke_context: &InvokeContext) -> DurableNonce { +fn get_durable_nonce(invoke_context: &InvokeContext) -> (DurableNonce, /*separate_domains:*/ bool) { let separate_nonce_from_blockhash = invoke_context .feature_set .is_active(&feature_set::separate_nonce_from_blockhash::id()); - DurableNonce::from_blockhash(&invoke_context.blockhash, separate_nonce_from_blockhash) + let durable_nonce = + DurableNonce::from_blockhash(&invoke_context.blockhash, separate_nonce_from_blockhash); + (durable_nonce, separate_nonce_from_blockhash) } pub fn advance_nonce_account( @@ -46,7 +48,7 @@ pub fn advance_nonce_account( } let state: Versions = account.get_state()?; - match state.convert_to_current() { + match state.state() { State::Initialized(data) => { if !signers.contains(&data.authority) { ic_msg!( @@ -56,7 +58,7 @@ pub fn advance_nonce_account( ); return Err(InstructionError::MissingRequiredSignature); } - let next_durable_nonce = get_durable_nonce(invoke_context); + let (next_durable_nonce, separate_domains) = get_durable_nonce(invoke_context); if data.durable_nonce == next_durable_nonce { ic_msg!( invoke_context, @@ -73,9 +75,12 @@ pub fn advance_nonce_account( next_durable_nonce, invoke_context.lamports_per_signature, ); - account.set_state(&Versions::new_current(State::Initialized(new_data))) + account.set_state(&Versions::new( + State::Initialized(new_data), + separate_domains, + )) } - _ => { + State::Uninitialized => { ic_msg!( invoke_context, "Advance nonce account: Account {} state is invalid", @@ -119,7 +124,7 @@ pub fn withdraw_nonce_account( } let state: Versions = from.get_state()?; - let signer = match state.convert_to_current() { + let signer = match state.state() { State::Uninitialized => { if lamports > from.get_lamports() { ic_msg!( @@ -134,7 +139,8 @@ pub fn withdraw_nonce_account( } State::Initialized(ref data) => { if lamports == from.get_lamports() { - if data.durable_nonce == get_durable_nonce(invoke_context) { + let (durable_nonce, separate_domains) = get_durable_nonce(invoke_context); + if data.durable_nonce == durable_nonce { ic_msg!( invoke_context, "Withdraw nonce account: nonce can only advance once per slot" @@ -144,7 +150,7 @@ pub fn withdraw_nonce_account( merge_nonce_error_into_system_error, )); } - from.set_state(&Versions::new_current(State::Uninitialized))?; + from.set_state(&Versions::new(State::Uninitialized, separate_domains))?; } else { let min_balance = rent.minimum_balance(from.get_data().len()); let amount = checked_add(lamports, min_balance)?; @@ -204,8 +210,7 @@ pub fn initialize_nonce_account( return Err(InstructionError::InvalidArgument); } - let state: Versions = account.get_state()?; - match state.convert_to_current() { + match account.get_state::()?.state() { State::Uninitialized => { let min_balance = rent.minimum_balance(account.get_data().len()); if account.get_lamports() < min_balance { @@ -217,14 +222,16 @@ pub fn initialize_nonce_account( ); return Err(InstructionError::InsufficientFunds); } + let (durable_nonce, separate_domains) = get_durable_nonce(invoke_context); let data = nonce::state::Data::new( *nonce_authority, - get_durable_nonce(invoke_context), + durable_nonce, invoke_context.lamports_per_signature, ); - account.set_state(&Versions::new_current(State::Initialized(data))) + let state = State::Initialized(data); + account.set_state(&Versions::new(state, separate_domains)) } - _ => { + State::Initialized(_) => { ic_msg!( invoke_context, "Initialize nonce account: Account {} state is invalid", @@ -262,7 +269,8 @@ pub fn authorize_nonce_account( } let state: Versions = account.get_state()?; - match state.convert_to_current() { + let separate_domains = state.separate_domains(); + match state.state() { State::Initialized(data) => { if !signers.contains(&data.authority) { ic_msg!( @@ -277,9 +285,12 @@ pub fn authorize_nonce_account( data.durable_nonce, data.get_lamports_per_signature(), ); - account.set_state(&Versions::new_current(State::Initialized(new_data))) + account.set_state(&Versions::new( + State::Initialized(new_data), + separate_domains, + )) } - _ => { + State::Uninitialized => { ic_msg!( invoke_context, "Authorize nonce account: Account {} state is invalid", @@ -297,6 +308,7 @@ pub fn authorize_nonce_account( mod test { use { super::*, + assert_matches::assert_matches, solana_program_runtime::invoke_context::InvokeContext, solana_sdk::{ account::AccountSharedData, @@ -334,9 +346,12 @@ mod test { let accounts = vec![ ( Pubkey::new_unique(), - create_account(from_lamports).into_inner(), + create_account(from_lamports, /*separate_domains:*/ true).into_inner(), + ), + ( + Pubkey::new_unique(), + create_account(42, /*separate_domains:*/ true).into_inner(), ), - (Pubkey::new_unique(), create_account(42).into_inner()), (system_program::id(), AccountSharedData::default()), ]; let $instruction_accounts = vec![ @@ -388,52 +403,40 @@ mod test { }; let mut signers = HashSet::new(); signers.insert(*nonce_account.get_key()); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); + let versions = nonce_account.get_state::().unwrap(); // New is in Uninitialzed state - assert_eq!(state, State::Uninitialized); + assert_eq!(versions.state(), &State::Uninitialized); set_invoke_context_blockhash!(invoke_context, 95); let authorized = *nonce_account.get_key(); initialize_nonce_account(&mut nonce_account, &authorized, &rent, &invoke_context).unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); + let versions = nonce_account.get_state::().unwrap(); let data = nonce::state::Data::new( data.authority, - get_durable_nonce(&invoke_context), + get_durable_nonce(&invoke_context).0, invoke_context.lamports_per_signature, ); // First nonce instruction drives state from Uninitialized to Initialized - assert_eq!(state, State::Initialized(data.clone())); + assert_eq!(versions.state(), &State::Initialized(data.clone())); set_invoke_context_blockhash!(invoke_context, 63); advance_nonce_account(&mut nonce_account, &signers, &invoke_context).unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); + let versions = nonce_account.get_state::().unwrap(); let data = nonce::state::Data::new( data.authority, - get_durable_nonce(&invoke_context), + get_durable_nonce(&invoke_context).0, invoke_context.lamports_per_signature, ); // Second nonce instruction consumes and replaces stored nonce - assert_eq!(state, State::Initialized(data.clone())); + assert_eq!(versions.state(), &State::Initialized(data.clone())); set_invoke_context_blockhash!(invoke_context, 31); advance_nonce_account(&mut nonce_account, &signers, &invoke_context).unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); + let versions = nonce_account.get_state::().unwrap(); let data = nonce::state::Data::new( data.authority, - get_durable_nonce(&invoke_context), + get_durable_nonce(&invoke_context).0, invoke_context.lamports_per_signature, ); // Third nonce instruction for fun and profit - assert_eq!(state, State::Initialized(data)); + assert_eq!(versions.state(), &State::Initialized(data)); set_invoke_context_blockhash!(invoke_context, 0); let to_account = instruction_context @@ -465,12 +468,9 @@ mod test { assert_eq!(nonce_account.get_lamports(), expect_nonce_lamports); // Account balance goes to `to` assert_eq!(to_account.get_lamports(), expect_to_lamports); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); + let versions = nonce_account.get_state::().unwrap(); // Empty balance deinitializes data - assert_eq!(state, State::Uninitialized); + assert_eq!(versions.state(), &State::Uninitialized); } #[test] @@ -488,16 +488,13 @@ mod test { set_invoke_context_blockhash!(invoke_context, 31); let authority = *nonce_account.get_key(); initialize_nonce_account(&mut nonce_account, &authority, &rent, &invoke_context).unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); + let versions = nonce_account.get_state::().unwrap(); let data = nonce::state::Data::new( authority, - get_durable_nonce(&invoke_context), + get_durable_nonce(&invoke_context).0, invoke_context.lamports_per_signature, ); - assert_eq!(state, State::Initialized(data)); + assert_eq!(versions.state(), &State::Initialized(data)); // Nonce account did not sign let signers = HashSet::new(); set_invoke_context_blockhash!(invoke_context, 0); @@ -611,11 +608,8 @@ mod test { let to_account = instruction_context .try_borrow_instruction_account(transaction_context, WITHDRAW_TO_ACCOUNT_INDEX) .unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = nonce_account.get_state::().unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); let mut signers = HashSet::new(); signers.insert(*nonce_account.get_key()); set_invoke_context_blockhash!(invoke_context, 0); @@ -641,11 +635,8 @@ mod test { let to_account = instruction_context .try_borrow_instruction_account(transaction_context, WITHDRAW_TO_ACCOUNT_INDEX) .unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = nonce_account.get_state::().unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); assert_eq!(nonce_account.get_lamports(), expect_from_lamports); assert_eq!(to_account.get_lamports(), expect_to_lamports); } @@ -665,11 +656,8 @@ mod test { let to_account = instruction_context .try_borrow_instruction_account(transaction_context, WITHDRAW_TO_ACCOUNT_INDEX) .unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = nonce_account.get_state::().unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); let signers = HashSet::new(); set_invoke_context_blockhash!(invoke_context, 0); let withdraw_lamports = nonce_account.get_lamports(); @@ -700,11 +688,8 @@ mod test { let nonce_account = instruction_context .try_borrow_instruction_account(transaction_context, NONCE_ACCOUNT_INDEX) .unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = nonce_account.get_state::().unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); let mut signers = HashSet::new(); signers.insert(*nonce_account.get_key()); set_invoke_context_blockhash!(invoke_context, 0); @@ -763,11 +748,8 @@ mod test { let to_account = instruction_context .try_borrow_instruction_account(transaction_context, WITHDRAW_TO_ACCOUNT_INDEX) .unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = nonce_account.get_state::().unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); assert_eq!(nonce_account.get_lamports(), from_expect_lamports); assert_eq!(to_account.get_lamports(), to_expect_lamports); let withdraw_lamports = nonce_account.get_lamports(); @@ -792,11 +774,8 @@ mod test { let to_account = instruction_context .try_borrow_instruction_account(transaction_context, WITHDRAW_TO_ACCOUNT_INDEX) .unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = nonce_account.get_state::().unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); assert_eq!(nonce_account.get_lamports(), from_expect_lamports); assert_eq!(to_account.get_lamports(), to_expect_lamports); } @@ -821,16 +800,13 @@ mod test { set_invoke_context_blockhash!(invoke_context, 31); let authority = *nonce_account.get_key(); initialize_nonce_account(&mut nonce_account, &authority, &rent, &invoke_context).unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); + let versions = nonce_account.get_state::().unwrap(); let data = nonce::state::Data::new( authority, - get_durable_nonce(&invoke_context), + get_durable_nonce(&invoke_context).0, invoke_context.lamports_per_signature, ); - assert_eq!(state, State::Initialized(data.clone())); + assert_eq!(versions.state(), &State::Initialized(data.clone())); let withdraw_lamports = 42; let from_expect_lamports = nonce_account.get_lamports() - withdraw_lamports; let to_expect_lamports = to_account.get_lamports() + withdraw_lamports; @@ -853,16 +829,13 @@ mod test { let to_account = instruction_context .try_borrow_instruction_account(transaction_context, WITHDRAW_TO_ACCOUNT_INDEX) .unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); + let versions = nonce_account.get_state::().unwrap(); let data = nonce::state::Data::new( data.authority, - get_durable_nonce(&invoke_context), + get_durable_nonce(&invoke_context).0, invoke_context.lamports_per_signature, ); - assert_eq!(state, State::Initialized(data)); + assert_eq!(versions.state(), &State::Initialized(data)); assert_eq!(nonce_account.get_lamports(), from_expect_lamports); assert_eq!(to_account.get_lamports(), to_expect_lamports); set_invoke_context_blockhash!(invoke_context, 0); @@ -888,11 +861,8 @@ mod test { let to_account = instruction_context .try_borrow_instruction_account(transaction_context, WITHDRAW_TO_ACCOUNT_INDEX) .unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = nonce_account.get_state::().unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); assert_eq!(nonce_account.get_lamports(), from_expect_lamports); assert_eq!(to_account.get_lamports(), to_expect_lamports); } @@ -1044,11 +1014,8 @@ mod test { let mut nonce_account = instruction_context .try_borrow_instruction_account(transaction_context, NONCE_ACCOUNT_INDEX) .unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = nonce_account.get_state::().unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); let mut signers = HashSet::new(); signers.insert(*nonce_account.get_key()); set_invoke_context_blockhash!(invoke_context, 0); @@ -1057,15 +1024,12 @@ mod test { initialize_nonce_account(&mut nonce_account, &authorized, &rent, &invoke_context); let data = nonce::state::Data::new( authorized, - get_durable_nonce(&invoke_context), + get_durable_nonce(&invoke_context).0, invoke_context.lamports_per_signature, ); assert_eq!(result, Ok(())); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Initialized(data)); + let versions = nonce_account.get_state::().unwrap(); + assert_eq!(versions.state(), &State::Initialized(data)); } #[test] @@ -1129,15 +1093,12 @@ mod test { let authority = Pubkey::default(); let data = nonce::state::Data::new( authority, - get_durable_nonce(&invoke_context), + get_durable_nonce(&invoke_context).0, invoke_context.lamports_per_signature, ); authorize_nonce_account(&mut nonce_account, &authority, &signers, &invoke_context).unwrap(); - let state = nonce_account - .get_state::() - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Initialized(data)); + let versions = nonce_account.get_state::().unwrap(); + assert_eq!(versions.state(), &State::Initialized(data)); } #[test] @@ -1199,21 +1160,24 @@ mod test { .unwrap(); let mut signers = HashSet::new(); signers.insert(nonce_account.get_key()); - let state: State = nonce_account.get_state().unwrap(); + let versions: Versions = nonce_account.get_state().unwrap(); // New is in Uninitialzed state - assert_eq!(state, State::Uninitialized); + assert_eq!(versions.state(), &State::Uninitialized); set_invoke_context_blockhash!(invoke_context, 0); let authorized = *nonce_account.get_key(); initialize_nonce_account(&mut nonce_account, &authorized, &rent, &invoke_context).unwrap(); drop(nonce_account); - assert!(verify_nonce_account( - &transaction_context - .get_account_at_index(NONCE_ACCOUNT_INDEX) - .unwrap() - .borrow(), - get_durable_nonce(&invoke_context).as_hash(), - ) - .is_some()); + assert_matches!( + verify_nonce_account( + &transaction_context + .get_account_at_index(NONCE_ACCOUNT_INDEX) + .unwrap() + .borrow(), + get_durable_nonce(&invoke_context).0.as_hash(), + true, // separate_domins + ), + Some(_) + ); } #[test] @@ -1225,14 +1189,17 @@ mod test { _instruction_context, instruction_accounts ); - assert!(verify_nonce_account( - &transaction_context - .get_account_at_index(NONCE_ACCOUNT_INDEX) - .unwrap() - .borrow(), - &Hash::default() - ) - .is_none()); + assert_eq!( + verify_nonce_account( + &transaction_context + .get_account_at_index(NONCE_ACCOUNT_INDEX) + .unwrap() + .borrow(), + &Hash::default(), + true, // separate_domins + ), + None + ); } #[test] @@ -1249,9 +1216,9 @@ mod test { .unwrap(); let mut signers = HashSet::new(); signers.insert(nonce_account.get_key()); - let state: State = nonce_account.get_state().unwrap(); + let versions: Versions = nonce_account.get_state().unwrap(); // New is in Uninitialzed state - assert_eq!(state, State::Uninitialized); + assert_eq!(versions.state(), &State::Uninitialized); set_invoke_context_blockhash!(invoke_context, 0); let authorized = *nonce_account.get_key(); initialize_nonce_account( @@ -1263,13 +1230,16 @@ mod test { .unwrap(); set_invoke_context_blockhash!(invoke_context, 1); drop(nonce_account); - assert!(verify_nonce_account( - &transaction_context - .get_account_at_index(NONCE_ACCOUNT_INDEX) - .unwrap() - .borrow(), - get_durable_nonce(&invoke_context).as_hash(), - ) - .is_none()); + assert_eq!( + verify_nonce_account( + &transaction_context + .get_account_at_index(NONCE_ACCOUNT_INDEX) + .unwrap() + .borrow(), + get_durable_nonce(&invoke_context).0.as_hash(), + true, // separate_domins + ), + None + ); } } diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index ca87c715a8ed9e..905d5595eca0b2 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -477,11 +477,10 @@ pub fn get_system_account_kind(account: &AccountSharedData) -> Option match *state { - nonce::State::Initialized(_) => Some(SystemAccountKind::Nonce), - _ => None, - }, + let nonce_versions: nonce::state::Versions = account.state().ok()?; + match nonce_versions.state() { + nonce::State::Uninitialized => None, + nonce::State::Initialized(_) => Some(SystemAccountKind::Nonce), } } else { None @@ -1118,9 +1117,10 @@ mod tests { let nonce = Pubkey::new_unique(); let nonce_account = AccountSharedData::new_data( 42, - &nonce::state::Versions::new_current(nonce::State::Initialized( - nonce::state::Data::default(), - )), + &nonce::state::Versions::new( + nonce::State::Initialized(nonce::state::Data::default()), + true, // separate_domains + ), &system_program::id(), ) .unwrap(); @@ -1409,10 +1409,13 @@ mod tests { let from = Pubkey::new_unique(); let from_account = AccountSharedData::new_data( 100, - &nonce::state::Versions::new_current(nonce::State::Initialized(nonce::state::Data { - authority: from, - ..nonce::state::Data::default() - })), + &nonce::state::Versions::new( + nonce::State::Initialized(nonce::state::Data { + authority: from, + ..nonce::state::Data::default() + }), + true, // separate_domains + ), &system_program::id(), ) .unwrap(); @@ -1691,7 +1694,8 @@ mod tests { #[test] fn test_process_nonce_ix_ok() { let nonce_address = Pubkey::new_unique(); - let nonce_account = nonce_account::create_account(1_000_000).into_inner(); + let nonce_account = + nonce_account::create_account(1_000_000, /*separate_domains:*/ true).into_inner(); #[allow(deprecated)] let blockhash_id = sysvar::recent_blockhashes::id(); let accounts = process_instruction( @@ -1798,7 +1802,8 @@ mod tests { #[test] fn test_process_withdraw_ix_ok() { let nonce_address = Pubkey::new_unique(); - let nonce_account = nonce_account::create_account(1_000_000).into_inner(); + let nonce_account = + nonce_account::create_account(1_000_000, /*separate_domains:*/ true).into_inner(); let pubkey = Pubkey::new_unique(); #[allow(deprecated)] let blockhash_id = sysvar::recent_blockhashes::id(); @@ -1851,7 +1856,8 @@ mod tests { #[test] fn test_process_initialize_ix_only_nonce_acc_fail() { let nonce_address = Pubkey::new_unique(); - let nonce_account = nonce_account::create_account(1_000_000).into_inner(); + let nonce_account = + nonce_account::create_account(1_000_000, /*separate_domains:*/ true).into_inner(); process_instruction( &serialize(&SystemInstruction::InitializeNonceAccount(nonce_address)).unwrap(), vec![(nonce_address, nonce_account)], @@ -1868,7 +1874,8 @@ mod tests { #[test] fn test_process_initialize_ix_ok() { let nonce_address = Pubkey::new_unique(); - let nonce_account = nonce_account::create_account(1_000_000).into_inner(); + let nonce_account = + nonce_account::create_account(1_000_000, /*separate_domains:*/ true).into_inner(); #[allow(deprecated)] let blockhash_id = sysvar::recent_blockhashes::id(); process_instruction( @@ -1903,7 +1910,8 @@ mod tests { #[test] fn test_process_authorize_ix_ok() { let nonce_address = Pubkey::new_unique(); - let nonce_account = nonce_account::create_account(1_000_000).into_inner(); + let nonce_account = + nonce_account::create_account(1_000_000, /*separate_domains:*/ true).into_inner(); #[allow(deprecated)] let blockhash_id = sysvar::recent_blockhashes::id(); let accounts = process_instruction( @@ -1972,9 +1980,10 @@ mod tests { fn test_get_system_account_kind_nonce_ok() { let nonce_account = AccountSharedData::new_data( 42, - &nonce::state::Versions::new_current(nonce::State::Initialized( - nonce::state::Data::default(), - )), + &nonce::state::Versions::new( + nonce::State::Initialized(nonce::state::Data::default()), + true, // separate_domains + ), &system_program::id(), ) .unwrap(); @@ -1987,7 +1996,9 @@ mod tests { #[test] fn test_get_system_account_kind_uninitialized_nonce_account_fail() { assert_eq!( - get_system_account_kind(&nonce_account::create_account(42).borrow()), + get_system_account_kind( + &nonce_account::create_account(42, /*separate_domains:*/ true).borrow() + ), None ); } @@ -2003,9 +2014,10 @@ mod tests { fn test_get_system_account_kind_nonsystem_owner_with_nonce_data_fail() { let nonce_account = AccountSharedData::new_data( 42, - &nonce::state::Versions::new_current(nonce::State::Initialized( - nonce::state::Data::default(), - )), + &nonce::state::Versions::new( + nonce::State::Initialized(nonce::state::Data::default()), + true, // separate_domains + ), &Pubkey::new_unique(), ) .unwrap(); @@ -2015,7 +2027,8 @@ mod tests { #[test] fn test_nonce_initialize_with_empty_recent_blockhashes_fail() { let nonce_address = Pubkey::new_unique(); - let nonce_account = nonce_account::create_account(1_000_000).into_inner(); + let nonce_account = + nonce_account::create_account(1_000_000, /*separate_domains:*/ true).into_inner(); #[allow(deprecated)] let blockhash_id = sysvar::recent_blockhashes::id(); #[allow(deprecated)] @@ -2055,7 +2068,8 @@ mod tests { #[test] fn test_nonce_advance_with_empty_recent_blockhashes_fail() { let nonce_address = Pubkey::new_unique(); - let nonce_account = nonce_account::create_account(1_000_000).into_inner(); + let nonce_account = + nonce_account::create_account(1_000_000, /*separate_domains:*/ true).into_inner(); #[allow(deprecated)] let blockhash_id = sysvar::recent_blockhashes::id(); let accounts = process_instruction( diff --git a/sdk/program/src/nonce/state/current.rs b/sdk/program/src/nonce/state/current.rs index a8034c4d2396bf..003ec82ea5fd0d 100644 --- a/sdk/program/src/nonce/state/current.rs +++ b/sdk/program/src/nonce/state/current.rs @@ -109,4 +109,14 @@ mod test { fn default_is_uninitialized() { assert_eq!(State::default(), State::Uninitialized) } + + #[test] + fn test_nonce_state_size() { + let data = Versions::new( + State::Initialized(Data::default()), + true, // separate_domains + ); + let size = bincode::serialized_size(&data).unwrap(); + assert_eq!(State::size() as u64, size); + } } diff --git a/sdk/program/src/nonce/state/mod.rs b/sdk/program/src/nonce/state/mod.rs index 859dfe7d32686a..0c2813b72e29f1 100644 --- a/sdk/program/src/nonce/state/mod.rs +++ b/sdk/program/src/nonce/state/mod.rs @@ -2,21 +2,178 @@ mod current; pub use current::{Data, DurableNonce, State}; -use serde_derive::{Deserialize, Serialize}; +use { + crate::hash::Hash, + serde_derive::{Deserialize, Serialize}, +}; #[derive(Debug, Serialize, Deserialize, PartialEq, Clone)] pub enum Versions { + Legacy(Box), + /// Current variants have durable nonce and blockhash domains separated. Current(Box), } impl Versions { - pub fn new_current(state: State) -> Self { - Self::Current(Box::new(state)) + pub fn new(state: State, separate_domains: bool) -> Self { + if separate_domains { + Self::Current(Box::new(state)) + } else { + Self::Legacy(Box::new(state)) + } + } + + pub fn state(&self) -> &State { + match self { + Self::Legacy(state) => state, + Self::Current(state) => state, + } } - pub fn convert_to_current(self) -> State { + /// Returns true if the durable nonce is not in the blockhash domain. + pub fn separate_domains(&self) -> bool { match self { - Self::Current(state) => *state, + Self::Legacy(_) => false, + Self::Current(_) => true, + } + } + + /// Checks if the recent_blockhash field in Transaction verifies, and + /// returns nonce account data if so. + pub fn verify_recent_blockhash( + &self, + recent_blockhash: &Hash, // Transaction.message.recent_blockhash + separate_domains: bool, + ) -> Option<&Data> { + let state = match self { + Self::Legacy(state) => { + if separate_domains { + // Legacy durable nonces are invalid and should not + // allow durable transactions. + return None; + } else { + state + } + } + Self::Current(state) => state, + }; + match **state { + State::Uninitialized => None, + State::Initialized(ref data) => (recent_blockhash == &data.blockhash()).then(|| data), + } + } +} + +impl From for State { + fn from(versions: Versions) -> Self { + match versions { + Versions::Legacy(state) => *state, + Versions::Current(state) => *state, + } + } +} + +#[cfg(test)] +mod tests { + use { + super::*, + crate::{fee_calculator::FeeCalculator, pubkey::Pubkey}, + }; + + #[test] + fn test_verify_recent_blockhash() { + let blockhash = Hash::from([171; 32]); + let versions = Versions::Legacy(Box::new(State::Uninitialized)); + for separate_domains in [false, true] { + assert_eq!( + versions.verify_recent_blockhash(&blockhash, separate_domains), + None + ); + assert_eq!( + versions.verify_recent_blockhash(&Hash::default(), separate_domains), + None + ); + } + let versions = Versions::Current(Box::new(State::Uninitialized)); + for separate_domains in [false, true] { + assert_eq!( + versions.verify_recent_blockhash(&blockhash, separate_domains), + None + ); + assert_eq!( + versions.verify_recent_blockhash(&Hash::default(), separate_domains), + None + ); + } + 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 separate_domains = false; + assert_eq!( + versions.verify_recent_blockhash(&Hash::default(), separate_domains), + None + ); + assert_eq!( + versions.verify_recent_blockhash(&blockhash, separate_domains), + Some(&data) + ); + assert_eq!( + versions.verify_recent_blockhash(&data.blockhash(), separate_domains), + Some(&data) + ); + assert_eq!( + versions.verify_recent_blockhash(durable_nonce.as_hash(), separate_domains), + Some(&data) + ); + let separate_domains = true; + assert_eq!( + versions.verify_recent_blockhash(&Hash::default(), separate_domains), + None + ); + assert_eq!( + versions.verify_recent_blockhash(&blockhash, separate_domains), + None + ); + assert_eq!( + versions.verify_recent_blockhash(&data.blockhash(), separate_domains), + None + ); + assert_eq!( + versions.verify_recent_blockhash(durable_nonce.as_hash(), separate_domains), + None + ); + 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::Current(Box::new(State::Initialized(data.clone()))); + for separate_domains in [false, true] { + assert_eq!( + versions.verify_recent_blockhash(&blockhash, separate_domains), + None + ); + assert_eq!( + versions.verify_recent_blockhash(&Hash::default(), separate_domains), + None + ); + assert_eq!( + versions.verify_recent_blockhash(&data.blockhash(), separate_domains), + Some(&data) + ); + assert_eq!( + versions.verify_recent_blockhash(durable_nonce.as_hash(), separate_domains), + Some(&data) + ); } } } diff --git a/sdk/src/nonce_account.rs b/sdk/src/nonce_account.rs index bce8c8604821cd..4b64b6d15daa0c 100644 --- a/sdk/src/nonce_account.rs +++ b/sdk/src/nonce_account.rs @@ -11,11 +11,11 @@ use { std::cell::RefCell, }; -pub fn create_account(lamports: u64) -> RefCell { +pub fn create_account(lamports: u64, separate_domains: bool) -> RefCell { RefCell::new( AccountSharedData::new_data_with_space( lamports, - &Versions::new_current(State::Uninitialized), + &Versions::new(State::Uninitialized, separate_domains), State::size(), &crate::system_program::id(), ) @@ -23,30 +23,41 @@ pub fn create_account(lamports: u64) -> RefCell { ) } -// TODO: Consider changing argument from Hash to DurableNonce. -pub fn verify_nonce_account(acc: &AccountSharedData, hash: &Hash) -> Option { - if acc.owner() != &crate::system_program::id() { - return None; - } - match StateMut::::state(acc).map(|v| v.convert_to_current()) { - Ok(State::Initialized(data)) => (hash == &data.blockhash()).then(|| data), - _ => None, - } +/// Checks if the recent_blockhash field in Transaction verifies, and returns +/// nonce account data if so. +pub fn verify_nonce_account( + account: &AccountSharedData, + recent_blockhash: &Hash, // Transaction.message.recent_blockhash + separate_domains: bool, +) -> Option { + (account.owner() == &crate::system_program::id()) + .then(|| { + StateMut::::state(account) + .ok()? + .verify_recent_blockhash(recent_blockhash, separate_domains) + .cloned() + }) + .flatten() } pub fn lamports_per_signature_of(account: &AccountSharedData) -> Option { - let state = StateMut::::state(account) - .ok()? - .convert_to_current(); - match state { + match StateMut::::state(account).ok()?.state() { State::Initialized(data) => Some(data.fee_calculator.lamports_per_signature), - _ => None, + State::Uninitialized => None, } } #[cfg(test)] mod tests { - use {super::*, crate::pubkey::Pubkey}; + use { + super::*, + crate::{ + fee_calculator::FeeCalculator, + nonce::state::{Data, DurableNonce}, + pubkey::Pubkey, + system_program, + }, + }; #[test] fn test_verify_bad_account_owner_fails() { @@ -54,11 +65,126 @@ mod tests { assert_ne!(program_id, crate::system_program::id()); let account = AccountSharedData::new_data_with_space( 42, - &Versions::new_current(State::Uninitialized), + &Versions::new(State::Uninitialized, /*separate_domains:*/ true), State::size(), &program_id, ) .expect("nonce_account"); - assert!(verify_nonce_account(&account, &Hash::default()).is_none()); + for separate_domains in [false, true] { + assert_eq!( + verify_nonce_account(&account, &Hash::default(), separate_domains), + None + ); + } + } + + fn new_nonce_account(versions: Versions) -> AccountSharedData { + AccountSharedData::new_data( + 1_000_000, // lamports + &versions, // state + &system_program::id(), // owner + ) + .unwrap() + } + + #[test] + fn test_verify_nonce_account() { + let blockhash = Hash::from([171; 32]); + let versions = Versions::Legacy(Box::new(State::Uninitialized)); + let account = new_nonce_account(versions); + for separate_domains in [false, true] { + assert_eq!( + verify_nonce_account(&account, &blockhash, separate_domains), + None + ); + assert_eq!( + verify_nonce_account(&account, &Hash::default(), separate_domains), + None + ); + } + let versions = Versions::Current(Box::new(State::Uninitialized)); + let account = new_nonce_account(versions); + for separate_domains in [false, true] { + assert_eq!( + verify_nonce_account(&account, &blockhash, separate_domains), + None + ); + assert_eq!( + verify_nonce_account(&account, &Hash::default(), separate_domains), + None + ); + } + 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 account = new_nonce_account(versions); + let separate_domains = false; + assert_eq!( + verify_nonce_account(&account, &blockhash, separate_domains), + Some(data.clone()) + ); + assert_eq!( + verify_nonce_account(&account, &Hash::default(), separate_domains), + None + ); + assert_eq!( + verify_nonce_account(&account, &data.blockhash(), separate_domains), + Some(data.clone()) + ); + assert_eq!( + verify_nonce_account(&account, durable_nonce.as_hash(), separate_domains), + Some(data.clone()) + ); + let separate_domains = true; + assert_eq!( + verify_nonce_account(&account, &blockhash, separate_domains), + None + ); + assert_eq!( + verify_nonce_account(&account, &Hash::default(), separate_domains), + None + ); + assert_eq!( + verify_nonce_account(&account, &data.blockhash(), separate_domains), + None + ); + assert_eq!( + verify_nonce_account(&account, durable_nonce.as_hash(), separate_domains), + None + ); + 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::Current(Box::new(State::Initialized(data.clone()))); + let account = new_nonce_account(versions); + for separate_domains in [false, true] { + assert_eq!( + verify_nonce_account(&account, &blockhash, separate_domains), + None + ); + assert_eq!( + verify_nonce_account(&account, &Hash::default(), separate_domains), + None + ); + assert_eq!( + verify_nonce_account(&account, &data.blockhash(), separate_domains), + Some(data.clone()) + ); + assert_eq!( + verify_nonce_account(&account, durable_nonce.as_hash(), separate_domains), + Some(data.clone()) + ); + } } } diff --git a/send-transaction-service/src/send_transaction_service.rs b/send-transaction-service/src/send_transaction_service.rs index f41a171c5ac3fa..67222a33d21a0f 100644 --- a/send-transaction-service/src/send_transaction_service.rs +++ b/send-transaction-service/src/send_transaction_service.rs @@ -398,10 +398,12 @@ impl SendTransactionService { .last_sent_time .map(|last| now.duration_since(last) >= retry_rate) .unwrap_or(false); - if nonce_account::verify_nonce_account(&nonce_account, &durable_nonce).is_none() - && signature_status.is_none() - && expired - { + let verify_nonce_account = nonce_account::verify_nonce_account( + &nonce_account, + &durable_nonce, + working_bank.separate_nonce_from_blockhash(), + ); + if verify_nonce_account.is_none() && signature_status.is_none() && expired { info!("Dropping expired durable-nonce transaction: {}", signature); result.expired += 1; inc_new_counter_info!("send_transaction_service-expired", 1); @@ -864,9 +866,14 @@ mod test { let nonce_address = Pubkey::new_unique(); let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique(), /*separate_domains:*/ true); - let nonce_state = nonce::state::Versions::new_current(nonce::State::Initialized( - nonce::state::Data::new(Pubkey::default(), durable_nonce, 42), - )); + let nonce_state = nonce::state::Versions::new( + nonce::State::Initialized(nonce::state::Data::new( + Pubkey::default(), + durable_nonce, + 42, + )), + true, // separate_domains + ); let nonce_account = AccountSharedData::new_data(43, &nonce_state, &system_program::id()).unwrap(); root_bank.store_account(&nonce_address, &nonce_account); @@ -1099,9 +1106,14 @@ mod test { } let new_durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique(), /*separate_domains:*/ true); - let new_nonce_state = nonce::state::Versions::new_current(nonce::State::Initialized( - nonce::state::Data::new(Pubkey::default(), new_durable_nonce, 42), - )); + let new_nonce_state = nonce::state::Versions::new( + nonce::State::Initialized(nonce::state::Data::new( + Pubkey::default(), + new_durable_nonce, + 42, + )), + true, // separate_domains + ); let nonce_account = AccountSharedData::new_data(43, &new_nonce_state, &system_program::id()).unwrap(); working_bank.store_account(&nonce_address, &nonce_account);