Skip to content

Commit

Permalink
adds system instruction to upgrade legacy nonce versions (backport #2…
Browse files Browse the repository at this point in the history
…5789) (#25891)

* feat(nonce): adds system instruction to upgrade legacy nonce versions (#25789)

#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.

(cherry picked from commit b419031)

# Conflicts:
#	sdk/program/src/system_instruction.rs
#	web3.js/src/system-program.ts

* removes mergify merge conflicts

* backport UpgradeNonceAccount

Co-authored-by: behzad nouri <[email protected]>
  • Loading branch information
mergify[bot] and behzadnouri authored Jun 10, 2022
1 parent af6693e commit 2cbad31
Show file tree
Hide file tree
Showing 8 changed files with 351 additions and 8 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 @@ -632,6 +636,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 @@ -1018,6 +1023,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
83 changes: 80 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,20 @@ 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,
},
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 +683,39 @@ 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);
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 Expand Up @@ -925,6 +985,23 @@ mod tests {
],
}
);

// Test UpgradeNonceAccount Subcommand.
let test_upgrade_nonce_account = test_commands.clone().get_matches_from(vec![
"test",
"upgrade-nonce-account",
&nonce_account_string,
]);
assert_eq!(
parse_command(&test_upgrade_nonce_account, &default_signer, &mut None).unwrap(),
CliCommandInfo {
command: CliCommand::UpgradeNonceAccount {
nonce_account: nonce_account_pubkey,
memo: None,
},
signers: CliSigners::default(),
}
);
}

#[test]
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
174 changes: 171 additions & 3 deletions runtime/src/system_instruction_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use {
},
solana_sdk::{
account::{AccountSharedData, ReadableAccount, WritableAccount},
account_utils::StateMut,
account_utils::{State as _, StateMut},
feature_set,
instruction::InstructionError,
keyed_account::{get_signers, keyed_account_at_index, KeyedAccount},
Expand Down Expand Up @@ -401,6 +401,26 @@ pub fn process_instruction(
let me = &mut keyed_account_at_index(keyed_accounts, first_instruction_account)?;
me.authorize_nonce_account(&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);
}
let nonce_account = keyed_account_at_index(keyed_accounts, first_instruction_account)?;
if !system_program::check_id(&nonce_account.owner()?) {
return Err(InstructionError::InvalidAccountOwner);
}
if !nonce_account.is_writable() {
return Err(InstructionError::InvalidArgument);
}
let nonce_versions: nonce::state::Versions = nonce_account.state()?;
match nonce_versions.upgrade() {
None => Err(InstructionError::InvalidArgument),
Some(nonce_versions) => nonce_account.set_state(&nonce_versions),
}
}
SystemInstruction::Allocate { space } => {
let keyed_account = keyed_account_at_index(keyed_accounts, first_instruction_account)?;
let mut account = keyed_account.try_account_ref_mut()?;
Expand Down Expand Up @@ -472,11 +492,18 @@ mod tests {
use solana_sdk::{
account::{self, Account, AccountSharedData},
client::SyncClient,
fee_calculator::FeeCalculator,
genesis_config::create_genesis_config,
hash::{hash, Hash},
hash::{hash, hashv, 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 @@ -2116,4 +2143,145 @@ 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: true,
}],
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: true,
}],
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: true,
}],
Err(InstructionError::InvalidArgument),
super::process_instruction,
);
assert_eq!(accounts.len(), 1);
assert_eq!(accounts[0], nonce_account);
let blockhash = hashv(&[&[171u8; 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 accounts = process_instruction(
&serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(),
vec![(nonce_address, nonce_account.clone())],
vec![AccountMeta {
pubkey: nonce_address,
is_signer: false,
is_writable: false, // Should fail!
}],
Err(InstructionError::InvalidArgument),
super::process_instruction,
);
assert_eq!(accounts.len(), 1);
assert_eq!(accounts[0], nonce_account);
let mut accounts = process_instruction(
&serialize(&SystemInstruction::UpgradeNonceAccount).unwrap(),
vec![(nonce_address, nonce_account)],
vec![AccountMeta {
pubkey: nonce_address,
is_signer: false,
is_writable: true,
}],
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: true,
}],
Err(InstructionError::InvalidArgument),
super::process_instruction,
);
assert_eq!(accounts.len(), 1);
assert_eq!(
accounts[0].deserialize_data::<NonceVersions>().unwrap(),
upgraded_nonce_account
);
}
}
Loading

0 comments on commit 2cbad31

Please sign in to comment.