Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

cleanup feature code after activated everywhere #34359

Merged
merged 1 commit into from
Dec 14, 2023
Merged

cleanup feature code after activated everywhere #34359

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 50 additions & 112 deletions runtime/src/accounts/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,6 @@ fn load_transaction_accounts(
i as IndexOfAccount,
error_counters,
rent_collector,
feature_set,
fee,
)?;

Expand Down Expand Up @@ -492,7 +491,6 @@ fn validate_fee_payer(
payer_index: IndexOfAccount,
error_counters: &mut TransactionErrorMetrics,
rent_collector: &RentCollector,
feature_set: &FeatureSet,
fee: u64,
) -> Result<()> {
if payer_account.lamports() == 0 {
Expand All @@ -511,23 +509,14 @@ fn validate_fee_payer(
}
};

// allow collapsible-else-if to make removing the feature gate safer once activated
#[allow(clippy::collapsible_else_if)]
if feature_set.is_active(&feature_set::checked_arithmetic_in_fee_validation::id()) {
payer_account
.lamports()
.checked_sub(min_balance)
.and_then(|v| v.checked_sub(fee))
.ok_or_else(|| {
error_counters.insufficient_funds += 1;
TransactionError::InsufficientFundsForFee
})?;
} else {
if payer_account.lamports() < fee + min_balance {
payer_account
.lamports()
.checked_sub(min_balance)
.and_then(|v| v.checked_sub(fee))
.ok_or_else(|| {
error_counters.insufficient_funds += 1;
return Err(TransactionError::InsufficientFundsForFee);
}
}
TransactionError::InsufficientFundsForFee
})?;

let payer_pre_rent_state = RentState::from_account(payer_account, &rent_collector.rent);
payer_account
Expand Down Expand Up @@ -1661,7 +1650,6 @@ mod tests {
fee: u64,
expected_result: Result<()>,
payer_post_balance: u64,
feature_checked_arithmmetic_enable: bool,
}
fn validate_fee_payer_account(
test_parameter: ValidateFeePayerTestParameter,
Expand All @@ -1678,17 +1666,12 @@ mod tests {
} else {
AccountSharedData::new(test_parameter.payer_init_balance, 0, &system_program::id())
};
let mut feature_set = FeatureSet::default();
if test_parameter.feature_checked_arithmmetic_enable {
feature_set.activate(&feature_set::checked_arithmetic_in_fee_validation::id(), 0);
};
let result = validate_fee_payer(
&payer_account_keys.pubkey(),
&mut account,
0,
&mut TransactionErrorMetrics::default(),
rent_collector,
&feature_set,
test_parameter.fee,
);

Expand All @@ -1713,80 +1696,68 @@ mod tests {
// If payer account has sufficient balance, expect successful fee deduction,
// regardless feature gate status, or if payer is nonce account.
{
for feature_checked_arithmmetic_enable in [true, false] {
for (is_nonce, min_balance) in [(true, min_balance), (false, 0)] {
validate_fee_payer_account(
ValidateFeePayerTestParameter {
is_nonce,
payer_init_balance: min_balance + fee,
fee,
expected_result: Ok(()),
payer_post_balance: min_balance,
feature_checked_arithmmetic_enable,
},
&rent_collector,
);
}
for (is_nonce, min_balance) in [(true, min_balance), (false, 0)] {
validate_fee_payer_account(
ValidateFeePayerTestParameter {
is_nonce,
payer_init_balance: min_balance + fee,
fee,
expected_result: Ok(()),
payer_post_balance: min_balance,
},
&rent_collector,
);
}
}

// If payer account has no balance, expected AccountNotFound Error
// regardless feature gate status, or if payer is nonce account.
{
for feature_checked_arithmmetic_enable in [true, false] {
for is_nonce in [true, false] {
validate_fee_payer_account(
ValidateFeePayerTestParameter {
is_nonce,
payer_init_balance: 0,
fee,
expected_result: Err(TransactionError::AccountNotFound),
payer_post_balance: 0,
feature_checked_arithmmetic_enable,
},
&rent_collector,
);
}
for is_nonce in [true, false] {
validate_fee_payer_account(
ValidateFeePayerTestParameter {
is_nonce,
payer_init_balance: 0,
fee,
expected_result: Err(TransactionError::AccountNotFound),
payer_post_balance: 0,
},
&rent_collector,
);
}
}

// If payer account has insufficent balance, expect InsufficientFundsForFee error
// regardless feature gate status, or if payer is nonce account.
{
for feature_checked_arithmmetic_enable in [true, false] {
for (is_nonce, min_balance) in [(true, min_balance), (false, 0)] {
validate_fee_payer_account(
ValidateFeePayerTestParameter {
is_nonce,
payer_init_balance: min_balance + fee - 1,
fee,
expected_result: Err(TransactionError::InsufficientFundsForFee),
payer_post_balance: min_balance + fee - 1,
feature_checked_arithmmetic_enable,
},
&rent_collector,
);
}
}
}

// normal payer account has balance of u64::MAX, so does fee; since it does not require
// min_balance, expect successful fee deduction, regardless of feature gate status
{
for feature_checked_arithmmetic_enable in [true, false] {
for (is_nonce, min_balance) in [(true, min_balance), (false, 0)] {
validate_fee_payer_account(
ValidateFeePayerTestParameter {
is_nonce: false,
payer_init_balance: u64::MAX,
fee: u64::MAX,
expected_result: Ok(()),
payer_post_balance: 0,
feature_checked_arithmmetic_enable,
is_nonce,
payer_init_balance: min_balance + fee - 1,
fee,
expected_result: Err(TransactionError::InsufficientFundsForFee),
payer_post_balance: min_balance + fee - 1,
},
&rent_collector,
);
}
}

// normal payer account has balance of u64::MAX, so does fee; since it does not require
// min_balance, expect successful fee deduction, regardless of feature gate status
{
validate_fee_payer_account(
ValidateFeePayerTestParameter {
is_nonce: false,
payer_init_balance: u64::MAX,
fee: u64::MAX,
expected_result: Ok(()),
payer_post_balance: 0,
},
&rent_collector,
);
}
}

#[test]
Expand All @@ -1811,39 +1782,6 @@ mod tests {
fee: u64::MAX,
expected_result: Err(TransactionError::InsufficientFundsForFee),
payer_post_balance: u64::MAX,
feature_checked_arithmmetic_enable: true,
},
&rent_collector,
);
}

#[test]
#[should_panic]
fn test_validate_nonce_fee_payer_without_checked_arithmetic() {
let rent_collector = RentCollector::new(
0,
EpochSchedule::default(),
500_000.0,
Rent {
lamports_per_byte_year: 1,
..Rent::default()
},
);

// same test setup as `test_validate_nonce_fee_payer_with_checked_arithmetic`:
// nonce payer account has balance of u64::MAX, so does fee; and nonce account
// requires additional min_balance, if feature gate is not enabled, in `debug`
// mode, `u64::MAX + min_balance` would panic on "attempt to add with overflow";
// in `release` mode, the addition will wrap, so the expected result would be
// `Ok(())` with post payer balance `0`, therefore fails test with a panic.
validate_fee_payer_account(
ValidateFeePayerTestParameter {
is_nonce: true,
payer_init_balance: u64::MAX,
fee: u64::MAX,
expected_result: Err(TransactionError::InsufficientFundsForFee),
payer_post_balance: u64::MAX,
feature_checked_arithmmetic_enable: false,
},
&rent_collector,
);
Expand Down