Skip to content

Commit

Permalink
feat(nonce): adds system instruction to upgrade legacy nonce versions
Browse files Browse the repository at this point in the history
#25788
permanently disables durable transactions with legacy nonce versions
which are within chain blockhash domain.

This commit adds a new system instruction for a one-time idempotent
upgrade of legacy nonce accounts in order to bump them out of chain
blockhash domain.
  • Loading branch information
behzadnouri committed Jun 6, 2022
1 parent 83cb5f0 commit 9d46737
Show file tree
Hide file tree
Showing 8 changed files with 323 additions and 6 deletions.
10 changes: 10 additions & 0 deletions cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ pub enum CliCommand {
destination_account_pubkey: Pubkey,
lamports: u64,
},
UpgradeNonceAccount {
nonce_account: Pubkey,
memo: Option<String>,
},
// Program Deployment
Deploy {
program_location: String,
Expand Down Expand Up @@ -633,6 +637,7 @@ pub fn parse_command(
("withdraw-from-nonce-account", Some(matches)) => {
parse_withdraw_from_nonce_account(matches, default_signer, wallet_manager)
}
("upgrade-nonce-account", Some(matches)) => parse_upgrade_nonce_account(matches),
// Program Deployment
("deploy", Some(matches)) => {
let (address_signer, _address) = signer_of(matches, "address_signer", wallet_manager)?;
Expand Down Expand Up @@ -1019,6 +1024,11 @@ pub fn process_command(config: &CliConfig) -> ProcessResult {
destination_account_pubkey,
*lamports,
),
// Upgrade nonce account out of blockhash domain.
CliCommand::UpgradeNonceAccount {
nonce_account,
memo,
} => process_upgrade_nonce_account(&rpc_client, config, *nonce_account, memo.as_ref()),

// Program Deployment

Expand Down
68 changes: 65 additions & 3 deletions cli/src/nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use {
solana_clap_utils::{
input_parsers::*,
input_validators::*,
keypair::{DefaultSigner, SignerIndex},
keypair::{CliSigners, DefaultSigner, SignerIndex},
memo::{memo_arg, MEMO_ARG},
nonce::*,
},
Expand All @@ -30,8 +30,8 @@ use {
pubkey::Pubkey,
system_instruction::{
advance_nonce_account, authorize_nonce_account, create_nonce_account,
create_nonce_account_with_seed, instruction_to_nonce_error, withdraw_nonce_account,
NonceError, SystemError,
create_nonce_account_with_seed, instruction_to_nonce_error, upgrade_nonce_account,
withdraw_nonce_account, NonceError, SystemError,
},
system_program,
transaction::{Transaction, TransactionError},
Expand Down Expand Up @@ -173,6 +173,19 @@ impl NonceSubCommands for App<'_, '_> {
.arg(nonce_authority_arg())
.arg(memo_arg()),
)
.subcommand(
SubCommand::with_name("upgrade-nonce-account")
.about("One-time idempotent upgrade of legacy nonce versions \
in order to bump them out of chain blockhash domain.")
.arg(
pubkey!(Arg::with_name("nonce_account_pubkey")
.index(1)
.value_name("NONCE_ACCOUNT_ADDRESS")
.required(true),
"Nonce account to upgrade. "),
)
.arg(memo_arg()),
)
}
}

Expand Down Expand Up @@ -325,6 +338,21 @@ pub fn parse_withdraw_from_nonce_account(
})
}

pub(crate) fn parse_upgrade_nonce_account(
matches: &ArgMatches<'_>,
) -> Result<CliCommandInfo, CliError> {
let nonce_account = pubkey_of(matches, "nonce_account_pubkey").unwrap();
let memo = matches.value_of(MEMO_ARG.name).map(String::from);
Ok(CliCommandInfo {
command: CliCommand::UpgradeNonceAccount {
nonce_account,
memo,
},
// TODO: need signers.
signers: CliSigners::default(),
})
}

/// Check if a nonce account is initialized with the given authority and hash
pub fn check_nonce_account(
nonce_account: &Account,
Expand Down Expand Up @@ -656,6 +684,40 @@ pub fn process_withdraw_from_nonce_account(
}
}

pub(crate) fn process_upgrade_nonce_account(
rpc_client: &RpcClient,
config: &CliConfig,
nonce_account: Pubkey,
memo: Option<&String>,
) -> ProcessResult {
let latest_blockhash = rpc_client.get_latest_blockhash()?;
let ixs = vec![upgrade_nonce_account(nonce_account)].with_memo(memo);
// TODO what signers?
let message = Message::new(&ixs, Some(&config.signers[0].pubkey()));
let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, latest_blockhash)?;
check_account_for_fee_with_commitment(
rpc_client,
&config.signers[0].pubkey(),
&tx.message,
config.commitment,
)?;
let merge_errors =
get_feature_is_active(rpc_client, &merge_nonce_error_into_system_error::id())?;
let result = rpc_client.send_and_confirm_transaction_with_spinner(&tx);
if merge_errors {
log_instruction_custom_error::<SystemError>(result, config)
} else {
log_instruction_custom_error_ex::<NonceError, _>(result, config, |ix_error| {
if let InstructionError::Custom(_) = ix_error {
instruction_to_nonce_error(ix_error, merge_errors)
} else {
None
}
})
}
}

