Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add feature gate to collect and reward priority fee fully #583

Merged
merged 3 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,7 @@ use {
solana_program_runtime::compute_budget_processor::process_compute_budget_instructions,
solana_runtime::{bank::Bank, bank_forks::BankForks},
solana_sdk::{
clock::MAX_PROCESSING_AGE,
feature_set::{
include_loaded_accounts_data_size_in_fee_calculation,
remove_rounding_in_fee_calculation,
},
fee::FeeBudgetLimits,
saturating_add_assign,
clock::MAX_PROCESSING_AGE, fee::FeeBudgetLimits, saturating_add_assign,
transaction::SanitizedTransaction,
},
solana_svm::transaction_error_metrics::TransactionErrorMetrics,
Expand Down Expand Up @@ -494,15 +488,7 @@ impl SchedulerController {
bank: &Bank,
) -> (u64, u64) {
let cost = CostModel::calculate_cost(transaction, &bank.feature_set).sum();
let fee = bank.fee_structure.calculate_fee(
transaction.message(),
5_000, // this just needs to be non-zero
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()),
);
let reward = bank.calculate_reward_for_transaction(transaction, fee_budget_limits);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply sharing bank's reward calculation

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there will be some oddity here when the feature gets activated. txs received before feature-activation will still have old calculation (prio 50% burned) those after will not.
This will only be an effect around the epoch boundary.


// We need a multiplier here to avoid rounding down too aggressively.
// For many transactions, the cost will be greater than the fees in terms of raw lamports.
Expand All @@ -511,7 +497,8 @@ impl SchedulerController {
// An offset of 1 is used in the denominator to explicitly avoid division by zero.
const MULTIPLIER: u64 = 1_000_000;
(
fee.saturating_mul(MULTIPLIER)
reward
.saturating_mul(MULTIPLIER)
.saturating_div(cost.saturating_add(1)),
cost,
)
Expand Down
30 changes: 25 additions & 5 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ use {
feature,
feature_set::{
self, include_loaded_accounts_data_size_in_fee_calculation,
remove_rounding_in_fee_calculation, FeatureSet,
remove_rounding_in_fee_calculation, reward_full_priority_fee, FeatureSet,
},
fee::{FeeDetails, FeeStructure},
fee_calculator::{FeeCalculator, FeeRateGovernor},
Expand Down Expand Up @@ -281,6 +281,19 @@ impl CollectorFeeDetails {
.priority_fee
.saturating_add(fee_details.prioritization_fee());
}

pub(crate) fn total(&self) -> u64 {
self.transaction_fee.saturating_add(self.priority_fee)
}
}

impl From<FeeDetails> for CollectorFeeDetails {
fn from(fee_details: FeeDetails) -> Self {
CollectorFeeDetails {
transaction_fee: fee_details.transaction_fee(),
priority_fee: fee_details.prioritization_fee(),
}
}
}

#[derive(Debug)]
Expand Down Expand Up @@ -2863,7 +2876,11 @@ impl Bank {
if *hash == Hash::default() {
// finish up any deferred changes to account state
self.collect_rent_eagerly();
self.distribute_transaction_fees();
if self.feature_set.is_active(&reward_full_priority_fee::id()) {
self.distribute_transaction_fee_details();
} else {
self.distribute_transaction_fees();
}
self.distribute_rent_fees();
self.update_slot_history();
self.run_incinerator();
Expand Down Expand Up @@ -3994,7 +4011,6 @@ impl Bank {
}

// Note: this function is not yet used; next PR will call it behind a feature gate
#[allow(dead_code)]
fn filter_program_errors_and_collect_fee_details(
&self,
txs: &[SanitizedTransaction],
Expand Down Expand Up @@ -4207,8 +4223,12 @@ impl Bank {

let mut update_transaction_statuses_time = Measure::start("update_transaction_statuses");
self.update_transaction_statuses(sanitized_txs, &execution_results);
let fee_collection_results =
self.filter_program_errors_and_collect_fee(sanitized_txs, &execution_results);
let fee_collection_results = if self.feature_set.is_active(&reward_full_priority_fee::id())
{
self.filter_program_errors_and_collect_fee_details(sanitized_txs, &execution_results)
} else {
self.filter_program_errors_and_collect_fee(sanitized_txs, &execution_results)
};
update_transaction_statuses_time.stop();
timings.saturating_add_in_place(
ExecuteTimingType::UpdateTransactionStatuses,
Expand Down
75 changes: 56 additions & 19 deletions runtime/src/bank/fee_distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,16 @@ use {
log::{debug, warn},
solana_sdk::{
account::{ReadableAccount, WritableAccount},
feature_set::{
include_loaded_accounts_data_size_in_fee_calculation,
remove_rounding_in_fee_calculation, reward_full_priority_fee,
},
fee::FeeBudgetLimits,
pubkey::Pubkey,
reward_info::RewardInfo,
reward_type::RewardType,
system_program,
transaction::SanitizedTransaction,
},
solana_svm::account_rent_state::RentState,
solana_vote::vote_account::VoteAccountsHashMap,
Expand Down Expand Up @@ -49,44 +55,75 @@ impl Bank {
pub(super) fn distribute_transaction_fees(&self) {
let collector_fees = self.collector_fees.load(Relaxed);
if collector_fees != 0 {
let (deposit, mut burn) = self.fee_rate_governor.burn(collector_fees);
let (deposit, mut burn) = self.calculate_reward_and_burn_fees(collector_fees);
if deposit > 0 {
self.deposit_or_burn_fee(deposit, &mut burn);
}
self.capitalization.fetch_sub(burn, Relaxed);
}
}

// NOTE: to replace `distribute_transaction_fees()`, it applies different burn/reward rate
// on different fees:
// transaction fee: same fee_rate_governor rule
// priority fee: 100% reward
// next PR will call it behind a feature gate
#[allow(dead_code)]
// Replace `distribute_transaction_fees()` after Feature Gate: Reward full priority fee to
// validators #34731;
pub(super) fn distribute_transaction_fee_details(&self) {
let CollectorFeeDetails {
transaction_fee,
priority_fee,
} = *self.collector_fee_details.read().unwrap();

if transaction_fee.saturating_add(priority_fee) == 0 {
let fee_details = self.collector_fee_details.read().unwrap();
if fee_details.total() == 0 {
// nothing to distribute, exit early
return;
}

let (mut deposit, mut burn) = if transaction_fee != 0 {
self.fee_rate_governor.burn(transaction_fee)
} else {
(0, 0)
};
deposit = deposit.saturating_add(priority_fee);
let (deposit, mut burn) = self.calculate_reward_and_burn_fee_details(&fee_details);

if deposit > 0 {
self.deposit_or_burn_fee(deposit, &mut burn);
}
self.capitalization.fetch_sub(burn, Relaxed);
}

pub fn calculate_reward_for_transaction(
&self,
transaction: &SanitizedTransaction,
fee_budget_limits: &FeeBudgetLimits,
) -> u64 {
let (reward, _burn) = if self.feature_set.is_active(&reward_full_priority_fee::id()) {
let fee_details = self.fee_structure.calculate_fee_details(
transaction.message(),
fee_budget_limits,
self.feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
);
self.calculate_reward_and_burn_fee_details(&CollectorFeeDetails::from(fee_details))
} else {
let fee = self.fee_structure.calculate_fee(
transaction.message(),
5_000, // this just needs to be non-zero
fee_budget_limits,
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()),
);
self.calculate_reward_and_burn_fees(fee)
};
reward
}

fn calculate_reward_and_burn_fees(&self, fee: u64) -> (u64, u64) {
self.fee_rate_governor.burn(fee)
}

fn calculate_reward_and_burn_fee_details(
&self,
fee_details: &CollectorFeeDetails,
) -> (u64, u64) {
let (deposit, burn) = if fee_details.transaction_fee != 0 {
self.fee_rate_governor.burn(fee_details.transaction_fee)
} else {
(0, 0)
};
(deposit.saturating_add(fee_details.priority_fee), burn)
}

fn deposit_or_burn_fee(&self, deposit: u64, burn: &mut u64) {
let validate_fee_collector = self.validate_fee_collector_account();
match self.deposit_fees(
Expand Down
8 changes: 6 additions & 2 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2868,7 +2868,9 @@ fn test_filter_program_errors_and_collect_fee() {
..
} = create_genesis_config_with_leader(100_000, &leader, 3);
genesis_config.fee_rate_governor = FeeRateGovernor::new(5000, 0);
let bank = Bank::new_for_tests(&genesis_config);
let mut bank = Bank::new_for_tests(&genesis_config);
// this test is only for when `feature_set::reward_full_priority_fee` inactivated
bank.deactivate_feature(&feature_set::reward_full_priority_fee::id());

let key = solana_sdk::pubkey::new_rand();
let tx1 = SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer(
Expand Down Expand Up @@ -2919,7 +2921,9 @@ fn test_filter_program_errors_and_collect_compute_unit_fee() {
..
} = create_genesis_config_with_leader(1000000, &leader, 3);
genesis_config.fee_rate_governor = FeeRateGovernor::new(2, 0);
let bank = Bank::new_for_tests(&genesis_config);
let mut bank = Bank::new_for_tests(&genesis_config);
// this test is only for when `feature_set::reward_full_priority_fee` inactivated
bank.deactivate_feature(&feature_set::reward_full_priority_fee::id());

let key = solana_sdk::pubkey::new_rand();
let tx1 = SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer(
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 @@ -793,6 +793,10 @@ pub mod deprecate_unused_legacy_vote_plumbing {
solana_sdk::declare_id!("6Uf8S75PVh91MYgPQSHnjRAPQq6an5BDv9vomrCwDqLe");
}

pub mod reward_full_priority_fee {
solana_sdk::declare_id!("3opE3EzAKnUftUDURkzMgwpNgimBAypW1mNDYH4x4Zg7");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -986,6 +990,7 @@ lazy_static! {
(deprecate_unused_legacy_vote_plumbing::id(), "Deprecate unused legacy vote tx plumbing"),
(enable_tower_sync_ix::id(), "Enable tower sync vote instruction"),
(chained_merkle_conflict_duplicate_proofs::id(), "generate duplicate proofs for chained merkle root conflicts"),
(reward_full_priority_fee::id(), "Reward full priority fee to validators #34731"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down
Loading