diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 193c7352b6..547498a096 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -3530,7 +3530,8 @@ mod tests { assert!(nonce_account::verify_nonce_account( &collected_nonce_account, durable_nonce.as_hash(), - )); + ) + .is_some()); } #[test] @@ -3635,7 +3636,8 @@ mod tests { assert!(nonce_account::verify_nonce_account( &collected_nonce_account, durable_nonce.as_hash(), - )); + ) + .is_some()); } #[test] diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 040abaa3fd..c09a459e02 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -124,7 +124,7 @@ use { message::{AccountKeys, SanitizedMessage}, native_loader, native_token::sol_to_lamports, - nonce::{self, state::DurableNonce}, + nonce::{self, state::DurableNonce, NONCED_TX_MARKER_IX_INDEX}, nonce_account, packet::PACKET_DATA_SIZE, precompiles::get_precompiles, @@ -4103,15 +4103,25 @@ impl Bank { } fn check_message_for_nonce(&self, message: &SanitizedMessage) -> Option { - message - .get_durable_nonce(self.feature_set.is_active(&nonce_must_be_writable::id())) - .and_then(|nonce_address| { - self.get_account_with_fixed_root(nonce_address) - .map(|nonce_account| (*nonce_address, nonce_account)) - }) - .filter(|(_, nonce_account)| { - nonce_account::verify_nonce_account(nonce_account, message.recent_blockhash()) - }) + 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())?; + + if self + .feature_set + .is_active(&feature_set::nonce_must_be_authorized::ID) + { + let nonce_is_authorized = message + .get_ix_signers(NONCED_TX_MARKER_IX_INDEX as usize) + .any(|signer| signer == &nonce_data.authority); + if !nonce_is_authorized { + return None; + } + } + + Some((*nonce_address, nonce_account)) } fn check_transaction_for_nonce( @@ -12963,22 +12973,16 @@ pub(crate) mod tests { let initial_custodian_balance = custodian_account.lamports(); assert_eq!( bank.process_transaction(&nonce_tx), - Err(TransactionError::InstructionError( - 0, - InstructionError::MissingRequiredSignature, - )) + Err(TransactionError::BlockhashNotFound), ); - /* Check fee charged and nonce has *not* advanced */ + /* Check fee was *not* charged and nonce has *not* advanced */ let mut recent_message = nonce_tx.message; recent_message.recent_blockhash = bank.last_blockhash(); assert_eq!( bank.get_balance(&custodian_pubkey), initial_custodian_balance - - bank - .get_fee_for_message(&recent_message.try_into().unwrap()) - .unwrap() ); - assert_ne!( + assert_eq!( nonce_hash, get_nonce_blockhash(&bank, &nonce_pubkey).unwrap() ); diff --git a/runtime/src/nonce_keyed_account.rs b/runtime/src/nonce_keyed_account.rs index 3ffd3541ec..9a06ac61e9 100644 --- a/runtime/src/nonce_keyed_account.rs +++ b/runtime/src/nonce_keyed_account.rs @@ -1214,7 +1214,8 @@ mod test { .unwrap() .borrow(), get_durable_nonce(&invoke_context).as_hash(), - )); + ) + .is_some()); } #[test] @@ -1226,13 +1227,14 @@ mod test { _instruction_context, instruction_accounts ); - assert!(!verify_nonce_account( + assert!(verify_nonce_account( &transaction_context .get_account_at_index(NONCE_ACCOUNT_INDEX) .unwrap() .borrow(), &Hash::default() - )); + ) + .is_none()); } #[test] @@ -1263,12 +1265,13 @@ mod test { .unwrap(); set_invoke_context_blockhash!(invoke_context, 1); drop(nonce_account); - assert!(!verify_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()); } } diff --git a/sdk/program/src/message/sanitized.rs b/sdk/program/src/message/sanitized.rs index 449395f4aa..4e938eb93f 100644 --- a/sdk/program/src/message/sanitized.rs +++ b/sdk/program/src/message/sanitized.rs @@ -225,6 +225,21 @@ impl SanitizedMessage { } } + /// Get a list of signers for the instruction at the given index + pub fn get_ix_signers(&self, ix_index: usize) -> impl Iterator { + self.instructions() + .get(ix_index) + .into_iter() + .flat_map(|ix| { + ix.accounts + .iter() + .copied() + .map(usize::from) + .filter(|index| self.is_signer(*index)) + .filter_map(|signer_index| self.account_keys().get(signer_index)) + }) + } + /// If the message uses a durable nonce, return the pubkey of the nonce account pub fn get_durable_nonce(&self, nonce_must_be_writable: bool) -> Option<&Pubkey> { self.instructions() @@ -256,7 +271,7 @@ impl SanitizedMessage { #[cfg(test)] mod tests { - use {super::*, crate::message::v0}; + use {super::*, crate::message::v0, std::collections::HashSet}; #[test] fn test_try_from_message() { @@ -336,4 +351,44 @@ mod tests { assert_eq!(v0_message.num_readonly_accounts(), 3); } + + #[test] + fn test_get_ix_signers() { + let signer0 = Pubkey::new_unique(); + let signer1 = Pubkey::new_unique(); + let non_signer = Pubkey::new_unique(); + let loader_key = Pubkey::new_unique(); + let instructions = vec![ + CompiledInstruction::new(3, &(), vec![2, 0]), + CompiledInstruction::new(3, &(), vec![0, 1]), + CompiledInstruction::new(3, &(), vec![0, 0]), + ]; + + let message = SanitizedMessage::try_from(LegacyMessage::new_with_compiled_instructions( + 2, + 1, + 2, + vec![signer0, signer1, non_signer, loader_key], + Hash::default(), + instructions, + )) + .unwrap(); + + assert_eq!( + message.get_ix_signers(0).collect::>(), + HashSet::from_iter([&signer0]) + ); + assert_eq!( + message.get_ix_signers(1).collect::>(), + HashSet::from_iter([&signer0, &signer1]) + ); + assert_eq!( + message.get_ix_signers(2).collect::>(), + HashSet::from_iter([&signer0]) + ); + assert_eq!( + message.get_ix_signers(3).collect::>(), + HashSet::default() + ); + } } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index ecfc8fc0a0..3b9416a18f 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -432,6 +432,10 @@ pub mod quick_bail_on_panic { solana_sdk::declare_id!("DpJREPyuMZ5nDfU6H3WTqSqUFSXAfw8u7xqmWtEwJDcP"); } +pub mod nonce_must_be_authorized { + solana_sdk::declare_id!("HxrEu1gXuH7iD3Puua1ohd5n4iUKJyFNtNxk9DVJkvgr"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -533,6 +537,7 @@ lazy_static! { (enable_durable_nonce::id(), "enable durable nonce #25744"), (vote_state_update_credit_per_dequeue::id(), "Calculate vote credits for VoteStateUpdate per vote dequeue to match credit awards for Vote instruction"), (quick_bail_on_panic::id(), "quick bail on panic"), + (nonce_must_be_authorized::id(), "nonce must be authorized"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/sdk/src/nonce_account.rs b/sdk/src/nonce_account.rs index 7dd642d96d..bce8c86048 100644 --- a/sdk/src/nonce_account.rs +++ b/sdk/src/nonce_account.rs @@ -3,7 +3,10 @@ use { account::{AccountSharedData, ReadableAccount}, account_utils::StateMut, hash::Hash, - nonce::{state::Versions, State}, + nonce::{ + state::{Data, Versions}, + State, + }, }, std::cell::RefCell, }; @@ -21,13 +24,13 @@ pub fn create_account(lamports: u64) -> RefCell { } // TODO: Consider changing argument from Hash to DurableNonce. -pub fn verify_nonce_account(acc: &AccountSharedData, hash: &Hash) -> bool { +pub fn verify_nonce_account(acc: &AccountSharedData, hash: &Hash) -> Option { if acc.owner() != &crate::system_program::id() { - return false; + return None; } match StateMut::::state(acc).map(|v| v.convert_to_current()) { - Ok(State::Initialized(ref data)) => hash == &data.blockhash(), - _ => false, + Ok(State::Initialized(data)) => (hash == &data.blockhash()).then(|| data), + _ => None, } } @@ -56,6 +59,6 @@ mod tests { &program_id, ) .expect("nonce_account"); - assert!(!verify_nonce_account(&account, &Hash::default())); + assert!(verify_nonce_account(&account, &Hash::default()).is_none()); } } diff --git a/send-transaction-service/src/send_transaction_service.rs b/send-transaction-service/src/send_transaction_service.rs index 24caa185c2..9c2e82936f 100644 --- a/send-transaction-service/src/send_transaction_service.rs +++ b/send-transaction-service/src/send_transaction_service.rs @@ -605,7 +605,7 @@ 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) + if nonce_account::verify_nonce_account(&nonce_account, &durable_nonce).is_none() && signature_status.is_none() && expired {