From 72a5c66e0d13ef16004985a7dd06bb594c189ade Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Thu, 27 Mar 2025 14:52:46 +0000 Subject: [PATCH 1/2] SIMD-0242: Static Nonce Account Only --- Cargo.lock | 1 + cost-model/src/transaction_cost.rs | 4 + feature-set/src/lib.rs | 5 + .../src/runtime_transaction.rs | 4 + runtime/src/bank/check_transactions.rs | 92 ++++++++++++++++++- svm-transaction/Cargo.toml | 1 + svm-transaction/src/svm_message.rs | 9 +- .../src/svm_message/sanitized_message.rs | 4 + .../src/svm_message/sanitized_transaction.rs | 4 + svm-transaction/src/tests.rs | 42 ++++++--- .../src/resolved_transaction_view.rs | 4 + 11 files changed, 153 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 10d6f00f937..be3bdec8075 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10438,6 +10438,7 @@ dependencies = [ "solana-system-interface", "solana-transaction", "static_assertions", + "test-case", ] [[package]] diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index 1d199526557..7569db8ba10 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -209,6 +209,10 @@ impl solana_svm_transaction::svm_message::SVMMessage for WritableKeysTransaction core::iter::empty() } + fn static_account_keys(&self) -> &[Pubkey] { + &self.0 + } + fn account_keys(&self) -> solana_message::AccountKeys { solana_message::AccountKeys::new(&self.0, None) } diff --git a/feature-set/src/lib.rs b/feature-set/src/lib.rs index b790aea5eb0..96a4352d080 100644 --- a/feature-set/src/lib.rs +++ b/feature-set/src/lib.rs @@ -1028,6 +1028,10 @@ pub mod enable_vote_address_leader_schedule { solana_pubkey::declare_id!("5JsG4NWH8Jbrqdd8uL6BNwnyZK3dQSoieRXG5vmofj9y"); } +pub mod require_static_nonce_account { + solana_pubkey::declare_id!("7VVhpg5oAjAmnmz1zCcSHb2Z9ecZB2FQqpnEwReka9Zm"); +} + pub static FEATURE_NAMES: LazyLock> = LazyLock::new(|| { [ (secp256k1_program_enabled::id(), "secp256k1 program"), @@ -1260,6 +1264,7 @@ pub static FEATURE_NAMES: LazyLock> = LazyLock::n (create_slashing_program::id(), "creates an enshrined slashing program SIMD-0204"), (disable_partitioned_rent_collection::id(), "Disable partitioned rent collection SIMD-0175 #4562"), (enable_vote_address_leader_schedule::id(), "Enable vote address leader schedule SIMD-0180 #4573"), + (require_static_nonce_account::id(), "SIMD-0242: Static Nonce Account Only"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/runtime-transaction/src/runtime_transaction.rs b/runtime-transaction/src/runtime_transaction.rs index 7273a67d670..aac2cbf110b 100644 --- a/runtime-transaction/src/runtime_transaction.rs +++ b/runtime-transaction/src/runtime_transaction.rs @@ -103,6 +103,10 @@ impl SVMMessage for RuntimeTransaction { self.transaction.program_instructions_iter() } + fn static_account_keys(&self) -> &[Pubkey] { + self.transaction.static_account_keys() + } + fn account_keys(&self) -> AccountKeys { self.transaction.account_keys() } diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index 87ea340d048..5c871700205 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -237,7 +237,10 @@ impl Bank { &self, message: &impl SVMMessage, ) -> Option<(Pubkey, AccountSharedData, NonceData)> { - let nonce_address = message.get_durable_nonce()?; + let require_static_nonce_account = self + .feature_set + .is_active(&agave_feature_set::require_static_nonce_account::id()); + let nonce_address = message.get_durable_nonce(require_static_nonce_account)?; let nonce_account = self.get_account_with_fixed_root(nonce_address)?; let nonce_data = nonce_account::verify_nonce_account(&nonce_account, message.recent_blockhash())?; @@ -300,8 +303,20 @@ mod tests { setup_nonce_with_bank, }, solana_sdk::{ - hash::Hash, message::Message, signature::Keypair, signer::Signer, system_instruction, + hash::Hash, + instruction::CompiledInstruction, + message::{ + v0::{self, LoadedAddresses, MessageAddressTableLookup}, + Message, MessageHeader, SanitizedMessage, SanitizedVersionedMessage, + SimpleAddressLoader, VersionedMessage, + }, + signature::Keypair, + signer::Signer, + system_instruction::{self, SystemInstruction}, + system_program, }, + std::collections::HashSet, + test_case::test_case, }; #[test] @@ -491,8 +506,79 @@ mod tests { .check_load_and_advance_message_nonce_account( &message, &bank.next_durable_nonce(), - lamports_per_signature + lamports_per_signature, ) .is_none()); } + + #[test_case(true; "test_check_and_load_message_nonce_account_nonce_is_alt_disallowed")] + #[test_case(false; "test_check_and_load_message_nonce_account_nonce_is_alt_allowed")] + fn test_check_and_load_message_nonce_account_nonce_is_alt(require_static_nonce_account: bool) { + let feature_set = if require_static_nonce_account { + FeatureSet::all_enabled() + } else { + FeatureSet::default() + }; + let nonce_authority = Pubkey::new_unique(); + let (bank, _mint_keypair, _custodian_keypair, nonce_keypair, _) = setup_nonce_with_bank( + 10_000_000, + |_| {}, + 5_000_000, + 250_000, + Some(nonce_authority), + feature_set, + ) + .unwrap(); + + let nonce_pubkey = nonce_keypair.pubkey(); + let nonce_hash = get_nonce_blockhash(&bank, &nonce_pubkey).unwrap(); + let loaded_addresses = LoadedAddresses { + writable: vec![nonce_pubkey], + readonly: vec![], + }; + + let message = SanitizedMessage::try_new( + SanitizedVersionedMessage::try_new(VersionedMessage::V0(v0::Message { + header: MessageHeader { + num_required_signatures: 1, + num_readonly_signed_accounts: 0, + num_readonly_unsigned_accounts: 1, + }, + account_keys: vec![nonce_authority, system_program::id()], + recent_blockhash: nonce_hash, + instructions: vec![CompiledInstruction::new( + 1, // index of system program + &SystemInstruction::AdvanceNonceAccount, + vec![ + 2, // index of alt nonce account + 0, // index of nonce_authority + ], + )], + address_table_lookups: vec![MessageAddressTableLookup { + account_key: Pubkey::new_unique(), + writable_indexes: (0..loaded_addresses.writable.len()) + .map(|x| x as u8) + .collect(), + readonly_indexes: (0..loaded_addresses.readonly.len()) + .map(|x| (loaded_addresses.writable.len() + x) as u8) + .collect(), + }], + })) + .unwrap(), + SimpleAddressLoader::Enabled(loaded_addresses), + &HashSet::new(), + ) + .unwrap(); + + let (_, lamports_per_signature) = bank.last_blockhash_and_lamports_per_signature(); + assert_eq!( + bank.check_load_and_advance_message_nonce_account( + &message, + &bank.next_durable_nonce(), + lamports_per_signature + ) + .is_none(), + require_static_nonce_account, + ); + } } diff --git a/svm-transaction/Cargo.toml b/svm-transaction/Cargo.toml index e6d690a5013..dc689d97c28 100644 --- a/svm-transaction/Cargo.toml +++ b/svm-transaction/Cargo.toml @@ -22,3 +22,4 @@ solana-message = { workspace = true, features = ["bincode"] } solana-nonce = { workspace = true } solana-system-interface = { workspace = true } static_assertions = { workspace = true } +test-case = { workspace = true } diff --git a/svm-transaction/src/svm_message.rs b/svm-transaction/src/svm_message.rs index dfd02a131b0..698d0fda2e9 100644 --- a/svm-transaction/src/svm_message.rs +++ b/svm-transaction/src/svm_message.rs @@ -53,6 +53,9 @@ pub trait SVMMessage: Debug { /// the pubkey of the program. fn program_instructions_iter(&self) -> impl Iterator + Clone; + /// Return the list of static account keys. + fn static_account_keys(&self) -> &[Pubkey]; + /// Return the account keys. fn account_keys(&self) -> AccountKeys; @@ -81,7 +84,7 @@ pub trait SVMMessage: Debug { } /// If the message uses a durable nonce, return the pubkey of the nonce account - fn get_durable_nonce(&self) -> Option<&Pubkey> { + fn get_durable_nonce(&self, require_static_nonce_account: bool) -> Option<&Pubkey> { let account_keys = self.account_keys(); self.instructions_iter() .nth(usize::from(NONCED_TX_MARKER_IX_INDEX)) @@ -104,7 +107,9 @@ pub trait SVMMessage: Debug { .and_then(|ix| { ix.accounts.first().and_then(|idx| { let index = usize::from(*idx); - if !self.is_writable(index) { + if (require_static_nonce_account && index >= self.static_account_keys().len()) + || !self.is_writable(index) + { None } else { account_keys.get(index) diff --git a/svm-transaction/src/svm_message/sanitized_message.rs b/svm-transaction/src/svm_message/sanitized_message.rs index 7224c1fe45a..b7be248f3d0 100644 --- a/svm-transaction/src/svm_message/sanitized_message.rs +++ b/svm-transaction/src/svm_message/sanitized_message.rs @@ -37,6 +37,10 @@ impl SVMMessage for SanitizedMessage { .map(|(pubkey, ix)| (pubkey, SVMInstruction::from(ix))) } + fn static_account_keys(&self) -> &[Pubkey] { + SanitizedMessage::static_account_keys(self) + } + fn account_keys(&self) -> AccountKeys { SanitizedMessage::account_keys(self) } diff --git a/svm-transaction/src/svm_message/sanitized_transaction.rs b/svm-transaction/src/svm_message/sanitized_transaction.rs index 5537412bcc3..d68a3d221f0 100644 --- a/svm-transaction/src/svm_message/sanitized_transaction.rs +++ b/svm-transaction/src/svm_message/sanitized_transaction.rs @@ -34,6 +34,10 @@ impl SVMMessage for SanitizedTransaction { SVMMessage::program_instructions_iter(SanitizedTransaction::message(self)) } + fn static_account_keys(&self) -> &[Pubkey] { + SVMMessage::static_account_keys(SanitizedTransaction::message(self)) + } + fn account_keys(&self) -> AccountKeys { SVMMessage::account_keys(SanitizedTransaction::message(self)) } diff --git a/svm-transaction/src/tests.rs b/svm-transaction/src/tests.rs index 36c80eece4d..0813e2359a7 100644 --- a/svm-transaction/src/tests.rs +++ b/svm-transaction/src/tests.rs @@ -1,4 +1,5 @@ #![cfg(test)] +#![allow(clippy::arithmetic_side_effects)] use { crate::svm_message::SVMMessage, solana_hash::Hash, @@ -13,10 +14,12 @@ use { solana_sdk_ids::system_program, solana_system_interface::instruction::SystemInstruction, std::collections::HashSet, + test_case::test_case, }; -#[test] -fn test_get_durable_nonce() { +#[test_case(true; "test_get_durable_nonce_static_nonce_account")] +#[test_case(false; "test_get_durable_nonce_alt_nonce_account")] +fn test_get_durable_nonce(require_static_nonce_account: bool) { fn create_message_for_test( num_signers: u8, num_writable: u8, @@ -71,7 +74,7 @@ fn test_get_durable_nonce() { { let message = create_message_for_test(1, 1, vec![Pubkey::new_unique()], vec![], None); assert!(SanitizedMessage::get_durable_nonce(&message).is_none()); - assert!(SVMMessage::get_durable_nonce(&message).is_none()); + assert!(SVMMessage::get_durable_nonce(&message, require_static_nonce_account).is_none()); } // system program id instruction - invalid @@ -84,7 +87,7 @@ fn test_get_durable_nonce() { None, ); assert!(SanitizedMessage::get_durable_nonce(&message).is_none()); - assert!(SVMMessage::get_durable_nonce(&message).is_none()); + assert!(SVMMessage::get_durable_nonce(&message, require_static_nonce_account).is_none()); } // system program id instruction - not nonce @@ -101,7 +104,7 @@ fn test_get_durable_nonce() { None, ); assert!(SanitizedMessage::get_durable_nonce(&message).is_none()); - assert!(SVMMessage::get_durable_nonce(&message).is_none()); + assert!(SVMMessage::get_durable_nonce(&message, require_static_nonce_account).is_none()); } // system program id - nonce instruction (no accounts) @@ -118,7 +121,7 @@ fn test_get_durable_nonce() { None, ); assert!(SanitizedMessage::get_durable_nonce(&message).is_none()); - assert!(SVMMessage::get_durable_nonce(&message).is_none()); + assert!(SVMMessage::get_durable_nonce(&message, require_static_nonce_account).is_none()); } // system program id - nonce instruction (non-fee-payer, non-writable) @@ -137,7 +140,7 @@ fn test_get_durable_nonce() { None, ); assert!(SanitizedMessage::get_durable_nonce(&message).is_none()); - assert!(SVMMessage::get_durable_nonce(&message).is_none()); + assert!(SVMMessage::get_durable_nonce(&message, require_static_nonce_account).is_none()); } // system program id - nonce instruction fee-payer @@ -158,7 +161,10 @@ fn test_get_durable_nonce() { SanitizedMessage::get_durable_nonce(&message), Some(&payer_nonce) ); - assert_eq!(SVMMessage::get_durable_nonce(&message), Some(&payer_nonce)); + assert_eq!( + SVMMessage::get_durable_nonce(&message, require_static_nonce_account), + Some(&payer_nonce) + ); } // system program id - nonce instruction w/ trailing bytes fee-payer @@ -181,7 +187,10 @@ fn test_get_durable_nonce() { SanitizedMessage::get_durable_nonce(&message), Some(&payer_nonce) ); - assert_eq!(SVMMessage::get_durable_nonce(&message), Some(&payer_nonce)); + assert_eq!( + SVMMessage::get_durable_nonce(&message, require_static_nonce_account), + Some(&payer_nonce) + ); } // system program id - nonce instruction (non-fee-payer) @@ -200,7 +209,10 @@ fn test_get_durable_nonce() { None, ); assert_eq!(SanitizedMessage::get_durable_nonce(&message), Some(&nonce)); - assert_eq!(SVMMessage::get_durable_nonce(&message), Some(&nonce)); + assert_eq!( + SVMMessage::get_durable_nonce(&message, require_static_nonce_account), + Some(&nonce) + ); } // system program id - nonce instruction (non-fee-payer, multiple accounts) @@ -220,7 +232,10 @@ fn test_get_durable_nonce() { None, ); assert_eq!(SanitizedMessage::get_durable_nonce(&message), Some(&nonce)); - assert_eq!(SVMMessage::get_durable_nonce(&message), Some(&nonce)); + assert_eq!( + SVMMessage::get_durable_nonce(&message, require_static_nonce_account), + Some(&nonce) + ); } // system program id - nonce instruction (non-fee-payer, loaded account) @@ -242,7 +257,10 @@ fn test_get_durable_nonce() { }), ); assert_eq!(SanitizedMessage::get_durable_nonce(&message), Some(&nonce)); - assert_eq!(SVMMessage::get_durable_nonce(&message), Some(&nonce)); + assert_eq!( + SVMMessage::get_durable_nonce(&message, require_static_nonce_account), + (!require_static_nonce_account).then_some(&nonce) + ); } } diff --git a/transaction-view/src/resolved_transaction_view.rs b/transaction-view/src/resolved_transaction_view.rs index 039af79284b..057991fabbd 100644 --- a/transaction-view/src/resolved_transaction_view.rs +++ b/transaction-view/src/resolved_transaction_view.rs @@ -183,6 +183,10 @@ impl SVMMessage for ResolvedTransactionView { self.view.program_instructions_iter() } + fn static_account_keys(&self) -> &[Pubkey] { + self.view.static_account_keys() + } + fn account_keys(&self) -> AccountKeys { AccountKeys::new( self.view.static_account_keys(), From cf4f96a308ff20981efa7ab61238aa6ff45b14d6 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Fri, 28 Mar 2025 16:41:33 +0000 Subject: [PATCH 2/2] clean up arithmetic --- svm-transaction/src/tests.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/svm-transaction/src/tests.rs b/svm-transaction/src/tests.rs index 0813e2359a7..60f61ebb574 100644 --- a/svm-transaction/src/tests.rs +++ b/svm-transaction/src/tests.rs @@ -1,5 +1,4 @@ #![cfg(test)] -#![allow(clippy::arithmetic_side_effects)] use { crate::svm_message::SVMMessage, solana_hash::Hash, @@ -30,8 +29,10 @@ fn test_get_durable_nonce(require_static_nonce_account: bool) { let header = MessageHeader { num_required_signatures: num_signers, num_readonly_signed_accounts: 0, - num_readonly_unsigned_accounts: u8::try_from(account_keys.len()).unwrap() - - num_writable, + num_readonly_unsigned_accounts: u8::try_from(account_keys.len()) + .unwrap() + .checked_sub(num_writable) + .unwrap(), }; let (versioned_message, loader) = match loaded_addresses { None => ( @@ -55,7 +56,7 @@ fn test_get_durable_nonce(require_static_nonce_account: bool) { .map(|x| x as u8) .collect(), readonly_indexes: (0..loaded_addresses.readonly.len()) - .map(|x| (loaded_addresses.writable.len() + x) as u8) + .map(|x| loaded_addresses.writable.len().saturating_add(x) as u8) .collect(), }], }),