Skip to content

Commit

Permalink
Include account index in rent paying account tx error (backport #25189)…
Browse files Browse the repository at this point in the history
… (#25246)

* Include account index in rent paying account tx error (#25189)

(cherry picked from commit f81c5df)

# Conflicts:
#	sdk/src/feature_set.rs

* conflicts

Co-authored-by: Justin Starry <[email protected]>
  • Loading branch information
mergify[bot] and jstarry authored May 18, 2022
1 parent 3b51dfb commit 12a12b9
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 77 deletions.
13 changes: 11 additions & 2 deletions runtime/src/account_rent_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ pub(crate) fn check_rent_state(
transaction_context: &TransactionContext,
index: usize,
do_support_realloc: bool,
include_account_index_in_err: bool,
) -> Result<()> {
if let Some((pre_rent_state, post_rent_state)) = pre_rent_state.zip(post_rent_state) {
let expect_msg = "account must exist at TransactionContext index if rent-states are Some";
Expand All @@ -87,6 +88,7 @@ pub(crate) fn check_rent_state(
.expect(expect_msg)
.borrow(),
do_support_realloc,
include_account_index_in_err.then(|| index),
)?;
}
Ok(())
Expand All @@ -98,6 +100,7 @@ pub(crate) fn check_rent_state_with_account(
address: &Pubkey,
account_state: &AccountSharedData,
do_support_realloc: bool,
account_index: Option<usize>,
) -> Result<()> {
submit_rent_state_metrics(pre_rent_state, post_rent_state);
if !solana_sdk::incinerator::check_id(address)
Expand All @@ -107,9 +110,15 @@ pub(crate) fn check_rent_state_with_account(
"Account {} not rent exempt, state {:?}",
address, account_state,
);
return Err(TransactionError::InvalidRentPayingAccount);
if let Some(account_index) = account_index {
let account_index = account_index as u8;
Err(TransactionError::InsufficientFundsForRent { account_index })
} else {
Err(TransactionError::InvalidRentPayingAccount)
}
} else {
Ok(())
}
Ok(())
}

#[cfg(test)]
Expand Down
3 changes: 3 additions & 0 deletions runtime/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,9 @@ impl Accounts {
payer_address,
payer_account,
feature_set.is_active(&feature_set::do_support_realloc::id()),
feature_set
.is_active(&feature_set::include_account_index_in_rent_error::ID)
.then(|| payer_index),
);
// Feature gate only wraps the actual error return so that the metrics and debug
// logging generated by `check_rent_state_with_account()` can be examined before
Expand Down
87 changes: 59 additions & 28 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl RentDebits {
}

type BankStatusCache = StatusCache<Result<()>>;
#[frozen_abi(digest = "HSBFEjhoubTjeGKeBJvRDAiCBTVeFfcWNPZSbDW1w4H4")]
#[frozen_abi(digest = "2YZk2K45HmmAafmxPJnYVXyQ7uA7WuBrRkpwrCawdK31")]
pub type BankSlotDelta = SlotDelta<Result<()>>;

// Eager rent collection repeats in cyclic manner.
Expand Down Expand Up @@ -3958,7 +3958,8 @@ impl Bank {
})
.map_err(|err| {
match err {
TransactionError::InvalidRentPayingAccount => {
TransactionError::InvalidRentPayingAccount
| TransactionError::InsufficientFundsForRent { .. } => {
error_counters.invalid_rent_paying_account += 1;
}
_ => {
Expand Down Expand Up @@ -16890,9 +16891,9 @@ pub(crate) mod tests {
recent_blockhash,
);
let result = bank.process_transaction(&tx);
assert_ne!(
assert_eq!(
result.unwrap_err(),
TransactionError::InvalidRentPayingAccount
TransactionError::InstructionError(0, InstructionError::Custom(1))
);
assert_ne!(
fee_payer_balance,
Expand Down Expand Up @@ -16935,7 +16936,7 @@ pub(crate) mod tests {
let result = bank.process_transaction(&tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InvalidRentPayingAccount
TransactionError::InsufficientFundsForRent { account_index: 0 }
);
assert!(check_account_is_rent_exempt(
&rent_exempt_fee_payer.pubkey()
Expand All @@ -16957,7 +16958,7 @@ pub(crate) mod tests {
let result = bank.process_transaction(&tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InvalidRentPayingAccount
TransactionError::InsufficientFundsForRent { account_index: 0 }
);
assert!(check_account_is_rent_exempt(
&rent_exempt_fee_payer.pubkey()
Expand All @@ -16984,7 +16985,7 @@ pub(crate) mod tests {
let result = bank.process_transaction(&tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InvalidRentPayingAccount
TransactionError::InsufficientFundsForRent { account_index: 0 }
);
assert_eq!(
fee_payer_balance - fee,
Expand Down Expand Up @@ -17037,7 +17038,7 @@ pub(crate) mod tests {
let result = bank.process_transaction(&tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InvalidRentPayingAccount
TransactionError::InsufficientFundsForRent { account_index: 0 }
);
assert!(check_account_is_rent_exempt(
&rent_exempt_fee_payer.pubkey()
Expand Down Expand Up @@ -17314,10 +17315,16 @@ pub(crate) mod tests {
mock_program_id,
recent_blockhash,
);
assert_eq!(
bank.process_transaction(&tx).unwrap_err(),
TransactionError::InvalidRentPayingAccount,
);
let expected_err = {
let account_index = tx
.message
.account_keys
.iter()
.position(|key| key == &rent_paying_pubkey)
.unwrap() as u8;
TransactionError::InsufficientFundsForRent { account_index }
};
assert_eq!(bank.process_transaction(&tx).unwrap_err(), expected_err);
assert_eq!(
rent_exempt_minimum_small - 1,
bank.get_account(&rent_paying_pubkey).unwrap().lamports()
Expand Down Expand Up @@ -17350,10 +17357,16 @@ pub(crate) mod tests {
mock_program_id,
recent_blockhash,
);
assert_eq!(
bank.process_transaction(&tx).unwrap_err(),
TransactionError::InvalidRentPayingAccount,
);
let expected_err = {
let account_index = tx
.message
.account_keys
.iter()
.position(|key| key == &rent_paying_pubkey)
.unwrap() as u8;
TransactionError::InsufficientFundsForRent { account_index }
};
assert_eq!(bank.process_transaction(&tx).unwrap_err(), expected_err);
assert_eq!(
rent_exempt_minimum_large,
bank.get_account(&rent_paying_pubkey).unwrap().lamports()
Expand Down Expand Up @@ -17386,10 +17399,16 @@ pub(crate) mod tests {
mock_program_id,
recent_blockhash,
);
assert_eq!(
bank.process_transaction(&tx).unwrap_err(),
TransactionError::InvalidRentPayingAccount,
);
let expected_err = {
let account_index = tx
.message
.account_keys
.iter()
.position(|key| key == &rent_paying_pubkey)
.unwrap() as u8;
TransactionError::InsufficientFundsForRent { account_index }
};
assert_eq!(bank.process_transaction(&tx).unwrap_err(), expected_err);
assert_eq!(
rent_exempt_minimum_small,
bank.get_account(&rent_paying_pubkey).unwrap().lamports()
Expand Down Expand Up @@ -17423,10 +17442,16 @@ pub(crate) mod tests {
account_data_size_small as u64,
&system_program::id(),
);
assert_eq!(
bank.process_transaction(&tx).unwrap_err(),
TransactionError::InvalidRentPayingAccount,
);
let expected_err = {
let account_index = tx
.message
.account_keys
.iter()
.position(|key| key == &created_keypair.pubkey())
.unwrap() as u8;
TransactionError::InsufficientFundsForRent { account_index }
};
assert_eq!(bank.process_transaction(&tx).unwrap_err(), expected_err);

// create account, rent exempt
let tx = system_transaction::create_account(
Expand Down Expand Up @@ -17472,10 +17497,16 @@ pub(crate) mod tests {
recent_blockhash,
(account_data_size_small + 1) as u64,
);
assert_eq!(
bank.process_transaction(&tx).unwrap_err(),
TransactionError::InvalidRentPayingAccount,
);
let expected_err = {
let account_index = tx
.message
.account_keys
.iter()
.position(|key| key == &created_keypair.pubkey())
.unwrap() as u8;
TransactionError::InsufficientFundsForRent { account_index }
};
assert_eq!(bank.process_transaction(&tx).unwrap_err(), expected_err);

// bring balance of account up to rent exemption
let tx = system_transaction::transfer(
Expand Down
4 changes: 4 additions & 0 deletions runtime/src/bank/transaction_account_state_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ impl Bank {
let do_support_realloc = self
.feature_set
.is_active(&feature_set::do_support_realloc::id());
let include_account_index_in_err = self
.feature_set
.is_active(&feature_set::include_account_index_in_rent_error::id());
for (i, (pre_state_info, post_state_info)) in
pre_state_infos.iter().zip(post_state_infos).enumerate()
{
Expand All @@ -70,6 +73,7 @@ impl Bank {
transaction_context,
i,
do_support_realloc,
include_account_index_in_err,
) {
// Feature gate only wraps the actual error return so that the metrics and debug
// logging generated by `check_rent_state()` can be examined before feature
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,10 @@ pub mod add_set_compute_unit_price_ix {
solana_sdk::declare_id!("98std1NSHqXi9WYvFShfVepRdCoq1qvsp8fsR2XZtG8g");
}

pub mod include_account_index_in_rent_error {
solana_sdk::declare_id!("2R72wpcQ7qV7aTJWUumdn8u5wmmTyXbK7qzEy7YSAgyY");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -433,6 +437,7 @@ lazy_static! {
(stake_allow_zero_undelegated_amount::id(), "Allow zero-lamport undelegated amount for initialized stakes #24670"),
(require_static_program_ids_in_transaction::id(), "require static program ids in versioned transactions"),
(add_set_compute_unit_price_ix::id(), "add compute budget ix for setting a compute unit price"),
(include_account_index_in_rent_error::id(), "include account index in rent tx error #25190"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down
6 changes: 6 additions & 0 deletions sdk/src/transaction/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ pub enum TransactionError {
/// Transaction contains a duplicate instruction that is not allowed
#[error("Transaction contains a duplicate instruction ({0}) that is not allowed")]
DuplicateInstruction(u8),

/// Transaction results in an account without insufficient funds for rent
#[error(
"Transaction results in an account ({account_index}) without insufficient funds for rent"
)]
InsufficientFundsForRent { account_index: u8 },
}

impl From<SanitizeError> for TransactionError {
Expand Down
5 changes: 3 additions & 2 deletions storage-proto/proto/transaction_by_addr.proto
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ message Memo {
message TransactionError {
TransactionErrorType transaction_error = 1;
InstructionError instruction_error = 2;
DuplicateInstructionError duplicate_instruction_error = 3;
TransactionDetails transaction_details = 3;
}

enum TransactionErrorType {
Expand Down Expand Up @@ -56,6 +56,7 @@ enum TransactionErrorType {
WOULD_EXCEED_MAX_VOTE_COST_LIMIT = 28;
WOULD_EXCEED_ACCOUNT_DATA_TOTAL_LIMIT = 29;
DUPLICATE_INSTRUCTION = 30;
INSUFFICIENT_FUNDS_FOR_RENT = 31;
}

message InstructionError {
Expand All @@ -64,7 +65,7 @@ message InstructionError {
CustomError custom = 3;
}

message DuplicateInstructionError {
message TransactionDetails {
uint32 index = 1;
}

Expand Down
Loading

0 comments on commit 12a12b9

Please sign in to comment.