Skip to content

Commit

Permalink
Cleanup - merge_nonce_error_into_system_error (solana-labs#31773)
Browse files Browse the repository at this point in the history 
* merge_nonce_error_into_system_error

* more cleanup

---------

Co-authored-by: Trent Nelson <trent@solana.com>
  • Loading branch information
@Lichtso @t-nelson @jeffwashington
2 people authored and jeffwashington committed May 24, 2023
1 parent 75ab9e8 commit e499928
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 400 deletions.
102 changes: 10 additions & 92 deletions cli/src/nonce.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ use {
crate::{
checks::{check_account_for_fee_with_commitment, check_unique_pubkeys},
cli::{
log_instruction_custom_error, log_instruction_custom_error_ex, CliCommand,
CliCommandInfo, CliConfig, CliError, ProcessResult,
log_instruction_custom_error, CliCommand, CliCommandInfo, CliConfig, CliError,
ProcessResult,
},
compute_unit_price::WithComputeUnitPrice,
feature::get_feature_is_active,
memo::WithMemo,
spend_utils::{resolve_spend_tx_and_check_account_balance, SpendAmount},
},
Expand All @@ -25,19 +24,17 @@ use {
solana_rpc_client_nonce_utils::*,
solana_sdk::{
account::Account,
feature_set::merge_nonce_error_into_system_error,
hash::Hash,
instruction::InstructionError,
message::Message,
nonce::{self, State},
pubkey::Pubkey,
system_instruction::{
advance_nonce_account, authorize_nonce_account, create_nonce_account,
create_nonce_account_with_seed, instruction_to_nonce_error, upgrade_nonce_account,
withdraw_nonce_account, NonceError, SystemError,
create_nonce_account_with_seed, upgrade_nonce_account, withdraw_nonce_account,
SystemError,
},
system_program,
transaction::{Transaction, TransactionError},
transaction::Transaction,
},
std::sync::Arc,
};
Expand Down Expand Up @@ -427,21 +424,9 @@ pub fn process_authorize_nonce_account(
&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
}
})
}
log_instruction_custom_error::<SystemError>(result, config)
}

pub fn process_create_nonce_account(
Expand Down Expand Up @@ -524,40 +509,9 @@ pub fn process_create_nonce_account(

let mut tx = Transaction::new_unsigned(message);
tx.try_sign(&config.signers, latest_blockhash)?;
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);

let err_ix_index = if let Err(err) = &result {
err.get_transaction_error().and_then(|tx_err| {
if let TransactionError::InstructionError(ix_index, _) = tx_err {
Some(ix_index)
} else {
None
}
})
} else {
None
};

match err_ix_index {
// SystemInstruction::InitializeNonceAccount failed
Some(1) => {
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
}
})
}
}
// SystemInstruction::CreateAccount{,WithSeed} failed
_ => log_instruction_custom_error::<SystemError>(result, config),
}
log_instruction_custom_error::<SystemError>(result, config)
}

pub fn process_get_nonce(
Expand Down Expand Up @@ -611,21 +565,9 @@ pub fn process_new_nonce(
&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
}
})
}
log_instruction_custom_error::<SystemError>(result, config)
}

pub fn process_show_nonce_account(
Expand Down Expand Up @@ -688,21 +630,9 @@ pub fn process_withdraw_from_nonce_account(
&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
}
})
}
log_instruction_custom_error::<SystemError>(result, config)
}

pub(crate) fn process_upgrade_nonce_account(
Expand All @@ -725,20 +655,8 @@ pub(crate) fn process_upgrade_nonce_account(
&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
}
})
}
log_instruction_custom_error::<SystemError>(result, config)
}

