blockworks-foundation
/
solana
mirror of https://github.com/blockworks-foundation/solana.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

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

This commit is contained in:
Justin Starry 2022-05-16 23:35:34 +08:00 committed by GitHub
parent 389d78e424
commit f81c5df1f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 165 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
runtime/src/account_rent_state.rs
View File

@ -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";
@ -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(())
@ -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)
@ -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)]

3
runtime/src/accounts.rs
View File

@ -385,6 +385,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

87
runtime/src/bank.rs
View File

@ -240,7 +240,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.
@ -4245,7 +4245,8 @@ impl Bank {
})
.map_err(|err| {
match err {
TransactionError::InvalidRentPayingAccount => {
TransactionError::InvalidRentPayingAccount
| TransactionError::InsufficientFundsForRent { .. } => {
error_counters.invalid_rent_paying_account += 1;
}
_ => {
@ -17421,9 +17422,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,
@ -17466,7 +17467,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()
@ -17488,7 +17489,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()
@ -17515,7 +17516,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,
@ -17568,7 +17569,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()
@ -17935,10 +17936,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()
@ -17971,10 +17978,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()
@ -18007,10 +18020,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()
@ -18044,10 +18063,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(
@ -18093,10 +18118,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(

4
runtime/src/bank/transaction_account_state_info.rs
View File

@ -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()
{
@ -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

5
sdk/src/feature_set.rs
View File

@ -404,6 +404,10 @@ pub mod disable_deploy_of_alloc_free_syscall {
solana_sdk::declare_id!("79HWsX9rpnnJBPcdNURVqygpMAfxdrAirzAGAVmf92im");
}
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> = [
@ -498,6 +502,7 @@ lazy_static! {
(stake_raise_minimum_delegation_to_1_sol::id(), "Raise minimum stake delegation to 1.0 SOL #24357"),
(add_set_compute_unit_price_ix::id(), "add compute budget ix for setting a compute unit price"),
(disable_deploy_of_alloc_free_syscall::id(), "disable new deployments of deprecated sol_alloc_free_ syscall"),
(include_account_index_in_rent_error::id(), "include account index in rent tx error #25190"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()

6
sdk/src/transaction/error.rs
View File

@ -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 {

5
storage-proto/proto/transaction_by_addr.proto
View File

@ -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 {
@ -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 {
@ -64,7 +65,7 @@ message InstructionError {
CustomError custom = 3;
}
message DuplicateInstructionError {
message TransactionDetails {
uint32 index = 1;
}

119
storage-proto/src/convert.rs
View File

@ -716,10 +716,20 @@ impl TryFrom<tx_by_addr::TransactionError> for TransactionError {
}
}
if let Some(duplicate_instruction) = transaction_error.duplicate_instruction_error {
return Ok(TransactionError::DuplicateInstruction(
duplicate_instruction.index as u8,
));
if let Some(transaction_details) = transaction_error.transaction_details {
match transaction_error.transaction_error {
30 => {
return Ok(TransactionError::DuplicateInstruction(
transaction_details.index as u8,
));
}
31 => {
return Ok(TransactionError::InsufficientFundsForRent {
account_index: transaction_details.index as u8,
});
}
_ => {}
}
}
Ok(match transaction_error.transaction_error {
@ -852,6 +862,9 @@ impl From<TransactionError> for tx_by_addr::TransactionError {
TransactionError::DuplicateInstruction(_) => {
tx_by_addr::TransactionErrorType::DuplicateInstruction
}
TransactionError::InsufficientFundsForRent { .. } => {
tx_by_addr::TransactionErrorType::InsufficientFundsForRent
}
} as i32,
instruction_error: match transaction_error {
TransactionError::InstructionError(index, ref instruction_error) => {
@ -1023,12 +1036,17 @@ impl From<TransactionError> for tx_by_addr::TransactionError {
}
_ => None,
},
duplicate_instruction_error: match transaction_error {
transaction_details: match transaction_error {
TransactionError::DuplicateInstruction(index) => {
Some(tx_by_addr::DuplicateInstructionError {
Some(tx_by_addr::TransactionDetails {
index: index as u32,
})
}
TransactionError::InsufficientFundsForRent { account_index } => {
Some(tx_by_addr::TransactionDetails {
index: account_index as u32,
})
}
_ => None,
},
}
@ -1706,6 +1724,14 @@ mod test {
transaction_error,
tx_by_addr_transaction_error.try_into().unwrap()
);
let transaction_error = TransactionError::InsufficientFundsForRent { account_index: 10 };
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]
@ -1713,12 +1739,13 @@ mod test {
let ix_index = 1;
let custom_error = 42;
for error in tx_by_addr::TransactionErrorType::into_enum_iter() {
if error != tx_by_addr::TransactionErrorType::InstructionError {
if error == tx_by_addr::TransactionErrorType::DuplicateInstruction {
match error {
tx_by_addr::TransactionErrorType::DuplicateInstruction
| tx_by_addr::TransactionErrorType::InsufficientFundsForRent => {
let tx_by_addr_error = tx_by_addr::TransactionError {
transaction_error: error as i32,
instruction_error: None,
duplicate_instruction_error: Some(tx_by_addr::DuplicateInstructionError {
transaction_details: Some(tx_by_addr::TransactionDetails {
index: ix_index,
}),
};
@ -1727,11 +1754,47 @@ mod test {
.try_into()
.unwrap_or_else(|_| panic!("{:?} conversion implemented?", error));
assert_eq!(tx_by_addr_error, transaction_error.into());
} else {
}
tx_by_addr::TransactionErrorType::InstructionError => {
for ix_error in tx_by_addr::InstructionErrorType::into_enum_iter() {
if ix_error != tx_by_addr::InstructionErrorType::Custom {
let tx_by_addr_error = tx_by_addr::TransactionError {
transaction_error: error as i32,
instruction_error: Some(tx_by_addr::InstructionError {
index: ix_index,
error: ix_error as i32,
custom: None,
}),
transaction_details: None,
};
let transaction_error: TransactionError =
tx_by_addr_error.clone().try_into().unwrap_or_else(|_| {
panic!("{:?} conversion implemented?", ix_error)
});
assert_eq!(tx_by_addr_error, transaction_error.into());
} else {
let tx_by_addr_error = tx_by_addr::TransactionError {
transaction_error: error as i32,
instruction_error: Some(tx_by_addr::InstructionError {
index: ix_index,
error: ix_error as i32,
custom: Some(tx_by_addr::CustomError {
custom: custom_error,
}),
}),
transaction_details: None,
};
let transaction_error: TransactionError =
tx_by_addr_error.clone().try_into().unwrap();
assert_eq!(tx_by_addr_error, transaction_error.into());
}
}
}
_ => {
let tx_by_addr_error = tx_by_addr::TransactionError {
transaction_error: error as i32,
instruction_error: None,
duplicate_instruction_error: None,
transaction_details: None,
};
let transaction_error: TransactionError = tx_by_addr_error
.clone()
@ -1739,40 +1802,6 @@ mod test {
.unwrap_or_else(|_| panic!("{:?} conversion implemented?", error));
assert_eq!(tx_by_addr_error, transaction_error.into());
}
} else {
for ix_error in tx_by_addr::InstructionErrorType::into_enum_iter() {
if ix_error != tx_by_addr::InstructionErrorType::Custom {
let tx_by_addr_error = tx_by_addr::TransactionError {
transaction_error: error as i32,
instruction_error: Some(tx_by_addr::InstructionError {
index: ix_index,
error: ix_error as i32,
custom: None,
}),
duplicate_instruction_error: None,
};
let transaction_error: TransactionError = tx_by_addr_error
.clone()
.try_into()
.unwrap_or_else(|_| panic!("{:?} conversion implemented?", ix_error));
assert_eq!(tx_by_addr_error, transaction_error.into());
} else {
let tx_by_addr_error = tx_by_addr::TransactionError {
transaction_error: error as i32,
instruction_error: Some(tx_by_addr::InstructionError {
index: ix_index,
error: ix_error as i32,
custom: Some(tx_by_addr::CustomError {
custom: custom_error,
}),
}),
duplicate_instruction_error: None,
};
let transaction_error: TransactionError =
tx_by_addr_error.clone().try_into().unwrap();
assert_eq!(tx_by_addr_error, transaction_error.into());
}
}
}
}
}