Skip to content

Commit

Permalink
separates durable nonce and blockhash domains
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
behzadnouri authored and jeffwashington committed Jun 28, 2022
1 parent b9a7f83 commit 1f89625
Show file tree
Hide file tree
Showing 19 changed files with 322 additions and 118 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
36 changes: 24 additions & 12 deletions cli/src/nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,10 @@ 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 {
provided: *nonce_hash,
expected: data.blockhash,
expected: data.blockhash(),
}
.into())
} else if nonce_authority != &data.authority {
Expand Down Expand Up @@ -524,7 +524,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 @@ -598,7 +598,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 @@ -665,7 +665,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 @@ -925,11 +929,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 @@ -949,9 +955,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).unwrap();
Expand All @@ -962,15 +970,15 @@ mod tests {
err,
Error::InvalidHash {
provided: blockhash,
expected: hash(b"invalid"),
expected: *invalid_durable_nonce.as_hash(),
}
);
}

let new_nonce_authority = solana_sdk::pubkey::new_rand();
let data = Versions::new_current(State::Initialized(nonce::state::Data::new(
new_nonce_authority,
blockhash,
durable_nonce,
0,
)));
let invalid_authority = Account::new_data(1, &data, &system_program::ID);
Expand Down Expand Up @@ -1019,7 +1027,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 @@ -1048,7 +1058,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
2 changes: 1 addition & 1 deletion client/src/nonce_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ pub fn state_from_account<T: ReadableAccount + StateMut<Versions>>(
/// // network's latest blockhash.
/// let nonce_account = client.get_account(&nonce_account_pubkey)?;
/// let nonce_data = nonce_utils::data_from_account(&nonce_account)?;
/// let blockhash = nonce_data.blockhash;
/// let blockhash = nonce_data.blockhash();
///
/// tx.try_sign(&[payer], blockhash)?;
///
Expand Down
5 changes: 3 additions & 2 deletions rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4432,7 +4432,8 @@ pub mod tests {
hash::{hash, Hash},
instruction::InstructionError,
message::{v0, v0::MessageAddressTableLookup, MessageHeader, VersionedMessage},
nonce, rpc_port,
nonce::{self, state::DurableNonce},
rpc_port,
signature::{Keypair, Signer},
slot_hashes::SlotHashes,
system_program, system_transaction,
Expand Down Expand Up @@ -5406,7 +5407,7 @@ pub mod tests {
42,
&nonce::state::Versions::new_current(nonce::State::new_initialized(
&authority,
&Hash::default(),
DurableNonce::default(),
1000,
)),
&system_program::id(),
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 @@ -220,7 +220,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 @@ -319,7 +320,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 1f89625

Please sign in to comment.