#[cfg(test)]
Expand Down
45 changes: 6 additions & 39 deletions programs/system/src/system_instruction.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
use {
solana_program_runtime::{ic_msg, invoke_context::InvokeContext},
solana_sdk::{
feature_set,
instruction::{checked_add, InstructionError},
nonce::{
self,
state::{AuthorizeNonceError, DurableNonce, Versions},
State,
},
pubkey::Pubkey,
system_instruction::{nonce_to_instruction_error, NonceError},
system_instruction::SystemError,
sysvar::rent::Rent,
transaction_context::{
BorrowedAccount, IndexOfAccount, InstructionContext, TransactionContext,
Expand All @@ -23,10 +22,6 @@ pub fn advance_nonce_account(
signers: &HashSet<Pubkey>,
invoke_context: &InvokeContext,
) -> Result<(), InstructionError> {
let merge_nonce_error_into_system_error = invoke_context
.feature_set
.is_active(&feature_set::merge_nonce_error_into_system_error::id());

if !account.is_writable() {
ic_msg!(
invoke_context,
Expand All @@ -53,10 +48,7 @@ pub fn advance_nonce_account(
invoke_context,
"Advance nonce account: nonce can only advance once per slot"
);
return Err(nonce_to_instruction_error(
NonceError::NotExpired,
merge_nonce_error_into_system_error,
));
return Err(SystemError::NonceBlockhashNotExpired.into());
}

let new_data = nonce::state::Data::new(
Expand All @@ -72,10 +64,7 @@ pub fn advance_nonce_account(
"Advance nonce account: Account {} state is invalid",
account.get_key()
);
Err(nonce_to_instruction_error(
NonceError::BadAccountState,
merge_nonce_error_into_system_error,
))
Err(InstructionError::InvalidAccountData)
}
}
}
Expand All @@ -92,10 +81,6 @@ pub fn withdraw_nonce_account(
) -> Result<(), InstructionError> {
let mut from = instruction_context
.try_borrow_instruction_account(transaction_context, from_account_index)?;
let merge_nonce_error_into_system_error = invoke_context
.feature_set
.is_active(&feature_set::merge_nonce_error_into_system_error::id());

if !from.is_writable() {
ic_msg!(
invoke_context,
Expand Down Expand Up @@ -127,10 +112,7 @@ pub fn withdraw_nonce_account(
invoke_context,
"Withdraw nonce account: nonce can only advance once per slot"
);
return Err(nonce_to_instruction_error(
NonceError::NotExpired,
merge_nonce_error_into_system_error,
));
return Err(SystemError::NonceBlockhashNotExpired.into());
}
from.set_state(&Versions::new(State::Uninitialized))?;
} else {
Expand Down Expand Up @@ -174,10 +156,6 @@ pub fn initialize_nonce_account(
rent: &Rent,
invoke_context: &InvokeContext,
) -> Result<(), InstructionError> {
let merge_nonce_error_into_system_error = invoke_context
.feature_set
.is_active(&feature_set::merge_nonce_error_into_system_error::id());

if !account.is_writable() {
ic_msg!(
invoke_context,
Expand Down Expand Up @@ -214,10 +192,7 @@ pub fn initialize_nonce_account(
"Initialize nonce account: Account {} state is invalid",
account.get_key()
);
Err(nonce_to_instruction_error(
NonceError::BadAccountState,
merge_nonce_error_into_system_error,
))
Err(InstructionError::InvalidAccountData)
}
}
}
Expand All @@ -228,10 +203,6 @@ pub fn authorize_nonce_account(
signers: &HashSet<Pubkey>,
invoke_context: &InvokeContext,
) -> Result<(), InstructionError> {
let merge_nonce_error_into_system_error = invoke_context
.feature_set
.is_active(&feature_set::merge_nonce_error_into_system_error::id());

if !account.is_writable() {
ic_msg!(
invoke_context,
Expand All @@ -251,10 +222,7 @@ pub fn authorize_nonce_account(
"Authorize nonce account: Account {} state is invalid",
account.get_key()
);
Err(nonce_to_instruction_error(
NonceError::BadAccountState,
merge_nonce_error_into_system_error,
))
Err(InstructionError::InvalidAccountData)
}
Err(AuthorizeNonceError::MissingRequiredSignature(account_authority)) => {
ic_msg!(
Expand All @@ -278,7 +246,6 @@ mod test {
hash::hash,
nonce::{self, State},
nonce_account::{create_account, verify_nonce_account},
system_instruction::SystemError,
system_program,
transaction_context::InstructionAccount,
},
Expand Down
12 changes: 5 additions & 7 deletions programs/system/src/system_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ use {
nonce,
program_utils::limited_deserialize,
pubkey::Pubkey,
system_instruction::{
NonceError, SystemError, SystemInstruction, MAX_PERMITTED_DATA_LENGTH,
},
system_instruction::{SystemError, SystemInstruction, MAX_PERMITTED_DATA_LENGTH},
system_program,
transaction_context::{
BorrowedAccount, IndexOfAccount, InstructionContext, TransactionContext,
Expand Down Expand Up @@ -435,7 +433,7 @@ declare_process_instruction!(process_instruction, 150, |invoke_context| {
invoke_context,
"Advance nonce account: recent blockhash list is empty",
);
return Err(NonceError::NoRecentBlockhashes.into());
return Err(SystemError::NonceNoRecentBlockhashes.into());
}
advance_nonce_account(&mut me, &signers, invoke_context)
}
Expand Down Expand Up @@ -474,7 +472,7 @@ declare_process_instruction!(process_instruction, 150, |invoke_context| {
invoke_context,
"Initialize nonce account: recent blockhash list is empty",
);
return Err(NonceError::NoRecentBlockhashes.into());
return Err(SystemError::NonceNoRecentBlockhashes.into());
}
let rent = get_sysvar_with_account_check::rent(invoke_context, instruction_context, 2)?;
initialize_nonce_account(&mut me, &authorized, &rent, invoke_context)
Expand Down Expand Up @@ -1884,7 +1882,7 @@ mod tests {
is_writable: false,
},
],
Err(NonceError::NoRecentBlockhashes.into()),
Err(SystemError::NonceNoRecentBlockhashes.into()),
);
}

Expand Down Expand Up @@ -1945,7 +1943,7 @@ mod tests {
is_writable: false,
},
],
Err(NonceError::NoRecentBlockhashes.into()),
Err(SystemError::NonceNoRecentBlockhashes.into()),
super::process_instruction,
|invoke_context: &mut InvokeContext| {
invoke_context.blockhash = hash(&serialize(&0).unwrap());
Expand Down
Loading

0 comments on commit e499928

Please sign in to comment.