Skip to content

Commit

Permalink
separates durable nonce and blockhash domains (backport #25744) (#25780)
Browse files Browse the repository at this point in the history
* separates durable nonce and blockhash domains

AdvanceNonceAccount instruction updates nonce to blockhash. This makes it
possible that a durable transaction is executed twice both as a normal
transaction and a nonce transaction if it uses blockhash (as opposed to nonce)
for its recent_blockhash field.

The commit prevents this double execution by separating nonce and blockhash
domains; when advancing nonce account, blockhash is hashed with a fixed string.
As a result a blockhash cannot be a valid nonce value; and if transaction was
once executed as a normal transaction it cannot be re-executed as a durable
transaction again and vice-versa.

(cherry picked from commit 5ee157f)

# Conflicts:
#	cli/src/nonce.rs
#	client/src/nonce_utils.rs
#	rpc/src/rpc.rs
#	runtime/src/nonce_keyed_account.rs
#	sdk/program/src/example_mocks.rs
#	sdk/program/src/nonce/state/current.rs
#	sdk/program/src/system_instruction.rs
#	send-transaction-service/src/send_transaction_service.rs

* adds feature gate enabling durable nonce

Previous commit separates durable nonce and blockhash domains with a
feature gate. A 2nd feature added in this commit enables durable nonce
at least one epoch after the 1st feature.
By the time 2nd feature is activated, some nonce accounts will have an
old blockhash, but no nonce account can have a recent blockhash.
As a result no transaction (durable or normal) can be executed twice.

(cherry picked from commit 9851774)

# Conflicts:
#	runtime/src/bank.rs

* removes mergify merge conflicts

Co-authored-by: behzad nouri <[email protected]>
  • Loading branch information
mergify[bot] and behzadnouri authored Jun 4, 2022
1 parent 9a88655 commit 7215dcb
Show file tree
Hide file tree
Showing 15 changed files with 254 additions and 125 deletions.
2 changes: 1 addition & 1 deletion account-decoder/src/parse_nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub fn parse_nonce(data: &[u8]) -> Result<UiNonceState, ParseAccountError> {
)),
State::Initialized(data) => Ok(UiNonceState::Initialized(UiNonceData {
authority: data.authority.to_string(),
blockhash: data.blockhash.to_string(),
blockhash: data.blockhash().to_string(),
fee_calculator: data.fee_calculator.into(),
})),
}
Expand Down
32 changes: 22 additions & 10 deletions cli/src/nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ pub fn check_nonce_account(
) -> Result<(), CliError> {
match state_from_account(nonce_account)? {
State::Initialized(ref data) => {
if &data.blockhash != nonce_hash {
if &data.blockhash() != nonce_hash {
Err(Error::InvalidHash.into())
} else if nonce_authority != &data.authority {
Err(Error::InvalidAuthority.into())
Expand Down Expand Up @@ -516,7 +516,7 @@ pub fn process_get_nonce(
.and_then(|ref a| state_from_account(a))?
{
State::Uninitialized => Ok("Nonce account is uninitialized".to_string()),
State::Initialized(ref data) => Ok(format!("{:?}", data.blockhash)),
State::Initialized(ref data) => Ok(format!("{:?}", data.blockhash())),
}
}

Expand Down Expand Up @@ -590,7 +590,7 @@ pub fn process_show_nonce_account(
..CliNonceAccount::default()
};
if let Some(data) = data {
nonce_account.nonce = Some(data.blockhash.to_string());
nonce_account.nonce = Some(data.blockhash().to_string());
nonce_account.lamports_per_signature = Some(data.fee_calculator.lamports_per_signature);
nonce_account.authority = Some(data.authority.to_string());
}
Expand Down Expand Up @@ -657,7 +657,11 @@ mod tests {
account::Account,
account_utils::StateMut,
hash::hash,
nonce::{self, state::Versions, State},
nonce::{
self,
state::{DurableNonce, Versions},
State,
},
nonce_account,
signature::{read_keypair_file, write_keypair, Keypair, Signer},
system_program,
Expand Down Expand Up @@ -917,11 +921,13 @@ mod tests {

#[test]
fn test_check_nonce_account() {
let blockhash = Hash::default();
let durable_nonce =
DurableNonce::from_blockhash(&Hash::default(), /*separate_domains:*/ true);
let blockhash = *durable_nonce.as_hash();
let nonce_pubkey = solana_sdk::pubkey::new_rand();
let data = Versions::new_current(State::Initialized(nonce::state::Data::new(
nonce_pubkey,
blockhash,
durable_nonce,
0,
)));
let valid = Account::new_data(1, &data, &system_program::ID);
Expand All @@ -941,9 +947,11 @@ mod tests {
assert_eq!(err, Error::InvalidAccountData,);
}

let invalid_durable_nonce =
DurableNonce::from_blockhash(&hash(b"invalid"), /*separate_domains:*/ true);
let data = Versions::new_current(State::Initialized(nonce::state::Data::new(
nonce_pubkey,
hash(b"invalid"),
invalid_durable_nonce,
0,
)));
let invalid_hash = Account::new_data(1, &data, &system_program::ID);
Expand All @@ -955,7 +963,7 @@ mod tests {

let data = Versions::new_current(State::Initialized(nonce::state::Data::new(
solana_sdk::pubkey::new_rand(),
blockhash,
durable_nonce,
0,
)));
let invalid_authority = Account::new_data(1, &data, &system_program::ID);
Expand Down Expand Up @@ -998,7 +1006,9 @@ mod tests {
let mut nonce_account = nonce_account::create_account(1).into_inner();
assert_eq!(state_from_account(&nonce_account), Ok(State::Uninitialized));

let data = nonce::state::Data::new(Pubkey::new(&[1u8; 32]), Hash::new(&[42u8; 32]), 42);
let durable_nonce =
DurableNonce::from_blockhash(&Hash::new(&[42u8; 32]), /*separate_domains:*/ true);
let data = nonce::state::Data::new(Pubkey::new(&[1u8; 32]), durable_nonce, 42);
nonce_account
.set_state(&Versions::new_current(State::Initialized(data.clone())))
.unwrap();
Expand Down Expand Up @@ -1027,7 +1037,9 @@ mod tests {
Err(Error::InvalidStateForOperation)
);

let data = nonce::state::Data::new(Pubkey::new(&[1u8; 32]), Hash::new(&[42u8; 32]), 42);
let durable_nonce =
DurableNonce::from_blockhash(&Hash::new(&[42u8; 32]), /*separate_domains:*/ true);
let data = nonce::state::Data::new(Pubkey::new(&[1u8; 32]), durable_nonce, 42);
nonce_account
.set_state(&Versions::new_current(State::Initialized(data.clone())))
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ fn test_create_account_with_seed() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Test by creating transfer TX with nonce, fully offline
let mut authority_config = CliConfig::recent_for_tests();
Expand Down
18 changes: 9 additions & 9 deletions cli/tests/stake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ fn test_nonced_stake_delegation_and_deactivation() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Delegate stake
config.signers = vec![&config_keypair];
Expand Down Expand Up @@ -539,7 +539,7 @@ fn test_nonced_stake_delegation_and_deactivation() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Deactivate stake
config.command = CliCommand::DeactivateStake {
Expand Down Expand Up @@ -802,7 +802,7 @@ fn test_stake_authorize() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Nonced assignment of new online stake authority
let online_authority = Keypair::new();
Expand Down Expand Up @@ -870,7 +870,7 @@ fn test_stake_authorize() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();
assert_ne!(nonce_hash, new_nonce_hash);
}

Expand Down Expand Up @@ -1126,7 +1126,7 @@ fn test_stake_split() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Nonced offline split
let split_account = keypair_from_seed(&[2u8; 32]).unwrap();
Expand Down Expand Up @@ -1400,7 +1400,7 @@ fn test_stake_set_lockup() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Nonced offline set lockup
let lockup = LockupArgs {
Expand Down Expand Up @@ -1515,7 +1515,7 @@ fn test_offline_nonced_create_stake_account_and_withdraw() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Create stake account offline
let stake_keypair = keypair_from_seed(&[4u8; 32]).unwrap();
Expand Down Expand Up @@ -1576,7 +1576,7 @@ fn test_offline_nonced_create_stake_account_and_withdraw() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Offline, nonced stake-withdraw
let recipient = keypair_from_seed(&[5u8; 32]).unwrap();
Expand Down Expand Up @@ -1630,7 +1630,7 @@ fn test_offline_nonced_create_stake_account_and_withdraw() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Create another stake account. This time with seed
let seed = "seedy";
Expand Down
6 changes: 3 additions & 3 deletions cli/tests/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ fn test_transfer() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Nonced transfer
config.signers = vec![&default_signer];
Expand Down Expand Up @@ -237,7 +237,7 @@ fn test_transfer() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();
assert_ne!(nonce_hash, new_nonce_hash);

// Assign nonce authority to offline
Expand All @@ -263,7 +263,7 @@ fn test_transfer() {
)
.and_then(|ref a| nonce_utils::data_from_account(a))
.unwrap()
.blockhash;
.blockhash();

// Offline, nonced transfer
offline.signers = vec![&default_offline_signer];
Expand Down
19 changes: 13 additions & 6 deletions client/src/blockhash_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl Source {
#[allow(clippy::redundant_closure)]
let data = nonce_utils::get_account_with_commitment(rpc_client, pubkey, commitment)
.and_then(|ref a| nonce_utils::data_from_account(a))?;
Ok((data.blockhash, data.fee_calculator))
Ok((data.blockhash(), data.fee_calculator))
}
}
}
Expand All @@ -64,7 +64,7 @@ impl Source {
let res = nonce_utils::get_account_with_commitment(rpc_client, pubkey, commitment)?;
let res = nonce_utils::data_from_account(&res)?;
Ok(Some(res)
.filter(|d| d.blockhash == *blockhash)
.filter(|d| d.blockhash() == *blockhash)
.map(|d| d.fee_calculator))
}
}
Expand All @@ -84,7 +84,7 @@ impl Source {
#[allow(clippy::redundant_closure)]
let data = nonce_utils::get_account_with_commitment(rpc_client, pubkey, commitment)
.and_then(|ref a| nonce_utils::data_from_account(a))?;
Ok(data.blockhash)
Ok(data.blockhash())
}
}
}
Expand Down Expand Up @@ -193,7 +193,12 @@ mod tests {
clap::App,
serde_json::{self, json},
solana_account_decoder::{UiAccount, UiAccountEncoding},
solana_sdk::{account::Account, hash::hash, nonce, system_program},
solana_sdk::{
account::Account,
hash::hash,
nonce::{self, state::DurableNonce},
system_program,
},
std::collections::HashMap,
};

Expand Down Expand Up @@ -405,11 +410,13 @@ mod tests {
.get_blockhash_and_fee_calculator(&rpc_client, CommitmentConfig::default())
.is_err());

let nonce_blockhash = Hash::new(&[2u8; 32]);
let durable_nonce =
DurableNonce::from_blockhash(&Hash::new(&[2u8; 32]), /*separate_domains:*/ true);
let nonce_blockhash = *durable_nonce.as_hash();
let nonce_fee_calc = FeeCalculator::new(4242);
let data = nonce::state::Data {
authority: Pubkey::new(&[3u8; 32]),
blockhash: nonce_blockhash,
durable_nonce,
fee_calculator: nonce_fee_calc,
};
let nonce_account = Account::new_data_with_space(
Expand Down
7 changes: 5 additions & 2 deletions rpc/src/transaction_status_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ pub(crate) mod tests {
hash::Hash,
instruction::CompiledInstruction,
message::{Message, MessageHeader, SanitizedMessage},
nonce, nonce_account,
nonce::{self, state::DurableNonce},
nonce_account,
pubkey::Pubkey,
signature::{Keypair, Signature, Signer},
system_transaction,
Expand Down Expand Up @@ -306,7 +307,9 @@ pub(crate) mod tests {
let pubkey = Pubkey::new_unique();

let mut nonce_account = nonce_account::create_account(1).into_inner();
let data = nonce::state::Data::new(Pubkey::new(&[1u8; 32]), Hash::new(&[42u8; 32]), 42);
let durable_nonce =
DurableNonce::from_blockhash(&Hash::new(&[42u8; 32]), /*separate_domains:*/ true);
let data = nonce::state::Data::new(Pubkey::new(&[1u8; 32]), durable_nonce, 42);
nonce_account
.set_state(&nonce::state::Versions::new_current(
nonce::State::Initialized(data),
Expand Down
Loading

0 comments on commit 7215dcb

Please sign in to comment.