gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

fix rewards points (#10914)

Browse Source 
* fix rewards points

* fixups

* * verify that we don't spend more in rewards than we've allocated for rewards
* purge f64s from calculations that could be done with integers

* test typical values

* simplify iteration over delegations some

* fixups

* Use try_from

* Add a comment for commission_split()

* Add assertion to detect inconsistent reward dist.

* Fix vote_balance_and_staked

* Don't overwrite accounts with stale copies

* Fix CI...

* Add tests for vote_balance_and_staked

* Add test for the determinism of update_rewards

* Revert "Don't overwrite accounts with stale copies"

This reverts commit 9886d085a6.

* Make stake_delegation_accounts to return hashmap

Co-authored-by: Ryo Onodera <ryoqun@gmail.com>
This commit is contained in:
Rob Walker
2020-07-20 21:57:25 -07:00
committed by GitHub
parent b0d1c70718
commit 7cc2a6801b
5 changed files with 562 additions and 239 deletions
Download Patch File Download Diff File Expand all files Collapse all files

277
runtime/src/bank.rs
View File

@ -55,11 +55,12 @@ use solana_sdk::{
timing::years_as_slots,
transaction::{Result, Transaction, TransactionError},
};
use solana_stake_program::stake_state::{self, Delegation};
use solana_stake_program::stake_state::{self, Delegation, PointValue};
use solana_vote_program::{vote_instruction::VoteInstruction, vote_state::VoteState};
use std::{
cell::RefCell,
collections::{HashMap, HashSet},
convert::TryFrom,
mem,
ops::RangeInclusive,
path::PathBuf,
@ -898,11 +899,13 @@ impl Bank {
let inflation = self.inflation.read().unwrap();
(*inflation).validator(year) * self.capitalization() as f64 * period
};
} as u64;
let validator_points = self.stakes.write().unwrap().claim_points();
let validator_point_value =
self.check_point_value(validator_rewards / validator_points as f64);
let vote_balance_and_staked = self.stakes.read().unwrap().vote_balance_and_staked();
let validator_point_value = self.pay_validator_rewards(validator_rewards);
// this sysvar could be retired...
self.update_sysvar_account(&sysvar::rewards::id(), |account| {
sysvar::rewards::create_account(
self.inherit_sysvar_account_balance(account),
@ -910,65 +913,117 @@ impl Bank {
)
});
let validator_rewards = self.pay_validator_rewards(validator_point_value);
let validator_rewards_paid =
self.stakes.read().unwrap().vote_balance_and_staked() - vote_balance_and_staked;
if let Some(rewards) = self.rewards.as_ref() {
assert_eq!(
validator_rewards_paid,
u64::try_from(rewards.iter().map(|(_pubkey, reward)| reward).sum::<i64>()).unwrap()
);
}
// verify that we didn't pay any more than we expected to
assert!(validator_rewards >= validator_rewards_paid);
self.capitalization
.fetch_add(validator_rewards as u64, Ordering::Relaxed);
.fetch_add(validator_rewards_paid, Ordering::Relaxed);
}
/// iterate over all stakes, redeem vote credits for each stake we can
/// successfully load and parse, return total payout
fn pay_validator_rewards(&mut self, point_value: f64) -> u64 {
let stake_history = self.stakes.read().unwrap().history().clone();
let mut validator_rewards = HashMap::new();
/// map stake delegations into resolved (pubkey, account) pairs
/// returns a map (has to be copied) of loaded
/// ( Vec<(staker info)> (voter account) ) keyed by voter pubkey
///
fn stake_delegation_accounts(&self) -> HashMap<Pubkey, (Vec<(Pubkey, Account)>, Account)> {
let mut accounts = HashMap::new();
let total_validator_rewards = self
self.stakes
.read()
.unwrap()
.stake_delegations()
.iter()
.map(|(stake_pubkey, delegation)| {
.for_each(|(stake_pubkey, delegation)| {
match (
self.get_account(&stake_pubkey),
self.get_account(&delegation.voter_pubkey),
) {
(Some(mut stake_account), Some(mut vote_account)) => {
let rewards = stake_state::redeem_rewards(
&mut stake_account,
&mut vote_account,
point_value,
Some(&stake_history),
);
if let Ok((stakers_reward, voters_reward)) = rewards {
self.store_account(&stake_pubkey, &stake_account);
self.store_account(&delegation.voter_pubkey, &vote_account);
if voters_reward > 0 {
*validator_rewards
.entry(delegation.voter_pubkey)
.or_insert(0i64) += voters_reward as i64;
}
if stakers_reward > 0 {
*validator_rewards.entry(*stake_pubkey).or_insert(0i64) +=
stakers_reward as i64;
}
stakers_reward + voters_reward
} else {
debug!(
"stake_state::redeem_rewards() failed for {}: {:?}",
stake_pubkey, rewards
);
0
}
(Some(stake_account), Some(vote_account)) => {
let entry = accounts
.entry(delegation.voter_pubkey)
.or_insert((Vec::new(), vote_account));
entry.0.push((*stake_pubkey, stake_account));
}
(_, _) => 0,
(_, _) => {}
}
});
accounts
}
/// iterate over all stakes, redeem vote credits for each stake we can
/// successfully load and parse, return total payout
fn pay_validator_rewards(&mut self, rewards: u64) -> f64 {
let stake_history = self.stakes.read().unwrap().history().clone();
let mut stake_delegation_accounts = self.stake_delegation_accounts();
let points: u128 = stake_delegation_accounts
.iter()
.flat_map(|(_vote_pubkey, (stake_group, vote_account))| {
stake_group
.iter()
.map(move |(_stake_pubkey, stake_account)| (stake_account, vote_account))
})
.map(|(stake_account, vote_account)| {
stake_state::calculate_points(&stake_account, &vote_account, Some(&stake_history))
.unwrap_or(0)
})
.sum();
if points == 0 {
return 0.0;
}
let point_value = PointValue { rewards, points };
let mut rewards = HashMap::new();
// pay according to point value
for (vote_pubkey, (stake_group, vote_account)) in stake_delegation_accounts.iter_mut() {
let mut vote_account_changed = false;
for (stake_pubkey, stake_account) in stake_group.iter_mut() {
let redeemed = stake_state::redeem_rewards(
stake_account,
vote_account,
&point_value,
Some(&stake_history),
);
if let Ok((stakers_reward, voters_reward)) = redeemed {
self.store_account(&stake_pubkey, &stake_account);
vote_account_changed = true;
if voters_reward > 0 {
*rewards.entry(*vote_pubkey).or_insert(0i64) += voters_reward as i64;
}
if stakers_reward > 0 {
*rewards.entry(*stake_pubkey).or_insert(0i64) += stakers_reward as i64;
}
} else {
debug!(
"stake_state::redeem_rewards() failed for {}: {:?}",
stake_pubkey, redeemed
);
}
}
if vote_account_changed {
self.store_account(&vote_pubkey, &vote_account);
}
}
assert_eq!(self.rewards, None);
self.rewards = Some(validator_rewards.drain().collect());
total_validator_rewards
self.rewards = Some(rewards.drain().collect());
point_value.rewards as f64 / point_value.points as f64
}
fn update_recent_blockhashes_locked(&self, locked_blockhash_queue: &BlockhashQueue) {
@ -986,21 +1041,6 @@ impl Bank {
self.update_recent_blockhashes_locked(&blockhash_queue);
}
// If the point values are not `normal`, bring them back into range and
// set them to the last value or 0.
fn check_point_value(&self, mut validator_point_value: f64) -> f64 {
let rewards = sysvar::rewards::Rewards::from_account(
&self
.get_account(&sysvar::rewards::id())
.unwrap_or_else(|| sysvar::rewards::create_account(1, 0.0)),
)
.unwrap_or_else(Default::default);
if !validator_point_value.is_normal() {
validator_point_value = rewards.validator_point_value;
}
validator_point_value
}
fn collect_fees(&self) {
let collector_fees = self.collector_fees.load(Ordering::Relaxed) as u64;
@ -4730,7 +4770,18 @@ mod tests {
}
bank.store_account(&vote_id, &vote_account);
let validator_points = bank.stakes.read().unwrap().points();
let validator_points: u128 = bank
.stake_delegation_accounts()
.iter()
.flat_map(|(_vote_pubkey, (stake_group, vote_account))| {
stake_group
.iter()
.map(move |(_stake_pubkey, stake_account)| (stake_account, vote_account))
})
.map(|(stake_account, vote_account)| {
stake_state::calculate_points(&stake_account, &vote_account, None).unwrap_or(0)
})
.sum();
// put a child bank in epoch 1, which calls update_rewards()...
let bank1 = Bank::new_from_parent(
@ -4774,6 +4825,89 @@ mod tests {
);
}
fn do_test_bank_update_rewards_determinism() -> u64 {
// create a bank that ticks really slowly...
let bank = Arc::new(Bank::new(&GenesisConfig {
accounts: (0..42)
.map(|_| {
(
Pubkey::new_rand(),
Account::new(1_000_000_000, 0, &Pubkey::default()),
)
})
.collect(),
// set it up so the first epoch is a full year long
poh_config: PohConfig {
target_tick_duration: Duration::from_secs(
SECONDS_PER_YEAR as u64
/ MINIMUM_SLOTS_PER_EPOCH as u64
/ DEFAULT_TICKS_PER_SLOT,
),
hashes_per_tick: None,
target_tick_count: None,
},
..GenesisConfig::default()
}));
// enable lazy rent collection because this test depends on rent-due accounts
// not being eagerly-collected for exact rewards calculation
bank.lazy_rent_collection.store(true, Ordering::Relaxed);
assert_eq!(bank.capitalization(), 42 * 1_000_000_000);
assert_eq!(bank.rewards, None);
let vote_id = Pubkey::new_rand();
let mut vote_account = vote_state::create_account(&vote_id, &Pubkey::new_rand(), 50, 100);
let (stake_id1, stake_account1) = crate::stakes::tests::create_stake_account(123, &vote_id);
let (stake_id2, stake_account2) = crate::stakes::tests::create_stake_account(456, &vote_id);
// set up accounts
bank.store_account(&stake_id1, &stake_account1);
bank.store_account(&stake_id2, &stake_account2);
// generate some rewards
let mut vote_state = Some(VoteState::from(&vote_account).unwrap());
for i in 0..MAX_LOCKOUT_HISTORY + 42 {
if let Some(v) = vote_state.as_mut() {
v.process_slot_vote_unchecked(i as u64)
}
let versioned = VoteStateVersions::Current(Box::new(vote_state.take().unwrap()));
VoteState::to(&versioned, &mut vote_account).unwrap();
bank.store_account(&vote_id, &vote_account);
match versioned {
VoteStateVersions::Current(v) => {
vote_state = Some(*v);
}
_ => panic!("Has to be of type Current"),
};
}
bank.store_account(&vote_id, &vote_account);
// put a child bank in epoch 1, which calls update_rewards()...
let bank1 = Bank::new_from_parent(
&bank,
&Pubkey::default(),
bank.get_slots_in_epoch(bank.epoch()) + 1,
);
// verify that there's inflation
assert_ne!(bank1.capitalization(), bank.capitalization());
bank1.capitalization()
}
#[test]
fn test_bank_update_rewards_determinism() {
// The same reward should be distributed given same credits
let expected_capitalization = do_test_bank_update_rewards_determinism();
// Repeat somewhat large number of iterations to expose possible different behavior
// depending on the randamly-seeded HashMap ordering
for _ in 0..30 {
let actual_capitalization = do_test_bank_update_rewards_determinism();
assert_eq!(actual_capitalization, expected_capitalization);
}
}
// Test that purging 0 lamports accounts works.
#[test]
fn test_purge_empty_accounts() {
@ -6364,23 +6498,6 @@ mod tests {
assert!(bank.is_delta.load(Ordering::Relaxed));
}
#[test]
#[allow(clippy::float_cmp)]
fn test_check_point_value() {
let (genesis_config, _) = create_genesis_config(500);
let bank = Arc::new(Bank::new(&genesis_config));
// check that point values are 0 if no previous value was known and current values are not normal
assert_eq!(bank.check_point_value(std::f64::INFINITY), 0.0);
bank.store_account(
&sysvar::rewards::id(),
&sysvar::rewards::create_account(1, 1.0),
);
// check that point values are the previous value if current values are not normal
assert_eq!(bank.check_point_value(std::f64::INFINITY), 1.0);
}
#[test]
fn test_bank_get_program_accounts() {
let (genesis_config, mint_keypair) = create_genesis_config(500);

4
runtime/src/serde_snapshot/tests.rs
View File

@ -282,7 +282,7 @@ mod test_bank_serialize {
// These some what long test harness is required to freeze the ABI of
// Bank's serialization due to versioned nature
#[frozen_abi(digest = "6MnT4MzuLHe4Uq96YaF3JF2gL1RvprzQHCaV9TaWgYLe")]
#[frozen_abi(digest = "FaZaic5p7bvdsKDxGJmaPVyp12AbAmURyYoGiUdx1Ksu")]
#[derive(Serialize, AbiExample)]
pub struct BankAbiTestWrapperFuture {
#[serde(serialize_with = "wrapper_future")]
@ -305,7 +305,7 @@ mod test_bank_serialize {
.serialize(s)
}
#[frozen_abi(digest = "92KVEUQ8PwKe5DgZ6AyDQGAi9pvi7kfHdMBE4hcs5EQ4")]
#[frozen_abi(digest = "9g4bYykzsC86fULgu9iUh4kpvb1pxvAmipvyZPChLhws")]
#[derive(Serialize, AbiExample)]
pub struct BankAbiTestWrapperLegacy {
#[serde(serialize_with = "wrapper_legacy")]

185
runtime/src/stakes.rs
View File

@ -15,9 +15,8 @@ pub struct Stakes {
/// stake_delegations
stake_delegations: HashMap<Pubkey, Delegation>,
/// unclaimed points.
// a point is a credit multiplied by the stake
points: u64,
/// unused
unused: u64,
/// current epoch, used to calculate current stake
epoch: Epoch,
@ -49,7 +48,7 @@ impl Stakes {
Stakes {
stake_delegations: self.stake_delegations.clone(),
points: self.points,
unused: self.unused,
epoch,
vote_accounts: self
.vote_accounts
@ -88,6 +87,18 @@ impl Stakes {
.sum()
}
pub fn vote_balance_and_staked(&self) -> u64 {
self.stake_delegations
.iter()
.map(|(_, stake_delegation)| stake_delegation.stake)
.sum::<u64>()
+ self
.vote_accounts
.iter()
.map(|(_pubkey, (_staked, vote_account))| vote_account.lamports)
.sum::<u64>()
}
pub fn is_stake(account: &Account) -> bool {
solana_vote_program::check_id(&account.owner)
|| solana_stake_program::check_id(&account.owner)
@ -106,15 +117,6 @@ impl Stakes {
|v| v.0,
);
// count any increase in points, can only go forward
let old_credits = old
.and_then(|(_stake, old_account)| VoteState::credits_from(old_account))
.unwrap_or(0);
let credits = VoteState::credits_from(account).unwrap_or(old_credits);
self.points += credits.saturating_sub(old_credits) * stake;
self.vote_accounts.insert(*pubkey, (stake, account.clone()));
}
} else if solana_stake_program::check_id(&account.owner) {
@ -176,18 +178,6 @@ impl Stakes {
.and_then(|(_k, (_stake, account))| VoteState::from(account))
.map(|vote_state| vote_state.node_pubkey)
}
/// currently unclaimed points
pub fn points(&self) -> u64 {
self.points
}
/// "claims" points, resets points to 0
pub fn claim_points(&mut self) -> u64 {
let points = self.points;
self.points = 0;
points
}
}
#[cfg(test)]
@ -195,9 +185,7 @@ pub mod tests {
use super::*;
use solana_sdk::{pubkey::Pubkey, rent::Rent};
use solana_stake_program::stake_state;
use solana_vote_program::vote_state::{
self, VoteState, VoteStateVersions, MAX_LOCKOUT_HISTORY,
};
use solana_vote_program::vote_state::{self, VoteState};
// set up some dummies for a staked node (( vote ) ( stake ))
pub fn create_staked_node_accounts(stake: u64) -> ((Pubkey, Account), (Pubkey, Account)) {
@ -224,6 +212,38 @@ pub mod tests {
)
}
pub fn create_warming_staked_node_accounts(
stake: u64,
epoch: Epoch,
) -> ((Pubkey, Account), (Pubkey, Account)) {
let vote_pubkey = Pubkey::new_rand();
let vote_account = vote_state::create_account(&vote_pubkey, &Pubkey::new_rand(), 0, 1);
(
(vote_pubkey, vote_account),
create_warming_stake_account(stake, epoch, &vote_pubkey),
)
}
// add stake to a vote_pubkey ( stake )
pub fn create_warming_stake_account(
stake: u64,
epoch: Epoch,
vote_pubkey: &Pubkey,
) -> (Pubkey, Account) {
let stake_pubkey = Pubkey::new_rand();
(
stake_pubkey,
stake_state::create_account_with_activation_epoch(
&stake_pubkey,
&vote_pubkey,
&vote_state::create_account(&vote_pubkey, &Pubkey::new_rand(), 0, 1),
&Rent::free(),
stake,
epoch,
),
)
}
#[test]
fn test_stakes_basic() {
for i in 0..4 {
@ -302,76 +322,6 @@ pub mod tests {
assert_eq!(stakes.highest_staked_node(), Some(vote11_node_pubkey))
}
#[test]
fn test_stakes_points() {
let mut stakes = Stakes::default();
stakes.epoch = 4;
let stake = 42;
assert_eq!(stakes.points(), 0);
assert_eq!(stakes.claim_points(), 0);
assert_eq!(stakes.claim_points(), 0);
let ((vote_pubkey, mut vote_account), (stake_pubkey, stake_account)) =
create_staked_node_accounts(stake);
stakes.store(&vote_pubkey, &vote_account);
stakes.store(&stake_pubkey, &stake_account);
assert_eq!(stakes.points(), 0);
assert_eq!(stakes.claim_points(), 0);
let mut vote_state = Some(VoteState::from(&vote_account).unwrap());
for i in 0..MAX_LOCKOUT_HISTORY + 42 {
if let Some(v) = vote_state.as_mut() {
v.process_slot_vote_unchecked(i as u64)
}
let versioned = VoteStateVersions::Current(Box::new(vote_state.take().unwrap()));
VoteState::to(&versioned, &mut vote_account).unwrap();
match versioned {
VoteStateVersions::Current(v) => {
vote_state = Some(*v);
}
_ => panic!("Has to be of type Current"),
};
stakes.store(&vote_pubkey, &vote_account);
assert_eq!(
stakes.points(),
vote_state.as_ref().unwrap().credits() * stake
);
}
vote_account.lamports = 0;
stakes.store(&vote_pubkey, &vote_account);
assert_eq!(
stakes.points(),
vote_state.as_ref().unwrap().credits() * stake
);
assert_eq!(
stakes.claim_points(),
vote_state.as_ref().unwrap().credits() * stake
);
assert_eq!(stakes.claim_points(), 0);
assert_eq!(stakes.claim_points(), 0);
// points come out of nowhere, but don't care here ;)
vote_account.lamports = 1;
stakes.store(&vote_pubkey, &vote_account);
assert_eq!(
stakes.points(),
vote_state.as_ref().unwrap().credits() * stake
);
// test going backwards, should never go backwards
let old_vote_state = vote_state;
let vote_account = vote_state::create_account(&vote_pubkey, &Pubkey::new_rand(), 0, 1);
stakes.store(&vote_pubkey, &vote_account);
assert_eq!(
stakes.points(),
old_vote_state.as_ref().unwrap().credits() * stake
);
}
#[test]
fn test_stakes_vote_account_disappear_reappear() {
let mut stakes = Stakes::default();
@ -528,4 +478,43 @@ pub mod tests {
assert_eq!(vote_accounts.get(&vote_pubkey).unwrap().0, 0);
}
}
#[test]
fn test_vote_balance_and_staked_empty() {
let stakes = Stakes::default();
assert_eq!(stakes.vote_balance_and_staked(), 0);
}
#[test]
fn test_vote_balance_and_staked_normal() {
let mut stakes = Stakes::default();
impl Stakes {
pub fn vote_balance_and_warmed_staked(&self) -> u64 {
self.vote_accounts
.iter()
.map(|(_pubkey, (staked, account))| staked + account.lamports)
.sum()
}
}
let genesis_epoch = 0;
let ((vote_pubkey, vote_account), (stake_pubkey, stake_account)) =
create_warming_staked_node_accounts(10, genesis_epoch);
stakes.store(&vote_pubkey, &vote_account);
stakes.store(&stake_pubkey, &stake_account);
assert_eq!(stakes.vote_balance_and_staked(), 11);
assert_eq!(stakes.vote_balance_and_warmed_staked(), 1);
for (epoch, expected_warmed_stake) in ((genesis_epoch + 1)..=3).zip(&[2, 3, 4]) {
stakes = stakes.clone_with_epoch(epoch);
// vote_balance_and_staked() always remain to return same lamports
// while vote_balance_and_warmed_staked() gradually increases
assert_eq!(stakes.vote_balance_and_staked(), 11);
assert_eq!(
stakes.vote_balance_and_warmed_staked(),
*expected_warmed_stake
);
}
}
}