Skip to content

Commit

Permalink
Feature cleanup: Removing unwanted rounding in fee calculation #21 (#…
Browse files Browse the repository at this point in the history
…4676)

cleanup activated feature: Removing unwanted rounding in fee calculation #21
  • Loading branch information
tao-stones authored Jan 30, 2025
1 parent 1d77f88 commit dad81b9
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 57 deletions.
8 changes: 1 addition & 7 deletions fee/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use {
solana_feature_set::{
enable_secp256r1_precompile, remove_rounding_in_fee_calculation, FeatureSet,
},
solana_feature_set::{enable_secp256r1_precompile, FeatureSet},
solana_fee_structure::FeeDetails,
solana_svm_transaction::svm_message::SVMMessage,
};
Expand All @@ -14,15 +12,12 @@ use {
// in the future. Keeping this struct will help keep things organized.
#[derive(Copy, Clone)]
pub struct FeeFeatures {
pub remove_rounding_in_fee_calculation: bool,
pub enable_secp256r1_precompile: bool,
}

impl From<&FeatureSet> for FeeFeatures {
fn from(feature_set: &FeatureSet) -> Self {
Self {
remove_rounding_in_fee_calculation: feature_set
.is_active(&remove_rounding_in_fee_calculation::ID),
enable_secp256r1_precompile: feature_set.is_active(&enable_secp256r1_precompile::ID),
}
}
Expand Down Expand Up @@ -64,7 +59,6 @@ pub fn calculate_fee_details(
fee_features.enable_secp256r1_precompile,
),
prioritization_fee,
fee_features.remove_rounding_in_fee_calculation,
)
}

