Skip to content

Commit

Permalink
Add fee_details to fee calculation (solana-labs#35021)
Browse files Browse the repository at this point in the history
* add fee_details to fee calculation

* fix - no need to round after summing u64

* feature gate on removing unwanted rounding
  • Loading branch information
tao-stones authored and jeffwashington committed Feb 27, 2024
1 parent 2332613 commit ce1875d
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 13 deletions.
2 changes: 2 additions & 0 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,8 @@ impl Consumer {
bank.feature_set.is_active(
&feature_set::include_loaded_accounts_data_size_in_fee_calculation::id(),
),
bank.feature_set
.is_active(&feature_set::remove_rounding_in_fee_calculation::id()),
);
let (mut fee_payer_account, _slot) = bank
.rc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,13 @@ use {
solana_runtime::{bank::Bank, bank_forks::BankForks},
solana_sdk::{
clock::MAX_PROCESSING_AGE,
feature_set::include_loaded_accounts_data_size_in_fee_calculation, fee::FeeBudgetLimits,
saturating_add_assign, transaction::SanitizedTransaction,
feature_set::{
include_loaded_accounts_data_size_in_fee_calculation,
remove_rounding_in_fee_calculation,
},
fee::FeeBudgetLimits,
saturating_add_assign,
transaction::SanitizedTransaction,
},
solana_svm::transaction_error_metrics::TransactionErrorMetrics,
std::{
Expand Down Expand Up @@ -422,6 +427,8 @@ impl SchedulerController {
fee_budget_limits,
bank.feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
bank.feature_set
.is_active(&remove_rounding_in_fee_calculation::id()),
);

// We need a multiplier here to avoid rounding down too aggressively.
Expand Down
2 changes: 2 additions & 0 deletions programs/sbf/tests/programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3713,6 +3713,7 @@ fn test_program_fees() {
.unwrap_or_default()
.into(),
false,
true,
);
bank_client
.send_and_confirm_message(&[&mint_keypair], message)
Expand All @@ -3736,6 +3737,7 @@ fn test_program_fees() {
.unwrap_or_default()
.into(),
false,
true,
);
assert!(expected_normal_fee < expected_prioritized_fee);

Expand Down
7 changes: 6 additions & 1 deletion runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,10 @@ use {
epoch_info::EpochInfo,
epoch_schedule::EpochSchedule,
feature,
feature_set::{self, include_loaded_accounts_data_size_in_fee_calculation, FeatureSet},
feature_set::{
self, include_loaded_accounts_data_size_in_fee_calculation,
remove_rounding_in_fee_calculation, FeatureSet,
},
fee::FeeStructure,
fee_calculator::{FeeCalculator, FeeRateGovernor},
genesis_config::{ClusterType, GenesisConfig},
Expand Down Expand Up @@ -4016,6 +4019,8 @@ impl Bank {
.into(),
self.feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
self.feature_set
.is_active(&remove_rounding_in_fee_calculation::id()),
)
}

Expand Down
3 changes: 1 addition & 2 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3333,7 +3333,6 @@ fn test_bank_parent_account_spend() {
let key2 = Keypair::new();
let (parent, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);
let amount = genesis_config.rent.minimum_balance(0);
println!("==== amount {}", amount);

let tx =
system_transaction::transfer(&mint_keypair, &key1.pubkey(), amount, genesis_config.hash());
Expand Down Expand Up @@ -10029,7 +10028,7 @@ fn calculate_test_fee(
.unwrap_or_default()
.into();

fee_structure.calculate_fee(message, lamports_per_signature, &budget_limits, false)
fee_structure.calculate_fee(message, lamports_per_signature, &budget_limits, false, true)
}

#[test]
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,10 @@ pub mod enable_chained_merkle_shreds {
solana_sdk::declare_id!("7uZBkJXJ1HkuP6R3MJfZs7mLwymBcDbKdqbF51ZWLier");
}

pub mod remove_rounding_in_fee_calculation {
solana_sdk::declare_id!("BtVN7YjDzNE6Dk7kTT7YTDgMNUZTNgiSJgsdzAeTg2jF");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -970,6 +974,7 @@ lazy_static! {
(cost_model_requested_write_lock_cost::id(), "cost model uses number of requested write locks #34819"),
(enable_gossip_duplicate_proof_ingestion::id(), "enable gossip duplicate proof ingestion #32963"),
(enable_chained_merkle_shreds::id(), "Enable chained Merkle shreds #34916"),
(remove_rounding_in_fee_calculation::id(), "Removing unwanted rounding in fee calculation #34982"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down
64 changes: 57 additions & 7 deletions sdk/src/fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,24 @@ pub struct FeeStructure {
pub compute_fee_bins: Vec<FeeBin>,
}

#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct FeeDetails {
transaction_fee: u64,
prioritization_fee: u64,
}

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

pub const ACCOUNT_DATA_COST_PAGE_SIZE: u64 = 32_u64.saturating_mul(1024);

impl FeeStructure {
Expand Down Expand Up @@ -83,6 +101,7 @@ impl FeeStructure {
lamports_per_signature: u64,
budget_limits: &FeeBudgetLimits,
include_loaded_account_data_size_in_fee: bool,
remove_rounding_in_fee_calculation: bool,
) -> u64 {
// Fee based on compute units and signatures
let congestion_multiplier = if lamports_per_signature == 0 {
Expand All @@ -91,6 +110,23 @@ impl FeeStructure {
1.0 // multiplier that has no effect
};

self.calculate_fee_details(
message,
budget_limits,
include_loaded_account_data_size_in_fee,
)
.total_fee(remove_rounding_in_fee_calculation)
.saturating_mul(congestion_multiplier as u64)
}

/// Calculate fee details for `SanitizedMessage`
#[cfg(not(target_os = "solana"))]
pub fn calculate_fee_details(
&self,
message: &SanitizedMessage,
budget_limits: &FeeBudgetLimits,
include_loaded_account_data_size_in_fee: bool,
) -> FeeDetails {
let signature_fee = message
.num_signatures()
.saturating_mul(self.lamports_per_signature);
Expand Down Expand Up @@ -122,13 +158,12 @@ impl FeeStructure {
.unwrap_or_default()
});

((budget_limits
.prioritization_fee
.saturating_add(signature_fee)
.saturating_add(write_lock_fee)
.saturating_add(compute_fee) as f64)
* congestion_multiplier)
.round() as u64
FeeDetails {
transaction_fee: signature_fee
.saturating_add(write_lock_fee)
.saturating_add(compute_fee),
prioritization_fee: budget_limits.prioritization_fee,
}
}
}

Expand Down Expand Up @@ -180,4 +215,19 @@ 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 large_fee_details = FeeDetails {
transaction_fee: u64::MAX - 11,
prioritization_fee: 1,
};
let expected_large_fee = u64::MAX - 10;

assert_eq!(large_fee_details.total_fee(true), expected_large_fee);
assert_ne!(large_fee_details.total_fee(false), expected_large_fee);
}
}
8 changes: 7 additions & 1 deletion svm/src/account_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ use {
create_executable_meta, is_builtin, is_executable, Account, AccountSharedData,
ReadableAccount, WritableAccount,
},
feature_set::{self, include_loaded_accounts_data_size_in_fee_calculation},
feature_set::{
self, include_loaded_accounts_data_size_in_fee_calculation,
remove_rounding_in_fee_calculation,
},
fee::FeeStructure,
message::SanitizedMessage,
native_loader,
Expand Down Expand Up @@ -74,6 +77,7 @@ pub fn load_accounts<CB: TransactionProcessingCallback>(
.into(),
feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
feature_set.is_active(&remove_rounding_in_fee_calculation::id()),
)
} else {
return (Err(TransactionError::BlockhashNotFound), None);
Expand Down Expand Up @@ -682,6 +686,7 @@ mod tests {
.unwrap_or_default()
.into(),
false,
true,
);
assert_eq!(fee, lamports_per_signature);

Expand Down Expand Up @@ -1210,6 +1215,7 @@ mod tests {
.unwrap_or_default()
.into(),
false,
true,
);
assert_eq!(fee, lamports_per_signature + prioritization_fee);

Expand Down

0 comments on commit ce1875d

Please sign in to comment.