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

Commit

Permalink
feature gate on removing unwanted rounding
Browse files Browse the repository at this point in the history
  • Loading branch information
tao-stones committed Feb 22, 2024
1 parent 7c296d1 commit fbd3e82
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 16 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
2 changes: 1 addition & 1 deletion runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10031,7 +10031,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
42 changes: 31 additions & 11 deletions sdk/src/fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,14 @@ pub struct FeeDetails {
}

impl FeeDetails {
pub fn total_fee(&self) -> u64 {
self.transaction_fee.saturating_add(self.prioritization_fee)
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
}
}
}

Expand Down Expand Up @@ -95,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 @@ -103,15 +110,13 @@ 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() as f64
* congestion_multiplier)
.round() as u64
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`
Expand Down Expand Up @@ -210,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 fbd3e82

Please sign in to comment.