Feature - better error codes for tx lamport check (#33343)
Replaces `TransactionError::InstructionError(0, InstructionError::UnbalancedInstruction)` with `TransactionError::UnbalancedTransaction`. Co-authored-by: Alexander Meißner <AlexanderMeissner@gmx.net>
This commit is contained in:
parent
c750ac5d38
commit
1840fd7ab3
|
@ -236,7 +236,7 @@ struct RentMetrics {
|
|||
}
|
||||
|
||||
pub type BankStatusCache = StatusCache<Result<()>>;
|
||||
#[frozen_abi(digest = "3FiwE61TtjxHenszm3oFTzmHtGQGohJz3YN3TSTwcbUM")]
|
||||
#[frozen_abi(digest = "EzAXfE2xG3ZqdAj8KMC8CeqoSxjo5hxrEaP7fta8LT9u")]
|
||||
pub type BankSlotDelta = SlotDelta<Result<()>>;
|
||||
|
||||
#[derive(Default, Copy, Clone, Debug, PartialEq, Eq)]
|
||||
|
@ -4804,6 +4804,24 @@ impl Bank {
|
|||
) -> TransactionExecutionResult {
|
||||
let prev_accounts_data_len = self.load_accounts_data_size();
|
||||
let transaction_accounts = std::mem::take(&mut loaded_transaction.accounts);
|
||||
|
||||
fn transaction_accounts_lamports_sum(
|
||||
accounts: &[(Pubkey, AccountSharedData)],
|
||||
message: &SanitizedMessage,
|
||||
) -> Option<u128> {
|
||||
let mut lamports_sum = 0u128;
|
||||
for i in 0..message.account_keys().len() {
|
||||
let Some((_, account)) = accounts.get(i) else {
|
||||
return None;
|
||||
};
|
||||
lamports_sum = lamports_sum.checked_add(u128::from(account.lamports()))?;
|
||||
}
|
||||
Some(lamports_sum)
|
||||
}
|
||||
|
||||
let lamports_before_tx =
|
||||
transaction_accounts_lamports_sum(&transaction_accounts, tx.message()).unwrap_or(0);
|
||||
|
||||
let mut transaction_context = TransactionContext::new(
|
||||
transaction_accounts,
|
||||
if self
|
||||
|
@ -4884,7 +4902,7 @@ impl Bank {
|
|||
process_message_time.as_us()
|
||||
);
|
||||
|
||||
let status = process_result
|
||||
let mut status = process_result
|
||||
.and_then(|info| {
|
||||
let post_account_state_info =
|
||||
self.get_transaction_account_state_info(&transaction_context, tx.message());
|
||||
|
@ -4910,10 +4928,6 @@ impl Bank {
|
|||
}
|
||||
err
|
||||
});
|
||||
let mut accounts_data_len_delta = status
|
||||
.as_ref()
|
||||
.map_or(0, |info| info.accounts_data_len_delta);
|
||||
let status = status.map(|_| ());
|
||||
|
||||
let log_messages: Option<TransactionLogMessages> =
|
||||
log_collector.and_then(|log_collector| {
|
||||
|
@ -4936,6 +4950,19 @@ impl Bank {
|
|||
touched_account_count,
|
||||
accounts_resize_delta,
|
||||
} = transaction_context.into();
|
||||
|
||||
if status.is_ok()
|
||||
&& transaction_accounts_lamports_sum(&accounts, tx.message())
|
||||
.filter(|lamports_after_tx| lamports_before_tx == *lamports_after_tx)
|
||||
.is_none()
|
||||
{
|
||||
status = Err(TransactionError::UnbalancedTransaction);
|
||||
}
|
||||
let mut accounts_data_len_delta = status
|
||||
.as_ref()
|
||||
.map_or(0, |info| info.accounts_data_len_delta);
|
||||
let status = status.map(|_| ());
|
||||
|
||||
loaded_transaction.accounts = accounts;
|
||||
if self
|
||||
.feature_set
|
||||
|
|
|
@ -6577,10 +6577,9 @@ fn test_same_program_id_uses_unqiue_executable_accounts() {
|
|||
declare_process_instruction!(process_instruction, 1, |invoke_context| {
|
||||
let transaction_context = &invoke_context.transaction_context;
|
||||
let instruction_context = transaction_context.get_current_instruction_context()?;
|
||||
let _ = instruction_context
|
||||
.try_borrow_program_account(transaction_context, 1)?
|
||||
.checked_add_lamports(1);
|
||||
Ok(())
|
||||
instruction_context
|
||||
.try_borrow_program_account(transaction_context, 0)?
|
||||
.set_data_length(2)
|
||||
});
|
||||
|
||||
let (genesis_config, mint_keypair) = create_genesis_config(50000);
|
||||
|
@ -6592,7 +6591,7 @@ fn test_same_program_id_uses_unqiue_executable_accounts() {
|
|||
|
||||
// Add a new program owned by the first
|
||||
let program2_pubkey = solana_sdk::pubkey::new_rand();
|
||||
let mut program2_account = AccountSharedData::new(42, 1, &program1_pubkey);
|
||||
let mut program2_account = AccountSharedData::new(1, 1, &program1_pubkey);
|
||||
program2_account.set_executable(true);
|
||||
bank.store_account(&program2_pubkey, &program2_account);
|
||||
|
||||
|
@ -6604,8 +6603,8 @@ fn test_same_program_id_uses_unqiue_executable_accounts() {
|
|||
bank.last_blockhash(),
|
||||
);
|
||||
assert!(bank.process_transaction(&tx).is_ok());
|
||||
assert_eq!(1, bank.get_balance(&program1_pubkey));
|
||||
assert_eq!(42, bank.get_balance(&program2_pubkey));
|
||||
assert_eq!(6, bank.get_account(&program1_pubkey).unwrap().data().len());
|
||||
assert_eq!(1, bank.get_account(&program2_pubkey).unwrap().data().len());
|
||||
}
|
||||
|
||||
fn get_shrink_account_size() -> usize {
|
||||
|
|
|
@ -693,6 +693,10 @@ pub mod require_rent_exempt_split_destination {
|
|||
solana_sdk::declare_id!("D2aip4BBr8NPWtU9vLrwrBvbuaQ8w1zV38zFLxx4pfBV");
|
||||
}
|
||||
|
||||
pub mod better_error_codes_for_tx_lamport_check {
|
||||
solana_sdk::declare_id!("Ffswd3egL3tccB6Rv3XY6oqfdzn913vUcjCSnpvCKpfx");
|
||||
}
|
||||
|
||||
lazy_static! {
|
||||
/// Map of feature identifiers to user-visible description
|
||||
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
|
||||
|
@ -861,6 +865,7 @@ lazy_static! {
|
|||
(remaining_compute_units_syscall_enabled::id(), "enable the remaining_compute_units syscall"),
|
||||
(enable_program_runtime_v2_and_loader_v4::id(), "Enable Program-Runtime-v2 and Loader-v4 #33293"),
|
||||
(require_rent_exempt_split_destination::id(), "Require stake split destination account to be rent exempt"),
|
||||
(better_error_codes_for_tx_lamport_check::id(), "better error codes for tx lamport check #33353"),
|
||||
/*************** ADD NEW FEATURES HERE ***************/
|
||||
]
|
||||
.iter()
|
||||
|
|
|
@ -165,6 +165,10 @@ pub enum TransactionError {
|
|||
/// Program execution is temporarily restricted on an account.
|
||||
#[error("Execution of the program referenced by account at index {account_index} is temporarily restricted.")]
|
||||
ProgramExecutionTemporarilyRestricted { account_index: u8 },
|
||||
|
||||
/// The total balance before the transaction does not equal the total balance after the transaction
|
||||
#[error("Sum of account balances before and after transaction do not match")]
|
||||
UnbalancedTransaction,
|
||||
}
|
||||
|
||||
impl From<SanitizeError> for TransactionError {
|
||||
|
|
|
@ -61,6 +61,7 @@ enum TransactionErrorType {
|
|||
INVALID_LOADED_ACCOUNTS_DATA_SIZE_LIMIT = 33;
|
||||
RESANITIZATION_NEEDED = 34;
|
||||
PROGRAM_EXECUTION_TEMPORARILY_RESTRICTED = 35;
|
||||
UNBALANCED_TRANSACTION = 36;
|
||||
}
|
||||
|
||||
message InstructionError {
|
||||
|
|
|
@ -812,6 +812,7 @@ impl TryFrom<tx_by_addr::TransactionError> for TransactionError {
|
|||
32 => TransactionError::MaxLoadedAccountsDataSizeExceeded,
|
||||
33 => TransactionError::InvalidLoadedAccountsDataSizeLimit,
|
||||
34 => TransactionError::ResanitizationNeeded,
|
||||
36 => TransactionError::UnbalancedTransaction,
|
||||
_ => return Err("Invalid TransactionError"),
|
||||
})
|
||||
}
|
||||
|
@ -927,6 +928,9 @@ impl From<TransactionError> for tx_by_addr::TransactionError {
|
|||
TransactionError::ProgramExecutionTemporarilyRestricted { .. } => {
|
||||
tx_by_addr::TransactionErrorType::ProgramExecutionTemporarilyRestricted
|
||||
}
|
||||
TransactionError::UnbalancedTransaction => {
|
||||
tx_by_addr::TransactionErrorType::UnbalancedTransaction
|
||||
}
|
||||
} as i32,
|
||||
instruction_error: match transaction_error {
|
||||
TransactionError::InstructionError(index, ref instruction_error) => {
|
||||
|
@ -1811,6 +1815,14 @@ mod test {
|
|||
transaction_error,
|
||||
tx_by_addr_transaction_error.try_into().unwrap()
|
||||
);
|
||||
|
||||
let transaction_error = TransactionError::UnbalancedTransaction;
|
||||
let tx_by_addr_transaction_error: tx_by_addr::TransactionError =
|
||||
transaction_error.clone().into();
|
||||
assert_eq!(
|
||||
transaction_error,
|
||||
tx_by_addr_transaction_error.try_into().unwrap()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
Loading…
Reference in New Issue