From 356a827087a09ca68a3152c7005a553d12bf2d4c Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Wed, 5 Jul 2023 10:44:54 -0700 Subject: [PATCH] Move NonceInfo, NonceFull and NoncePartial out of bank.rs (#32375) * Move NonceInfo, NonceFull and NoncePartial out of bank.rs * fix imports * move test_nonce_info to nonce_info.rs --- rpc/src/transaction_status_service.rs | 3 +- runtime/src/accounts.rs | 3 +- runtime/src/bank.rs | 103 +--------- runtime/src/bank/tests.rs | 138 +------------ runtime/src/lib.rs | 1 + runtime/src/nonce_info.rs | 273 ++++++++++++++++++++++++++ 6 files changed, 280 insertions(+), 241 deletions(-) create mode 100644 runtime/src/nonce_info.rs diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 6e6a9eb22..d591c329a 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -227,7 +227,8 @@ pub(crate) mod tests { solana_account_decoder::parse_token::token_amount_to_ui_amount, solana_ledger::{genesis_utils::create_genesis_config, get_tmp_ledger_path}, solana_runtime::{ - bank::{Bank, NonceFull, NoncePartial, TransactionBalancesSet}, + bank::{Bank, TransactionBalancesSet}, + nonce_info::{NonceFull, NoncePartial}, rent_debits::RentDebits, }, solana_sdk::{ diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index d12f5405a..c5aad8207 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -13,8 +13,9 @@ use { }, accounts_update_notifier_interface::AccountsUpdateNotifier, ancestors::Ancestors, - bank::{Bank, NonceFull, NonceInfo, TransactionCheckResult, TransactionExecutionResult}, + bank::{Bank, TransactionCheckResult, TransactionExecutionResult}, blockhash_queue::BlockhashQueue, + nonce_info::{NonceFull, NonceInfo}, rent_collector::RentCollector, rent_debits::RentDebits, storable_accounts::StorableAccounts, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 30d4795de..429d75ca9 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -62,6 +62,7 @@ use { epoch_accounts_hash::{self, EpochAccountsHash}, epoch_rewards_hasher::hash_rewards_into_partitions, epoch_stakes::{EpochStakes, NodeVoteAccounts}, + nonce_info::{NonceFull, NonceInfo, NoncePartial}, partitioned_rewards::PartitionedEpochRewardsConfig, rent_collector::{CollectedInfo, RentCollector}, rent_debits::RentDebits, @@ -552,108 +553,6 @@ impl TransactionLogCollector { } } -pub trait NonceInfo { - fn address(&self) -> &Pubkey; - fn account(&self) -> &AccountSharedData; - fn lamports_per_signature(&self) -> Option; - fn fee_payer_account(&self) -> Option<&AccountSharedData>; -} - -/// Holds limited nonce info available during transaction checks -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct NoncePartial { - address: Pubkey, - account: AccountSharedData, -} -impl NoncePartial { - pub fn new(address: Pubkey, account: AccountSharedData) -> Self { - Self { address, account } - } -} -impl NonceInfo for NoncePartial { - fn address(&self) -> &Pubkey { - &self.address - } - fn account(&self) -> &AccountSharedData { - &self.account - } - fn lamports_per_signature(&self) -> Option { - nonce_account::lamports_per_signature_of(&self.account) - } - fn fee_payer_account(&self) -> Option<&AccountSharedData> { - None - } -} - -/// Holds fee subtracted nonce info -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct NonceFull { - address: Pubkey, - account: AccountSharedData, - fee_payer_account: Option, -} -impl NonceFull { - pub fn new( - address: Pubkey, - account: AccountSharedData, - fee_payer_account: Option, - ) -> Self { - Self { - address, - account, - fee_payer_account, - } - } - pub fn from_partial( - partial: NoncePartial, - message: &SanitizedMessage, - accounts: &[TransactionAccount], - rent_debits: &RentDebits, - ) -> Result { - let fee_payer = (0..message.account_keys().len()).find_map(|i| { - if let Some((k, a)) = &accounts.get(i) { - if message.is_non_loader_key(i) { - return Some((k, a)); - } - } - None - }); - - if let Some((fee_payer_address, fee_payer_account)) = fee_payer { - let mut fee_payer_account = fee_payer_account.clone(); - let rent_debit = rent_debits.get_account_rent_debit(fee_payer_address); - fee_payer_account.set_lamports(fee_payer_account.lamports().saturating_add(rent_debit)); - - let nonce_address = *partial.address(); - if *fee_payer_address == nonce_address { - Ok(Self::new(nonce_address, fee_payer_account, None)) - } else { - Ok(Self::new( - nonce_address, - partial.account().clone(), - Some(fee_payer_account), - )) - } - } else { - Err(TransactionError::AccountNotFound) - } - } -} -impl NonceInfo for NonceFull { - fn address(&self) -> &Pubkey { - &self.address - } - fn account(&self) -> &AccountSharedData { - &self.account - } - fn lamports_per_signature(&self) -> Option { - nonce_account::lamports_per_signature_of(&self.account) - } - fn fee_payer_account(&self) -> Option<&AccountSharedData> { - self.fee_payer_account.as_ref() - } -} - // Bank's common fields shared by all supported snapshot versions for deserialization. // Sync fields with BankFieldsToSerialize! This is paired with it. // All members are made public to remain Bank's members private and to make versioned deserializer workable on this diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index c8269207e..58538f090 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -23,6 +23,7 @@ use { genesis_sysvar_and_builtin_program_lamports, GenesisConfigInfo, ValidatorVoteKeypairs, }, inline_spl_token, + nonce_info::NonceFull, rent_collector::RENT_EXEMPT_RENT_EPOCH, status_cache::MAX_CACHE_ENTRIES, transaction_error_metrics::TransactionErrorMetrics, @@ -225,10 +226,6 @@ fn test_race_register_tick_freeze() { } } -fn new_sanitized_message(instructions: &[Instruction], payer: Option<&Pubkey>) -> SanitizedMessage { - Message::new(instructions, payer).try_into().unwrap() -} - fn new_execution_result( status: Result<()>, nonce: Option<&NonceFull>, @@ -254,139 +251,6 @@ impl Bank { } } -#[test] -fn test_nonce_info() { - let lamports_per_signature = 42; - - let nonce_authority = keypair_from_seed(&[0; 32]).unwrap(); - let nonce_address = nonce_authority.pubkey(); - let from = keypair_from_seed(&[1; 32]).unwrap(); - let from_address = from.pubkey(); - let to_address = Pubkey::new_unique(); - - let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); - let nonce_account = AccountSharedData::new_data( - 43, - &nonce::state::Versions::new(nonce::State::Initialized(nonce::state::Data::new( - Pubkey::default(), - durable_nonce, - lamports_per_signature, - ))), - &system_program::id(), - ) - .unwrap(); - let from_account = AccountSharedData::new(44, 0, &Pubkey::default()); - let to_account = AccountSharedData::new(45, 0, &Pubkey::default()); - let recent_blockhashes_sysvar_account = AccountSharedData::new(4, 0, &Pubkey::default()); - - const TEST_RENT_DEBIT: u64 = 1; - let rent_collected_nonce_account = { - let mut account = nonce_account.clone(); - account.set_lamports(nonce_account.lamports() - TEST_RENT_DEBIT); - account - }; - let rent_collected_from_account = { - let mut account = from_account.clone(); - account.set_lamports(from_account.lamports() - TEST_RENT_DEBIT); - account - }; - - let instructions = vec![ - system_instruction::advance_nonce_account(&nonce_address, &nonce_authority.pubkey()), - system_instruction::transfer(&from_address, &to_address, 42), - ]; - - // NoncePartial create + NonceInfo impl - let partial = NoncePartial::new(nonce_address, rent_collected_nonce_account.clone()); - assert_eq!(*partial.address(), nonce_address); - assert_eq!(*partial.account(), rent_collected_nonce_account); - assert_eq!( - partial.lamports_per_signature(), - Some(lamports_per_signature) - ); - assert_eq!(partial.fee_payer_account(), None); - - // Add rent debits to ensure the rollback captures accounts without rent fees - let mut rent_debits = RentDebits::default(); - rent_debits.insert( - &from_address, - TEST_RENT_DEBIT, - rent_collected_from_account.lamports(), - ); - rent_debits.insert( - &nonce_address, - TEST_RENT_DEBIT, - rent_collected_nonce_account.lamports(), - ); - - // NonceFull create + NonceInfo impl - { - let message = new_sanitized_message(&instructions, Some(&from_address)); - let accounts = [ - ( - *message.account_keys().get(0).unwrap(), - rent_collected_from_account.clone(), - ), - ( - *message.account_keys().get(1).unwrap(), - rent_collected_nonce_account.clone(), - ), - (*message.account_keys().get(2).unwrap(), to_account.clone()), - ( - *message.account_keys().get(3).unwrap(), - recent_blockhashes_sysvar_account.clone(), - ), - ]; - - let full = - NonceFull::from_partial(partial.clone(), &message, &accounts, &rent_debits).unwrap(); - assert_eq!(*full.address(), nonce_address); - assert_eq!(*full.account(), rent_collected_nonce_account); - assert_eq!(full.lamports_per_signature(), Some(lamports_per_signature)); - assert_eq!( - full.fee_payer_account(), - Some(&from_account), - "rent debit should be refunded in captured fee account" - ); - } - - // Nonce account is fee-payer - { - let message = new_sanitized_message(&instructions, Some(&nonce_address)); - let accounts = [ - ( - *message.account_keys().get(0).unwrap(), - rent_collected_nonce_account, - ), - ( - *message.account_keys().get(1).unwrap(), - rent_collected_from_account, - ), - (*message.account_keys().get(2).unwrap(), to_account), - ( - *message.account_keys().get(3).unwrap(), - recent_blockhashes_sysvar_account, - ), - ]; - - let full = - NonceFull::from_partial(partial.clone(), &message, &accounts, &rent_debits).unwrap(); - assert_eq!(*full.address(), nonce_address); - assert_eq!(*full.account(), nonce_account); - assert_eq!(full.lamports_per_signature(), Some(lamports_per_signature)); - assert_eq!(full.fee_payer_account(), None); - } - - // NonceFull create, fee-payer not in account_keys fails - { - let message = new_sanitized_message(&instructions, Some(&nonce_address)); - assert_eq!( - NonceFull::from_partial(partial, &message, &[], &RentDebits::default()).unwrap_err(), - TransactionError::AccountNotFound, - ); - } -} - #[test] fn test_bank_unix_timestamp_from_genesis() { let (genesis_config, _mint_keypair) = create_genesis_config(1); diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 3dd7eee23..0f7947889 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -49,6 +49,7 @@ pub mod inline_spl_token; pub mod inline_spl_token_2022; pub mod loader_utils; pub mod non_circulating_supply; +pub mod nonce_info; pub mod partitioned_rewards; pub mod prioritization_fee; pub mod prioritization_fee_cache; diff --git a/runtime/src/nonce_info.rs b/runtime/src/nonce_info.rs new file mode 100644 index 000000000..16917be15 --- /dev/null +++ b/runtime/src/nonce_info.rs @@ -0,0 +1,273 @@ +use { + crate::rent_debits::RentDebits, + solana_sdk::{ + account::{AccountSharedData, ReadableAccount, WritableAccount}, + message::SanitizedMessage, + nonce_account, + pubkey::Pubkey, + transaction::{self, TransactionError}, + transaction_context::TransactionAccount, + }, +}; + +pub trait NonceInfo { + fn address(&self) -> &Pubkey; + fn account(&self) -> &AccountSharedData; + fn lamports_per_signature(&self) -> Option; + fn fee_payer_account(&self) -> Option<&AccountSharedData>; +} + +/// Holds limited nonce info available during transaction checks +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct NoncePartial { + address: Pubkey, + account: AccountSharedData, +} + +impl NoncePartial { + pub fn new(address: Pubkey, account: AccountSharedData) -> Self { + Self { address, account } + } +} + +impl NonceInfo for NoncePartial { + fn address(&self) -> &Pubkey { + &self.address + } + fn account(&self) -> &AccountSharedData { + &self.account + } + fn lamports_per_signature(&self) -> Option { + nonce_account::lamports_per_signature_of(&self.account) + } + fn fee_payer_account(&self) -> Option<&AccountSharedData> { + None + } +} + +/// Holds fee subtracted nonce info +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct NonceFull { + address: Pubkey, + account: AccountSharedData, + fee_payer_account: Option, +} + +impl NonceFull { + pub fn new( + address: Pubkey, + account: AccountSharedData, + fee_payer_account: Option, + ) -> Self { + Self { + address, + account, + fee_payer_account, + } + } + pub fn from_partial( + partial: NoncePartial, + message: &SanitizedMessage, + accounts: &[TransactionAccount], + rent_debits: &RentDebits, + ) -> transaction::Result { + let fee_payer = (0..message.account_keys().len()).find_map(|i| { + if let Some((k, a)) = &accounts.get(i) { + if message.is_non_loader_key(i) { + return Some((k, a)); + } + } + None + }); + + if let Some((fee_payer_address, fee_payer_account)) = fee_payer { + let mut fee_payer_account = fee_payer_account.clone(); + let rent_debit = rent_debits.get_account_rent_debit(fee_payer_address); + fee_payer_account.set_lamports(fee_payer_account.lamports().saturating_add(rent_debit)); + + let nonce_address = *partial.address(); + if *fee_payer_address == nonce_address { + Ok(Self::new(nonce_address, fee_payer_account, None)) + } else { + Ok(Self::new( + nonce_address, + partial.account().clone(), + Some(fee_payer_account), + )) + } + } else { + Err(TransactionError::AccountNotFound) + } + } +} + +impl NonceInfo for NonceFull { + fn address(&self) -> &Pubkey { + &self.address + } + fn account(&self) -> &AccountSharedData { + &self.account + } + fn lamports_per_signature(&self) -> Option { + nonce_account::lamports_per_signature_of(&self.account) + } + fn fee_payer_account(&self) -> Option<&AccountSharedData> { + self.fee_payer_account.as_ref() + } +} + +#[cfg(test)] +mod tests { + use { + super::*, + solana_sdk::{ + hash::Hash, + instruction::Instruction, + message::Message, + nonce::{self, state::DurableNonce}, + signature::{keypair_from_seed, Signer}, + system_instruction, system_program, + }, + }; + + fn new_sanitized_message( + instructions: &[Instruction], + payer: Option<&Pubkey>, + ) -> SanitizedMessage { + Message::new(instructions, payer).try_into().unwrap() + } + + #[test] + fn test_nonce_info() { + let lamports_per_signature = 42; + + let nonce_authority = keypair_from_seed(&[0; 32]).unwrap(); + let nonce_address = nonce_authority.pubkey(); + let from = keypair_from_seed(&[1; 32]).unwrap(); + let from_address = from.pubkey(); + let to_address = Pubkey::new_unique(); + + let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique()); + let nonce_account = AccountSharedData::new_data( + 43, + &nonce::state::Versions::new(nonce::State::Initialized(nonce::state::Data::new( + Pubkey::default(), + durable_nonce, + lamports_per_signature, + ))), + &system_program::id(), + ) + .unwrap(); + let from_account = AccountSharedData::new(44, 0, &Pubkey::default()); + let to_account = AccountSharedData::new(45, 0, &Pubkey::default()); + let recent_blockhashes_sysvar_account = AccountSharedData::new(4, 0, &Pubkey::default()); + + const TEST_RENT_DEBIT: u64 = 1; + let rent_collected_nonce_account = { + let mut account = nonce_account.clone(); + account.set_lamports(nonce_account.lamports() - TEST_RENT_DEBIT); + account + }; + let rent_collected_from_account = { + let mut account = from_account.clone(); + account.set_lamports(from_account.lamports() - TEST_RENT_DEBIT); + account + }; + + let instructions = vec![ + system_instruction::advance_nonce_account(&nonce_address, &nonce_authority.pubkey()), + system_instruction::transfer(&from_address, &to_address, 42), + ]; + + // NoncePartial create + NonceInfo impl + let partial = NoncePartial::new(nonce_address, rent_collected_nonce_account.clone()); + assert_eq!(*partial.address(), nonce_address); + assert_eq!(*partial.account(), rent_collected_nonce_account); + assert_eq!( + partial.lamports_per_signature(), + Some(lamports_per_signature) + ); + assert_eq!(partial.fee_payer_account(), None); + + // Add rent debits to ensure the rollback captures accounts without rent fees + let mut rent_debits = RentDebits::default(); + rent_debits.insert( + &from_address, + TEST_RENT_DEBIT, + rent_collected_from_account.lamports(), + ); + rent_debits.insert( + &nonce_address, + TEST_RENT_DEBIT, + rent_collected_nonce_account.lamports(), + ); + + // NonceFull create + NonceInfo impl + { + let message = new_sanitized_message(&instructions, Some(&from_address)); + let accounts = [ + ( + *message.account_keys().get(0).unwrap(), + rent_collected_from_account.clone(), + ), + ( + *message.account_keys().get(1).unwrap(), + rent_collected_nonce_account.clone(), + ), + (*message.account_keys().get(2).unwrap(), to_account.clone()), + ( + *message.account_keys().get(3).unwrap(), + recent_blockhashes_sysvar_account.clone(), + ), + ]; + + let full = NonceFull::from_partial(partial.clone(), &message, &accounts, &rent_debits) + .unwrap(); + assert_eq!(*full.address(), nonce_address); + assert_eq!(*full.account(), rent_collected_nonce_account); + assert_eq!(full.lamports_per_signature(), Some(lamports_per_signature)); + assert_eq!( + full.fee_payer_account(), + Some(&from_account), + "rent debit should be refunded in captured fee account" + ); + } + + // Nonce account is fee-payer + { + let message = new_sanitized_message(&instructions, Some(&nonce_address)); + let accounts = [ + ( + *message.account_keys().get(0).unwrap(), + rent_collected_nonce_account, + ), + ( + *message.account_keys().get(1).unwrap(), + rent_collected_from_account, + ), + (*message.account_keys().get(2).unwrap(), to_account), + ( + *message.account_keys().get(3).unwrap(), + recent_blockhashes_sysvar_account, + ), + ]; + + let full = NonceFull::from_partial(partial.clone(), &message, &accounts, &rent_debits) + .unwrap(); + assert_eq!(*full.address(), nonce_address); + assert_eq!(*full.account(), nonce_account); + assert_eq!(full.lamports_per_signature(), Some(lamports_per_signature)); + assert_eq!(full.fee_payer_account(), None); + } + + // NonceFull create, fee-payer not in account_keys fails + { + let message = new_sanitized_message(&instructions, Some(&nonce_address)); + assert_eq!( + NonceFull::from_partial(partial, &message, &[], &RentDebits::default()) + .unwrap_err(), + TransactionError::AccountNotFound, + ); + } + } +}