Expand Down
9 changes: 4 additions & 5 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3044,7 +3044,7 @@ fn test_filter_program_errors_and_collect_fee() {
bank.deactivate_feature(&feature_set::reward_full_priority_fee::id());

let tx_fee = 42;
let fee_details = FeeDetails::new(tx_fee, 0, false);
let fee_details = FeeDetails::new(tx_fee, 0);
let processing_results = vec![
Err(TransactionError::AccountNotFound),
new_executed_processing_result(Ok(()), fee_details),
Expand Down Expand Up @@ -3083,7 +3083,7 @@ fn test_filter_program_errors_and_collect_priority_fee() {
bank.deactivate_feature(&feature_set::reward_full_priority_fee::id());

let priority_fee = 42;
let fee_details: FeeDetails = FeeDetails::new(0, priority_fee, false);
let fee_details: FeeDetails = FeeDetails::new(0, priority_fee);
let processing_results = vec![
Err(TransactionError::AccountNotFound),
new_executed_processing_result(Ok(()), fee_details),
Expand Down Expand Up @@ -3344,7 +3344,7 @@ fn test_load_and_execute_commit_transactions_fees_only(enable_fees_only_txs: boo
inner_instructions: None,
return_data: None,
executed_units: 0,
fee_details: FeeDetails::new(5000, 0, true),
fee_details: FeeDetails::new(5000, 0),
rent_debits: RentDebits::default(),
loaded_account_stats: TransactionLoadedAccountsStats {
loaded_accounts_count: 2,
Expand Down Expand Up @@ -10392,7 +10392,6 @@ fn calculate_test_fee(
fee_structure.lamports_per_signature,
fee_budget_limits.prioritization_fee,
FeeFeatures {
remove_rounding_in_fee_calculation: true,
enable_secp256r1_precompile: true,
},
)
Expand Down Expand Up @@ -13234,7 +13233,7 @@ fn test_filter_program_errors_and_collect_fee_details() {
let initial_payer_balance = 7_000;
let tx_fee = 5000;
let priority_fee = 1000;
let fee_details = FeeDetails::new(tx_fee, priority_fee, false);
let fee_details = FeeDetails::new(tx_fee, priority_fee);
let expected_collected_fee_details = CollectorFeeDetails {
transaction_fee: 3 * tx_fee,
priority_fee: 3 * priority_fee,
Expand Down
44 changes: 2 additions & 42 deletions sdk/fee-structure/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,30 +41,18 @@ pub struct FeeStructure {
pub struct FeeDetails {
transaction_fee: u64,
prioritization_fee: u64,
remove_rounding_in_fee_calculation: bool,
}

impl FeeDetails {
pub fn new(
transaction_fee: u64,
prioritization_fee: u64,
remove_rounding_in_fee_calculation: bool,
) -> Self {
pub fn new(transaction_fee: u64, prioritization_fee: u64) -> Self {
Self {
transaction_fee,
prioritization_fee,
remove_rounding_in_fee_calculation,
}
}

pub fn total_fee(&self) -> u64 {
let total_fee = self.transaction_fee.saturating_add(self.prioritization_fee);
if self.remove_rounding_in_fee_calculation {
total_fee
} else {
// backward compatible behavior
(total_fee as f64).round() as u64
}
self.transaction_fee.saturating_add(self.prioritization_fee)
}

pub fn accumulate(&mut self, fee_details: &FeeDetails) {
Expand Down Expand Up @@ -141,15 +129,13 @@ impl FeeStructure {
lamports_per_signature: u64,
budget_limits: &FeeBudgetLimits,
include_loaded_account_data_size_in_fee: bool,
remove_rounding_in_fee_calculation: bool,
) -> u64 {
#[allow(deprecated)]
self.calculate_fee_details(
message,
lamports_per_signature,
budget_limits,
include_loaded_account_data_size_in_fee,
remove_rounding_in_fee_calculation,
)
.total_fee()
}
Expand All @@ -166,7 +152,6 @@ impl FeeStructure {
lamports_per_signature: u64,
budget_limits: &FeeBudgetLimits,
include_loaded_account_data_size_in_fee: bool,
remove_rounding_in_fee_calculation: bool,
) -> FeeDetails {
// Backward compatibility - lamports_per_signature == 0 means to clear
// transaction fee to zero
Expand Down Expand Up @@ -210,7 +195,6 @@ impl FeeStructure {
.saturating_add(write_lock_fee)
.saturating_add(compute_fee),
prioritization_fee: budget_limits.prioritization_fee,
remove_rounding_in_fee_calculation,
}
}
}
Expand Down Expand Up @@ -263,28 +247,4 @@ mod tests {
FeeStructure::calculate_memory_usage_cost(64 * K, heap_cost)
);
}

#[test]
fn test_total_fee_rounding() {
// round large `f64` can lost precision, see feature gate:
// "Removing unwanted rounding in fee calculation #34982"

let transaction_fee = u64::MAX - 11;
let prioritization_fee = 1;
let expected_large_fee = u64::MAX - 10;

let details_with_rounding = FeeDetails {
transaction_fee,
prioritization_fee,
remove_rounding_in_fee_calculation: false,
};
let details_without_rounding = FeeDetails {
transaction_fee,
prioritization_fee,
remove_rounding_in_fee_calculation: true,
};

assert_eq!(details_without_rounding.total_fee(), expected_large_fee);
assert_ne!(details_with_rounding.total_fee(), expected_large_fee);
}
}
6 changes: 3 additions & 3 deletions svm/src/transaction_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2194,7 +2194,7 @@ mod tests {
fee_payer_rent_epoch
),
compute_budget_limits,
fee_details: FeeDetails::new(transaction_fee, priority_fee, false),
fee_details: FeeDetails::new(transaction_fee, priority_fee),
loaded_fee_payer_account: LoadedTransactionAccount {
loaded_size: fee_payer_account.data().len(),
account: post_validation_fee_payer_account,
Expand Down Expand Up @@ -2271,7 +2271,7 @@ mod tests {
0, // rent epoch
),
compute_budget_limits,
fee_details: FeeDetails::new(transaction_fee, 0, false),
fee_details: FeeDetails::new(transaction_fee, 0),
loaded_fee_payer_account: LoadedTransactionAccount {
loaded_size: fee_payer_account.data().len(),
account: post_validation_fee_payer_account,
Expand Down Expand Up @@ -2520,7 +2520,7 @@ mod tests {
0, // fee_payer_rent_epoch
),
compute_budget_limits,
fee_details: FeeDetails::new(transaction_fee, priority_fee, false),
fee_details: FeeDetails::new(transaction_fee, priority_fee),
loaded_fee_payer_account: LoadedTransactionAccount {
loaded_size: fee_payer_account.data().len(),
account: post_validation_fee_payer_account,
Expand Down

0 comments on commit dad81b9

Please sign in to comment.