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

Commit

Permalink
Burn fees collected into invalid accounts (#33887)
Browse files Browse the repository at this point in the history 
* refactor: create bank::fee_distribution module

* feature: add checks to fee distribution

* refactor: move Bank::deposit fn into test_utils

* feedback

* feedback 2

* add datapoints

* change to datapoint_warn

* typo
  • Loading branch information
@jstarry
jstarry authored Nov 6, 2023
1 parent 662ac8b commit ebe8afb
Show file tree
Hide file tree
Showing 6 changed files with 964 additions and 515 deletions.
4 changes: 2 additions & 2 deletions runtime/benches/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn deposit_many(bank: &Bank, pubkeys: &mut Vec<Pubkey>, num: usize) -> Result<()
AccountSharedData::new((t + 1) as u64, 0, AccountSharedData::default().owner());
pubkeys.push(pubkey);
assert!(bank.get_account(&pubkey).is_none());
bank.deposit(&pubkey, (t + 1) as u64)?;
test_utils::deposit(bank, &pubkey, (t + 1) as u64)?;
assert_eq!(bank.get_account(&pubkey).unwrap(), account);
}
Ok(())
Expand Down Expand Up @@ -80,7 +80,7 @@ fn test_accounts_squash(bencher: &mut Bencher) {
&Pubkey::default(),
slot,
));
next_bank.deposit(&pubkeys[0], 1).unwrap();
test_utils::deposit(&next_bank, &pubkeys[0], 1).unwrap();
next_bank.squash();
slot += 1;
prev_bank = next_bank;
Expand Down
286 changes: 32 additions & 254 deletions runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ use {
},
solana_accounts_db::{
account_overrides::AccountOverrides,
account_rent_state::RentState,
accounts::{
AccountAddressFilter, Accounts, LoadedTransaction, PubkeyAccountSlot, RewardInterval,
TransactionLoadResult,
Expand Down Expand Up @@ -150,7 +149,6 @@ use {
incinerator,
inflation::Inflation,
instruction::InstructionError,
lamports::LamportsError,
loader_v4::{self, LoaderV4State, LoaderV4Status},
message::{AccountKeys, SanitizedMessage},
native_loader,
Expand Down Expand Up @@ -186,7 +184,7 @@ use {
borrow::Cow,
cell::RefCell,
collections::{HashMap, HashSet},
convert::{TryFrom, TryInto},
convert::TryFrom,
fmt, mem,
ops::{AddAssign, RangeInclusive},
path::PathBuf,
Expand Down Expand Up @@ -217,6 +215,7 @@ mod address_lookup_table;
pub mod bank_hash_details;
mod builtin_programs;
pub mod epoch_accounts_hash_utils;
mod fee_distribution;
mod metrics;
mod serde_snapshot;
mod sysvar_cache;
Expand Down Expand Up @@ -3679,62 +3678,6 @@ impl Bank {
stake_weighted_timestamp
}

// Distribute collected transaction fees for this slot to collector_id (= current leader).
//
// Each validator is incentivized to process more transactions to earn more transaction fees.
// Transaction fees are rewarded for the computing resource utilization cost, directly
// proportional to their actual processing power.
//
// collector_id is rotated according to stake-weighted leader schedule. So the opportunity of
// earning transaction fees are fairly distributed by stake. And missing the opportunity
// (not producing a block as a leader) earns nothing. So, being online is incentivized as a
// form of transaction fees as well.
//
// On the other hand, rent fees are distributed under slightly different philosophy, while
// still being stake-weighted.
// Ref: distribute_rent_to_validators
fn collect_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);
// burn a portion of fees
debug!(
"distributed fee: {} (rounded from: {}, burned: {})",
deposit, collector_fees, burn
);

match self.deposit(&self.collector_id, deposit) {
Ok(post_balance) => {
if deposit != 0 {
self.rewards.write().unwrap().push((
self.collector_id,
RewardInfo {
reward_type: RewardType::Fee,
lamports: deposit as i64,
post_balance,
commission: None,
},
));
}
}
Err(_) => {
error!(
"Burning {} fee instead of crediting {}",
deposit, self.collector_id
);
datapoint_error!(
"bank-burned_fee",
("slot", self.slot(), i64),
("num_lamports", deposit, i64)
);
burn += deposit;
}
}
self.capitalization.fetch_sub(burn, Relaxed);
}
}

pub fn rehash(&self) {
let mut hash = self.hash.write().unwrap();
let new = self.hash_internal_state();
Expand All @@ -3760,8 +3703,8 @@ impl Bank {
if *hash == Hash::default() {
// finish up any deferred changes to account state
self.collect_rent_eagerly();
self.collect_fees();
self.distribute_rent();
self.distribute_transaction_fees();
self.distribute_rent_fees();
self.update_slot_history();
self.run_incinerator();

Expand Down Expand Up @@ -3864,12 +3807,14 @@ impl Bank {
self.accounts_data_size_initial += account.data().len() as u64;
}

// highest staked node is the first collector
// Highest staked node is the first collector but if a genesis config
// doesn't define any staked nodes, we assume this genesis config is for
// testing and set the collector id to a unique pubkey.
self.collector_id = self
.stakes_cache
.stakes()
.highest_staked_node()
.unwrap_or_default();
.unwrap_or_else(Pubkey::new_unique);

self.blockhash_queue.write().unwrap().genesis_hash(
&genesis_config.hash(),
Expand Down Expand Up @@ -5666,183 +5611,6 @@ impl Bank {
}
}

// Distribute collected rent fees for this slot to staked validators (excluding stakers)
// according to stake.
//
// The nature of rent fee is the cost of doing business, every validator has to hold (or have
// access to) the same list of accounts, so we pay according to stake, which is a rough proxy for
// value to the network.
//
// Currently, rent distribution doesn't consider given validator's uptime at all (this might
// change). That's because rent should be rewarded for the storage resource utilization cost.
// It's treated differently from transaction fees, which is for the computing resource
// utilization cost.
//
// We can't use collector_id (which is rotated according to stake-weighted leader schedule)
// as an approximation to the ideal rent distribution to simplify and avoid this per-slot
// computation for the distribution (time: N log N, space: N acct. stores; N = # of
// validators).
// The reason is that rent fee doesn't need to be incentivized for throughput unlike transaction
// fees
//
// Ref: collect_fees
#[allow(clippy::needless_collect)]
fn distribute_rent_to_validators(
&self,
vote_accounts: &VoteAccountsHashMap,
rent_to_be_distributed: u64,
) {
let mut total_staked = 0;

// Collect the stake associated with each validator.
// Note that a validator may be present in this vector multiple times if it happens to have
// more than one staked vote account somehow
let mut validator_stakes = vote_accounts
.iter()
.filter_map(|(_vote_pubkey, (staked, account))| {
if *staked == 0 {
None
} else {
total_staked += *staked;
Some((account.node_pubkey()?, *staked))
}
})
.collect::<Vec<(Pubkey, u64)>>();

#[cfg(test)]
if validator_stakes.is_empty() {
// some tests bank.freezes() with bad staking state
self.capitalization
.fetch_sub(rent_to_be_distributed, Relaxed);
return;
}
#[cfg(not(test))]
assert!(!validator_stakes.is_empty());

// Sort first by stake and then by validator identity pubkey for determinism.
// If two items are still equal, their relative order does not matter since
// both refer to the same validator.
validator_stakes.sort_unstable_by(|(pubkey1, staked1), (pubkey2, staked2)| {
(staked1, pubkey1).cmp(&(staked2, pubkey2)).reverse()
});

let enforce_fix = self.no_overflow_rent_distribution_enabled();

let mut rent_distributed_in_initial_round = 0;
let validator_rent_shares = validator_stakes
.into_iter()
.map(|(pubkey, staked)| {
let rent_share = if !enforce_fix {
(((staked * rent_to_be_distributed) as f64) / (total_staked as f64)) as u64
} else {
(((staked as u128) * (rent_to_be_distributed as u128)) / (total_staked as u128))
.try_into()
.unwrap()
};
rent_distributed_in_initial_round += rent_share;
(pubkey, rent_share)
})
.collect::<Vec<(Pubkey, u64)>>();

// Leftover lamports after fraction calculation, will be paid to validators starting from highest stake
// holder
let mut leftover_lamports = rent_to_be_distributed - rent_distributed_in_initial_round;

let mut rewards = vec![];
validator_rent_shares
.into_iter()
.for_each(|(pubkey, rent_share)| {
let rent_to_be_paid = if leftover_lamports > 0 {
leftover_lamports -= 1;
rent_share + 1
} else {
rent_share
};
if !enforce_fix || rent_to_be_paid > 0 {
let mut account = self
.get_account_with_fixed_root(&pubkey)
.unwrap_or_default();
let rent = self.rent_collector().rent;
let recipient_pre_rent_state = RentState::from_account(&account, &rent);
let distribution = account.checked_add_lamports(rent_to_be_paid);
let recipient_post_rent_state = RentState::from_account(&account, &rent);
let rent_state_transition_allowed = recipient_post_rent_state
.transition_allowed_from(&recipient_pre_rent_state);
if !rent_state_transition_allowed {
warn!(
"Rent distribution of {rent_to_be_paid} to {pubkey} results in \
invalid RentState: {recipient_post_rent_state:?}"
);
datapoint_warn!(
"bank-rent_distribution_invalid_state",
("slot", self.slot(), i64),
("pubkey", pubkey.to_string(), String),
("rent_to_be_paid", rent_to_be_paid, i64)
);
}
if distribution.is_err()
|| (self.prevent_rent_paying_rent_recipients()
&& !rent_state_transition_allowed)
{
// overflow adding lamports or resulting account is not rent-exempt
self.capitalization.fetch_sub(rent_to_be_paid, Relaxed);
error!(
"Burned {} rent lamports instead of sending to {}",
rent_to_be_paid, pubkey
);
datapoint_error!(
"bank-burned_rent",
("slot", self.slot(), i64),
("num_lamports", rent_to_be_paid, i64)
);
} else {
self.store_account(&pubkey, &account);
rewards.push((
pubkey,
RewardInfo {
reward_type: RewardType::Rent,
lamports: rent_to_be_paid as i64,
post_balance: account.lamports(),
commission: None,
},
));
}
}
});
self.rewards.write().unwrap().append(&mut rewards);

if enforce_fix {
assert_eq!(leftover_lamports, 0);
} else if leftover_lamports != 0 {
warn!(
"There was leftover from rent distribution: {}",
leftover_lamports
);
self.capitalization.fetch_sub(leftover_lamports, Relaxed);
}
}

fn distribute_rent(&self) {
let total_rent_collected = self.collected_rent.load(Relaxed);

let (burned_portion, rent_to_be_distributed) = self
.rent_collector
.rent
.calculate_burn(total_rent_collected);

debug!(
"distributed rent: {} (rounded from: {}, burned: {})",
rent_to_be_distributed, total_rent_collected, burned_portion
);
self.capitalization.fetch_sub(burned_portion, Relaxed);

if rent_to_be_distributed == 0 {
return;
}

self.distribute_rent_to_validators(&self.vote_accounts(), rent_to_be_distributed);
}

fn collect_rent(
&self,
execution_results: &[TransactionExecutionResult],
Expand Down Expand Up @@ -6753,19 +6521,6 @@ impl Bank {
}
}

pub fn deposit(
&self,
pubkey: &Pubkey,
lamports: u64,
) -> std::result::Result<u64, LamportsError> {
// This doesn't collect rents intentionally.
// Rents should only be applied to actual TXes
let mut account = self.get_account_with_fixed_root(pubkey).unwrap_or_default();
account.checked_add_lamports(lamports)?;
self.store_account(pubkey, &account);
Ok(account.lamports())
}

pub fn accounts(&self) -> Arc<Accounts> {
self.rc.accounts.clone()
}
Expand Down Expand Up @@ -7982,6 +7737,11 @@ impl Bank {
.is_active(&feature_set::prevent_rent_paying_rent_recipients::id())
}

pub fn validate_fee_collector_account(&self) -> bool {
self.feature_set
.is_active(&feature_set::validate_fee_collector_account::id())
}

pub fn read_cost_tracker(&self) -> LockResult<RwLockReadGuard<CostTracker>> {
self.cost_tracker.read()
}
Expand Down Expand Up @@ -8548,7 +8308,12 @@ pub mod test_utils {
use {
super::Bank,
crate::installed_scheduler_pool::BankWithScheduler,
solana_sdk::{hash::hashv, pubkey::Pubkey},
solana_sdk::{
account::{ReadableAccount, WritableAccount},
hash::hashv,
lamports::LamportsError,
pubkey::Pubkey,
},
solana_vote_program::vote_state::{self, BlockTimestamp, VoteStateVersions},
std::sync::Arc,
};
Expand Down Expand Up @@ -8580,4 +8345,17 @@ pub mod test_utils {
vote_state::to(&versioned, &mut vote_account).unwrap();
bank.store_account(vote_pubkey, &vote_account);
}

pub fn deposit(
bank: &Bank,
pubkey: &Pubkey,
lamports: u64,
) -> std::result::Result<u64, LamportsError> {
// This doesn't collect rents intentionally.
// Rents should only be applied to actual TXes
let mut account = bank.get_account_with_fixed_root(pubkey).unwrap_or_default();
account.checked_add_lamports(lamports)?;
bank.store_account(pubkey, &account);
Ok(account.lamports())
}
}
Loading

0 comments on commit ebe8afb

Please sign in to comment.