Avoid overflow when computing rent distribution (#12112)
* Avoid overflow when computing rent distribution * Use assert_eq!.... * Fix tests * Add test * Use FeatureSet * Add comments * Address review comments * Tweak a bit. * Fix fmt
This commit is contained in:
parent
8f10e407ee
commit
e3773d919c
|
@ -67,7 +67,7 @@ use solana_vote_program::{vote_instruction::VoteInstruction, vote_state::VoteSta
|
||||||
use std::{
|
use std::{
|
||||||
cell::RefCell,
|
cell::RefCell,
|
||||||
collections::{HashMap, HashSet},
|
collections::{HashMap, HashSet},
|
||||||
convert::TryFrom,
|
convert::{TryFrom, TryInto},
|
||||||
mem,
|
mem,
|
||||||
ops::RangeInclusive,
|
ops::RangeInclusive,
|
||||||
path::PathBuf,
|
path::PathBuf,
|
||||||
|
@ -1161,6 +1161,11 @@ impl Bank {
|
||||||
// verify that we didn't pay any more than we expected to
|
// verify that we didn't pay any more than we expected to
|
||||||
assert!(validator_rewards >= validator_rewards_paid);
|
assert!(validator_rewards >= validator_rewards_paid);
|
||||||
|
|
||||||
|
info!(
|
||||||
|
"distributed inflation: {} (rounded from: {})",
|
||||||
|
validator_rewards_paid, validator_rewards
|
||||||
|
);
|
||||||
|
|
||||||
self.capitalization
|
self.capitalization
|
||||||
.fetch_add(validator_rewards_paid, Relaxed);
|
.fetch_add(validator_rewards_paid, Relaxed);
|
||||||
|
|
||||||
|
@ -1306,12 +1311,30 @@ impl Bank {
|
||||||
self.update_recent_blockhashes_locked(&blockhash_queue);
|
self.update_recent_blockhashes_locked(&blockhash_queue);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 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) {
|
fn collect_fees(&self) {
|
||||||
let collector_fees = self.collector_fees.load(Relaxed) as u64;
|
let collector_fees = self.collector_fees.load(Relaxed) as u64;
|
||||||
|
|
||||||
if collector_fees != 0 {
|
if collector_fees != 0 {
|
||||||
let (unburned, burned) = self.fee_rate_governor.burn(collector_fees);
|
let (unburned, burned) = self.fee_rate_governor.burn(collector_fees);
|
||||||
// burn a portion of fees
|
// burn a portion of fees
|
||||||
|
debug!(
|
||||||
|
"distributed fee: {} (rounded from: {}, burned: {})",
|
||||||
|
unburned, collector_fees, burned
|
||||||
|
);
|
||||||
self.deposit(&self.collector_id, unburned);
|
self.deposit(&self.collector_id, unburned);
|
||||||
self.capitalization.fetch_sub(burned, Relaxed);
|
self.capitalization.fetch_sub(burned, Relaxed);
|
||||||
}
|
}
|
||||||
|
@ -2361,14 +2384,33 @@ 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)]
|
#[allow(clippy::needless_collect)]
|
||||||
fn distribute_rent_to_validators(
|
fn distribute_rent_to_validators(
|
||||||
&self,
|
&self,
|
||||||
vote_account_hashmap: &HashMap<Pubkey, (u64, Account)>,
|
vote_account_hashmap: &HashMap<Pubkey, (u64, Account)>,
|
||||||
rent_to_be_distributed: u64,
|
rent_to_be_distributed: u64,
|
||||||
) -> u64 {
|
) {
|
||||||
let mut total_staked = 0;
|
let mut total_staked = 0;
|
||||||
let mut rent_distributed_in_initial_round = 0;
|
|
||||||
|
|
||||||
// Collect the stake associated with each validator.
|
// Collect the stake associated with each validator.
|
||||||
// Note that a validator may be present in this vector multiple times if it happens to have
|
// Note that a validator may be present in this vector multiple times if it happens to have
|
||||||
|
@ -2387,6 +2429,16 @@ impl Bank {
|
||||||
})
|
})
|
||||||
.collect::<Vec<(Pubkey, u64)>>();
|
.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
|
// Sort first by stake and then by validator identity pubkey for determinism
|
||||||
validator_stakes.sort_by(|(pubkey1, staked1), (pubkey2, staked2)| {
|
validator_stakes.sort_by(|(pubkey1, staked1), (pubkey2, staked2)| {
|
||||||
match staked2.cmp(staked1) {
|
match staked2.cmp(staked1) {
|
||||||
|
@ -2395,11 +2447,19 @@ impl Bank {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
let enforce_fix = self.no_overflow_rent_distribution_enabled();
|
||||||
|
|
||||||
|
let mut rent_distributed_in_initial_round = 0;
|
||||||
let validator_rent_shares = validator_stakes
|
let validator_rent_shares = validator_stakes
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|(pubkey, staked)| {
|
.map(|(pubkey, staked)| {
|
||||||
let rent_share =
|
let rent_share = if !enforce_fix {
|
||||||
(((staked * rent_to_be_distributed) as f64) / (total_staked as f64)) as u64;
|
(((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;
|
rent_distributed_in_initial_round += rent_share;
|
||||||
(pubkey, rent_share)
|
(pubkey, rent_share)
|
||||||
})
|
})
|
||||||
|
@ -2422,7 +2482,16 @@ impl Bank {
|
||||||
account.lamports += rent_to_be_paid;
|
account.lamports += rent_to_be_paid;
|
||||||
self.store_account(&pubkey, &account);
|
self.store_account(&pubkey, &account);
|
||||||
});
|
});
|
||||||
leftover_lamports
|
|
||||||
|
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) {
|
fn distribute_rent(&self) {
|
||||||
|
@ -2433,18 +2502,17 @@ impl Bank {
|
||||||
.rent
|
.rent
|
||||||
.calculate_burn(total_rent_collected);
|
.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);
|
self.capitalization.fetch_sub(burned_portion, Relaxed);
|
||||||
|
|
||||||
if rent_to_be_distributed == 0 {
|
if rent_to_be_distributed == 0 {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
let leftover =
|
self.distribute_rent_to_validators(&self.vote_accounts(), rent_to_be_distributed);
|
||||||
self.distribute_rent_to_validators(&self.vote_accounts(), rent_to_be_distributed);
|
|
||||||
if leftover != 0 {
|
|
||||||
warn!("There was leftover from rent distribution: {}", leftover);
|
|
||||||
self.capitalization.fetch_sub(leftover, Relaxed);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn collect_rent(
|
fn collect_rent(
|
||||||
|
@ -3611,6 +3679,11 @@ impl Bank {
|
||||||
.is_active(&feature_set::secp256k1_program_enabled::id())
|
.is_active(&feature_set::secp256k1_program_enabled::id())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn no_overflow_rent_distribution_enabled(&self) -> bool {
|
||||||
|
self.feature_set
|
||||||
|
.is_active(&feature_set::no_overflow_rent_distribution::id())
|
||||||
|
}
|
||||||
|
|
||||||
// This is called from snapshot restore AND for each epoch boundary
|
// This is called from snapshot restore AND for each epoch boundary
|
||||||
// The entire code path herein must be idempotent
|
// The entire code path herein must be idempotent
|
||||||
fn apply_feature_activations(&mut self, init_finish_or_warp: bool) {
|
fn apply_feature_activations(&mut self, init_finish_or_warp: bool) {
|
||||||
|
@ -3993,6 +4066,8 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_credit_debit_rent_no_side_effect_on_hash() {
|
fn test_credit_debit_rent_no_side_effect_on_hash() {
|
||||||
|
solana_logger::setup();
|
||||||
|
|
||||||
let (mut genesis_config, _mint_keypair) = create_genesis_config(10);
|
let (mut genesis_config, _mint_keypair) = create_genesis_config(10);
|
||||||
let keypair1: Keypair = Keypair::new();
|
let keypair1: Keypair = Keypair::new();
|
||||||
let keypair2: Keypair = Keypair::new();
|
let keypair2: Keypair = Keypair::new();
|
||||||
|
@ -4484,6 +4559,50 @@ mod tests {
|
||||||
assert!(bank.calculate_and_verify_capitalization());
|
assert!(bank.calculate_and_verify_capitalization());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_distribute_rent_to_validators_overflow() {
|
||||||
|
solana_logger::setup();
|
||||||
|
|
||||||
|
// These values are taken from the real cluster (testnet)
|
||||||
|
const RENT_TO_BE_DISTRIBUTED: u64 = 120_525;
|
||||||
|
const VALIDATOR_STAKE: u64 = 374_999_998_287_840;
|
||||||
|
|
||||||
|
let validator_pubkey = Pubkey::new_rand();
|
||||||
|
let mut genesis_config =
|
||||||
|
create_genesis_config_with_leader(10, &validator_pubkey, VALIDATOR_STAKE)
|
||||||
|
.genesis_config;
|
||||||
|
|
||||||
|
let bank = Bank::new(&genesis_config);
|
||||||
|
let old_validator_lamports = bank.get_balance(&validator_pubkey);
|
||||||
|
bank.distribute_rent_to_validators(&bank.vote_accounts(), RENT_TO_BE_DISTRIBUTED);
|
||||||
|
let new_validator_lamports = bank.get_balance(&validator_pubkey);
|
||||||
|
assert_eq!(
|
||||||
|
new_validator_lamports,
|
||||||
|
old_validator_lamports + RENT_TO_BE_DISTRIBUTED
|
||||||
|
);
|
||||||
|
|
||||||
|
genesis_config
|
||||||
|
.accounts
|
||||||
|
.remove(&feature_set::no_overflow_rent_distribution::id())
|
||||||
|
.unwrap();
|
||||||
|
let bank = std::panic::AssertUnwindSafe(Bank::new(&genesis_config));
|
||||||
|
let old_validator_lamports = bank.get_balance(&validator_pubkey);
|
||||||
|
let new_validator_lamports = std::panic::catch_unwind(|| {
|
||||||
|
bank.distribute_rent_to_validators(&bank.vote_accounts(), RENT_TO_BE_DISTRIBUTED);
|
||||||
|
bank.get_balance(&validator_pubkey)
|
||||||
|
});
|
||||||
|
|
||||||
|
if let Ok(new_validator_lamports) = new_validator_lamports {
|
||||||
|
info!("asserting overflowing incorrect rent distribution");
|
||||||
|
assert_ne!(
|
||||||
|
new_validator_lamports,
|
||||||
|
old_validator_lamports + RENT_TO_BE_DISTRIBUTED
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
info!("NOT-asserting overflowing incorrect rent distribution");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_rent_exempt_executable_account() {
|
fn test_rent_exempt_executable_account() {
|
||||||
let (mut genesis_config, mint_keypair) = create_genesis_config(100_000);
|
let (mut genesis_config, mint_keypair) = create_genesis_config(100_000);
|
||||||
|
|
|
@ -37,6 +37,10 @@ pub mod sha256_syscall_enabled {
|
||||||
solana_sdk::declare_id!("D7KfP7bZxpkYtD4Pc38t9htgs1k5k47Yhxe4rp6WDVi8");
|
solana_sdk::declare_id!("D7KfP7bZxpkYtD4Pc38t9htgs1k5k47Yhxe4rp6WDVi8");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub mod no_overflow_rent_distribution {
|
||||||
|
solana_sdk::declare_id!("4kpdyrcj5jS47CZb2oJGfVxjYbsMm2Kx97gFyZrxxwXz");
|
||||||
|
}
|
||||||
|
|
||||||
lazy_static! {
|
lazy_static! {
|
||||||
/// Map of feature identifiers to user-visible description
|
/// Map of feature identifiers to user-visible description
|
||||||
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
|
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
|
||||||
|
@ -47,7 +51,8 @@ lazy_static! {
|
||||||
(spl_token_v2_multisig_fix::id(), "spl-token multisig fix"),
|
(spl_token_v2_multisig_fix::id(), "spl-token multisig fix"),
|
||||||
(bpf_loader2_program::id(), "bpf_loader2 program"),
|
(bpf_loader2_program::id(), "bpf_loader2 program"),
|
||||||
(compute_budget_config2::id(), "1ms compute budget"),
|
(compute_budget_config2::id(), "1ms compute budget"),
|
||||||
(sha256_syscall_enabled::id(), "sha256 syscall")
|
(sha256_syscall_enabled::id(), "sha256 syscall"),
|
||||||
|
(no_overflow_rent_distribution::id(), "no overflow rent distribution"),
|
||||||
/*************** ADD NEW FEATURES HERE ***************/
|
/*************** ADD NEW FEATURES HERE ***************/
|
||||||
]
|
]
|
||||||
.iter()
|
.iter()
|
||||||
|
|
|
@ -607,6 +607,7 @@ pub fn bank_from_archive<P: AsRef<Path>>(
|
||||||
panic!("Snapshot bank for slot {} failed to verify", bank.slot());
|
panic!("Snapshot bank for slot {} failed to verify", bank.slot());
|
||||||
}
|
}
|
||||||
if genesis_config.cluster_type == ClusterType::Testnet {
|
if genesis_config.cluster_type == ClusterType::Testnet {
|
||||||
|
// remove me after we transitions to the fixed rent distribution with no overflow
|
||||||
let old = bank.set_capitalization();
|
let old = bank.set_capitalization();
|
||||||
if old != bank.capitalization() {
|
if old != bank.capitalization() {
|
||||||
warn!(
|
warn!(
|
||||||
|
|
Loading…
Reference in New Issue