From 96249691b4b7c873220b27376f271ead38392541 Mon Sep 17 00:00:00 2001 From: Joe C Date: Fri, 8 Nov 2024 10:07:52 +0400 Subject: [PATCH] SVM: Clarify usage of `lamports_per_signature` (#3216) * SVM: use lps from `FeeStructure` directly * SVM: rename `lamports_per_signature` to `blockhash_lamports_per_signature` * program-runtime: rename `lamports_per_signature` to `blockhash_lamports_per_signature` --- program-runtime/src/invoke_context.rs | 8 +-- programs/bpf_loader/src/syscalls/mod.rs | 4 +- programs/system/src/system_instruction.rs | 45 ++++++++---- runtime/src/bank.rs | 7 +- .../bank/builtins/core_bpf_migration/mod.rs | 2 +- .../json-rpc/server/src/rpc_process.rs | 4 +- svm/examples/paytube/src/lib.rs | 10 +-- svm/src/message_processor.rs | 14 ++-- svm/src/transaction_processor.rs | 71 +++++++++++-------- svm/tests/conformance.rs | 4 +- svm/tests/integration_test.rs | 3 +- 11 files changed, 105 insertions(+), 67 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 1beda1c1d8a288..a54e0839ab8036 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -150,27 +150,27 @@ impl BpfAllocator { pub struct EnvironmentConfig<'a> { pub blockhash: Hash, + pub blockhash_lamports_per_signature: u64, epoch_total_stake: Option, epoch_vote_accounts: Option<&'a VoteAccountsHashMap>, pub feature_set: Arc, - pub lamports_per_signature: u64, sysvar_cache: &'a SysvarCache, } impl<'a> EnvironmentConfig<'a> { pub fn new( blockhash: Hash, + blockhash_lamports_per_signature: u64, epoch_total_stake: Option, epoch_vote_accounts: Option<&'a VoteAccountsHashMap>, feature_set: Arc, - lamports_per_signature: u64, sysvar_cache: &'a SysvarCache, ) -> Self { Self { blockhash, + blockhash_lamports_per_signature, epoch_total_stake, epoch_vote_accounts, feature_set, - lamports_per_signature, sysvar_cache, } } @@ -764,10 +764,10 @@ macro_rules! with_mock_invoke_context { }); let environment_config = EnvironmentConfig::new( Hash::default(), + 0, None, None, Arc::new(FeatureSet::all_enabled()), - 0, &sysvar_cache, ); let mut program_cache_for_tx_batch = ProgramCacheForTxBatch::default(); diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index b131ec712da101..334272896ad177 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -4800,10 +4800,10 @@ mod tests { with_mock_invoke_context!(invoke_context, transaction_context, vec![]); invoke_context.environment_config = EnvironmentConfig::new( Hash::default(), + 0, Some(expected_total_stake), None, // Vote accounts are not needed for this test. Arc::::default(), - 0, &sysvar_cache, ); @@ -4854,10 +4854,10 @@ mod tests { with_mock_invoke_context!(invoke_context, transaction_context, vec![]); invoke_context.environment_config = EnvironmentConfig::new( Hash::default(), + 0, None, // Total stake is not needed for this test. Some(&vote_accounts_map), Arc::::default(), - 0, &sysvar_cache, ); diff --git a/programs/system/src/system_instruction.rs b/programs/system/src/system_instruction.rs index c75f8d7d0c9f86..18e27c0a71ba74 100644 --- a/programs/system/src/system_instruction.rs +++ b/programs/system/src/system_instruction.rs @@ -56,7 +56,9 @@ pub fn advance_nonce_account( let new_data = nonce::state::Data::new( data.authority, next_durable_nonce, - invoke_context.environment_config.lamports_per_signature, + invoke_context + .environment_config + .blockhash_lamports_per_signature, ); account.set_state(&Versions::new(State::Initialized(new_data))) } @@ -185,7 +187,9 @@ pub fn initialize_nonce_account( let data = nonce::state::Data::new( *nonce_authority, durable_nonce, - invoke_context.environment_config.lamports_per_signature, + invoke_context + .environment_config + .blockhash_lamports_per_signature, ); let state = State::Initialized(data); account.set_state(&Versions::new(state)) @@ -312,8 +316,9 @@ mod test { ($invoke_context:expr, $seed:expr) => { $invoke_context.environment_config.blockhash = hash(&bincode::serialize(&$seed).unwrap()); - $invoke_context.environment_config.lamports_per_signature = - ($seed as u64).saturating_mul(100); + $invoke_context + .environment_config + .blockhash_lamports_per_signature = ($seed as u64).saturating_mul(100); }; } @@ -350,7 +355,9 @@ mod test { let data = nonce::state::Data::new( data.authority, DurableNonce::from_blockhash(&invoke_context.environment_config.blockhash), - invoke_context.environment_config.lamports_per_signature, + invoke_context + .environment_config + .blockhash_lamports_per_signature, ); // First nonce instruction drives state from Uninitialized to Initialized assert_eq!(versions.state(), &State::Initialized(data.clone())); @@ -360,7 +367,9 @@ mod test { let data = nonce::state::Data::new( data.authority, DurableNonce::from_blockhash(&invoke_context.environment_config.blockhash), - invoke_context.environment_config.lamports_per_signature, + invoke_context + .environment_config + .blockhash_lamports_per_signature, ); // Second nonce instruction consumes and replaces stored nonce assert_eq!(versions.state(), &State::Initialized(data.clone())); @@ -370,7 +379,9 @@ mod test { let data = nonce::state::Data::new( data.authority, DurableNonce::from_blockhash(&invoke_context.environment_config.blockhash), - invoke_context.environment_config.lamports_per_signature, + invoke_context + .environment_config + .blockhash_lamports_per_signature, ); // Third nonce instruction for fun and profit assert_eq!(versions.state(), &State::Initialized(data)); @@ -429,7 +440,9 @@ mod test { let data = nonce::state::Data::new( authority, DurableNonce::from_blockhash(&invoke_context.environment_config.blockhash), - invoke_context.environment_config.lamports_per_signature, + invoke_context + .environment_config + .blockhash_lamports_per_signature, ); assert_eq!(versions.state(), &State::Initialized(data)); // Nonce account did not sign @@ -741,7 +754,9 @@ mod test { let data = nonce::state::Data::new( authority, DurableNonce::from_blockhash(&invoke_context.environment_config.blockhash), - invoke_context.environment_config.lamports_per_signature, + invoke_context + .environment_config + .blockhash_lamports_per_signature, ); assert_eq!(versions.state(), &State::Initialized(data.clone())); let withdraw_lamports = 42; @@ -770,7 +785,9 @@ mod test { let data = nonce::state::Data::new( data.authority, DurableNonce::from_blockhash(&invoke_context.environment_config.blockhash), - invoke_context.environment_config.lamports_per_signature, + invoke_context + .environment_config + .blockhash_lamports_per_signature, ); assert_eq!(versions.state(), &State::Initialized(data)); assert_eq!(nonce_account.get_lamports(), from_expect_lamports); @@ -962,7 +979,9 @@ mod test { let data = nonce::state::Data::new( authorized, DurableNonce::from_blockhash(&invoke_context.environment_config.blockhash), - invoke_context.environment_config.lamports_per_signature, + invoke_context + .environment_config + .blockhash_lamports_per_signature, ); assert_eq!(result, Ok(())); let versions = nonce_account.get_state::().unwrap(); @@ -1031,7 +1050,9 @@ mod test { let data = nonce::state::Data::new( authority, DurableNonce::from_blockhash(&invoke_context.environment_config.blockhash), - invoke_context.environment_config.lamports_per_signature, + invoke_context + .environment_config + .blockhash_lamports_per_signature, ); authorize_nonce_account(&mut nonce_account, &authority, &signers, &invoke_context).unwrap(); let versions = nonce_account.get_state::().unwrap(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 9f4697ff270b8e..2858836ae72f4e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3709,16 +3709,17 @@ impl Bank { )); timings.saturating_add_in_place(ExecuteTimingType::CheckUs, check_us); - let (blockhash, lamports_per_signature) = self.last_blockhash_and_lamports_per_signature(); + let (blockhash, blockhash_lamports_per_signature) = + self.last_blockhash_and_lamports_per_signature(); let rent_collector_with_metrics = RentCollectorWithMetrics::new(self.rent_collector.clone()); let processing_environment = TransactionProcessingEnvironment { blockhash, + blockhash_lamports_per_signature, epoch_total_stake: Some(self.get_current_epoch_total_stake()), epoch_vote_accounts: Some(self.get_current_epoch_vote_accounts()), feature_set: Arc::clone(&self.feature_set), - fee_structure: Some(&self.fee_structure), - lamports_per_signature, + fee_lamports_per_signature: self.fee_structure.lamports_per_signature, rent_collector: Some(&rent_collector_with_metrics), }; diff --git a/runtime/src/bank/builtins/core_bpf_migration/mod.rs b/runtime/src/bank/builtins/core_bpf_migration/mod.rs index 023f27ec8e0794..2785acfd2f7b71 100644 --- a/runtime/src/bank/builtins/core_bpf_migration/mod.rs +++ b/runtime/src/bank/builtins/core_bpf_migration/mod.rs @@ -201,10 +201,10 @@ impl Bank { &mut program_cache_for_tx_batch, EnvironmentConfig::new( Hash::default(), + 0, None, None, self.feature_set.clone(), - 0, &sysvar_cache, ), None, diff --git a/svm/examples/json-rpc/server/src/rpc_process.rs b/svm/examples/json-rpc/server/src/rpc_process.rs index 49f282d5c33f75..fdea14aab20369 100644 --- a/svm/examples/json-rpc/server/src/rpc_process.rs +++ b/svm/examples/json-rpc/server/src/rpc_process.rs @@ -546,11 +546,11 @@ impl JsonRpcRequestProcessor { let (blockhash, lamports_per_signature) = self.last_blockhash_and_lamports_per_signature(); let processing_environment = TransactionProcessingEnvironment { blockhash, + blockhash_lamports_per_signature: lamports_per_signature, epoch_total_stake: self.epoch_total_stake(Epoch::default()), epoch_vote_accounts: self.epoch_vote_accounts(Epoch::default()), feature_set: Arc::clone(&bank.feature_set), - fee_structure: None, - lamports_per_signature, + fee_lamports_per_signature: lamports_per_signature, rent_collector: None, }; diff --git a/svm/examples/paytube/src/lib.rs b/svm/examples/paytube/src/lib.rs index 7549e0261c6866..6db007ceb22c93 100644 --- a/svm/examples/paytube/src/lib.rs +++ b/svm/examples/paytube/src/lib.rs @@ -117,7 +117,6 @@ impl PayTubeChannel { let compute_budget = ComputeBudget::default(); let feature_set = FeatureSet::all_enabled(); let fee_structure = FeeStructure::default(); - let lamports_per_signature = fee_structure.lamports_per_signature; let rent_collector = RentCollector::default(); // PayTube loader/callback implementation. @@ -148,11 +147,11 @@ impl PayTubeChannel { // Again, these can be configurable or hoisted from the cluster. let processing_environment = TransactionProcessingEnvironment { blockhash: Hash::default(), + blockhash_lamports_per_signature: fee_structure.lamports_per_signature, epoch_total_stake: None, epoch_vote_accounts: None, feature_set: Arc::new(feature_set), - fee_structure: Some(&fee_structure), - lamports_per_signature, + fee_lamports_per_signature: fee_structure.lamports_per_signature, rent_collector: Some(&rent_collector), }; @@ -177,7 +176,10 @@ impl PayTubeChannel { let results = processor.load_and_execute_sanitized_transactions( &account_loader, &svm_transactions, - get_transaction_check_results(svm_transactions.len(), lamports_per_signature), + get_transaction_check_results( + svm_transactions.len(), + fee_structure.lamports_per_signature, + ), &processing_environment, &processing_config, ); diff --git a/svm/src/message_processor.rs b/svm/src/message_processor.rs index 589e1d1362a6bf..87ec0cf0e49d97 100644 --- a/svm/src/message_processor.rs +++ b/svm/src/message_processor.rs @@ -257,10 +257,10 @@ mod tests { let sysvar_cache = SysvarCache::default(); let environment_config = EnvironmentConfig::new( Hash::default(), + 0, None, None, Arc::new(FeatureSet::all_enabled()), - 0, &sysvar_cache, ); let mut invoke_context = InvokeContext::new( @@ -311,10 +311,10 @@ mod tests { )); let environment_config = EnvironmentConfig::new( Hash::default(), + 0, None, None, Arc::new(FeatureSet::all_enabled()), - 0, &sysvar_cache, ); let mut invoke_context = InvokeContext::new( @@ -355,10 +355,10 @@ mod tests { )); let environment_config = EnvironmentConfig::new( Hash::default(), + 0, None, None, Arc::new(FeatureSet::all_enabled()), - 0, &sysvar_cache, ); let mut invoke_context = InvokeContext::new( @@ -490,10 +490,10 @@ mod tests { let sysvar_cache = SysvarCache::default(); let environment_config = EnvironmentConfig::new( Hash::default(), + 0, None, None, Arc::new(FeatureSet::all_enabled()), - 0, &sysvar_cache, ); let mut invoke_context = InvokeContext::new( @@ -529,10 +529,10 @@ mod tests { )); let environment_config = EnvironmentConfig::new( Hash::default(), + 0, None, None, Arc::new(FeatureSet::all_enabled()), - 0, &sysvar_cache, ); let mut invoke_context = InvokeContext::new( @@ -565,10 +565,10 @@ mod tests { )); let environment_config = EnvironmentConfig::new( Hash::default(), + 0, None, None, Arc::new(FeatureSet::all_enabled()), - 0, &sysvar_cache, ); let mut invoke_context = InvokeContext::new( @@ -662,10 +662,10 @@ mod tests { ); let environment_config = EnvironmentConfig::new( Hash::default(), + 0, None, None, Arc::new(FeatureSet::all_enabled()), - 0, &sysvar_cache, ); let mut invoke_context = InvokeContext::new( diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 2f92d376152363..4c0373971dfec9 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -121,24 +121,42 @@ pub struct TransactionProcessingConfig<'a> { } /// Runtime environment for transaction batch processing. -#[derive(Default)] pub struct TransactionProcessingEnvironment<'a> { /// The blockhash to use for the transaction batch. pub blockhash: Hash, + /// Lamports per signature that corresponds to this blockhash. + /// + /// Note: This value is primarily used for nonce accounts. If set to zero, + /// it will disable transaction fees. However, any non-zero value will not + /// change transaction fees. For this reason, it is recommended to use the + /// `fee_per_signature` field to adjust transaction fees. + pub blockhash_lamports_per_signature: u64, /// The total stake for the current epoch. pub epoch_total_stake: Option, /// The vote accounts for the current epoch. pub epoch_vote_accounts: Option<&'a VoteAccountsHashMap>, /// Runtime feature set to use for the transaction batch. pub feature_set: Arc, - /// Fee structure to use for assessing transaction fees. - pub fee_structure: Option<&'a FeeStructure>, - /// Lamports per signature to charge per transaction. - pub lamports_per_signature: u64, + /// Transaction fee to charge per signature, in lamports. + pub fee_lamports_per_signature: u64, /// Rent collector to use for the transaction batch. pub rent_collector: Option<&'a dyn SVMRentCollector>, } +impl Default for TransactionProcessingEnvironment<'_> { + fn default() -> Self { + Self { + blockhash: Hash::default(), + blockhash_lamports_per_signature: 0, + epoch_total_stake: None, + epoch_vote_accounts: None, + feature_set: Arc::::default(), + fee_lamports_per_signature: FeeStructure::default().lamports_per_signature, // <-- Default fee. + rent_collector: None, + } + } +} + #[cfg_attr(feature = "frozen-abi", derive(AbiExample))] #[cfg_attr( feature = "dev-context-only-utils", @@ -270,9 +288,7 @@ impl TransactionBatchProcessor { sanitized_txs, check_results, &environment.feature_set, - environment - .fee_structure - .unwrap_or(&FeeStructure::default()), + environment.fee_lamports_per_signature, environment .rent_collector .unwrap_or(&RentCollector::default()), @@ -408,7 +424,7 @@ impl TransactionBatchProcessor { sanitized_txs: &[impl core::borrow::Borrow], check_results: Vec, feature_set: &FeatureSet, - fee_structure: &FeeStructure, + fee_lamports_per_signature: u64, rent_collector: &dyn SVMRentCollector, error_counters: &mut TransactionErrorMetrics, ) -> Vec { @@ -424,7 +440,7 @@ impl TransactionBatchProcessor { message, checked_details, feature_set, - fee_structure, + fee_lamports_per_signature, rent_collector, error_counters, ) @@ -443,7 +459,7 @@ impl TransactionBatchProcessor { message: &impl SVMMessage, checked_details: CheckedTransactionDetails, feature_set: &FeatureSet, - fee_structure: &FeeStructure, + fee_lamports_per_signature: u64, rent_collector: &dyn SVMRentCollector, error_counters: &mut TransactionErrorMetrics, ) -> transaction::Result { @@ -491,7 +507,7 @@ impl TransactionBatchProcessor { let fee_details = solana_fee::calculate_fee_details( message, lamports_per_signature == 0, - fee_structure.lamports_per_signature, + fee_lamports_per_signature, fee_budget_limits.prioritization_fee, feature_set.is_active(&remove_rounding_in_fee_calculation::id()), ); @@ -807,9 +823,6 @@ impl TransactionBatchProcessor { None }; - let blockhash = environment.blockhash; - let lamports_per_signature = environment.lamports_per_signature; - let mut executed_units = 0u64; let sysvar_cache = &self.sysvar_cache.read().unwrap(); @@ -817,11 +830,11 @@ impl TransactionBatchProcessor { &mut transaction_context, program_cache_for_tx_batch, EnvironmentConfig::new( - blockhash, + environment.blockhash, + environment.blockhash_lamports_per_signature, environment.epoch_total_stake, environment.epoch_vote_accounts, Arc::clone(&environment.feature_set), - lamports_per_signature, sysvar_cache, ), log_collector.clone(), @@ -1044,7 +1057,7 @@ mod tests { bpf_loader, compute_budget::ComputeBudgetInstruction, epoch_schedule::EpochSchedule, - fee::FeeDetails, + fee::{FeeDetails, FeeStructure}, fee_calculator::FeeCalculator, hash::Hash, message::{LegacyMessage, Message, MessageHeader, SanitizedMessage}, @@ -1978,7 +1991,7 @@ mod tests { lamports_per_signature, }, &FeatureSet::default(), - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &rent_collector, &mut error_counters, ); @@ -2056,7 +2069,7 @@ mod tests { lamports_per_signature, }, &FeatureSet::default(), - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &rent_collector, &mut error_counters, ); @@ -2107,7 +2120,7 @@ mod tests { lamports_per_signature, }, &FeatureSet::default(), - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &RentCollector::default(), &mut error_counters, ); @@ -2141,7 +2154,7 @@ mod tests { lamports_per_signature, }, &FeatureSet::default(), - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &RentCollector::default(), &mut error_counters, ); @@ -2179,7 +2192,7 @@ mod tests { lamports_per_signature, }, &FeatureSet::default(), - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &rent_collector, &mut error_counters, ); @@ -2215,7 +2228,7 @@ mod tests { lamports_per_signature, }, &FeatureSet::default(), - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &RentCollector::default(), &mut error_counters, ); @@ -2247,7 +2260,7 @@ mod tests { lamports_per_signature, }, &FeatureSet::default(), - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &RentCollector::default(), &mut error_counters, ); @@ -2314,7 +2327,7 @@ mod tests { lamports_per_signature, }, &feature_set, - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &rent_collector, &mut error_counters, ); @@ -2376,7 +2389,7 @@ mod tests { lamports_per_signature, }, &feature_set, - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &rent_collector, &mut error_counters, ); @@ -2430,7 +2443,7 @@ mod tests { lamports_per_signature, }, &FeatureSet::default(), - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &rent_collector, &mut error_counters, ); @@ -2478,7 +2491,7 @@ mod tests { lamports_per_signature: 5000, }, &FeatureSet::default(), - &FeeStructure::default(), + FeeStructure::default().lamports_per_signature, &RentCollector::default(), &mut TransactionErrorMetrics::default(), ) diff --git a/svm/tests/conformance.rs b/svm/tests/conformance.rs index 3828c6cdfc71a9..44dc89a02acebd 100644 --- a/svm/tests/conformance.rs +++ b/svm/tests/conformance.rs @@ -305,7 +305,7 @@ fn run_fixture(fixture: InstrFixture, filename: OsString, execute_as_instr: bool transaction_check, &TransactionProcessingEnvironment { blockhash, - lamports_per_signature, + blockhash_lamports_per_signature: lamports_per_signature, ..Default::default() }, &processor_config, @@ -452,10 +452,10 @@ fn execute_fixture_as_instr( let sysvar_cache = &batch_processor.sysvar_cache(); let env_config = EnvironmentConfig::new( Hash::default(), + 0, None, None, mock_bank.feature_set.clone(), - 0, sysvar_cache, ); diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 32802953b8374c..99b98214c1279a 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -112,7 +112,8 @@ impl SvmTestEnvironment<'_> { let processing_environment = TransactionProcessingEnvironment { blockhash: LAST_BLOCKHASH, feature_set: feature_set.into(), - lamports_per_signature: LAMPORTS_PER_SIGNATURE, + blockhash_lamports_per_signature: LAMPORTS_PER_SIGNATURE, + fee_lamports_per_signature: LAMPORTS_PER_SIGNATURE, ..TransactionProcessingEnvironment::default() };