Skip to content

Commit

Permalink
Reject durable nonce txs that don't use an advanceable nonce (backport
Browse files Browse the repository at this point in the history
…#25832) (#25855)

* Reject durable nonce txs that don't use an advanceable nonce (#25832)

* resolve conflicts

Co-authored-by: Justin Starry <[email protected]>
  • Loading branch information
mergify[bot] and jstarry authored Jun 9, 2022
1 parent 837cf71 commit d385261
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 5 deletions.
75 changes: 70 additions & 5 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3678,16 +3678,22 @@ impl Bank {
.feature_set
.is_active(&feature_set::enable_durable_nonce::id());
let hash_queue = self.blockhash_queue.read().unwrap();
let last_blockhash = hash_queue.last_hash();
let next_durable_nonce =
DurableNonce::from_blockhash(&last_blockhash, separate_nonce_from_blockhash);

txs.zip(lock_results)
.map(|(tx, lock_res)| match lock_res {
Ok(()) => {
let recent_blockhash = tx.message().recent_blockhash();
let hash_age = hash_queue.check_hash_age(recent_blockhash, max_age);
if hash_age == Some(true) {
(Ok(()), None)
} else if let Some((address, account)) =
self.check_transaction_for_nonce(tx, enable_durable_nonce)
{
} else if let Some((address, account)) = self.check_transaction_for_nonce(
tx,
enable_durable_nonce,
&next_durable_nonce,
) {
(Ok(()), Some(NoncePartial::new(address, account)))
} else if hash_age == Some(false) {
error_counters.blockhash_too_old += 1;
Expand Down Expand Up @@ -3773,10 +3779,16 @@ impl Bank {
&self,
tx: &SanitizedTransaction,
enable_durable_nonce: bool,
next_durable_nonce: &DurableNonce,
) -> Option<(Pubkey, AccountSharedData)> {
(enable_durable_nonce
let durable_nonces_enabled = enable_durable_nonce
|| self.slot() <= 135986379
|| self.cluster_type() != ClusterType::MainnetBeta)
|| self.cluster_type() != ClusterType::MainnetBeta;
let nonce_must_be_advanceable = self
.feature_set
.is_active(&feature_set::nonce_must_be_advanceable::ID);
let nonce_is_advanceable = tx.message().recent_blockhash() != next_durable_nonce.as_hash();
(durable_nonces_enabled && (nonce_is_advanceable || !nonce_must_be_advanceable))
.then(|| self.check_message_for_nonce(tx.message()))
.flatten()
}
Expand Down Expand Up @@ -11700,9 +11712,26 @@ pub(crate) mod tests {
nonce_lamports,
nonce_authority,
)?;

// The setup nonce is not valid to be used until the next bank
// so wait one more block
goto_end_of_slot(Arc::get_mut(&mut bank).unwrap());
bank = Arc::new(new_from_parent(&bank));

Ok((bank, mint_keypair, custodian_keypair, nonce_keypair))
}

impl Bank {
fn next_durable_nonce(&self) -> DurableNonce {
let separate_nonce_from_blockhash = self
.feature_set
.is_active(&feature_set::separate_nonce_from_blockhash::id());
let hash_queue = self.blockhash_queue.read().unwrap();
let last_blockhash = hash_queue.last_hash();
DurableNonce::from_blockhash(&last_blockhash, separate_nonce_from_blockhash)
}
}

#[test]
fn test_check_transaction_for_nonce_ok() {
let mut feature_set = FeatureSet::all_enabled();
Expand All @@ -11728,6 +11757,7 @@ pub(crate) mod tests {
bank.check_transaction_for_nonce(
&SanitizedTransaction::from_transaction_for_tests(tx),
true, // enable_durable_nonce
&bank.next_durable_nonce(),
),
Some((nonce_pubkey, nonce_account))
);
Expand Down Expand Up @@ -11757,6 +11787,7 @@ pub(crate) mod tests {
.check_transaction_for_nonce(
&SanitizedTransaction::from_transaction_for_tests(tx,),
true, // enable_durable_nonce
&bank.next_durable_nonce(),
)
.is_none());
}
Expand Down Expand Up @@ -11786,6 +11817,7 @@ pub(crate) mod tests {
.check_transaction_for_nonce(
&SanitizedTransaction::from_transaction_for_tests(tx),
true, // enable_durable_nonce
&bank.next_durable_nonce(),
)
.is_none());
}
Expand Down Expand Up @@ -11816,6 +11848,7 @@ pub(crate) mod tests {
.check_transaction_for_nonce(
&SanitizedTransaction::from_transaction_for_tests(tx),
true, // enable_durable_nonce
&bank.next_durable_nonce(),
)
.is_none());
}
Expand Down Expand Up @@ -11843,6 +11876,7 @@ pub(crate) mod tests {
.check_transaction_for_nonce(
&SanitizedTransaction::from_transaction_for_tests(tx),
true, // enable_durable_nonce
&bank.next_durable_nonce(),
)
.is_none());
}
Expand Down Expand Up @@ -11874,6 +11908,36 @@ pub(crate) mod tests {
assert_eq!(bank.process_transaction(&tx), expect);
}

#[test]
fn test_nonce_must_be_advanceable() {
let (genesis_config, _mint_keypair) = create_genesis_config(100_000_000);
let mut bank = Bank::new_for_tests(&genesis_config);
bank.feature_set = Arc::new(FeatureSet::all_enabled());
let bank = Arc::new(bank);
let nonce_keypair = Keypair::new();
let nonce_authority = nonce_keypair.pubkey();
let durable_nonce =
DurableNonce::from_blockhash(&bank.last_blockhash(), true /* separate domains */);
let nonce_account = AccountSharedData::new_data(
42_424_242,
&nonce::state::Versions::new_current(nonce::State::Initialized(
nonce::state::Data::new(nonce_authority, durable_nonce, 5000),
)),
&system_program::id(),
)
.unwrap();
bank.store_account(&nonce_keypair.pubkey(), &nonce_account);

let ix =
system_instruction::advance_nonce_account(&nonce_keypair.pubkey(), &nonce_authority);
let message = Message::new(&[ix], Some(&nonce_keypair.pubkey()));
let tx = Transaction::new(&[&nonce_keypair], message, *durable_nonce.as_hash());
assert_eq!(
bank.process_transaction(&tx),
Err(TransactionError::BlockhashNotFound)
);
}

#[test]
fn test_nonce_transaction() {
let mut feature_set = FeatureSet::all_enabled();
Expand Down Expand Up @@ -12512,6 +12576,7 @@ pub(crate) mod tests {
bank.check_transaction_for_nonce(
&SanitizedTransaction::from_transaction_for_tests(tx),
true, // enable_durable_nonce
&bank.next_durable_nonce(),
),
None
);
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,10 @@ pub mod nonce_must_be_authorized {
solana_sdk::declare_id!("HxrEu1gXuH7iD3Puua1ohd5n4iUKJyFNtNxk9DVJkvgr");
}

pub mod nonce_must_be_advanceable {
solana_sdk::declare_id!("3u3Er5Vc2jVcwz4xr2GJeSAXT3fAj6ADHZ4BJMZiScFd");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -413,6 +417,7 @@ lazy_static! {
(separate_nonce_from_blockhash::id(), "separate durable nonce and blockhash domains #25744"),
(enable_durable_nonce::id(), "enable durable nonce #25744"),
(nonce_must_be_authorized::id(), "nonce must be authorized"),
(nonce_must_be_advanceable::id(), "durable nonces must be advanceable"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down

0 comments on commit d385261

Please sign in to comment.