From 96cc01e1eb5aec75638ba4f719bf9081ba8d6e21 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Tue, 4 Jun 2024 14:57:13 -0600 Subject: [PATCH 01/15] Add num_partitions field to Rewards proto definition --- storage-proto/proto/confirmed_block.proto | 6 ++++++ storage-proto/src/convert.rs | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/storage-proto/proto/confirmed_block.proto b/storage-proto/proto/confirmed_block.proto index 47548ea13bc6a4..6d26ce7bce2cad 100644 --- a/storage-proto/proto/confirmed_block.proto +++ b/storage-proto/proto/confirmed_block.proto @@ -10,6 +10,7 @@ message ConfirmedBlock { repeated Reward rewards = 5; UnixTimestamp block_time = 6; BlockHeight block_height = 7; + NumPartitions num_partitions = 8; } message ConfirmedTransaction { @@ -130,6 +131,7 @@ message Reward { message Rewards { repeated Reward rewards = 1; + NumPartitions num_partitions = 2; } message UnixTimestamp { @@ -139,3 +141,7 @@ message UnixTimestamp { message BlockHeight { uint64 block_height = 1; } + +message NumPartitions { + uint64 num_partitions = 1; +} diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index 8d6669e44b43f1..b6dc895ea4dc50 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -47,6 +47,7 @@ impl From> for generated::Rewards { fn from(rewards: Vec) -> Self { Self { rewards: rewards.into_iter().map(|r| r.into()).collect(), + num_partitions: None, } } } @@ -67,6 +68,7 @@ impl From for generated::Rewards { r.into() }) .collect(), + num_partitions: None, } } } @@ -121,6 +123,12 @@ impl From for Reward { } } +impl From for generated::NumPartitions { + fn from(num_partitions: u64) -> Self { + Self { num_partitions } + } +} + impl From for generated::ConfirmedBlock { fn from(confirmed_block: VersionedConfirmedBlock) -> Self { let VersionedConfirmedBlock { @@ -139,6 +147,7 @@ impl From for generated::ConfirmedBlock { parent_slot, transactions: transactions.into_iter().map(|tx| tx.into()).collect(), rewards: rewards.into_iter().map(|r| r.into()).collect(), + num_partitions: None, block_time: block_time.map(|timestamp| generated::UnixTimestamp { timestamp }), block_height: block_height.map(|block_height| generated::BlockHeight { block_height }), } @@ -156,6 +165,7 @@ impl TryFrom for ConfirmedBlock { parent_slot, transactions, rewards, + num_partitions: _num_partitions, block_time, block_height, } = confirmed_block; From 39bbefb1f7139efced5f640a409207af294f79e0 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Tue, 4 Jun 2024 15:51:34 -0600 Subject: [PATCH 02/15] Add type to hold rewards plus num_partitions --- storage-proto/src/convert.rs | 14 ++++++++++++-- transaction-status/src/lib.rs | 5 +++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index b6dc895ea4dc50..81deac59216e41 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -16,8 +16,9 @@ use { }, solana_transaction_status::{ ConfirmedBlock, EntrySummary, InnerInstruction, InnerInstructions, Reward, RewardType, - TransactionByAddrInfo, TransactionStatusMeta, TransactionTokenBalance, - TransactionWithStatusMeta, VersionedConfirmedBlock, VersionedTransactionWithStatusMeta, + RewardsAndNumPartitions, TransactionByAddrInfo, TransactionStatusMeta, + TransactionTokenBalance, TransactionWithStatusMeta, VersionedConfirmedBlock, + VersionedTransactionWithStatusMeta, }, std::{ convert::{TryFrom, TryInto}, @@ -52,6 +53,15 @@ impl From> for generated::Rewards { } } +impl From for generated::Rewards { + fn from(input: RewardsAndNumPartitions) -> Self { + Self { + rewards: input.rewards.into_iter().map(|r| r.into()).collect(), + num_partitions: input.num_partitions.map(|n| n.into()), + } + } +} + impl From for Vec { fn from(rewards: generated::Rewards) -> Self { rewards.rewards.into_iter().map(|r| r.into()).collect() diff --git a/transaction-status/src/lib.rs b/transaction-status/src/lib.rs index 7779cfc5ae9353..79dd7a690faec3 100644 --- a/transaction-status/src/lib.rs +++ b/transaction-status/src/lib.rs @@ -628,6 +628,11 @@ pub struct Reward { pub type Rewards = Vec; +pub struct RewardsAndNumPartitions { + pub rewards: Rewards, + pub num_partitions: Option, +} + #[derive(Debug, Error)] pub enum ConvertBlockError { #[error("transactions missing after converted, before: {0}, after: {1}")] From 2da7053734e26ad0d3aee847350ddde9e644f165 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Tue, 4 Jun 2024 15:23:24 -0600 Subject: [PATCH 03/15] Add Bank method to get rewards plus num_partitions for recording --- runtime/src/bank.rs | 4 +++- .../src/bank/partitioned_epoch_rewards/mod.rs | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e89d4678166756..8f0f1062702375 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -35,7 +35,6 @@ //! already been signed and verified. #[allow(deprecated)] use solana_sdk::recent_blockhashes_account; -pub use solana_sdk::reward_type::RewardType; use { crate::{ bank::{ @@ -205,6 +204,9 @@ use { time::{Duration, Instant}, }, }; +pub use { + partitioned_epoch_rewards::KeyedRewardsAndNumPartitions, solana_sdk::reward_type::RewardType, +}; #[cfg(feature = "dev-context-only-utils")] use { solana_accounts_db::accounts_db::{ diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index f374b127050aef..5a26c3d6f2b910 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -150,7 +150,27 @@ pub(super) struct CalculateRewardsAndDistributeVoteRewardsResult { pub(crate) type StakeRewards = Vec; +pub struct KeyedRewardsAndNumPartitions { + pub keyed_rewards: Vec<(Pubkey, RewardInfo)>, + pub num_partitions: Option, +} + impl Bank { + pub fn get_rewards_and_num_partitions(&self) -> KeyedRewardsAndNumPartitions { + let rewards = self.rewards.read().unwrap(); + let epoch_rewards_sysvar = self.get_epoch_rewards_sysvar(); + // If partitioned epoch rewards are active and this Bank is the + // epoch-boundary block, populate num_partitions + let num_partitions = (epoch_rewards_sysvar.active + && (self.block_height().saturating_add(1) + == epoch_rewards_sysvar.distribution_starting_block_height)) + .then_some(epoch_rewards_sysvar.num_partitions); + KeyedRewardsAndNumPartitions { + keyed_rewards: rewards.clone(), + num_partitions, + } + } + pub(super) fn is_partitioned_rewards_feature_enabled(&self) -> bool { self.feature_set .is_active(&feature_set::enable_partitioned_epoch_reward::id()) From bbd598e6e736647960a647f5c90e61606c94056f Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Tue, 4 Jun 2024 16:05:55 -0600 Subject: [PATCH 04/15] Update Blockstore::write_rewards to use num_partitions --- core/src/rewards_recorder_service.rs | 10 ++++++++-- ledger/src/blockstore.rs | 7 ++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/core/src/rewards_recorder_service.rs b/core/src/rewards_recorder_service.rs index 3fc2c8dc5b5149..2a22ca55b2ba8e 100644 --- a/core/src/rewards_recorder_service.rs +++ b/core/src/rewards_recorder_service.rs @@ -2,7 +2,7 @@ use { crossbeam_channel::{Receiver, RecvTimeoutError, Sender}, solana_ledger::blockstore::Blockstore, solana_sdk::{clock::Slot, pubkey::Pubkey, reward_info::RewardInfo}, - solana_transaction_status::Reward, + solana_transaction_status::{Reward, RewardsAndNumPartitions}, std::{ sync::{ atomic::{AtomicBool, AtomicU64, Ordering}, @@ -68,7 +68,13 @@ impl RewardsRecorderService { .collect(); blockstore - .write_rewards(slot, rpc_rewards) + .write_rewards( + slot, + RewardsAndNumPartitions { + rewards: rpc_rewards, + num_partitions: None, + }, + ) .expect("Expect database write to succeed"); } RewardsMessage::Complete(slot) => { diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 450f7b4f005f3c..3cf3417e5faabf 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -57,8 +57,9 @@ use { solana_storage_proto::{StoredExtendedRewards, StoredTransactionStatusMeta}, solana_transaction_status::{ ConfirmedTransactionStatusWithSignature, ConfirmedTransactionWithStatusMeta, Rewards, - TransactionStatusMeta, TransactionWithStatusMeta, VersionedConfirmedBlock, - VersionedConfirmedBlockWithEntries, VersionedTransactionWithStatusMeta, + RewardsAndNumPartitions, TransactionStatusMeta, TransactionWithStatusMeta, + VersionedConfirmedBlock, VersionedConfirmedBlockWithEntries, + VersionedTransactionWithStatusMeta, }, std::{ borrow::Cow, @@ -3371,7 +3372,7 @@ impl Blockstore { .map(|result| result.map(|option| option.into())) } - pub fn write_rewards(&self, index: Slot, rewards: Rewards) -> Result<()> { + pub fn write_rewards(&self, index: Slot, rewards: RewardsAndNumPartitions) -> Result<()> { let rewards = rewards.into(); self.rewards_cf.put_protobuf(index, &rewards) } From 649b8c3dd5f67ac86518fa3e5330f0951027fe69 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Tue, 4 Jun 2024 16:21:28 -0600 Subject: [PATCH 05/15] Update RewardsRecorderService to handle num_partitions --- core/src/replay_stage.rs | 10 ++++++++-- core/src/rewards_recorder_service.rs | 15 +++++++++++---- runtime/src/bank/partitioned_epoch_rewards/mod.rs | 6 ++++++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 4861b7893e5554..ac2c3639f05123 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -58,7 +58,7 @@ use { solana_rpc_client_api::response::SlotUpdate, solana_runtime::{ accounts_background_service::AbsRequestSender, - bank::{bank_hash_details, Bank, NewBankOptions}, + bank::{bank_hash_details, Bank, KeyedRewardsAndNumPartitions, NewBankOptions}, bank_forks::{BankForks, SetRootError, MAX_ROOT_DISTANCE_FOR_VOTE_ONLY}, commitment::BlockCommitmentCache, installed_scheduler_pool::BankWithScheduler, @@ -4396,7 +4396,13 @@ impl ReplayStage { let rewards = bank.rewards.read().unwrap(); if !rewards.is_empty() { rewards_recorder_sender - .send(RewardsMessage::Batch((bank.slot(), rewards.clone()))) + .send(RewardsMessage::Batch(( + bank.slot(), + KeyedRewardsAndNumPartitions { + keyed_rewards: rewards.clone(), + num_partitions: None, + }, + ))) .unwrap_or_else(|err| warn!("rewards_recorder_sender failed: {:?}", err)); } rewards_recorder_sender diff --git a/core/src/rewards_recorder_service.rs b/core/src/rewards_recorder_service.rs index 2a22ca55b2ba8e..044fd2de53adc7 100644 --- a/core/src/rewards_recorder_service.rs +++ b/core/src/rewards_recorder_service.rs @@ -1,7 +1,8 @@ use { crossbeam_channel::{Receiver, RecvTimeoutError, Sender}, solana_ledger::blockstore::Blockstore, - solana_sdk::{clock::Slot, pubkey::Pubkey, reward_info::RewardInfo}, + solana_runtime::bank::KeyedRewardsAndNumPartitions, + solana_sdk::clock::Slot, solana_transaction_status::{Reward, RewardsAndNumPartitions}, std::{ sync::{ @@ -13,7 +14,7 @@ use { }, }; -pub type RewardsBatch = (Slot, Vec<(Pubkey, RewardInfo)>); +pub type RewardsBatch = (Slot, KeyedRewardsAndNumPartitions); pub type RewardsRecorderReceiver = Receiver; pub type RewardsRecorderSender = Sender; @@ -55,7 +56,13 @@ impl RewardsRecorderService { blockstore: &Blockstore, ) -> Result<(), RecvTimeoutError> { match rewards_receiver.recv_timeout(Duration::from_secs(1))? { - RewardsMessage::Batch((slot, rewards)) => { + RewardsMessage::Batch(( + slot, + KeyedRewardsAndNumPartitions { + keyed_rewards: rewards, + num_partitions, + }, + )) => { let rpc_rewards = rewards .into_iter() .map(|(pubkey, reward_info)| Reward { @@ -72,7 +79,7 @@ impl RewardsRecorderService { slot, RewardsAndNumPartitions { rewards: rpc_rewards, - num_partitions: None, + num_partitions, }, ) .expect("Expect database write to succeed"); diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index 5a26c3d6f2b910..becc24091eaac2 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -155,6 +155,12 @@ pub struct KeyedRewardsAndNumPartitions { pub num_partitions: Option, } +impl KeyedRewardsAndNumPartitions { + pub fn is_empty(&self) -> bool { + self.keyed_rewards.is_empty() && self.num_partitions.is_none() + } +} + impl Bank { pub fn get_rewards_and_num_partitions(&self) -> KeyedRewardsAndNumPartitions { let rewards = self.rewards.read().unwrap(); From 95d6de7429771585cdc29d41a71927830eefbdfd Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Tue, 4 Jun 2024 16:22:24 -0600 Subject: [PATCH 06/15] Populate num_partitions in ReplayStage::record_rewards --- core/src/replay_stage.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index ac2c3639f05123..37e6edb2c22caa 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -58,7 +58,7 @@ use { solana_rpc_client_api::response::SlotUpdate, solana_runtime::{ accounts_background_service::AbsRequestSender, - bank::{bank_hash_details, Bank, KeyedRewardsAndNumPartitions, NewBankOptions}, + bank::{bank_hash_details, Bank, NewBankOptions}, bank_forks::{BankForks, SetRootError, MAX_ROOT_DISTANCE_FOR_VOTE_ONLY}, commitment::BlockCommitmentCache, installed_scheduler_pool::BankWithScheduler, @@ -4393,16 +4393,10 @@ impl ReplayStage { fn record_rewards(bank: &Bank, rewards_recorder_sender: &Option) { if let Some(rewards_recorder_sender) = rewards_recorder_sender { - let rewards = bank.rewards.read().unwrap(); + let rewards = bank.get_rewards_and_num_partitions(); if !rewards.is_empty() { rewards_recorder_sender - .send(RewardsMessage::Batch(( - bank.slot(), - KeyedRewardsAndNumPartitions { - keyed_rewards: rewards.clone(), - num_partitions: None, - }, - ))) + .send(RewardsMessage::Batch((bank.slot(), rewards))) .unwrap_or_else(|err| warn!("rewards_recorder_sender failed: {:?}", err)); } rewards_recorder_sender From 16dfd3fbe7ace13231377d82a2fa7309d4259223 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Wed, 5 Jun 2024 15:02:45 -0600 Subject: [PATCH 07/15] Write num_partitions to Bigtable --- ledger/src/blockstore.rs | 6 +++++- rpc-client/src/mock_sender.rs | 1 + storage-bigtable/src/bigtable.rs | 4 ++++ storage-bigtable/src/lib.rs | 2 ++ storage-proto/src/convert.rs | 18 ++++++++++++++++-- transaction-status/src/lib.rs | 9 +++++++++ 6 files changed, 37 insertions(+), 3 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 3cf3417e5faabf..0342a323905876 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -2679,7 +2679,7 @@ impl Blockstore { Hash::default() }; - let rewards = self + let (rewards, num_partitions) = self .rewards_cf .get_protobuf_or_bincode::(slot)? .unwrap_or_default() @@ -2700,6 +2700,7 @@ impl Blockstore { transactions: self .map_transactions_to_statuses(slot, slot_transaction_iterator)?, rewards, + num_partitions, block_time, block_height, }; @@ -8303,6 +8304,7 @@ pub mod tests { blockhash: blockhash.to_string(), previous_blockhash: Hash::default().to_string(), rewards: vec![], + num_partitions: None, block_time: None, block_height: None, }; @@ -8317,6 +8319,7 @@ pub mod tests { blockhash: blockhash.to_string(), previous_blockhash: blockhash.to_string(), rewards: vec![], + num_partitions: None, block_time: None, block_height: None, }; @@ -8334,6 +8337,7 @@ pub mod tests { blockhash: blockhash.to_string(), previous_blockhash: blockhash.to_string(), rewards: vec![], + num_partitions: None, block_time: None, block_height: None, }; diff --git a/rpc-client/src/mock_sender.rs b/rpc-client/src/mock_sender.rs index 806d56d70a2f6d..8c5c58086e6fe6 100644 --- a/rpc-client/src/mock_sender.rs +++ b/rpc-client/src/mock_sender.rs @@ -406,6 +406,7 @@ impl RpcSender for MockSender { version: Some(TransactionVersion::LEGACY), }], rewards: Rewards::new(), + num_partitions: None, block_time: None, block_height: Some(428), })?, diff --git a/storage-bigtable/src/bigtable.rs b/storage-bigtable/src/bigtable.rs index fdebb5ab8d0214..b4bfe040a30963 100644 --- a/storage-bigtable/src/bigtable.rs +++ b/storage-bigtable/src/bigtable.rs @@ -985,6 +985,7 @@ mod tests { parent_slot, transactions, rewards, + num_partitions, block_time, block_height, } = confirmed_block; @@ -995,6 +996,8 @@ mod tests { parent_slot, transactions: transactions.into_iter().map(|tx| tx.into()).collect(), rewards: rewards.into_iter().map(|r| r.into()).collect(), + num_partitions: num_partitions + .map(|num_partitions| generated::NumPartitions { num_partitions }), block_time: block_time.map(|timestamp| generated::UnixTimestamp { timestamp }), block_height: block_height.map(|block_height| generated::BlockHeight { block_height }), } @@ -1028,6 +1031,7 @@ mod tests { blockhash: Hash::default().to_string(), previous_blockhash: Hash::default().to_string(), rewards: vec![], + num_partitions: None, block_time: Some(1_234_567_890), block_height: Some(1), }; diff --git a/storage-bigtable/src/lib.rs b/storage-bigtable/src/lib.rs index 240ae44c3d07fe..3af928a626d834 100644 --- a/storage-bigtable/src/lib.rs +++ b/storage-bigtable/src/lib.rs @@ -141,6 +141,7 @@ impl From for StoredConfirmedBlock { parent_slot, transactions, rewards, + num_partitions: _num_partitions, block_time, block_height, } = confirmed_block; @@ -175,6 +176,7 @@ impl From for ConfirmedBlock { parent_slot, transactions: transactions.into_iter().map(|tx| tx.into()).collect(), rewards: rewards.into_iter().map(|reward| reward.into()).collect(), + num_partitions: None, block_time, block_height, } diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index 81deac59216e41..8315bcf99a4dac 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -68,6 +68,17 @@ impl From for Vec { } } +impl From for (Vec, Option) { + fn from(rewards: generated::Rewards) -> Self { + ( + rewards.rewards.into_iter().map(|r| r.into()).collect(), + rewards + .num_partitions + .map(|generated::NumPartitions { num_partitions }| num_partitions), + ) + } +} + impl From for generated::Rewards { fn from(rewards: StoredExtendedRewards) -> Self { Self { @@ -147,6 +158,7 @@ impl From for generated::ConfirmedBlock { parent_slot, transactions, rewards, + num_partitions, block_time, block_height, } = confirmed_block; @@ -157,7 +169,7 @@ impl From for generated::ConfirmedBlock { parent_slot, transactions: transactions.into_iter().map(|tx| tx.into()).collect(), rewards: rewards.into_iter().map(|r| r.into()).collect(), - num_partitions: None, + num_partitions: num_partitions.map(Into::into), block_time: block_time.map(|timestamp| generated::UnixTimestamp { timestamp }), block_height: block_height.map(|block_height| generated::BlockHeight { block_height }), } @@ -175,7 +187,7 @@ impl TryFrom for ConfirmedBlock { parent_slot, transactions, rewards, - num_partitions: _num_partitions, + num_partitions, block_time, block_height, } = confirmed_block; @@ -189,6 +201,8 @@ impl TryFrom for ConfirmedBlock { .map(|tx| tx.try_into()) .collect::, Self::Error>>()?, rewards: rewards.into_iter().map(|r| r.into()).collect(), + num_partitions: num_partitions + .map(|generated::NumPartitions { num_partitions }| num_partitions), block_time: block_time.map(|generated::UnixTimestamp { timestamp }| timestamp), block_height: block_height.map(|generated::BlockHeight { block_height }| block_height), }) diff --git a/transaction-status/src/lib.rs b/transaction-status/src/lib.rs index 79dd7a690faec3..9467b32c3db4e0 100644 --- a/transaction-status/src/lib.rs +++ b/transaction-status/src/lib.rs @@ -646,6 +646,7 @@ pub struct ConfirmedBlock { pub parent_slot: Slot, pub transactions: Vec, pub rewards: Rewards, + pub num_partitions: Option, pub block_time: Option, pub block_height: Option, } @@ -659,6 +660,7 @@ pub struct VersionedConfirmedBlock { pub parent_slot: Slot, pub transactions: Vec, pub rewards: Rewards, + pub num_partitions: Option, pub block_time: Option, pub block_height: Option, } @@ -675,6 +677,7 @@ impl From for ConfirmedBlock { .map(TransactionWithStatusMeta::Complete) .collect(), rewards: block.rewards, + num_partitions: block.num_partitions, block_time: block.block_time, block_height: block.block_height, } @@ -709,6 +712,7 @@ impl TryFrom for VersionedConfirmedBlock { parent_slot: block.parent_slot, transactions: txs, rewards: block.rewards, + num_partitions: block.num_partitions, block_time: block.block_time, block_height: block.block_height, }) @@ -773,6 +777,7 @@ impl ConfirmedBlock { } else { None }, + num_partitions: self.num_partitions, block_time: self.block_time, block_height: self.block_height, }) @@ -787,6 +792,7 @@ pub struct EncodedConfirmedBlock { pub parent_slot: Slot, pub transactions: Vec, pub rewards: Rewards, + pub num_partitions: Option, pub block_time: Option, pub block_height: Option, } @@ -799,6 +805,7 @@ impl From for EncodedConfirmedBlock { parent_slot: block.parent_slot, transactions: block.transactions.unwrap_or_default(), rewards: block.rewards.unwrap_or_default(), + num_partitions: block.num_partitions, block_time: block.block_time, block_height: block.block_height, } @@ -817,6 +824,8 @@ pub struct UiConfirmedBlock { pub signatures: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] pub rewards: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub num_partitions: Option, pub block_time: Option, pub block_height: Option, } From 2fe773e9ca46e8c90b9d3737e4ba4a9b10a49479 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Wed, 12 Jun 2024 12:54:54 -0600 Subject: [PATCH 08/15] Reword KeyedRewardsAndNumPartitions method --- core/src/replay_stage.rs | 2 +- runtime/src/bank/partitioned_epoch_rewards/mod.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 37e6edb2c22caa..d547abc096d65d 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -4394,7 +4394,7 @@ impl ReplayStage { fn record_rewards(bank: &Bank, rewards_recorder_sender: &Option) { if let Some(rewards_recorder_sender) = rewards_recorder_sender { let rewards = bank.get_rewards_and_num_partitions(); - if !rewards.is_empty() { + if rewards.should_record() { rewards_recorder_sender .send(RewardsMessage::Batch((bank.slot(), rewards))) .unwrap_or_else(|err| warn!("rewards_recorder_sender failed: {:?}", err)); diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index becc24091eaac2..72b55a7f88e71b 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -156,8 +156,8 @@ pub struct KeyedRewardsAndNumPartitions { } impl KeyedRewardsAndNumPartitions { - pub fn is_empty(&self) -> bool { - self.keyed_rewards.is_empty() && self.num_partitions.is_none() + pub fn should_record(&self) -> bool { + !self.keyed_rewards.is_empty() || self.num_partitions.is_some() } } From bf922b3db59ce20351d1844429a3b69fc7dd3479 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Wed, 12 Jun 2024 12:57:11 -0600 Subject: [PATCH 09/15] Clone immediately --- runtime/src/bank/partitioned_epoch_rewards/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index 72b55a7f88e71b..b6a9ca6b4d0d4a 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -163,7 +163,7 @@ impl KeyedRewardsAndNumPartitions { impl Bank { pub fn get_rewards_and_num_partitions(&self) -> KeyedRewardsAndNumPartitions { - let rewards = self.rewards.read().unwrap(); + let keyed_rewards = self.rewards.read().unwrap().clone(); let epoch_rewards_sysvar = self.get_epoch_rewards_sysvar(); // If partitioned epoch rewards are active and this Bank is the // epoch-boundary block, populate num_partitions @@ -172,7 +172,7 @@ impl Bank { == epoch_rewards_sysvar.distribution_starting_block_height)) .then_some(epoch_rewards_sysvar.num_partitions); KeyedRewardsAndNumPartitions { - keyed_rewards: rewards.clone(), + keyed_rewards, num_partitions, } } From d70ddfae4b5e1e4324b81d20069c6f125209eb1b Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Wed, 12 Jun 2024 13:17:08 -0600 Subject: [PATCH 10/15] Determine epoch boundary by checking parent epoch --- runtime/src/bank/partitioned_epoch_rewards/mod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index b6a9ca6b4d0d4a..44ff0b69a99ba9 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -167,9 +167,11 @@ impl Bank { let epoch_rewards_sysvar = self.get_epoch_rewards_sysvar(); // If partitioned epoch rewards are active and this Bank is the // epoch-boundary block, populate num_partitions - let num_partitions = (epoch_rewards_sysvar.active - && (self.block_height().saturating_add(1) - == epoch_rewards_sysvar.distribution_starting_block_height)) + let epoch_schedule = self.epoch_schedule(); + let parent_epoch = epoch_schedule.get_epoch(self.parent_slot()); + let is_first_block_in_epoch = self.epoch() > parent_epoch; + + let num_partitions = (epoch_rewards_sysvar.active && is_first_block_in_epoch) .then_some(epoch_rewards_sysvar.num_partitions); KeyedRewardsAndNumPartitions { keyed_rewards, From 848542515b0393eb7b11d5c67dba535849588e62 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Wed, 12 Jun 2024 13:23:21 -0600 Subject: [PATCH 11/15] Rename UiConfirmedBlock field --- transaction-status/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/transaction-status/src/lib.rs b/transaction-status/src/lib.rs index 9467b32c3db4e0..489a213c525a94 100644 --- a/transaction-status/src/lib.rs +++ b/transaction-status/src/lib.rs @@ -777,7 +777,7 @@ impl ConfirmedBlock { } else { None }, - num_partitions: self.num_partitions, + num_reward_partitions: self.num_partitions, block_time: self.block_time, block_height: self.block_height, }) @@ -805,7 +805,7 @@ impl From for EncodedConfirmedBlock { parent_slot: block.parent_slot, transactions: block.transactions.unwrap_or_default(), rewards: block.rewards.unwrap_or_default(), - num_partitions: block.num_partitions, + num_partitions: block.num_reward_partitions, block_time: block.block_time, block_height: block.block_height, } @@ -825,7 +825,7 @@ pub struct UiConfirmedBlock { #[serde(default, skip_serializing_if = "Option::is_none")] pub rewards: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub num_partitions: Option, + pub num_reward_partitions: Option, pub block_time: Option, pub block_height: Option, } From b06cd82417bb91e55de5f9ec0e26b964180ce0b6 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Wed, 12 Jun 2024 14:55:16 -0600 Subject: [PATCH 12/15] nit: fix comment typo --- runtime/src/bank/partitioned_epoch_rewards/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index 44ff0b69a99ba9..be2897de1365be 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -712,7 +712,7 @@ mod tests { /// Test that program execution that attempts to mutate a stake account /// incorrectly should fail during reward period. A credit should succeed, - /// but a withdrawal shoudl fail. + /// but a withdrawal should fail. #[test] fn test_program_execution_restricted_for_stake_account_in_reward_period() { use solana_sdk::transaction::TransactionError::InstructionError; From 2b7c10fe43d518acb6208d9b9b0c713cb3f21a77 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Mon, 17 Jun 2024 12:04:11 -0600 Subject: [PATCH 13/15] Add test_get_rewards_and_partitions --- .../src/bank/partitioned_epoch_rewards/mod.rs | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index be2897de1365be..87c13a73d77317 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -150,6 +150,7 @@ pub(super) struct CalculateRewardsAndDistributeVoteRewardsResult { pub(crate) type StakeRewards = Vec; +#[derive(Debug, PartialEq)] pub struct KeyedRewardsAndNumPartitions { pub keyed_rewards: Vec<(Pubkey, RewardInfo)>, pub num_partitions: Option, @@ -276,6 +277,7 @@ mod tests { account::Account, epoch_schedule::EpochSchedule, native_token::LAMPORTS_PER_SOL, + reward_type::RewardType, signature::Signer, signer::keypair::Keypair, stake::instruction::StakeError, @@ -828,4 +830,97 @@ mod tests { previous_bank = bank; } } + + #[test] + fn test_get_rewards_and_partitions() { + let starting_slot = SLOTS_PER_EPOCH - 1; + let num_rewards = 100; + let stake_account_stores_per_block = 50; + let RewardBank { bank, .. } = + create_reward_bank(num_rewards, stake_account_stores_per_block, starting_slot); + + assert!(bank.is_partitioned_rewards_feature_enabled()); + // Slot before the epoch boundary contains empty rewards (since fees are + // off), and no partitions because not at the epoch boundary + assert_eq!( + bank.get_rewards_and_num_partitions(), + KeyedRewardsAndNumPartitions { + keyed_rewards: vec![], + num_partitions: None, + } + ); + + let epoch_boundary_bank = Arc::new(Bank::new_from_parent( + bank, + &Pubkey::default(), + SLOTS_PER_EPOCH, + )); + assert!(epoch_boundary_bank.is_partitioned_rewards_feature_enabled()); + // Slot at the epoch boundary contains voting rewards only, as well as partition data + let KeyedRewardsAndNumPartitions { + keyed_rewards, + num_partitions, + } = epoch_boundary_bank.get_rewards_and_num_partitions(); + for (_pubkey, reward) in keyed_rewards.iter() { + assert_eq!(reward.reward_type, RewardType::Voting); + } + assert_eq!(keyed_rewards.len(), num_rewards); + assert_eq!( + num_partitions, + Some(num_rewards as u64 / stake_account_stores_per_block) + ); + + let mut total_staking_rewards = 0; + + let partition0_bank = Arc::new(Bank::new_from_parent( + epoch_boundary_bank, + &Pubkey::default(), + SLOTS_PER_EPOCH + 1, + )); + assert!(partition0_bank.is_partitioned_rewards_feature_enabled()); + // Slot after the epoch boundary contains first partition of staking + // rewards, and no partitions because not at the epoch boundary + let KeyedRewardsAndNumPartitions { + keyed_rewards, + num_partitions, + } = partition0_bank.get_rewards_and_num_partitions(); + for (_pubkey, reward) in keyed_rewards.iter() { + assert_eq!(reward.reward_type, RewardType::Staking); + } + total_staking_rewards += keyed_rewards.len(); + assert_eq!(num_partitions, None); + + let partition1_bank = Arc::new(Bank::new_from_parent( + partition0_bank, + &Pubkey::default(), + SLOTS_PER_EPOCH + 2, + )); + assert!(partition1_bank.is_partitioned_rewards_feature_enabled()); + // Slot 2 after the epoch boundary contains second partition of staking + // rewards, and no partitions because not at the epoch boundary + let KeyedRewardsAndNumPartitions { + keyed_rewards, + num_partitions, + } = partition1_bank.get_rewards_and_num_partitions(); + for (_pubkey, reward) in keyed_rewards.iter() { + assert_eq!(reward.reward_type, RewardType::Staking); + } + total_staking_rewards += keyed_rewards.len(); + assert_eq!(num_partitions, None); + + // All rewards are recorded + assert_eq!(total_staking_rewards, num_rewards); + + let bank = Bank::new_from_parent(partition1_bank, &Pubkey::default(), SLOTS_PER_EPOCH + 3); + assert!(bank.is_partitioned_rewards_feature_enabled()); + // Next slot contains empty rewards (since fees are off), and no + // partitions because not at the epoch boundary + assert_eq!( + bank.get_rewards_and_num_partitions(), + KeyedRewardsAndNumPartitions { + keyed_rewards: vec![], + num_partitions: None, + } + ); + } } From e5f10a4e78783c3b4f37b0cb6071f607ef94d785 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Mon, 17 Jun 2024 12:04:50 -0600 Subject: [PATCH 14/15] Add pre-activation test --- .../src/bank/partitioned_epoch_rewards/mod.rs | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index 87c13a73d77317..f8ff7048d47035 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -923,4 +923,115 @@ mod tests { } ); } + + #[test] + fn test_get_rewards_and_partitions_before_feature() { + let starting_slot = SLOTS_PER_EPOCH - 1; + let num_rewards = 100; + + let validator_keypairs = (0..num_rewards) + .map(|_| ValidatorVoteKeypairs::new_rand()) + .collect::>(); + + let GenesisConfigInfo { + mut genesis_config, .. + } = create_genesis_config_with_vote_accounts( + 1_000_000_000, + &validator_keypairs, + vec![2_000_000_000; num_rewards], + ); + genesis_config.epoch_schedule = EpochSchedule::new(SLOTS_PER_EPOCH); + + // Set feature to inactive + genesis_config + .accounts + .remove(&feature_set::enable_partitioned_epoch_reward::id()); + + let bank = Bank::new_for_tests(&genesis_config); + + for validator_vote_keypairs in &validator_keypairs { + let vote_id = validator_vote_keypairs.vote_keypair.pubkey(); + let mut vote_account = bank.get_account(&vote_id).unwrap(); + // generate some rewards + let mut vote_state = Some(vote_state::from(&vote_account).unwrap()); + for i in 0..MAX_LOCKOUT_HISTORY + 42 { + if let Some(v) = vote_state.as_mut() { + vote_state::process_slot_vote_unchecked(v, i as u64) + } + let versioned = VoteStateVersions::Current(Box::new(vote_state.take().unwrap())); + vote_state::to(&versioned, &mut vote_account).unwrap(); + match versioned { + VoteStateVersions::Current(v) => { + vote_state = Some(*v); + } + _ => panic!("Has to be of type Current"), + }; + } + bank.store_account_and_update_capitalization(&vote_id, &vote_account); + } + + let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); + let bank = new_bank_from_parent_with_bank_forks( + &bank_forks, + bank, + &Pubkey::default(), + starting_slot, + ); + + assert!(!bank.is_partitioned_rewards_feature_enabled()); + // Slot before the epoch boundary contains empty rewards (since fees are + // off), and no partitions because feature is inactive + assert_eq!( + bank.get_rewards_and_num_partitions(), + KeyedRewardsAndNumPartitions { + keyed_rewards: vec![], + num_partitions: None, + } + ); + + let epoch_boundary_bank = Arc::new(Bank::new_from_parent( + bank, + &Pubkey::default(), + SLOTS_PER_EPOCH, + )); + assert!(!epoch_boundary_bank.is_partitioned_rewards_feature_enabled()); + // Slot at the epoch boundary contains voting rewards and staking rewards; still no partitions + let KeyedRewardsAndNumPartitions { + keyed_rewards, + num_partitions, + } = epoch_boundary_bank.get_rewards_and_num_partitions(); + let mut voting_rewards_count = 0; + let mut staking_rewards_count = 0; + for (_pubkey, reward) in keyed_rewards.iter() { + match reward.reward_type { + RewardType::Voting => { + voting_rewards_count += 1; + } + RewardType::Staking => { + staking_rewards_count += 1; + } + _ => {} + } + } + assert_eq!( + keyed_rewards.len(), + voting_rewards_count + staking_rewards_count + ); + assert_eq!(voting_rewards_count, num_rewards); + assert_eq!(staking_rewards_count, num_rewards); + assert!(num_partitions.is_none()); + + let bank = + Bank::new_from_parent(epoch_boundary_bank, &Pubkey::default(), SLOTS_PER_EPOCH + 1); + assert!(!bank.is_partitioned_rewards_feature_enabled()); + // Slot after the epoch boundary contains empty rewards (since fees are + // off), and no partitions because feature is inactive + assert_eq!( + bank.get_rewards_and_num_partitions(), + KeyedRewardsAndNumPartitions { + keyed_rewards: vec![], + num_partitions: None, + } + ); + } } From f2b014aaedbc1e04d9c3526a131660afda289755 Mon Sep 17 00:00:00 2001 From: Tyera Eulberg Date: Mon, 17 Jun 2024 12:14:56 -0600 Subject: [PATCH 15/15] Add should_record unit test --- .../src/bank/partitioned_epoch_rewards/mod.rs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index f8ff7048d47035..99323c990a0c12 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -1034,4 +1034,38 @@ mod tests { } ); } + + #[test] + fn test_rewards_and_partitions_should_record() { + let reward = RewardInfo { + reward_type: RewardType::Voting, + lamports: 55, + post_balance: 5555, + commission: Some(5), + }; + + let rewards_and_partitions = KeyedRewardsAndNumPartitions { + keyed_rewards: vec![], + num_partitions: None, + }; + assert!(!rewards_and_partitions.should_record()); + + let rewards_and_partitions = KeyedRewardsAndNumPartitions { + keyed_rewards: vec![(Pubkey::new_unique(), reward)], + num_partitions: None, + }; + assert!(rewards_and_partitions.should_record()); + + let rewards_and_partitions = KeyedRewardsAndNumPartitions { + keyed_rewards: vec![], + num_partitions: Some(42), + }; + assert!(rewards_and_partitions.should_record()); + + let rewards_and_partitions = KeyedRewardsAndNumPartitions { + keyed_rewards: vec![(Pubkey::new_unique(), reward)], + num_partitions: Some(42), + }; + assert!(rewards_and_partitions.should_record()); + } }