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

Reward full priority fee #34980

Closed
Closed
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
82 changes: 53 additions & 29 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ use {
epoch_schedule::EpochSchedule,
feature,
feature_set::{self, include_loaded_accounts_data_size_in_fee_calculation, FeatureSet},
fee::FeeStructure,
fee::{FeeDetails, FeeStructure},
fee_calculator::{FeeCalculator, FeeRateGovernor},
genesis_config::{ClusterType, GenesisConfig},
hard_forks::HardForks,
Expand Down Expand Up @@ -270,6 +270,32 @@ impl AddAssign for SquashTiming {
}
}

#[derive(Serialize, Deserialize, PartialEq, Clone, Debug, Default, Copy, AbiExample)]
#[serde(rename_all = "camelCase")]
pub struct CollectorFeeDetails {
pub transaction_fee: u64,
pub priority_fee: u64,
// more fees such as write_lock_fee go here
}

impl CollectorFeeDetails {
// TODO - just a placeholder
pub fn add(&mut self, fee_details: &FeeDetails) {
self.transaction_fee = self
.transaction_fee
.saturating_add(fee_details.transaction_fee);
self.priority_fee = self
.priority_fee
.saturating_add(fee_details.prioritization_fee);
}
pub fn get(&self) -> Self {
*self
}
pub fn dummy(&self) -> u64 {
self.transaction_fee
}
}

#[derive(Debug)]
pub struct BankRc {
/// where all the Accounts are stored
Expand Down Expand Up @@ -456,7 +482,7 @@ pub struct BankFieldsToDeserialize {
pub(crate) epoch: Epoch,
pub(crate) block_height: u64,
pub(crate) collector_id: Pubkey,
pub(crate) collector_fees: u64,
pub(crate) collector_fees: CollectorFeeDetails,
pub(crate) fee_calculator: FeeCalculator,
pub(crate) fee_rate_governor: FeeRateGovernor,
pub(crate) collected_rent: u64,
Expand Down Expand Up @@ -502,7 +528,7 @@ pub(crate) struct BankFieldsToSerialize<'a> {
pub(crate) epoch: Epoch,
pub(crate) block_height: u64,
pub(crate) collector_id: Pubkey,
pub(crate) collector_fees: u64,
pub(crate) collector_fees: CollectorFeeDetails,
pub(crate) fee_calculator: FeeCalculator,
pub(crate) fee_rate_governor: FeeRateGovernor,
pub(crate) collected_rent: u64,
Expand Down Expand Up @@ -607,7 +633,7 @@ impl PartialEq for Bank {
&& epoch == &other.epoch
&& block_height == &other.block_height
&& collector_id == &other.collector_id
&& collector_fees.load(Relaxed) == other.collector_fees.load(Relaxed)
&& *collector_fees.read().unwrap() == *other.collector_fees.read().unwrap()
&& fee_rate_governor == &other.fee_rate_governor
&& collected_rent.load(Relaxed) == other.collected_rent.load(Relaxed)
&& rent_collector == &other.rent_collector
Expand Down Expand Up @@ -759,7 +785,7 @@ pub struct Bank {
collector_id: Pubkey,

/// Fees that have been collected
collector_fees: AtomicU64,
collector_fees: RwLock<CollectorFeeDetails>,

/// Track cluster signature throughput and adjust fee rate
pub(crate) fee_rate_governor: FeeRateGovernor,
Expand Down Expand Up @@ -1000,7 +1026,7 @@ impl Bank {
epoch: Epoch::default(),
block_height: u64::default(),
collector_id: Pubkey::default(),
collector_fees: AtomicU64::default(),
collector_fees: RwLock::new(CollectorFeeDetails::default()),
fee_rate_governor: FeeRateGovernor::default(),
collected_rent: AtomicU64::default(),
rent_collector: RentCollector::default(),
Expand Down Expand Up @@ -1309,7 +1335,7 @@ impl Bank {
parent_hash: parent.hash(),
parent_slot: parent.slot(),
collector_id: *collector_id,
collector_fees: AtomicU64::new(0),
collector_fees: RwLock::new(CollectorFeeDetails::default()),
ancestors: Ancestors::default(),
hash: RwLock::new(Hash::default()),
is_delta: AtomicBool::new(false),
Expand Down Expand Up @@ -1813,7 +1839,7 @@ impl Bank {
epoch: fields.epoch,
block_height: fields.block_height,
collector_id: fields.collector_id,
collector_fees: AtomicU64::new(fields.collector_fees),
collector_fees: RwLock::new(fields.collector_fees),
fee_rate_governor: fields.fee_rate_governor,
collected_rent: AtomicU64::new(fields.collected_rent),
// clone()-ing is needed to consider a gated behavior in rent_collector
Expand Down Expand Up @@ -1932,7 +1958,7 @@ impl Bank {
epoch: self.epoch,
block_height: self.block_height,
collector_id: self.collector_id,
collector_fees: self.collector_fees.load(Relaxed),
collector_fees: *self.collector_fees.read().unwrap(),
fee_calculator: FeeCalculator::default(),
fee_rate_governor: self.fee_rate_governor.clone(),
collected_rent: self.collected_rent.load(Relaxed),
Expand Down Expand Up @@ -5516,8 +5542,7 @@ impl Bank {
txs: &[SanitizedTransaction],
execution_results: &[TransactionExecutionResult],
) -> Vec<Result<()>> {
let hash_queue = self.blockhash_queue.read().unwrap();
let mut fees = 0;
let mut accumulated_fee_details = FeeDetails::default();

let results = txs
.iter()
Expand All @@ -5530,21 +5555,17 @@ impl Bank {
TransactionExecutionResult::NotExecuted(err) => Err(err.clone()),
}?;

let (lamports_per_signature, is_nonce) = durable_nonce_fee
.map(|durable_nonce_fee| durable_nonce_fee.lamports_per_signature())
.map(|maybe_lamports_per_signature| (maybe_lamports_per_signature, true))
.unwrap_or_else(|| {
(
hash_queue.get_lamports_per_signature(tx.message().recent_blockhash()),
false,
)
});

let lamports_per_signature =
lamports_per_signature.ok_or(TransactionError::BlockhashNotFound)?;
let fee = self.get_fee_for_message_with_lamports_per_signature(
tx.message(),
lamports_per_signature,
let is_nonce = durable_nonce_fee.is_some();

// TODO can refactor self.get_fee_for_message_with_lamports_per_signature() out
let message = tx.message();
let fee_details = self.fee_structure.calculate_fee_details(
message,
&process_compute_budget_instructions(message.program_instructions_iter())
.unwrap_or_default()
.into(),
self.feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
);

// In case of instruction error, even though no accounts
Expand All @@ -5555,15 +5576,18 @@ impl Bank {
// post-load, fee deducted, pre-execute account state
// stored
if execution_status.is_err() && !is_nonce {
self.withdraw(tx.message().fee_payer(), fee)?;
self.withdraw(tx.message().fee_payer(), fee_details.total_fee())?;
}

fees += fee;
accumulated_fee_details.accumulate(&fee_details);
Ok(())
})
.collect();

self.collector_fees.fetch_add(fees, Relaxed);
self.collector_fees
.write()
.unwrap()
.add(&accumulated_fee_details);
results
}

Expand Down
136 changes: 80 additions & 56 deletions runtime/src/bank/fee_distribution.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use {
super::Bank,
crate::svm::account_rent_state::RentState,
crate::{bank::CollectorFeeDetails, svm::account_rent_state::RentState},
log::{debug, warn},
solana_accounts_db::stake_rewards::RewardInfo,
solana_sdk::{
Expand Down Expand Up @@ -46,47 +46,53 @@ impl Bank {
// still being stake-weighted.
// Ref: distribute_rent_to_validators
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);
if deposit > 0 {
let validate_fee_collector = self.validate_fee_collector_account();
match self.deposit_fees(
&self.collector_id,
deposit,
DepositFeeOptions {
check_account_owner: validate_fee_collector,
check_rent_paying: validate_fee_collector,
},
) {
Ok(post_balance) => {
self.rewards.write().unwrap().push((
self.collector_id,
RewardInfo {
reward_type: RewardType::Fee,
lamports: deposit as i64,
post_balance,
commission: None,
},
));
}
Err(err) => {
debug!(
"Burned {} lamport tx fee instead of sending to {} due to {}",
deposit, self.collector_id, err
);
datapoint_warn!(
"bank-burned_fee",
("slot", self.slot(), i64),
("num_lamports", deposit, i64),
("error", err.to_string(), String),
);
burn += deposit;
}
let CollectorFeeDetails {
transaction_fee,
priority_fee,
} = self.collector_fees.read().unwrap().get();
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);
if deposit > 0 {
let validate_fee_collector = self.validate_fee_collector_account();
match self.deposit_fees(
&self.collector_id,
deposit,
DepositFeeOptions {
check_account_owner: validate_fee_collector,
check_rent_paying: validate_fee_collector,
},
) {
Ok(post_balance) => {
self.rewards.write().unwrap().push((
self.collector_id,
RewardInfo {
reward_type: RewardType::Fee,
lamports: deposit as i64,
post_balance,
commission: None,
},
));
}
Err(err) => {
debug!(
"Burned {} lamport tx fee instead of sending to {} due to {}",
deposit, self.collector_id, err
);
datapoint_warn!(
"bank-burned_fee",
("slot", self.slot(), i64),
("num_lamports", deposit, i64),
("error", err.to_string(), String),
);
burn = burn.saturating_add(deposit);
}
}
self.capitalization.fetch_sub(burn, Relaxed);
}
self.capitalization.fetch_sub(burn, Relaxed);
}

// Deposits fees into a specified account and if successful, returns the new balance of that account
Expand Down Expand Up @@ -295,8 +301,8 @@ pub mod tests {
create_genesis_config_with_vote_accounts, ValidatorVoteKeypairs,
},
solana_sdk::{
account::AccountSharedData, feature_set, native_token::sol_to_lamports, pubkey,
rent::Rent, signature::Signer,
account::AccountSharedData, feature_set, fee::FeeDetails,
native_token::sol_to_lamports, pubkey, rent::Rent, signature::Signer,
},
};

Expand Down Expand Up @@ -343,19 +349,25 @@ pub mod tests {
let min_rent_exempt_balance = rent.minimum_balance(0);
genesis.genesis_config.rent = rent; // Ensure rent is non-zero, as genesis_utils sets Rent::free by default
let bank = Bank::new_for_tests(&genesis.genesis_config);
let transaction_fees = 100;
bank.collector_fees.fetch_add(transaction_fees, Relaxed);
assert_eq!(transaction_fees, bank.collector_fees.load(Relaxed));
let fee_details = FeeDetails {
transaction_fee: 100,
prioritization_fee: 0,
};
bank.collector_fees.write().unwrap().add(&fee_details);
assert_eq!(
fee_details.transaction_fee,
bank.collector_fees.read().unwrap().dummy()
);
let (expected_collected_fees, burn_amount) =
bank.fee_rate_governor.burn(transaction_fees);
bank.fee_rate_governor.burn(fee_details.transaction_fee);
assert!(burn_amount > 0);

if test_case.scenario == Scenario::RentPaying {
// ensure that account balance + collected fees will make it rent-paying
let initial_balance = 100;
let account = AccountSharedData::new(initial_balance, 0, &system_program::id());
bank.store_account(bank.collector_id(), &account);
assert!(initial_balance + transaction_fees < min_rent_exempt_balance);
assert!(initial_balance + fee_details.transaction_fee < min_rent_exempt_balance);
} else if test_case.scenario == Scenario::InvalidOwner {
// ensure that account owner is invalid and fee distribution will fail
let account =
Expand All @@ -375,7 +387,7 @@ pub mod tests {
if test_case.scenario != Scenario::Normal && !test_case.disable_checks {
assert_eq!(initial_collector_id_balance, new_collector_id_balance);
assert_eq!(
initial_capitalization - transaction_fees,
initial_capitalization - fee_details.transaction_fee,
bank.capitalization()
);
let locked_rewards = bank.rewards.read().unwrap();
Expand Down Expand Up @@ -416,7 +428,7 @@ pub mod tests {
fn test_distribute_transaction_fees_zero() {
let genesis = create_genesis_config(0);
let bank = Bank::new_for_tests(&genesis.genesis_config);
assert_eq!(bank.collector_fees.load(Relaxed), 0);
assert_eq!(bank.collector_fees.read().unwrap().dummy(), 0);

let initial_capitalization = bank.capitalization();
let initial_collector_id_balance = bank.get_balance(bank.collector_id());
Expand All @@ -437,9 +449,15 @@ pub mod tests {
let mut genesis = create_genesis_config(0);
genesis.genesis_config.fee_rate_governor.burn_percent = 100;
let bank = Bank::new_for_tests(&genesis.genesis_config);
let transaction_fees = 100;
bank.collector_fees.fetch_add(transaction_fees, Relaxed);
assert_eq!(transaction_fees, bank.collector_fees.load(Relaxed));
let fee_details = FeeDetails {
transaction_fee: 100,
prioritization_fee: 0,
};
bank.collector_fees.write().unwrap().add(&fee_details);
assert_eq!(
fee_details.transaction_fee,
bank.collector_fees.read().unwrap().dummy()
);

let initial_capitalization = bank.capitalization();
let initial_collector_id_balance = bank.get_balance(bank.collector_id());
Expand All @@ -448,7 +466,7 @@ pub mod tests {

assert_eq!(initial_collector_id_balance, new_collector_id_balance);
assert_eq!(
initial_capitalization - transaction_fees,
initial_capitalization - fee_details.transaction_fee,
bank.capitalization()
);
let locked_rewards = bank.rewards.read().unwrap();
Expand All @@ -462,9 +480,15 @@ pub mod tests {
fn test_distribute_transaction_fees_overflow_failure() {
let genesis = create_genesis_config(0);
let bank = Bank::new_for_tests(&genesis.genesis_config);
let transaction_fees = 100;
bank.collector_fees.fetch_add(transaction_fees, Relaxed);
assert_eq!(transaction_fees, bank.collector_fees.load(Relaxed));
let fee_details = FeeDetails {
transaction_fee: 100,
prioritization_fee: 0,
};
bank.collector_fees.write().unwrap().add(&fee_details);
assert_eq!(
fee_details.transaction_fee,
bank.collector_fees.read().unwrap().dummy()
);

// ensure that account balance will overflow and fee distribution will fail
let account = AccountSharedData::new(u64::MAX, 0, &system_program::id());
Expand All @@ -477,7 +501,7 @@ pub mod tests {

assert_eq!(initial_collector_id_balance, new_collector_id_balance);
assert_eq!(
initial_capitalization - transaction_fees,
initial_capitalization - fee_details.transaction_fee,
bank.capitalization()
);
let locked_rewards = bank.rewards.read().unwrap();
Expand Down
Loading
Loading