diff --git a/runtime/src/bank/check_transactions.rs b/runtime/src/bank/check_transactions.rs index 41ac5c04620051..2170933a259c96 100644 --- a/runtime/src/bank/check_transactions.rs +++ b/runtime/src/bank/check_transactions.rs @@ -129,30 +129,27 @@ impl Bank { next_durable_nonce: &DurableNonce, ) -> Option<(NonceInfo, u64)> { let nonce_is_advanceable = message.recent_blockhash() != next_durable_nonce.as_hash(); - if nonce_is_advanceable { - if let Some((nonce_address, mut nonce_account, nonce_data)) = - self.load_message_nonce_account(message) - { - let lamports_per_signature = nonce_data.get_lamports_per_signature(); - let next_nonce_state = NonceState::new_initialized( - &nonce_data.authority, - *next_durable_nonce, - lamports_per_signature, - ); - nonce_account - .set_state(&NonceVersions::new(next_nonce_state)) - .ok()?; - - Some(( - NonceInfo::new(nonce_address, nonce_account), - lamports_per_signature, - )) - } else { - None - } - } else { - None + if !nonce_is_advanceable { + return None; } + + let (nonce_address, mut nonce_account, nonce_data) = + self.load_message_nonce_account(message)?; + + let lamports_per_signature = self.get_lamports_per_signature(); + let next_nonce_state = NonceState::new_initialized( + &nonce_data.authority, + *next_durable_nonce, + lamports_per_signature, + ); + nonce_account + .set_state(&NonceVersions::new(next_nonce_state)) + .ok()?; + + Some(( + NonceInfo::new(nonce_address, nonce_account), + lamports_per_signature, + )) } pub(super) fn load_message_nonce_account( @@ -238,6 +235,7 @@ mod tests { .unwrap(); let custodian_pubkey = custodian_keypair.pubkey(); let nonce_pubkey = nonce_keypair.pubkey(); + let stale_lamports_per_signature = 42; let nonce_hash = get_nonce_blockhash(&bank, &nonce_pubkey).unwrap(); let message = new_sanitized_message(Message::new_with_blockhash( @@ -248,18 +246,32 @@ mod tests { Some(&custodian_pubkey), &nonce_hash, )); - let nonce_account = bank.get_account(&nonce_pubkey).unwrap(); + + // set a spurious lamports_per_signature value + let mut nonce_account = bank.get_account(&nonce_pubkey).unwrap(); let nonce_data = get_nonce_data_from_account(&nonce_account).unwrap(); + nonce_account + .set_state(&NonceVersions::new(NonceState::new_initialized( + &nonce_data.authority, + nonce_data.durable_nonce, + stale_lamports_per_signature, + ))) + .unwrap(); + bank.store_account(&nonce_pubkey, &nonce_account); - let lamports_per_signature = nonce_data.get_lamports_per_signature(); + let nonce_account = bank.get_account(&nonce_pubkey).unwrap(); + let current_lamports_per_signature = bank.get_lamports_per_signature(); let mut expected_nonce_info = NonceInfo::new(nonce_pubkey, nonce_account); expected_nonce_info - .try_advance_nonce(bank.next_durable_nonce(), lamports_per_signature) + .try_advance_nonce(bank.next_durable_nonce(), current_lamports_per_signature) .unwrap(); + // we now expect to: + // * advance the nonce to the current value + // * set the bank's lamports_per_signature in the nonce data assert_eq!( bank.check_load_and_advance_message_nonce_account(&message, &bank.next_durable_nonce()), - Some((expected_nonce_info, lamports_per_signature)), + Some((expected_nonce_info, current_lamports_per_signature)), ); } diff --git a/svm/src/rollback_accounts.rs b/svm/src/rollback_accounts.rs index cbb1903936d6a9..1a9a764e131186 100644 --- a/svm/src/rollback_accounts.rs +++ b/svm/src/rollback_accounts.rs @@ -51,6 +51,10 @@ impl RollbackAccounts { if let Some(nonce) = nonce { if &fee_payer_address == nonce.address() { + // `nonce` contains an AccountSharedData which has already been advanced to the current DurableNonce + // `fee_payer_account` is an AccountSharedData as it currently exists on-chain + // thus if the nonce account is being used as the fee payer, we need to update that data here + // so we capture both the data change for the nonce and the lamports/rent epoch change for the fee payer fee_payer_account.set_data_from_slice(nonce.account().data()); RollbackAccounts::SameNonceAndFeePayer { @@ -65,7 +69,7 @@ impl RollbackAccounts { } else { // When rolling back failed transactions which don't use nonces, the // runtime should not update the fee payer's rent epoch so reset the - // rollback fee payer acocunt's rent epoch to its originally loaded + // rollback fee payer account's rent epoch to its originally loaded // rent epoch value. In the future, a feature gate could be used to // alter this behavior such that rent epoch updates are handled the // same for both nonce and non-nonce failed transactions. diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index e2d75bfa09c129..30ed3ce352b925 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -1001,7 +1001,7 @@ mod tests { fee_calculator::FeeCalculator, hash::Hash, message::{LegacyMessage, Message, MessageHeader, SanitizedMessage}, - nonce::{self, state::DurableNonce}, + nonce, rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH}, rent_debits::RentDebits, reserved_account_keys::ReservedAccountKeys, @@ -2180,12 +2180,10 @@ mod tests { let mut error_counters = TransactionErrorMetrics::default(); let batch_processor = TransactionBatchProcessor::::default(); - let durable_nonce = DurableNonce::from_blockhash(&last_blockhash); - let mut nonce_info = NonceInfo::new(*fee_payer_address, fee_payer_account.clone()); - nonce_info - .try_advance_nonce(durable_nonce, lamports_per_signature) - .unwrap(); - let nonce = Some(nonce_info); + let nonce = Some(NonceInfo::new( + *fee_payer_address, + fee_payer_account.clone(), + )); let result = batch_processor.validate_transaction_fee_payer( &mock_bank, diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs index 5e03430d1d97ee..f2b3f77162197e 100644 --- a/svm/tests/integration_test.rs +++ b/svm/tests/integration_test.rs @@ -639,7 +639,10 @@ fn simple_transfer() -> Vec { vec![test_entry] } -fn simple_nonce_fee_only(enable_fee_only_transactions: bool) -> Vec { +fn simple_nonce_fee_only( + enable_fee_only_transactions: bool, + fee_paying_nonce: bool, +) -> Vec { let mut test_entry = SvmTestEntry::default(); if enable_fee_only_transactions { test_entry @@ -655,19 +658,31 @@ fn simple_nonce_fee_only(enable_fee_only_transactions: bool) -> Vec Vec Vec Vec Vec) { for test_entry in test_entries { execute_test_entry(test_entry); diff --git a/svm/tests/mock_bank.rs b/svm/tests/mock_bank.rs index 7f89180f951c5c..836d007097d02c 100644 --- a/svm/tests/mock_bank.rs +++ b/svm/tests/mock_bank.rs @@ -217,7 +217,8 @@ pub fn create_executable_environment( .unwrap() .insert(Rent::id(), account_data); - // advance nonce asserts recent blockhashes is non-empty but then just gets the blockhash from ic + // SystemInstruction::AdvanceNonceAccount asserts RecentBlockhashes is non-empty + // but then just gets the blockhash from InvokeContext. so the sysvar doesnt need real entries #[allow(deprecated)] let recent_blockhashes = vec![BlockhashesEntry::default()];