#[cfg(test)]
mod tests {
use {
Expand Down
1 change: 1 addition & 0 deletions docs/src/developing/clients/javascript-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ pub enum SystemInstruction {
/** 9 **/AllocateWithSeed {/**/},
/** 10 **/AssignWithSeed {/**/},
/** 11 **/TransferWithSeed {/**/},
/** 12 **/UpgradeNonceAccount,
}
```

Expand Down
156 changes: 155 additions & 1 deletion runtime/src/system_instruction_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,25 @@ pub fn process_instruction(
instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
authorize_nonce_account(&mut me, &nonce_authority, &signers, invoke_context)
}
SystemInstruction::UpgradeNonceAccount => {
let separate_nonce_from_blockhash = invoke_context
.feature_set
.is_active(&feature_set::separate_nonce_from_blockhash::id());
if !separate_nonce_from_blockhash {
return Err(InstructionError::InvalidInstructionData);
}
instruction_context.check_number_of_instruction_accounts(1)?;
let mut nonce_account =
instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
if nonce_account.get_owner() != &system_program::id() {
return Err(InstructionError::InvalidAccountOwner);
}
let nonce_versions: nonce::state::Versions = nonce_account.get_state()?;
match nonce_versions.upgrade() {
None => Err(InstructionError::InvalidArgument),
Some(nonce_versions) => nonce_account.set_state(&nonce_versions),
}
}
SystemInstruction::Allocate { space } => {
instruction_context.check_number_of_instruction_accounts(1)?;
let mut account = instruction_context
Expand Down Expand Up @@ -568,11 +587,18 @@ mod tests {
use solana_sdk::{
account::{self, Account, AccountSharedData, ReadableAccount},
client::SyncClient,
fee_calculator::FeeCalculator,
genesis_config::create_genesis_config,
hash::{hash, Hash},
instruction::{AccountMeta, Instruction, InstructionError},
message::Message,
nonce, nonce_account, recent_blockhashes_account,
nonce::{
self,
state::{
Data as NonceData, DurableNonce, State as NonceState, Versions as NonceVersions,
},
},
nonce_account, recent_blockhashes_account,
signature::{Keypair, Signer},
system_instruction, system_program,
sysvar::{self, recent_blockhashes::IterItem, rent::Rent},
Expand Down Expand Up @@ -2202,4 +2228,132 @@ mod tests {
},
);
}

#[test]
fn test_nonce_account_upgrade_check_owner() {
let nonce_address = Pubkey::new_unique();
let versions = NonceVersions::Legacy(Box::new(NonceState::Uninitialized));
let nonce_account = AccountSharedData::new_data(
1_000_000, // lamports
&versions, // state
&Pubkey::new_unique(), // owner
)
.unwrap();
let accounts = process_instruction(
&serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(),
vec![(nonce_address, nonce_account.clone())],
vec![AccountMeta {
pubkey: nonce_address,
is_signer: false,
is_writable: false,
}],
Err(InstructionError::InvalidAccountOwner),
super::process_instruction,
);
assert_eq!(accounts.len(), 1);
assert_eq!(accounts[0], nonce_account);
}

fn new_nonce_account(versions: NonceVersions) -> AccountSharedData {
let nonce_account = AccountSharedData::new_data(
1_000_000, // lamports
&versions, // state
&system_program::id(), // owner
)
.unwrap();
assert_eq!(
nonce_account.deserialize_data::<NonceVersions>().unwrap(),
versions
);
nonce_account
}

#[test]
fn test_nonce_account_upgrade() {
let nonce_address = Pubkey::new_unique();
let versions = NonceVersions::Legacy(Box::new(NonceState::Uninitialized));
let nonce_account = new_nonce_account(versions);
let accounts = process_instruction(
&serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(),
vec![(nonce_address, nonce_account.clone())],
vec![AccountMeta {
pubkey: nonce_address,
is_signer: false,
is_writable: false,
}],
Err(InstructionError::InvalidArgument),
super::process_instruction,
);
assert_eq!(accounts.len(), 1);
assert_eq!(accounts[0], nonce_account);
let versions = NonceVersions::Current(Box::new(NonceState::Uninitialized));
let nonce_account = new_nonce_account(versions);
let accounts = process_instruction(
&serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(),
vec![(nonce_address, nonce_account.clone())],
vec![AccountMeta {
pubkey: nonce_address,
is_signer: false,
is_writable: false,
}],
Err(InstructionError::InvalidArgument),
super::process_instruction,
);
assert_eq!(accounts.len(), 1);
assert_eq!(accounts[0], nonce_account);
let blockhash = Hash::from([171; 32]);
let durable_nonce =
DurableNonce::from_blockhash(&blockhash, /*separate_domains:*/ false);
let data = NonceData {
authority: Pubkey::new_unique(),
durable_nonce,
fee_calculator: FeeCalculator {
lamports_per_signature: 2718,
},
};
let versions = NonceVersions::Legacy(Box::new(NonceState::Initialized(data.clone())));
let nonce_account = new_nonce_account(versions);
let mut accounts = process_instruction(
&serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(),
vec![(nonce_address, nonce_account)],
vec![AccountMeta {
pubkey: nonce_address,
is_signer: false,
is_writable: false,
}],
Ok(()),
super::process_instruction,
);
assert_eq!(accounts.len(), 1);
let nonce_account = accounts.remove(0);
let durable_nonce =
DurableNonce::from_blockhash(&blockhash, /*separate_domains:*/ true);
assert_ne!(data.durable_nonce, durable_nonce);
let data = NonceData {
durable_nonce,
..data
};
let upgraded_nonce_account =
NonceVersions::Current(Box::new(NonceState::Initialized(data)));
assert_eq!(
nonce_account.deserialize_data::<NonceVersions>().unwrap(),
upgraded_nonce_account
);
let accounts = process_instruction(
&serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(),
vec![(nonce_address, nonce_account)],
vec![AccountMeta {
pubkey: nonce_address,
is_signer: false,
is_writable: false,
}],
Err(InstructionError::InvalidArgument),
super::process_instruction,
);
assert_eq!(accounts.len(), 1);
assert_eq!(
accounts[0].deserialize_data::<NonceVersions>().unwrap(),
upgraded_nonce_account
);
}
}
55 changes: 55 additions & 0 deletions sdk/program/src/nonce/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,29 @@ impl Versions {
State::Initialized(ref data) => hash == &data.blockhash(),
}
}

// Upgrades legacy nonces out of chain blockhash domains.
pub fn upgrade(self) -> Option<Self> {
match self {
Self::Legacy(mut state) => {
match *state {
// An Uninitialized legacy nonce cannot verify a durable
// transaction. The nonce will be upgraded to Current
// version when initialized. Therefore there is no need to
// upgrade Uninitialized legacy nonces.
State::Uninitialized => None,
State::Initialized(ref mut data) => {
data.durable_nonce = DurableNonce::from_blockhash(
&data.blockhash(),
true, // separate_domains
);
Some(Self::Current(state))
}
}
}
Self::Current(_) => None,
}
}
}

impl From<Versions> for State {
Expand Down Expand Up @@ -123,4 +146,36 @@ mod tests {
assert!(versions.verify_recent_blockhash(durable_nonce.as_hash(), separate_domains));
}
}

#[test]
fn test_nonce_versions_upgrade() {
// Uninitialized
let versions = Versions::Legacy(Box::new(State::Uninitialized));
assert_eq!(versions.upgrade(), None);
// Initialized
let blockhash = Hash::from([171; 32]);
let durable_nonce =
DurableNonce::from_blockhash(&blockhash, /*separate_domains:*/ false);
let data = Data {
authority: Pubkey::new_unique(),
durable_nonce,
fee_calculator: FeeCalculator {
lamports_per_signature: 2718,
},
};
let versions = Versions::Legacy(Box::new(State::Initialized(data.clone())));
let durable_nonce =
DurableNonce::from_blockhash(&blockhash, /*separate_domains:*/ true);
assert_ne!(data.durable_nonce, durable_nonce);
let data = Data {
durable_nonce,
..data
};
let versions = versions.upgrade().unwrap();
assert_eq!(
versions,
Versions::Current(Box::new(State::Initialized(data)))
);
assert_eq!(versions.upgrade(), None);
}
}
Loading

0 comments on commit 9d46737

Please sign in to comment.