Skip to content

Commit

Permalink
add feature gate; updated tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@tao-stones
tao-stones committed Apr 20, 2023
1 parent 0a37011 commit 7e62a48
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 120 deletions.
273 changes: 153 additions & 120 deletions runtime/src/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,14 +594,20 @@ impl Accounts {
}
};

payer_account
.lamports()
.checked_sub(min_balance)
.and_then(|v| v.checked_sub(fee))
.ok_or_else(|| {
error_counters.insufficient_funds += 1;
TransactionError::InsufficientFundsForFee
})?;
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 {
error_counters.insufficient_funds += 1;
return Err(TransactionError::InsufficientFundsForFee);
}

let payer_pre_rent_state = RentState::from_account(payer_account, &rent_collector.rent);
payer_account
.checked_sub_lamports(fee)
Expand Down Expand Up @@ -4324,6 +4330,45 @@ mod tests {
);
}

struct TestParameter {
is_nonce: bool,
payer_init_balance: u64,
fee: u64,
expected_result: Result<()>,
payer_post_balance: u64,
feature_checked_arithmmetic_enable: bool,
}

fn validate_fee_payer_account(test_parameter: TestParameter, rent_collector: &RentCollector) {
let payer_account_keys = Keypair::new();
let mut account = if test_parameter.is_nonce {
AccountSharedData::new_data(
test_parameter.payer_init_balance,
&NonceVersions::new(NonceState::Initialized(nonce::state::Data::default())),
&system_program::id(),
)
.unwrap()
} 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 = Accounts::validate_fee_payer(
&payer_account_keys.pubkey(),
&mut account,
0,
&mut TransactionErrorMetrics::default(),
rent_collector,
&feature_set,
test_parameter.fee,
);

assert_eq!(result, test_parameter.expected_result);
assert_eq!(account.lamports(), test_parameter.payer_post_balance);
}

#[test]
fn test_validate_fee_payer() {
let rent_collector = RentCollector::new(
Expand All @@ -4338,126 +4383,100 @@ mod tests {
let min_balance = rent_collector.rent.minimum_balance(NonceState::size());
let fee = 5_000;

struct TestParameter {
is_nonce: bool,
payer_init_balance: u64,
fee: u64,
expected_result: Result<()>,
payer_post_balance: u64,
}

fn validate_fee_payer_account(
test_parameter: TestParameter,
rent_collector: &RentCollector,
) {
let payer_account_keys = Keypair::new();
let mut account = if test_parameter.is_nonce {
AccountSharedData::new_data(
test_parameter.payer_init_balance,
&NonceVersions::new(NonceState::Initialized(nonce::state::Data::default())),
&system_program::id(),
)
.unwrap()
} else {
AccountSharedData::new(test_parameter.payer_init_balance, 0, &system_program::id())
};
let result = Accounts::validate_fee_payer(
&payer_account_keys.pubkey(),
&mut account,
0,
&mut TransactionErrorMetrics::default(),
rent_collector,
&FeatureSet::default(),
test_parameter.fee,
);

assert_eq!(result, test_parameter.expected_result);
assert_eq!(account.lamports(), test_parameter.payer_post_balance);
}

// nonce payer account has sufficient balance, expect successful fee deduction
// If payer account has sufficient balance, expect successful fee deduction,
// regardless feature gate status, or if payer is nonce account.
{
validate_fee_payer_account(
TestParameter {
is_nonce: true,
payer_init_balance: min_balance + fee,
fee,
expected_result: Ok(()),
payer_post_balance: min_balance,
},
&rent_collector,
);
}
// normal payer account has sufficient balance, expect successful fee deduction
{
validate_fee_payer_account(
TestParameter {
is_nonce: false,
payer_init_balance: fee,
fee,
expected_result: Ok(()),
payer_post_balance: 0,
},
&rent_collector,
);
for feature_checked_arithmmetic_enable in [true, false] {
for (is_nonce, min_balance) in [(true, min_balance), (false, 0)] {
validate_fee_payer_account(
TestParameter {
is_nonce,
payer_init_balance: min_balance + fee,
fee,
expected_result: Ok(()),
payer_post_balance: min_balance,
feature_checked_arithmmetic_enable,
},
&rent_collector,
);
}
}
}

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

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

// 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(
TestParameter {
is_nonce: false,
payer_init_balance: fee - 1,
fee,
expected_result: Err(TransactionError::InsufficientFundsForFee),
payer_post_balance: fee - 1,
},
&rent_collector,
);
for feature_checked_arithmmetic_enable in [true, false] {
validate_fee_payer_account(
TestParameter {
is_nonce: false,
payer_init_balance: u64::MAX,
fee: u64::MAX,
expected_result: Ok(()),
payer_post_balance: 0,
feature_checked_arithmmetic_enable,
},
&rent_collector,
);
}
}
}

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

// nonce payer account has balance of u64::MAX, so does fee; due to nonce account
// requires additional min_balance, expect InsufficientFundsForFee error
// requires additional min_balance, expect InsufficientFundsForFee error if feature gate is
// enabled
{
validate_fee_payer_account(
TestParameter {
Expand All @@ -4466,20 +4485,34 @@ mod tests {
fee: u64::MAX,
expected_result: Err(TransactionError::InsufficientFundsForFee),
payer_post_balance: u64::MAX,
feature_checked_arithmmetic_enable: true,
},
&rent_collector,
);
}
// normal payer account has balance of u64::MAX, so does fee; without requiring
// min_balance, expect successful fee deduction
}

#[test]
#[should_panic(expected = "attempt to add with overflow")]
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()
},
);
{
validate_fee_payer_account(
TestParameter {
is_nonce: false,
is_nonce: true,
payer_init_balance: u64::MAX,
fee: u64::MAX,
expected_result: Ok(()),
payer_post_balance: 0,
payer_post_balance: u64::MAX,
feature_checked_arithmmetic_enable: false,
},
&rent_collector,
);
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,10 @@ pub mod vote_state_add_vote_latency {
solana_sdk::declare_id!("7axKe5BTYBDD87ftzWbk5DfzWMGyRvqmWTduuo22Yaqy");
}

pub mod checked_arithmetic_in_fee_validation {
solana_sdk::declare_id!("5Pecy6ie6XGm22pc9d4P9W5c31BugcFBuy6hsP2zkETv");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -817,6 +821,7 @@ lazy_static! {
(stop_truncating_strings_in_syscalls::id(), "Stop truncating strings in syscalls #31029"),
(clean_up_delegation_errors::id(), "Return InsufficientDelegation instead of InsufficientFunds or InsufficientStake where applicable #31206"),
(vote_state_add_vote_latency::id(), "replace Lockout with LandedVote (including vote latency) in vote state #31264"),
(checked_arithmetic_in_fee_validation::id(), "checked arithmetic in fee validation #31273"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down

0 comments on commit 7e62a48

Please sign in to comment.