From 69706be3d53fcfac17767a34df840b7bb31cc652 Mon Sep 17 00:00:00 2001 From: Tao Zhu <82401714+taozhu-chicago@users.noreply.github.com> Date: Thu, 14 Dec 2023 11:26:33 -0600 Subject: [PATCH] cleanup feature code after activated everywhere (#34359) --- runtime/src/accounts/mod.rs | 162 +++++++++++------------------------- 1 file changed, 50 insertions(+), 112 deletions(-) diff --git a/runtime/src/accounts/mod.rs b/runtime/src/accounts/mod.rs index 5027f82bede46e..da3231ba991129 100644 --- a/runtime/src/accounts/mod.rs +++ b/runtime/src/accounts/mod.rs @@ -274,7 +274,6 @@ fn load_transaction_accounts( i as IndexOfAccount, error_counters, rent_collector, - feature_set, fee, )?; @@ -484,7 +483,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 { @@ -503,23 +501,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 @@ -1641,7 +1630,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, @@ -1658,17 +1646,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, ); @@ -1693,80 +1676,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] @@ -1791,39 +1762,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, );