From 23da4af302ca59b94a219eed0efd318cf7f402ce Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 9 Jun 2022 20:32:08 +0000 Subject: [PATCH] permanently disables durable nonces with chain blockhash domain (backport #25788) (#25871) * 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. (cherry picked from commit 3c1ce3cc939cbc92bc8cfaac9ea5203e5e954682) # Conflicts: # cli/src/nonce.rs # rpc/src/rpc.rs # runtime/src/nonce_keyed_account.rs # runtime/src/system_instruction_processor.rs # sdk/program/src/nonce/state/current.rs # send-transaction-service/src/send_transaction_service.rs * removes mergify merge conflicts Co-authored-by: behzad nouri --- 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 | 2 +- rpc/src/transaction_status_service.rs | 6 +- runtime/src/accounts.rs | 152 +++++++----- runtime/src/bank.rs | 102 ++++---- runtime/src/nonce_keyed_account.rs | 217 +++++++++--------- runtime/src/system_instruction_processor.rs | 85 ++++--- sdk/program/src/nonce/state/current.rs | 5 +- sdk/program/src/nonce/state/mod.rs | 167 +++++++++++++- sdk/src/nonce_account.rs | 165 +++++++++++-- .../src/send_transaction_service.rs | 29 ++- 15 files changed, 703 insertions(+), 308 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 5baf452058cbcc..165ac8b7bbaee3 100644 --- a/cli/src/nonce.rs +++ b/cli/src/nonce.rs @@ -925,11 +925,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()); @@ -949,11 +948,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); if let CliError::InvalidNonce(err) = check_nonce_account(&invalid_hash.unwrap(), &nonce_pubkey, &blockhash).unwrap_err() @@ -961,11 +963,14 @@ mod tests { assert_eq!(err, Error::InvalidHash,); } - let data = Versions::new_current(State::Initialized(nonce::state::Data::new( - solana_sdk::pubkey::new_rand(), - durable_nonce, - 0, - ))); + let data = Versions::new( + State::Initialized(nonce::state::Data::new( + solana_sdk::pubkey::new_rand(), + 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() @@ -973,7 +978,7 @@ mod tests { assert_eq!(err, Error::InvalidAuthority,); } - 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() @@ -984,7 +989,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()); @@ -1003,14 +1009,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), @@ -1026,7 +1036,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), @@ -1041,7 +1052,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 de2cf412864494..f7b828ceacf3be 100644 --- a/client/src/nonce_utils.rs +++ b/client/src/nonce_utils.rs @@ -65,9 +65,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)) } pub fn data_from_account>( diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index b302f61710a30f..b6188be876dfff 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -5528,7 +5528,7 @@ pub mod tests { let json: Value = serde_json::from_str(&res.unwrap()).unwrap(); let accounts: Vec = serde_json::from_value(json["result"].clone()) .expect("actual response deserialization"); - assert_eq!(accounts.len(), 0); + assert_eq!(accounts.len(), 2); // Test dataSize filter let req = format!( diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 162338f05ee38d..019b2f9af95cf7 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -306,13 +306,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 83b939294dcc46..bd87382b18a9af 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1117,7 +1117,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, rent_for_sysvars: bool, leave_nonce_on_success: bool, @@ -1154,7 +1154,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, rent_for_sysvars: bool, leave_nonce_on_success: bool, @@ -1242,7 +1242,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 { @@ -1261,17 +1261,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 { @@ -1324,6 +1322,7 @@ mod tests { bank::{DurableNonceFee, TransactionExecutionDetails}, rent_collector::RentCollector, }, + assert_matches::assert_matches, solana_address_lookup_table_program::state::LookupTableMeta, solana_program_runtime::invoke_context::Executors, solana_sdk::{ @@ -1640,7 +1639,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(), @@ -2948,7 +2950,7 @@ mod tests { &execution_results, loaded.as_mut_slice(), &rent_collector, - &DurableNonce::default(), + &(DurableNonce::default(), /*separate_domains:*/ true), 0, true, true, // leave_nonce_on_success @@ -3095,20 +3097,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, ) @@ -3120,7 +3126,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 { @@ -3166,9 +3172,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( @@ -3252,9 +3263,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( @@ -3294,7 +3310,7 @@ mod tests { )), false, Some((&nonce, true)), - &DurableNonce::default(), + &(DurableNonce::default(), /*separate_domains:*/ true), 1, &post_fee_payer_account.clone(), )); @@ -3305,7 +3321,7 @@ mod tests { &Ok(()), true, Some((&nonce, true)), - &DurableNonce::default(), + &(DurableNonce::default(), /*separate_domains:*/ true), 1, &post_fee_payer_account.clone(), )); @@ -3319,7 +3335,7 @@ mod tests { )), true, None, - &DurableNonce::default(), + &(DurableNonce::default(), /*separate_domains:*/ true), 1, &post_fee_payer_account.clone(), )); @@ -3333,7 +3349,7 @@ mod tests { )), true, Some((&nonce, true)), - &DurableNonce::default(), + &(DurableNonce::default(), /*separate_domains:*/ true), 1, &pre_fee_payer_account, )); @@ -3350,9 +3366,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()); @@ -3377,9 +3398,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()); @@ -3424,7 +3450,7 @@ mod tests { &execution_results, loaded.as_mut_slice(), &rent_collector, - &durable_nonce, + &(durable_nonce, /*separate_domains:*/ true), 0, true, true, // leave_nonce_on_success @@ -3449,11 +3475,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] @@ -3467,9 +3496,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()); @@ -3494,9 +3528,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(); @@ -3540,7 +3579,7 @@ mod tests { &execution_results, loaded.as_mut_slice(), &rent_collector, - &durable_nonce, + &(durable_nonce, /*separate_domains:*/ true), 0, true, true, // leave_nonce_on_success @@ -3556,11 +3595,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 3d8be61e436099..5d9054fc28b747 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3663,6 +3663,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, @@ -3670,9 +3675,7 @@ impl Bank { max_age: usize, error_counters: &mut TransactionErrorMetrics, ) -> 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 @@ -3757,8 +3760,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 @@ -4520,10 +4526,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(), @@ -6941,9 +6947,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(); @@ -9467,9 +9478,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(); @@ -11638,14 +11650,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( @@ -11888,9 +11898,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(); @@ -11920,9 +11931,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(); @@ -12401,10 +12417,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, @@ -12438,10 +12453,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, @@ -12471,10 +12485,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, @@ -12508,10 +12521,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 00477ebce63ab4..2f49e541e4b30f 100644 --- a/runtime/src/nonce_keyed_account.rs +++ b/runtime/src/nonce_keyed_account.rs @@ -46,11 +46,13 @@ pub trait NonceKeyedAccount { ) -> Result<(), InstructionError>; } -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) } impl<'a> NonceKeyedAccount for KeyedAccount<'a> { @@ -76,8 +78,8 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { return Err(InstructionError::InvalidArgument); } - let state = AccountUtilsState::::state(self)?.convert_to_current(); - match state { + let state = AccountUtilsState::::state(self)?; + match state.state() { State::Initialized(data) => { if !signers.contains(&data.authority) { ic_msg!( @@ -87,7 +89,7 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { ); 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, @@ -104,9 +106,12 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { next_durable_nonce, invoke_context.lamports_per_signature, ); - self.set_state(&Versions::new_current(State::Initialized(new_data))) + self.set_state(&Versions::new( + State::Initialized(new_data), + separate_domains, + )) } - _ => { + State::Uninitialized => { ic_msg!( invoke_context, "Advance nonce account: Account {} state is invalid", @@ -145,7 +150,7 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { return Err(InstructionError::InvalidArgument); } - let signer = match AccountUtilsState::::state(self)?.convert_to_current() { + let signer = match AccountUtilsState::::state(self)?.state() { State::Uninitialized => { if lamports > self.lamports()? { ic_msg!( @@ -160,7 +165,8 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { } State::Initialized(ref data) => { if lamports == self.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" @@ -170,7 +176,7 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { merge_nonce_error_into_system_error, )); } - self.set_state(&Versions::new_current(State::Uninitialized))?; + self.set_state(&Versions::new(State::Uninitialized, separate_domains))?; } else { let min_balance = rent.minimum_balance(self.data_len()?); let amount = checked_add(lamports, min_balance)?; @@ -236,7 +242,7 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { return Err(InstructionError::InvalidArgument); } - match AccountUtilsState::::state(self)?.convert_to_current() { + match AccountUtilsState::::state(self)?.state() { State::Uninitialized => { let min_balance = rent.minimum_balance(self.data_len()?); if self.lamports()? < min_balance { @@ -248,14 +254,16 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { ); 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, ); - self.set_state(&Versions::new_current(State::Initialized(data))) + let state = State::Initialized(data); + self.set_state(&Versions::new(state, separate_domains)) } - _ => { + State::Initialized(_) => { ic_msg!( invoke_context, "Initialize nonce account: Account {} state is invalid", @@ -292,7 +300,9 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { return Err(InstructionError::InvalidArgument); } - match AccountUtilsState::::state(self)?.convert_to_current() { + let state = AccountUtilsState::::state(self)?; + let separate_domains = state.separate_domains(); + match state.state() { State::Initialized(data) => { if !signers.contains(&data.authority) { ic_msg!( @@ -307,9 +317,12 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { data.durable_nonce, data.get_lamports_per_signature(), ); - self.set_state(&Versions::new_current(State::Initialized(new_data))) + self.set_state(&Versions::new( + State::Initialized(new_data), + separate_domains, + )) } - _ => { + State::Uninitialized => { ic_msg!( invoke_context, "Authorize nonce account: Account {} state is invalid", @@ -328,6 +341,7 @@ impl<'a> NonceKeyedAccount for KeyedAccount<'a> { mod test { use { super::*, + assert_matches::assert_matches, solana_program_runtime::invoke_context::InvokeContext, solana_sdk::{ account::ReadableAccount, @@ -345,7 +359,7 @@ mod test { F: Fn(&KeyedAccount), { let pubkey = Pubkey::new_unique(); - let account = create_account(lamports); + let account = create_account(lamports, /*separate_domains:*/ true); let keyed_account = KeyedAccount::new(&pubkey, signer, &account); f(&keyed_account) } @@ -384,54 +398,46 @@ mod test { }; let mut signers = HashSet::new(); signers.insert(*keyed_account.signer_key().unwrap()); - let state = AccountUtilsState::::state(keyed_account) - .unwrap() - .convert_to_current(); + let versions = AccountUtilsState::::state(keyed_account).unwrap(); // New is in Uninitialzed state - assert_eq!(state, State::Uninitialized); + assert_eq!(versions.state(), &State::Uninitialized); let invoke_context = create_invoke_context_with_blockhash(95); let authorized = keyed_account.unsigned_key(); keyed_account .initialize_nonce_account(authorized, &rent, &invoke_context) .unwrap(); - let state = AccountUtilsState::::state(keyed_account) - .unwrap() - .convert_to_current(); + let versions = AccountUtilsState::::state(keyed_account).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())); let invoke_context = create_invoke_context_with_blockhash(63); keyed_account .advance_nonce_account(&signers, &invoke_context) .unwrap(); - let state = AccountUtilsState::::state(keyed_account) - .unwrap() - .convert_to_current(); + let versions = AccountUtilsState::::state(keyed_account).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())); let invoke_context = create_invoke_context_with_blockhash(31); keyed_account .advance_nonce_account(&signers, &invoke_context) .unwrap(); - let state = AccountUtilsState::::state(keyed_account) - .unwrap() - .convert_to_current(); + let versions = AccountUtilsState::::state(keyed_account).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)); with_test_keyed_account(42, false, |to_keyed| { let invoke_context = create_invoke_context_with_blockhash(0); let withdraw_lamports = keyed_account.account.borrow().lamports(); @@ -454,11 +460,9 @@ mod test { ); // Account balance goes to `to` assert_eq!(to_keyed.account.borrow().lamports(), expect_to_lamports); - let state = AccountUtilsState::::state(keyed_account) - .unwrap() - .convert_to_current(); + let versions = AccountUtilsState::::state(keyed_account).unwrap(); // Empty balance deinitializes data - assert_eq!(state, State::Uninitialized); + assert_eq!(versions.state(), &State::Uninitialized); }) }) } @@ -478,15 +482,13 @@ mod test { .unwrap(); let pubkey = *nonce_account.account.borrow().owner(); let nonce_account = KeyedAccount::new(&pubkey, false, nonce_account.account); - let state = AccountUtilsState::::state(&nonce_account) - .unwrap() - .convert_to_current(); + let versions = AccountUtilsState::::state(&nonce_account).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)); let signers = HashSet::new(); let invoke_context = create_invoke_context_with_blockhash(0); @@ -586,10 +588,8 @@ mod test { }; let min_lamports = rent.minimum_balance(State::size()); with_test_keyed_account(min_lamports + 42, true, |nonce_keyed| { - let state = AccountUtilsState::::state(nonce_keyed) - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let verions = AccountUtilsState::::state(nonce_keyed).unwrap(); + assert_eq!(verions.state(), &State::Uninitialized); with_test_keyed_account(42, false, |to_keyed| { let mut signers = HashSet::new(); signers.insert(*nonce_keyed.signer_key().unwrap()); @@ -607,12 +607,10 @@ mod test { &invoke_context, ) .unwrap(); - let state = AccountUtilsState::::state(nonce_keyed) - .unwrap() - .convert_to_current(); + let versions = AccountUtilsState::::state(nonce_keyed).unwrap(); // Withdraw instruction... // Deinitializes Account state - assert_eq!(state, State::Uninitialized); + assert_eq!(versions.state(), &State::Uninitialized); // Empties Account balance assert_eq!( nonce_keyed.account.borrow().lamports(), @@ -632,10 +630,8 @@ mod test { }; let min_lamports = rent.minimum_balance(State::size()); with_test_keyed_account(min_lamports + 42, false, |nonce_keyed| { - let state = AccountUtilsState::::state(nonce_keyed) - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = AccountUtilsState::::state(nonce_keyed).unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); with_test_keyed_account(42, false, |to_keyed| { let signers = HashSet::new(); let invoke_context = create_invoke_context_with_blockhash(0); @@ -660,10 +656,8 @@ mod test { }; let min_lamports = rent.minimum_balance(State::size()); with_test_keyed_account(min_lamports + 42, true, |nonce_keyed| { - let state = AccountUtilsState::::state(nonce_keyed) - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = AccountUtilsState::::state(nonce_keyed).unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); with_test_keyed_account(42, false, |to_keyed| { let mut signers = HashSet::new(); signers.insert(*nonce_keyed.signer_key().unwrap()); @@ -706,10 +700,8 @@ mod test { &invoke_context, ) .unwrap(); - let state = AccountUtilsState::::state(nonce_keyed) - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = AccountUtilsState::::state(nonce_keyed).unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); assert_eq!( nonce_keyed.account.borrow().lamports(), nonce_expect_lamports @@ -728,10 +720,8 @@ mod test { &invoke_context, ) .unwrap(); - let state = AccountUtilsState::::state(nonce_keyed) - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = AccountUtilsState::::state(nonce_keyed).unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); assert_eq!( nonce_keyed.account.borrow().lamports(), nonce_expect_lamports @@ -756,15 +746,13 @@ mod test { nonce_keyed .initialize_nonce_account(&authority, &rent, &invoke_context) .unwrap(); - let state = AccountUtilsState::::state(nonce_keyed) - .unwrap() - .convert_to_current(); + let versions = AccountUtilsState::::state(nonce_keyed).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())); with_test_keyed_account(42, false, |to_keyed| { let withdraw_lamports = nonce_keyed.account.borrow().lamports() - min_lamports; let nonce_expect_lamports = @@ -779,15 +767,13 @@ mod test { &invoke_context, ) .unwrap(); - let state = AccountUtilsState::::state(nonce_keyed) - .unwrap() - .convert_to_current(); + let versions = AccountUtilsState::::state(nonce_keyed).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_keyed.account.borrow().lamports(), nonce_expect_lamports @@ -807,10 +793,8 @@ mod test { &invoke_context, ) .unwrap(); - let state = AccountUtilsState::::state(nonce_keyed) - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = AccountUtilsState::::state(nonce_keyed).unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); assert_eq!( nonce_keyed.account.borrow().lamports(), nonce_expect_lamports @@ -947,10 +931,8 @@ mod test { }; let min_lamports = rent.minimum_balance(State::size()); with_test_keyed_account(min_lamports + 42, true, |keyed_account| { - let state = AccountUtilsState::::state(keyed_account) - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Uninitialized); + let versions = AccountUtilsState::::state(keyed_account).unwrap(); + assert_eq!(versions.state(), &State::Uninitialized); let mut signers = HashSet::new(); signers.insert(*keyed_account.signer_key().unwrap()); let invoke_context = create_invoke_context_with_blockhash(0); @@ -958,14 +940,12 @@ mod test { let result = keyed_account.initialize_nonce_account(&authority, &rent, &invoke_context); 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!(result, Ok(())); - let state = AccountUtilsState::::state(keyed_account) - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Initialized(data)); + let versions = AccountUtilsState::::state(keyed_account).unwrap(); + assert_eq!(versions.state(), &State::Initialized(data)); }) } @@ -1023,7 +1003,7 @@ 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, ); let result = nonce_account.authorize_nonce_account( @@ -1032,10 +1012,8 @@ mod test { &invoke_context, ); assert_eq!(result, Ok(())); - let state = AccountUtilsState::::state(nonce_account) - .unwrap() - .convert_to_current(); - assert_eq!(state, State::Initialized(data)); + let versions = AccountUtilsState::::state(nonce_account).unwrap(); + assert_eq!(versions.state(), &State::Initialized(data)); }) } @@ -1087,27 +1065,35 @@ mod test { with_test_keyed_account(42, true, |nonce_account| { let mut signers = HashSet::new(); signers.insert(nonce_account.signer_key().unwrap()); - let state: State = nonce_account.state().unwrap(); + let versions: Versions = nonce_account.state().unwrap(); // New is in Uninitialzed state - assert_eq!(state, State::Uninitialized); + assert_eq!(versions.state(), &State::Uninitialized); let invoke_context = create_invoke_context_with_blockhash(0); let authorized = nonce_account.unsigned_key(); nonce_account .initialize_nonce_account(authorized, &Rent::free(), &invoke_context) .unwrap(); - assert!(verify_nonce_account( - &nonce_account.account.borrow(), - get_durable_nonce(&invoke_context).as_hash(), - ) - .is_some()); + assert_matches!( + verify_nonce_account( + &nonce_account.account.borrow(), + get_durable_nonce(&invoke_context).0.as_hash(), + true, // separate_domains + ), + Some(_) + ); }); } #[test] fn verify_nonce_bad_acc_state_fail() { with_test_keyed_account(42, true, |nonce_account| { - assert!( - verify_nonce_account(&nonce_account.account.borrow(), &Hash::default()).is_none() + assert_eq!( + verify_nonce_account( + &nonce_account.account.borrow(), + &Hash::default(), + true, // separate_domains + ), + None, ); }); } @@ -1117,20 +1103,23 @@ mod test { with_test_keyed_account(42, true, |nonce_account| { let mut signers = HashSet::new(); signers.insert(nonce_account.signer_key().unwrap()); - let state: State = nonce_account.state().unwrap(); + let versions: Versions = nonce_account.state().unwrap(); // New is in Uninitialzed state - assert_eq!(state, State::Uninitialized); + assert_eq!(versions.state(), &State::Uninitialized); let invoke_context = create_invoke_context_with_blockhash(0); let authorized = nonce_account.unsigned_key(); nonce_account .initialize_nonce_account(authorized, &Rent::free(), &invoke_context) .unwrap(); let invoke_context = create_invoke_context_with_blockhash(1); - assert!(verify_nonce_account( - &nonce_account.account.borrow(), - get_durable_nonce(&invoke_context).as_hash(), - ) - .is_none()); + assert_eq!( + verify_nonce_account( + &nonce_account.account.borrow(), + get_durable_nonce(&invoke_context).0.as_hash(), + true, // separate_domains + ), + None + ); }); } } diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index 5161af7842c180..04fb20d4f50626 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -472,11 +472,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 @@ -1020,9 +1019,10 @@ mod tests { let nonce = Pubkey::new_unique(); let nonce_account = AccountSharedData::new_ref_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(); @@ -1259,10 +1259,13 @@ mod tests { let from = Pubkey::new_unique(); let from_account = AccountSharedData::new_ref_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(); @@ -1542,7 +1545,9 @@ mod tests { #[test] fn test_process_nonce_ix_ok() { let nonce_address = Pubkey::new_unique(); - let nonce_account = Rc::new(nonce_account::create_account(1_000_000)); + let nonce_account = Rc::new(nonce_account::create_account( + 1_000_000, /*separate_domains:*/ true, + )); process_instruction( &serialize(&SystemInstruction::InitializeNonceAccount(nonce_address)).unwrap(), &[ @@ -1665,7 +1670,9 @@ mod tests { true, false, Pubkey::default(), - Rc::new(nonce_account::create_account(1_000_000)), + Rc::new(nonce_account::create_account( + 1_000_000, /*separate_domains:*/ true + )), ), (true, false, Pubkey::default(), create_default_account()), ( @@ -1692,7 +1699,9 @@ mod tests { true, true, Pubkey::new_unique(), - Rc::new(nonce_account::create_account(1_000_000)), + Rc::new(nonce_account::create_account( + 1_000_000, /*separate_domains:*/ true + )), ), (true, false, Pubkey::default(), create_default_account()), ( @@ -1734,7 +1743,9 @@ mod tests { true, false, Pubkey::default(), - Rc::new(nonce_account::create_account(1_000_000)), + Rc::new(nonce_account::create_account( + 1_000_000, /*separate_domains:*/ true + )), )], ), Err(InstructionError::NotEnoughAccountKeys), @@ -1751,7 +1762,9 @@ mod tests { true, false, Pubkey::default(), - Rc::new(nonce_account::create_account(1_000_000)), + Rc::new(nonce_account::create_account( + 1_000_000, /*separate_domains:*/ true + )), ), ( true, @@ -1776,7 +1789,9 @@ mod tests { true, false, Pubkey::default(), - Rc::new(nonce_account::create_account(1_000_000)), + Rc::new(nonce_account::create_account( + 1_000_000, /*separate_domains:*/ true + )), ), ( false, @@ -1803,7 +1818,9 @@ mod tests { true, true, nonce_address, - Rc::new(nonce_account::create_account(1_000_000)), + Rc::new(nonce_account::create_account( + 1_000_000, /*separate_domains:*/ true + )), ), ( false, @@ -1827,7 +1844,9 @@ mod tests { #[test] fn test_process_authorize_ix_ok() { let nonce_address = Pubkey::new_unique(); - let nonce_account = Rc::new(nonce_account::create_account(1_000_000)); + let nonce_account = Rc::new(nonce_account::create_account( + 1_000_000, /*separate_domains:*/ true, + )); process_instruction( &serialize(&SystemInstruction::InitializeNonceAccount(nonce_address)).unwrap(), &[ @@ -1883,9 +1902,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(); @@ -1898,7 +1918,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 ); } @@ -1914,9 +1936,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(); @@ -1926,7 +1949,9 @@ mod tests { #[test] fn test_nonce_initialize_with_empty_recent_blockhashes_fail() { let nonce_address = Pubkey::new_unique(); - let nonce_account = Rc::new(nonce_account::create_account(1_000_000)); + let nonce_account = Rc::new(nonce_account::create_account( + 1_000_000, /*separate_domains:*/ true, + )); #[allow(deprecated)] let new_recent_blockhashes_account = Rc::new(RefCell::new( solana_sdk::recent_blockhashes_account::create_account_with_data_for_test( @@ -1960,7 +1985,9 @@ mod tests { #[test] fn test_nonce_advance_with_empty_recent_blockhashes_fail() { let nonce_address = Pubkey::new_unique(); - let nonce_account = Rc::new(nonce_account::create_account(1_000_000)); + let nonce_account = Rc::new(nonce_account::create_account( + 1_000_000, /*separate_domains:*/ true, + )); process_instruction( &serialize(&SystemInstruction::InitializeNonceAccount(nonce_address)).unwrap(), &[ diff --git a/sdk/program/src/nonce/state/current.rs b/sdk/program/src/nonce/state/current.rs index da9bf57db3ca44..778db58953b179 100644 --- a/sdk/program/src/nonce/state/current.rs +++ b/sdk/program/src/nonce/state/current.rs @@ -82,7 +82,10 @@ impl State { Self::Initialized(Data::new(*authority, durable_nonce, lamports_per_signature)) } pub fn size() -> usize { - let data = Versions::new_current(State::Initialized(Data::default())); + let data = Versions::new( + State::Initialized(Data::default()), + true, // separate_domains + ); bincode::serialized_size(&data).unwrap() as usize } } diff --git a/sdk/program/src/nonce/state/mod.rs b/sdk/program/src/nonce/state/mod.rs index ca6bc9919b14ad..6131dc341afd39 100644 --- a/sdk/program/src/nonce/state/mod.rs +++ b/sdk/program/src/nonce/state/mod.rs @@ -1,20 +1,177 @@ 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, hash::hashv, pubkey::Pubkey}, + }; + + #[test] + fn test_verify_recent_blockhash() { + let blockhash: Hash = hashv(&[&[171u8; 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..565e0990a528ba 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,42 @@ 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, + hash::hashv, + nonce::state::{Data, DurableNonce}, + pubkey::Pubkey, + system_program, + }, + }; #[test] fn test_verify_bad_account_owner_fails() { @@ -54,11 +66,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 = hashv(&[&[171u8; 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 503b90dc5bbdb8..5cd4c67a7ebce0 100644 --- a/send-transaction-service/src/send_transaction_service.rs +++ b/send-transaction-service/src/send_transaction_service.rs @@ -237,7 +237,12 @@ impl SendTransactionService { } if let Some((nonce_pubkey, durable_nonce)) = transaction_info.durable_nonce_info { let nonce_account = working_bank.get_account(&nonce_pubkey).unwrap_or_default(); - if nonce_account::verify_nonce_account(&nonce_account, &durable_nonce).is_none() + 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() && working_bank.get_signature_status_slot(signature).is_none() { info!("Dropping expired durable-nonce transaction: {}", signature); @@ -635,9 +640,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); @@ -865,9 +875,14 @@ mod test { // Advance nonce 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);