Fix nonce fee_calculator overwrite (#10973)
* Add failing test * Pass fee_calculator to prepare_if_nonce_account; only overwrite in error case
This commit is contained in:
parent
16eeea4f82
commit
25228ca957
|
@ -17,6 +17,7 @@ use rayon::slice::ParallelSliceMut;
|
|||
use solana_sdk::{
|
||||
account::Account,
|
||||
clock::Slot,
|
||||
fee_calculator::FeeCalculator,
|
||||
hash::Hash,
|
||||
message::Message,
|
||||
native_loader, nonce,
|
||||
|
@ -664,7 +665,7 @@ impl Accounts {
|
|||
res: &[TransactionProcessResult],
|
||||
loaded: &mut [(Result<TransactionLoadResult>, Option<HashAgeKind>)],
|
||||
rent_collector: &RentCollector,
|
||||
last_blockhash: &Hash,
|
||||
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
|
||||
) {
|
||||
let accounts_to_store = self.collect_accounts_to_store(
|
||||
txs,
|
||||
|
@ -672,7 +673,7 @@ impl Accounts {
|
|||
res,
|
||||
loaded,
|
||||
rent_collector,
|
||||
last_blockhash,
|
||||
last_blockhash_with_fee_calculator,
|
||||
);
|
||||
self.accounts_db.store(slot, &accounts_to_store);
|
||||
}
|
||||
|
@ -698,7 +699,7 @@ impl Accounts {
|
|||
res: &'a [TransactionProcessResult],
|
||||
loaded: &'a mut [(Result<TransactionLoadResult>, Option<HashAgeKind>)],
|
||||
rent_collector: &RentCollector,
|
||||
last_blockhash: &Hash,
|
||||
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
|
||||
) -> Vec<(&'a Pubkey, &'a Account)> {
|
||||
let mut accounts = Vec::with_capacity(loaded.len());
|
||||
for (i, ((raccs, _hash_age_kind), tx)) in loaded
|
||||
|
@ -734,7 +735,7 @@ impl Accounts {
|
|||
key,
|
||||
res,
|
||||
maybe_nonce,
|
||||
last_blockhash,
|
||||
last_blockhash_with_fee_calculator,
|
||||
);
|
||||
if message.is_writable(i) {
|
||||
if account.rent_epoch == 0 {
|
||||
|
@ -1689,7 +1690,7 @@ mod tests {
|
|||
&loaders,
|
||||
&mut loaded,
|
||||
&rent_collector,
|
||||
&Hash::default(),
|
||||
&(Hash::default(), FeeCalculator::default()),
|
||||
);
|
||||
assert_eq!(collected_accounts.len(), 2);
|
||||
assert!(collected_accounts
|
||||
|
|
|
@ -1578,7 +1578,7 @@ impl Bank {
|
|||
executed,
|
||||
loaded_accounts,
|
||||
&self.rent_collector,
|
||||
&self.last_blockhash(),
|
||||
&self.last_blockhash_with_fee_calculator(),
|
||||
);
|
||||
self.collect_rent(executed, loaded_accounts);
|
||||
|
||||
|
@ -6680,6 +6680,70 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_nonce_fee_calculator_updates() {
|
||||
let (mut genesis_config, mint_keypair) = create_genesis_config(1_000_000);
|
||||
genesis_config.rent.lamports_per_byte_year = 0;
|
||||
let mut bank = Arc::new(Bank::new(&genesis_config));
|
||||
|
||||
// Deliberately use bank 0 to initialize nonce account, so that nonce account fee_calculator indicates 0 fees
|
||||
let (custodian_keypair, nonce_keypair) =
|
||||
nonce_setup(&mut bank, &mint_keypair, 500_000, 100_000, None).unwrap();
|
||||
let custodian_pubkey = custodian_keypair.pubkey();
|
||||
let nonce_pubkey = nonce_keypair.pubkey();
|
||||
|
||||
// Grab the hash and fee_calculator stored in the nonce account
|
||||
let (stored_nonce_hash, stored_fee_calculator) = bank
|
||||
.get_account(&nonce_pubkey)
|
||||
.and_then(|acc| {
|
||||
let state =
|
||||
StateMut::<nonce::state::Versions>::state(&acc).map(|v| v.convert_to_current());
|
||||
match state {
|
||||
Ok(nonce::State::Initialized(ref data)) => {
|
||||
Some((data.blockhash, data.fee_calculator.clone()))
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
// Kick nonce hash off the blockhash_queue
|
||||
for _ in 0..MAX_RECENT_BLOCKHASHES + 1 {
|
||||
goto_end_of_slot(Arc::get_mut(&mut bank).unwrap());
|
||||
bank = Arc::new(new_from_parent(&bank));
|
||||
}
|
||||
|
||||
// Durable Nonce transfer
|
||||
let nonce_tx = Transaction::new_signed_with_payer(
|
||||
&[
|
||||
system_instruction::advance_nonce_account(&nonce_pubkey, &nonce_pubkey),
|
||||
system_instruction::transfer(&custodian_pubkey, &Pubkey::new_rand(), 100_000),
|
||||
],
|
||||
Some(&custodian_pubkey),
|
||||
&[&custodian_keypair, &nonce_keypair],
|
||||
stored_nonce_hash,
|
||||
);
|
||||
bank.process_transaction(&nonce_tx).unwrap();
|
||||
|
||||
// Grab the new hash and fee_calculator; both should be updated
|
||||
let (nonce_hash, fee_calculator) = bank
|
||||
.get_account(&nonce_pubkey)
|
||||
.and_then(|acc| {
|
||||
let state =
|
||||
StateMut::<nonce::state::Versions>::state(&acc).map(|v| v.convert_to_current());
|
||||
match state {
|
||||
Ok(nonce::State::Initialized(ref data)) => {
|
||||
Some((data.blockhash, data.fee_calculator.clone()))
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
assert_ne!(stored_nonce_hash, nonce_hash);
|
||||
assert_ne!(stored_fee_calculator, fee_calculator);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_collect_balances() {
|
||||
let (genesis_config, _mint_keypair) = create_genesis_config(500);
|
||||
|
|
|
@ -51,25 +51,26 @@ pub fn prepare_if_nonce_account(
|
|||
account_pubkey: &Pubkey,
|
||||
tx_result: &transaction::Result<()>,
|
||||
maybe_nonce: Option<(&Pubkey, &Account)>,
|
||||
last_blockhash: &Hash,
|
||||
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
|
||||
) {
|
||||
if let Some((nonce_key, nonce_acc)) = maybe_nonce {
|
||||
if account_pubkey == nonce_key {
|
||||
// Nonce TX failed with an InstructionError. Roll back
|
||||
// its account state
|
||||
if tx_result.is_err() {
|
||||
*account = nonce_acc.clone()
|
||||
}
|
||||
// Since hash_age_kind is DurableNonce, unwrap is safe here
|
||||
let state = StateMut::<Versions>::state(nonce_acc)
|
||||
.unwrap()
|
||||
.convert_to_current();
|
||||
if let State::Initialized(ref data) = state {
|
||||
let new_data = Versions::new_current(State::Initialized(nonce::state::Data {
|
||||
blockhash: *last_blockhash,
|
||||
..data.clone()
|
||||
}));
|
||||
account.set_state(&new_data).unwrap();
|
||||
*account = nonce_acc.clone();
|
||||
// Since hash_age_kind is DurableNonce, unwrap is safe here
|
||||
let state = StateMut::<Versions>::state(nonce_acc)
|
||||
.unwrap()
|
||||
.convert_to_current();
|
||||
if let State::Initialized(ref data) = state {
|
||||
let new_data = Versions::new_current(State::Initialized(nonce::state::Data {
|
||||
blockhash: last_blockhash_with_fee_calculator.0,
|
||||
fee_calculator: last_blockhash_with_fee_calculator.1.clone(),
|
||||
..data.clone()
|
||||
}));
|
||||
account.set_state(&new_data).unwrap();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -247,7 +248,8 @@ mod tests {
|
|||
});
|
||||
}
|
||||
|
||||
fn create_accounts_prepare_if_nonce_account() -> (Pubkey, Account, Account, Hash) {
|
||||
fn create_accounts_prepare_if_nonce_account() -> (Pubkey, Account, Account, Hash, FeeCalculator)
|
||||
{
|
||||
let data = Versions::new_current(State::Initialized(nonce::state::Data::default()));
|
||||
let account = Account::new_data(42, &data, &system_program::id()).unwrap();
|
||||
let pre_account = Account {
|
||||
|
@ -259,6 +261,9 @@ mod tests {
|
|||
pre_account,
|
||||
account,
|
||||
Hash::new(&[1u8; 32]),
|
||||
FeeCalculator {
|
||||
lamports_per_signature: 1234,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
|
@ -267,7 +272,7 @@ mod tests {
|
|||
account_pubkey: &Pubkey,
|
||||
tx_result: &transaction::Result<()>,
|
||||
maybe_nonce: Option<(&Pubkey, &Account)>,
|
||||
last_blockhash: &Hash,
|
||||
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
|
||||
expect_account: &Account,
|
||||
) -> bool {
|
||||
// Verify expect_account's relationship
|
||||
|
@ -275,7 +280,7 @@ mod tests {
|
|||
Some((nonce_pubkey, _nonce_account))
|
||||
if nonce_pubkey == account_pubkey && tx_result.is_ok() =>
|
||||
{
|
||||
assert_ne!(expect_account, account)
|
||||
assert_eq!(expect_account, account) // Account update occurs in system_instruction_processor
|
||||
}
|
||||
Some((nonce_pubkey, nonce_account)) if nonce_pubkey == account_pubkey => {
|
||||
assert_ne!(expect_account, nonce_account)
|
||||
|
@ -288,22 +293,24 @@ mod tests {
|
|||
account_pubkey,
|
||||
tx_result,
|
||||
maybe_nonce,
|
||||
last_blockhash,
|
||||
last_blockhash_with_fee_calculator,
|
||||
);
|
||||
expect_account == account
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_prepare_if_nonce_account_expected() {
|
||||
let (pre_account_pubkey, pre_account, mut post_account, last_blockhash) =
|
||||
create_accounts_prepare_if_nonce_account();
|
||||
let (
|
||||
pre_account_pubkey,
|
||||
pre_account,
|
||||
mut post_account,
|
||||
last_blockhash,
|
||||
last_fee_calculator,
|
||||
) = create_accounts_prepare_if_nonce_account();
|
||||
let post_account_pubkey = pre_account_pubkey;
|
||||
|
||||
let mut expect_account = post_account.clone();
|
||||
let data = Versions::new_current(State::Initialized(nonce::state::Data {
|
||||
blockhash: last_blockhash,
|
||||
..nonce::state::Data::default()
|
||||
}));
|
||||
let data = Versions::new_current(State::Initialized(nonce::state::Data::default()));
|
||||
expect_account.set_state(&data).unwrap();
|
||||
|
||||
assert!(run_prepare_if_nonce_account_test(
|
||||
|
@ -311,14 +318,14 @@ mod tests {
|
|||
&post_account_pubkey,
|
||||
&Ok(()),
|
||||
Some((&pre_account_pubkey, &pre_account)),
|
||||
&last_blockhash,
|
||||
&(last_blockhash, last_fee_calculator),
|
||||
&expect_account,
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_prepare_if_nonce_account_not_nonce_tx() {
|
||||
let (pre_account_pubkey, _pre_account, _post_account, last_blockhash) =
|
||||
let (pre_account_pubkey, _pre_account, _post_account, last_blockhash, last_fee_calculator) =
|
||||
create_accounts_prepare_if_nonce_account();
|
||||
let post_account_pubkey = pre_account_pubkey;
|
||||
|
||||
|
@ -329,15 +336,20 @@ mod tests {
|
|||
&post_account_pubkey,
|
||||
&Ok(()),
|
||||
None,
|
||||
&last_blockhash,
|
||||
&(last_blockhash, last_fee_calculator),
|
||||
&expect_account,
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_prepare_if_nonce_naccount_ot_nonce_pubkey() {
|
||||
let (pre_account_pubkey, pre_account, mut post_account, last_blockhash) =
|
||||
create_accounts_prepare_if_nonce_account();
|
||||
fn test_prepare_if_nonce_account_not_nonce_pubkey() {
|
||||
let (
|
||||
pre_account_pubkey,
|
||||
pre_account,
|
||||
mut post_account,
|
||||
last_blockhash,
|
||||
last_fee_calculator,
|
||||
) = create_accounts_prepare_if_nonce_account();
|
||||
|
||||
let expect_account = post_account.clone();
|
||||
// Wrong key
|
||||
|
@ -346,15 +358,20 @@ mod tests {
|
|||
&Pubkey::new(&[1u8; 32]),
|
||||
&Ok(()),
|
||||
Some((&pre_account_pubkey, &pre_account)),
|
||||
&last_blockhash,
|
||||
&(last_blockhash, last_fee_calculator),
|
||||
&expect_account,
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_prepare_if_nonce_account_tx_error() {
|
||||
let (pre_account_pubkey, pre_account, mut post_account, last_blockhash) =
|
||||
create_accounts_prepare_if_nonce_account();
|
||||
let (
|
||||
pre_account_pubkey,
|
||||
pre_account,
|
||||
mut post_account,
|
||||
last_blockhash,
|
||||
last_fee_calculator,
|
||||
) = create_accounts_prepare_if_nonce_account();
|
||||
let post_account_pubkey = pre_account_pubkey;
|
||||
|
||||
let mut expect_account = pre_account.clone();
|
||||
|
@ -362,6 +379,7 @@ mod tests {
|
|||
.set_state(&Versions::new_current(State::Initialized(
|
||||
nonce::state::Data {
|
||||
blockhash: last_blockhash,
|
||||
fee_calculator: last_fee_calculator.clone(),
|
||||
..nonce::state::Data::default()
|
||||
},
|
||||
)))
|
||||
|
@ -375,7 +393,7 @@ mod tests {
|
|||
InstructionError::InvalidArgument,
|
||||
)),
|
||||
Some((&pre_account_pubkey, &pre_account)),
|
||||
&last_blockhash,
|
||||
&(last_blockhash, last_fee_calculator),
|
||||
&expect_account,
|
||||
));
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue