From 20e4edec615b38e2e1da67450cbffd1f05bc738e Mon Sep 17 00:00:00 2001 From: Sagar Dhawan Date: Thu, 28 Feb 2019 17:08:45 -0800 Subject: [PATCH] Refactor Vote Program Account setup (#2992) --- fullnode/src/main.rs | 4 +- programs/native/rewards/src/lib.rs | 34 +--- programs/native/rewards/tests/rewards.rs | 18 +- .../rewards_api/src/rewards_instruction.rs | 8 +- .../rewards_api/src/rewards_transaction.rs | 3 +- programs/native/vote/src/lib.rs | 5 +- runtime/src/accounts.rs | 16 +- runtime/src/bank.rs | 16 +- sdk/src/vote_program.rs | 159 +++++++++++++----- sdk/src/vote_transaction.rs | 33 ++-- src/broadcast_service.rs | 2 +- src/cluster_info.rs | 2 +- src/fullnode.rs | 2 +- src/leader_confirmation_service.rs | 6 +- src/leader_schedule_utils.rs | 50 +++++- src/retransmit_stage.rs | 2 +- src/staking_utils.rs | 49 +++--- src/thin_client.rs | 12 +- src/voting_keypair.rs | 8 +- 19 files changed, 278 insertions(+), 151 deletions(-) diff --git a/fullnode/src/main.rs b/fullnode/src/main.rs index d693683903..5b00e5d5e8 100644 --- a/fullnode/src/main.rs +++ b/fullnode/src/main.rs @@ -77,7 +77,7 @@ fn create_and_fund_vote_account( let last_id = client.get_last_id(); info!("create_and_fund_vote_account last_id={:?}", last_id); let transaction = - VoteTransaction::new_account(node_keypair, vote_account, last_id, 1, 1); + VoteTransaction::fund_staking_account(node_keypair, vote_account, last_id, 1, 1); match client.transfer_signed(&transaction) { Ok(signature) => { @@ -110,7 +110,7 @@ fn create_and_fund_vote_account( let vote_account_user_data = client.get_account_userdata(&vote_account); if let Ok(Some(vote_account_user_data)) = vote_account_user_data { if let Ok(vote_state) = VoteState::deserialize(&vote_account_user_data) { - if vote_state.node_id == pubkey { + if vote_state.delegate_id == pubkey { return Ok(()); } } diff --git a/programs/native/rewards/src/lib.rs b/programs/native/rewards/src/lib.rs index 5358b68d7d..a7a10df458 100644 --- a/programs/native/rewards/src/lib.rs +++ b/programs/native/rewards/src/lib.rs @@ -46,16 +46,9 @@ fn redeem_vote_credits(keyed_accounts: &mut [KeyedAccount]) -> Result<(), Progra let vote_state = VoteState::deserialize(&keyed_accounts[0].account.userdata)?; - //// TODO: This assumes the staker_id is static. If not, it should use the staker_id - //// at the time of voting, not at credit redemption. - if vote_state.staker_id != *keyed_accounts[2].unsigned_key() { - error!("account[2] was not the VOTE_PROGRAM's staking account"); - Err(ProgramError::InvalidArgument)?; - } - // TODO: This assumes the stake is static. If not, it should use the account value // at the time of voting, not at credit redemption. - let stake = keyed_accounts[2].account.tokens; + let stake = keyed_accounts[0].account.tokens; if stake == 0 { error!("staking account has no stake"); Err(ProgramError::InvalidArgument)?; @@ -65,7 +58,7 @@ fn redeem_vote_credits(keyed_accounts: &mut [KeyedAccount]) -> Result<(), Progra // Transfer rewards from the rewards pool to the staking account. keyed_accounts[1].account.tokens -= lamports; - keyed_accounts[2].account.tokens += lamports; + keyed_accounts[0].account.tokens += lamports; Ok(()) } @@ -105,27 +98,24 @@ mod tests { rewards_account: &mut Account, vote_id: &Pubkey, vote_account: &mut Account, - to_id: &Pubkey, - to_account: &mut Account, ) -> Result<(), ProgramError> { let mut keyed_accounts = [ KeyedAccount::new(vote_id, true, vote_account), KeyedAccount::new(rewards_id, false, rewards_account), - KeyedAccount::new(to_id, false, to_account), ]; redeem_vote_credits(&mut keyed_accounts) } #[test] fn test_redeem_vote_credits_via_program() { - let from_id = Keypair::new().pubkey(); - let mut from_account = Account::new(100, 0, Pubkey::default()); + let staker_id = Keypair::new().pubkey(); + let mut staker_account = Account::new(100, 0, Pubkey::default()); let vote_id = Keypair::new().pubkey(); let mut vote_account = vote_program::create_vote_account(100); - vote_program::register_and_deserialize( - &from_id, - &mut from_account, + vote_program::initialize_and_deserialize( + &staker_id, + &mut staker_account, &vote_id, &mut vote_account, ) @@ -147,21 +137,15 @@ mod tests { let rewards_id = Keypair::new().pubkey(); let mut rewards_account = create_rewards_account(100); - // TODO: Add VoteInstruction::RegisterStakerId so that we don't need to point the "to" - // account to the "from" account. - let to_id = from_id; - let mut to_account = from_account; - let to_tokens = to_account.tokens; + let tokens_before = vote_account.tokens; redeem_vote_credits_( &rewards_id, &mut rewards_account, &vote_id, &mut vote_account, - &to_id, - &mut to_account, ) .unwrap(); - assert!(to_account.tokens > to_tokens); + assert!(vote_account.tokens > tokens_before); } } diff --git a/programs/native/rewards/tests/rewards.rs b/programs/native/rewards/tests/rewards.rs index 72cf3dda24..543aaa47cb 100644 --- a/programs/native/rewards/tests/rewards.rs +++ b/programs/native/rewards/tests/rewards.rs @@ -36,7 +36,7 @@ impl<'a> RewardsBank<'a> { lamports: u64, ) -> Result<()> { let last_id = self.bank.last_id(); - let tx = VoteTransaction::new_account(from_keypair, vote_id, last_id, lamports, 0); + let tx = VoteTransaction::fund_staking_account(from_keypair, vote_id, last_id, lamports, 0); self.bank.process_transaction(&tx) } @@ -50,15 +50,9 @@ impl<'a> RewardsBank<'a> { Ok(VoteState::deserialize(&vote_account.userdata).unwrap()) } - fn redeem_credits( - &self, - rewards_id: Pubkey, - vote_keypair: &Keypair, - to_id: Pubkey, - ) -> Result { + fn redeem_credits(&self, rewards_id: Pubkey, vote_keypair: &Keypair) -> Result { let last_id = self.bank.last_id(); - let tx = - RewardsTransaction::new_redeem_credits(&vote_keypair, rewards_id, to_id, last_id, 0); + let tx = RewardsTransaction::new_redeem_credits(&vote_keypair, rewards_id, last_id, 0); self.bank.process_transaction(&tx)?; let vote_account = self.bank.get_account(&vote_keypair.pubkey()).unwrap(); Ok(VoteState::deserialize(&vote_account.userdata).unwrap()) @@ -97,13 +91,13 @@ fn test_redeem_vote_credits_via_bank() { // TODO: Add VoteInstruction::RegisterStakerId so that we don't need to point the "to" // account to the "from" account. - let to_id = from_keypair.pubkey(); - let to_tokens = bank.get_balance(&to_id); + let to_id = vote_id; + let to_tokens = bank.get_balance(&vote_id); // Periodically, the staker sumbits its vote account to the rewards pool // to exchange its credits for lamports. let vote_state = rewards_bank - .redeem_credits(rewards_id, &vote_keypair, to_id) + .redeem_credits(rewards_id, &vote_keypair) .unwrap(); assert!(bank.get_balance(&to_id) > to_tokens); assert_eq!(vote_state.credits(), 0); diff --git a/programs/native/rewards_api/src/rewards_instruction.rs b/programs/native/rewards_api/src/rewards_instruction.rs index 90ded34142..89c0648f84 100644 --- a/programs/native/rewards_api/src/rewards_instruction.rs +++ b/programs/native/rewards_api/src/rewards_instruction.rs @@ -9,15 +9,11 @@ pub enum RewardsInstruction { } impl RewardsInstruction { - pub fn new_redeem_vote_credits( - vote_id: Pubkey, - rewards_id: Pubkey, - to_id: Pubkey, - ) -> BuilderInstruction { + pub fn new_redeem_vote_credits(vote_id: Pubkey, rewards_id: Pubkey) -> BuilderInstruction { BuilderInstruction::new( rewards_program::id(), &RewardsInstruction::RedeemVoteCredits, - vec![(vote_id, true), (rewards_id, false), (to_id, false)], + vec![(vote_id, true), (rewards_id, false)], ) } } diff --git a/programs/native/rewards_api/src/rewards_transaction.rs b/programs/native/rewards_api/src/rewards_transaction.rs index 9a38b80acc..1b03918b89 100644 --- a/programs/native/rewards_api/src/rewards_transaction.rs +++ b/programs/native/rewards_api/src/rewards_transaction.rs @@ -35,14 +35,13 @@ impl RewardsTransaction { pub fn new_redeem_credits( vote_keypair: &Keypair, rewards_id: Pubkey, - to_id: Pubkey, last_id: Hash, fee: u64, ) -> Transaction { let vote_id = vote_keypair.pubkey(); TransactionBuilder::new(fee) .push(RewardsInstruction::new_redeem_vote_credits( - vote_id, rewards_id, to_id, + vote_id, rewards_id, )) .push(VoteInstruction::new_clear_credits(vote_id)) .sign(&[vote_keypair], last_id) diff --git a/programs/native/vote/src/lib.rs b/programs/native/vote/src/lib.rs index 1cfe7d3e82..b981b696b9 100644 --- a/programs/native/vote/src/lib.rs +++ b/programs/native/vote/src/lib.rs @@ -28,7 +28,10 @@ fn entrypoint( } match deserialize(data).map_err(|_| ProgramError::InvalidUserdata)? { - VoteInstruction::RegisterAccount => vote_program::register(keyed_accounts), + VoteInstruction::InitializeAccount => vote_program::initialize_account(keyed_accounts), + VoteInstruction::DelegateStake(delegate_id) => { + vote_program::delegate_stake(keyed_accounts, delegate_id) + } VoteInstruction::Vote(vote) => { debug!("{:?} by {}", vote, keyed_accounts[0].signer_key().unwrap()); solana_metrics::submit( diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index b1e89f04d7..95b842ffb9 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -246,13 +246,19 @@ impl AccountsDB { ))) } - fn get_vote_accounts(&self, fork: Fork) -> Vec { + fn get_vote_accounts(&self, fork: Fork) -> HashMap { self.index_info .vote_index .read() .unwrap() .iter() - .filter_map(|pubkey| self.load(fork, pubkey, true)) + .filter_map(|pubkey| { + if let Some(account) = self.load(fork, pubkey, true) { + Some((*pubkey, account)) + } else { + None + } + }) .collect() } @@ -886,11 +892,11 @@ impl Accounts { self.accounts_db.squash(fork); } - pub fn get_vote_accounts(&self, fork: Fork) -> Vec { + pub fn get_vote_accounts(&self, fork: Fork) -> HashMap { self.accounts_db .get_vote_accounts(fork) .into_iter() - .filter(|acc| acc.tokens != 0) + .filter(|(_, acc)| acc.tokens != 0) .collect() } } @@ -1605,7 +1611,7 @@ mod tests { create_account(&accounts_db, &mut pubkeys, 100, 6); let accounts = accounts_db.get_vote_accounts(0); assert_eq!(accounts.len(), 6); - accounts.iter().for_each(|account| { + accounts.iter().for_each(|(_, account)| { assert_eq!(account.owner, vote_program::id()); }); let lastkey = Keypair::new().pubkey(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4412069706..4de99944fe 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8,6 +8,7 @@ use crate::last_id_queue::LastIdQueue; use crate::runtime::{self, RuntimeError}; use crate::status_cache::StatusCache; use bincode::serialize; +use hashbrown::HashMap; use log::*; use solana_metrics::counter::Counter; use solana_sdk::account::Account; @@ -225,10 +226,7 @@ impl Bank { executable: false, }; - let mut vote_state = VoteState::new( - genesis_block.bootstrap_leader_id, - genesis_block.bootstrap_leader_id, - ); + let mut vote_state = VoteState::new(genesis_block.bootstrap_leader_id); vote_state .votes .push_back(vote_program::Lockout::new(&vote_program::Vote::new(0))); @@ -717,17 +715,17 @@ impl Bank { self.slots_per_epoch } - pub fn vote_states(&self, cond: F) -> Vec + pub fn vote_states(&self, cond: F) -> HashMap where - F: Fn(&VoteState) -> bool, + F: Fn(&Pubkey, &VoteState) -> bool, { self.accounts() .get_vote_accounts(self.id) .iter() - .filter_map(|account| { + .filter_map(|(p, account)| { if let Ok(vote_state) = VoteState::deserialize(&account.userdata) { - if cond(&vote_state) { - return Some(vote_state); + if cond(&p, &vote_state) { + return Some((*p, vote_state)); } } None diff --git a/sdk/src/vote_program.rs b/sdk/src/vote_program.rs index a3ece45ec4..1ea4ea25d2 100644 --- a/sdk/src/vote_program.rs +++ b/sdk/src/vote_program.rs @@ -67,12 +67,12 @@ impl Lockout { #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub enum VoteInstruction { - /// Register a new "vote account" to represent a particular validator in the Vote Contract, - /// and initialize the VoteState for this "vote account" - /// * Transaction::keys[0] - the validator id - /// * Transaction::keys[1] - the new "vote account" to be associated with the validator - /// identified by keys[0] for voting - RegisterAccount, + /// Initialize the VoteState for this `vote account` + /// * Transaction::keys[0] - the staker id that is also assumed to be the delegate by default + /// * Transaction::keys[1] - the new "vote account" to be associated with the delegate + InitializeAccount, + /// `Delegate` or `Assign` A staking account to a particular node + DelegateStake(Pubkey), Vote(Vote), /// Clear the credits in the vote account /// * Transaction::keys[0] - the "vote account" @@ -83,13 +83,29 @@ impl VoteInstruction { pub fn new_clear_credits(vote_id: Pubkey) -> BuilderInstruction { BuilderInstruction::new(id(), &VoteInstruction::ClearCredits, vec![(vote_id, true)]) } + pub fn new_delegate_stake(vote_id: Pubkey, delegate_id: Pubkey) -> BuilderInstruction { + BuilderInstruction::new( + id(), + &VoteInstruction::DelegateStake(delegate_id), + vec![(vote_id, true)], + ) + } + pub fn new_initialize_account(vote_id: Pubkey, staker_id: Pubkey) -> BuilderInstruction { + BuilderInstruction::new( + id(), + &VoteInstruction::InitializeAccount, + vec![(staker_id, true), (vote_id, false)], + ) + } + pub fn new_vote(vote_id: Pubkey, vote: Vote) -> BuilderInstruction { + BuilderInstruction::new(id(), &VoteInstruction::Vote(vote), vec![(vote_id, true)]) + } } #[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq)] pub struct VoteState { pub votes: VecDeque, - pub node_id: Pubkey, - pub staker_id: Pubkey, + pub delegate_id: Pubkey, pub root_slot: Option, credits: u64, } @@ -104,14 +120,13 @@ pub fn get_max_size() -> usize { } impl VoteState { - pub fn new(node_id: Pubkey, staker_id: Pubkey) -> Self { + pub fn new(delegate_id: Pubkey) -> Self { let votes = VecDeque::new(); let credits = 0; let root_slot = None; Self { votes, - node_id, - staker_id, + delegate_id, credits, root_slot, } @@ -193,25 +208,60 @@ impl VoteState { } } -// TODO: Deprecate the RegisterAccount instruction and its awkward delegation -// semantics. -pub fn register(keyed_accounts: &mut [KeyedAccount]) -> Result<(), ProgramError> { +pub fn delegate_stake( + keyed_accounts: &mut [KeyedAccount], + node_id: Pubkey, +) -> Result<(), ProgramError> { + if !check_id(&keyed_accounts[0].account.owner) { + error!("account[0] is not assigned to the VOTE_PROGRAM"); + Err(ProgramError::InvalidArgument)?; + } + + if keyed_accounts[0].signer_key().is_none() { + error!("account[0] should sign the transaction"); + Err(ProgramError::InvalidArgument)?; + } + + let vote_state = VoteState::deserialize(&keyed_accounts[0].account.userdata); + if let Ok(mut vote_state) = vote_state { + vote_state.delegate_id = node_id; + vote_state.serialize(&mut keyed_accounts[0].account.userdata)?; + } else { + error!("account[0] does not valid userdata"); + Err(ProgramError::InvalidUserdata)?; + } + + Ok(()) +} + +/// Initialize the vote_state for a staking account +/// Assumes that the account is being init as part of a account creation or balance transfer and +/// that the transaction must be signed by the staker's keys +pub fn initialize_account(keyed_accounts: &mut [KeyedAccount]) -> Result<(), ProgramError> { if !check_id(&keyed_accounts[1].account.owner) { error!("account[1] is not assigned to the VOTE_PROGRAM"); Err(ProgramError::InvalidArgument)?; } - // TODO: This assumes keyed_accounts[0] is the SystemInstruction::CreateAccount - // that created keyed_accounts[1]. Putting any other signed instruction in - // keyed_accounts[0] would allow the owner to highjack the vote account and - // insert itself into the leader rotation. - let from_id = keyed_accounts[0].signer_key().unwrap(); + if keyed_accounts[0].signer_key().is_none() { + error!("account[0] should sign the transaction"); + Err(ProgramError::InvalidArgument)?; + } - // Awkwardly configure the voting account to claim that the account that - // initially funded it is both the identity of the staker and the fullnode - // that should sign blocks on behalf of the staker. - let vote_state = VoteState::new(*from_id, *from_id); - vote_state.serialize(&mut keyed_accounts[1].account.userdata)?; + let staker_id = keyed_accounts[0].signer_key().unwrap(); + let vote_state = VoteState::deserialize(&keyed_accounts[1].account.userdata); + if let Ok(vote_state) = vote_state { + if vote_state.delegate_id == Pubkey::default() { + let vote_state = VoteState::new(*staker_id); + vote_state.serialize(&mut keyed_accounts[1].account.userdata)?; + } else { + error!("account[1] userdata already initialized"); + Err(ProgramError::InvalidUserdata)?; + } + } else { + error!("account[1] does not have valid userdata"); + Err(ProgramError::InvalidUserdata)?; + } Ok(()) } @@ -245,7 +295,7 @@ pub fn create_vote_account(tokens: u64) -> Account { Account::new(tokens, space, id()) } -pub fn register_and_deserialize( +pub fn initialize_and_deserialize( from_id: &Pubkey, from_account: &mut Account, vote_id: &Pubkey, @@ -255,7 +305,7 @@ pub fn register_and_deserialize( KeyedAccount::new(from_id, true, from_account), KeyedAccount::new(vote_id, false, vote_account), ]; - register(&mut keyed_accounts)?; + initialize_account(&mut keyed_accounts)?; let vote_state = VoteState::deserialize(&vote_account.userdata).unwrap(); Ok(vote_state) } @@ -276,6 +326,42 @@ mod tests { use super::*; use crate::signature::{Keypair, KeypairUtil}; + #[test] + fn test_initialize_staking_account() { + let staker_id = Keypair::new().pubkey(); + let mut staker_account = Account::new(100, 0, Pubkey::default()); + let mut bogus_staker_account = Account::new(100, 0, Pubkey::default()); + + let staking_account_id = Keypair::new().pubkey(); + let mut staking_account = create_vote_account(100); + + let bogus_account_id = Keypair::new().pubkey(); + let mut bogus_account = Account::new(100, 0, id()); + + let mut keyed_accounts = [ + KeyedAccount::new(&staker_id, false, &mut bogus_staker_account), + KeyedAccount::new(&bogus_account_id, false, &mut bogus_account), + ]; + + //staker is not signer + let res = initialize_account(&mut keyed_accounts); + assert_eq!(res, Err(ProgramError::InvalidArgument)); + + //space was never initialized, deserialize will fail + keyed_accounts[0] = KeyedAccount::new(&staker_id, true, &mut staker_account); + let res = initialize_account(&mut keyed_accounts); + assert_eq!(res, Err(ProgramError::InvalidUserdata)); + + //init should pass + keyed_accounts[1] = KeyedAccount::new(&staking_account_id, false, &mut staking_account); + let res = initialize_account(&mut keyed_accounts); + assert_eq!(res, Ok(())); + + // reinit should fail + let res = initialize_account(&mut keyed_accounts); + assert_eq!(res, Err(ProgramError::InvalidUserdata)); + } + #[test] fn test_vote_serialize() { let mut buffer: Vec = vec![0; get_max_size()]; @@ -296,9 +382,9 @@ mod tests { let mut vote_account = create_vote_account(100); let vote_state = - register_and_deserialize(&from_id, &mut from_account, &vote_id, &mut vote_account) + initialize_and_deserialize(&from_id, &mut from_account, &vote_id, &mut vote_account) .unwrap(); - assert_eq!(vote_state.node_id, from_id); + assert_eq!(vote_state.delegate_id, from_id); assert!(vote_state.votes.is_empty()); } @@ -306,10 +392,10 @@ mod tests { fn test_vote() { let from_id = Keypair::new().pubkey(); let mut from_account = Account::new(100, 0, Pubkey::default()); - let vote_id = Keypair::new().pubkey(); let mut vote_account = create_vote_account(100); - register_and_deserialize(&from_id, &mut from_account, &vote_id, &mut vote_account).unwrap(); + initialize_and_deserialize(&from_id, &mut from_account, &vote_id, &mut vote_account) + .unwrap(); let vote = Vote::new(1); let vote_state = vote_and_deserialize(&vote_id, &mut vote_account, vote.clone()).unwrap(); @@ -318,21 +404,20 @@ mod tests { } #[test] - fn test_vote_without_registration() { + fn test_vote_without_initialization() { let vote_id = Keypair::new().pubkey(); let mut vote_account = create_vote_account(100); let vote = Vote::new(1); let vote_state = vote_and_deserialize(&vote_id, &mut vote_account, vote.clone()).unwrap(); - assert_eq!(vote_state.node_id, Pubkey::default()); + assert_eq!(vote_state.delegate_id, Pubkey::default()); assert_eq!(vote_state.votes, vec![Lockout::new(&vote)]); } #[test] fn test_vote_lockout() { let voter_id = Keypair::new().pubkey(); - let staker_id = Keypair::new().pubkey(); - let mut vote_state = VoteState::new(voter_id, staker_id); + let mut vote_state = VoteState::new(voter_id); for i in 0..(MAX_LOCKOUT_HISTORY + 1) { vote_state.process_vote(Vote::new((INITIAL_LOCKOUT as usize * i) as u64)); @@ -362,8 +447,7 @@ mod tests { #[test] fn test_vote_double_lockout_after_expiration() { let voter_id = Keypair::new().pubkey(); - let staker_id = Keypair::new().pubkey(); - let mut vote_state = VoteState::new(voter_id, staker_id); + let mut vote_state = VoteState::new(voter_id); for i in 0..3 { let vote = Vote::new(i as u64); @@ -389,8 +473,7 @@ mod tests { #[test] fn test_vote_credits() { let voter_id = Keypair::new().pubkey(); - let staker_id = Keypair::new().pubkey(); - let mut vote_state = VoteState::new(voter_id, staker_id); + let mut vote_state = VoteState::new(voter_id); for i in 0..MAX_LOCKOUT_HISTORY { vote_state.process_vote(Vote::new(i as u64)); diff --git a/sdk/src/vote_transaction.rs b/sdk/src/vote_transaction.rs index 4d7606aec8..efb8ef020f 100644 --- a/sdk/src/vote_transaction.rs +++ b/sdk/src/vote_transaction.rs @@ -6,6 +6,7 @@ use crate::signature::{Keypair, KeypairUtil}; use crate::system_instruction::SystemInstruction; use crate::system_program; use crate::transaction::{Instruction, Transaction}; +use crate::transaction_builder::TransactionBuilder; use crate::vote_program::{self, Vote, VoteInstruction}; use bincode::deserialize; @@ -19,18 +20,13 @@ impl VoteTransaction { fee: u64, ) -> Transaction { let vote = Vote { slot_height }; - let instruction = VoteInstruction::Vote(vote); - Transaction::new( - voting_keypair, - &[], - vote_program::id(), - &instruction, - last_id, - fee, - ) + TransactionBuilder::new(fee) + .push(VoteInstruction::new_vote(voting_keypair.pubkey(), vote)) + .sign(&[voting_keypair], last_id) } - pub fn new_account( + /// Fund or create the staking account with tokens + pub fn fund_staking_account( from_keypair: &Keypair, vote_account_id: Pubkey, last_id: Hash, @@ -50,11 +46,26 @@ impl VoteTransaction { vec![system_program::id(), vote_program::id()], vec![ Instruction::new(0, &create_tx, vec![0, 1]), - Instruction::new(1, &VoteInstruction::RegisterAccount, vec![0, 1]), + Instruction::new(1, &VoteInstruction::InitializeAccount, vec![0, 1]), ], ) } + /// Choose a node id to `delegate` or `assign` this vote account to + pub fn delegate_vote_account( + vote_keypair: &T, + last_id: Hash, + node_id: Pubkey, + fee: u64, + ) -> Transaction { + TransactionBuilder::new(fee) + .push(VoteInstruction::new_delegate_stake( + vote_keypair.pubkey(), + node_id, + )) + .sign(&[vote_keypair], last_id) + } + fn get_vote(tx: &Transaction, ix_index: usize) -> Option<(Pubkey, Vote, Hash)> { if !vote_program::check_id(&tx.program_id(ix_index)) { return None; diff --git a/src/broadcast_service.rs b/src/broadcast_service.rs index f323c403a6..c70c225e74 100644 --- a/src/broadcast_service.rs +++ b/src/broadcast_service.rs @@ -190,7 +190,7 @@ impl BroadcastService { let mut broadcast_table = cluster_info .read() .unwrap() - .sorted_tvu_peers(&staking_utils::staked_nodes(&bank)); + .sorted_tvu_peers(&staking_utils::node_stakes(&bank)); // Layer 1, leader nodes are limited to the fanout size. broadcast_table.truncate(DATA_PLANE_FANOUT); inc_new_counter_info!("broadcast_service-num_peers", broadcast_table.len() + 1); diff --git a/src/cluster_info.rs b/src/cluster_info.rs index 79c119a01c..0e0dd4418a 100644 --- a/src/cluster_info.rs +++ b/src/cluster_info.rs @@ -878,7 +878,7 @@ impl ClusterInfo { let start = timestamp(); let stakes: HashMap<_, _> = match bank_forks { Some(ref bank_forks) => { - staking_utils::staked_nodes(&bank_forks.read().unwrap().working_bank()) + staking_utils::node_stakes(&bank_forks.read().unwrap().working_bank()) } None => HashMap::new(), }; diff --git a/src/fullnode.rs b/src/fullnode.rs index 5e63ec209b..ccdc701771 100644 --- a/src/fullnode.rs +++ b/src/fullnode.rs @@ -462,7 +462,7 @@ pub fn make_active_set_entries( let vote_account_id = voting_keypair.pubkey(); let new_vote_account_tx = - VoteTransaction::new_account(active_keypair, vote_account_id, *last_id, 1, 1); + VoteTransaction::fund_staking_account(active_keypair, vote_account_id, *last_id, 1, 1); let new_vote_account_entry = next_entry_mut(&mut last_entry_id, 1, vec![new_vote_account_tx]); // 3) Create vote entry diff --git a/src/leader_confirmation_service.rs b/src/leader_confirmation_service.rs index 743c1831e4..6ca2f90e2d 100644 --- a/src/leader_confirmation_service.rs +++ b/src/leader_confirmation_service.rs @@ -35,12 +35,12 @@ impl LeaderConfirmationService { // Hold an accounts_db read lock as briefly as possible, just long enough to collect all // the vote states - let vote_states = bank.vote_states(|vote_state| leader_id != vote_state.node_id); + let vote_states = bank.vote_states(|_, vote_state| leader_id != vote_state.delegate_id); let mut ticks_and_stakes: Vec<(u64, u64)> = vote_states .iter() - .filter_map(|vote_state| { - let validator_stake = bank.get_balance(&vote_state.node_id); + .filter_map(|(_, vote_state)| { + let validator_stake = bank.get_balance(&vote_state.delegate_id); total_stake += validator_stake; // Filter out any validators that don't have at least one vote // by returning None diff --git a/src/leader_schedule_utils.rs b/src/leader_schedule_utils.rs index 354d0f6328..f61784a7c5 100644 --- a/src/leader_schedule_utils.rs +++ b/src/leader_schedule_utils.rs @@ -5,13 +5,30 @@ use solana_sdk::pubkey::Pubkey; /// Return the leader schedule for the given epoch. fn leader_schedule(epoch_height: u64, bank: &Bank) -> LeaderSchedule { - let stakes = staking_utils::staked_nodes_at_epoch(bank, epoch_height); + let stakes = staking_utils::node_stakes_at_epoch(bank, epoch_height); let mut seed = [0u8; 32]; seed[0..8].copy_from_slice(&epoch_height.to_le_bytes()); - let stakes: Vec<_> = stakes.into_iter().collect(); + let mut stakes: Vec<_> = stakes.into_iter().collect(); + sort_stakes(&mut stakes); LeaderSchedule::new(&stakes, seed, bank.slots_per_epoch()) } +fn sort_stakes(stakes: &mut Vec<(Pubkey, u64)>) { + // Sort first by stake. If stakes are the same, sort by pubkey to ensure a + // deterministic result. + // Note: Use unstable sort, because we dedup right after to remove the equal elements. + stakes.sort_unstable_by(|(l_id, l_stake), (r_id, r_stake)| { + if r_stake == l_stake { + r_id.cmp(&l_id) + } else { + r_stake.cmp(&l_stake) + } + }); + + // Now that it's sorted, we can do an O(n) dedup. + stakes.dedup(); +} + /// Return the leader for the slot at the slot_index and epoch_height returned /// by the given function. fn slot_leader_by(bank: &Bank, get_slot_index: F) -> Pubkey @@ -92,7 +109,7 @@ mod tests { let (genesis_block, _mint_keypair) = GenesisBlock::new_with_leader(2, pubkey, 2); let bank = Bank::new(&genesis_block); - let ids_and_stakes: Vec<_> = staking_utils::staked_nodes(&bank).into_iter().collect(); + let ids_and_stakes: Vec<_> = staking_utils::node_stakes(&bank).into_iter().collect(); let seed = [0u8; 32]; let leader_schedule = LeaderSchedule::new(&ids_and_stakes, seed, genesis_block.slots_per_epoch); @@ -122,4 +139,31 @@ mod tests { assert_eq!(next_slot_leader_index(0, 0, 2), (1, 0)); assert_eq!(next_slot_leader_index(1, 0, 2), (0, 1)); } + + #[test] + fn test_sort_stakes_basic() { + let pubkey0 = Keypair::new().pubkey(); + let pubkey1 = Keypair::new().pubkey(); + let mut stakes = vec![(pubkey0, 1), (pubkey1, 2)]; + sort_stakes(&mut stakes); + assert_eq!(stakes, vec![(pubkey1, 2), (pubkey0, 1)]); + } + + #[test] + fn test_sort_stakes_with_dup() { + let pubkey0 = Keypair::new().pubkey(); + let pubkey1 = Keypair::new().pubkey(); + let mut stakes = vec![(pubkey0, 1), (pubkey1, 2), (pubkey0, 1)]; + sort_stakes(&mut stakes); + assert_eq!(stakes, vec![(pubkey1, 2), (pubkey0, 1)]); + } + + #[test] + fn test_sort_stakes_with_equal_stakes() { + let pubkey0 = Pubkey::default(); + let pubkey1 = Keypair::new().pubkey(); + let mut stakes = vec![(pubkey0, 1), (pubkey1, 1)]; + sort_stakes(&mut stakes); + assert_eq!(stakes, vec![(pubkey1, 1), (pubkey0, 1)]); + } } diff --git a/src/retransmit_stage.rs b/src/retransmit_stage.rs index 768cda83db..d0f045e572 100644 --- a/src/retransmit_stage.rs +++ b/src/retransmit_stage.rs @@ -40,7 +40,7 @@ fn retransmit( .to_owned(), ); let (neighbors, children) = compute_retransmit_peers( - &staking_utils::staked_nodes(&bank_forks.read().unwrap().working_bank()), + &staking_utils::node_stakes(&bank_forks.read().unwrap().working_bank()), cluster_info, DATA_PLANE_FANOUT, NEIGHBORHOOD_SIZE, diff --git a/src/staking_utils.rs b/src/staking_utils.rs index 004bc6b7d7..aad248748a 100644 --- a/src/staking_utils.rs +++ b/src/staking_utils.rs @@ -15,18 +15,20 @@ pub fn get_supermajority_slot(bank: &Bank, epoch_height: u64) -> Option { find_supermajority_slot(supermajority_stake, stakes_and_lockouts.values()) } -pub fn staked_nodes(bank: &Bank) -> HashMap { - staked_nodes_extractor(bank, |stake, _| stake) +/// Collect the node Pubkey and staker account balance for nodes +/// that have non-zero balance in their corresponding staking accounts +pub fn node_stakes(bank: &Bank) -> HashMap { + node_stakes_extractor(bank, |stake, _| stake) } /// Return the checkpointed stakes that should be used to generate a leader schedule. -pub fn staked_nodes_at_epoch(bank: &Bank, epoch_height: u64) -> HashMap { - staked_nodes_at_epoch_extractor(bank, epoch_height, |stake, _| stake) +pub fn node_stakes_at_epoch(bank: &Bank, epoch_height: u64) -> HashMap { + node_stakes_at_epoch_extractor(bank, epoch_height, |stake, _| stake) } /// Return the checkpointed stakes that should be used to generate a leader schedule. /// state_extractor takes (stake, vote_state) and maps to an output. -fn staked_nodes_at_epoch_extractor( +fn node_stakes_at_epoch_extractor( bank: &Bank, epoch_height: u64, state_extractor: F, @@ -35,12 +37,12 @@ where F: Fn(u64, &VoteState) -> T, { let epoch_slot_height = epoch_height * bank.slots_per_epoch(); - staked_nodes_at_slot_extractor(bank, epoch_slot_height, state_extractor) + node_stakes_at_slot_extractor(bank, epoch_slot_height, state_extractor) } /// Return the checkpointed stakes that should be used to generate a leader schedule. /// state_extractor takes (stake, vote_state) and maps to an output -fn staked_nodes_at_slot_extractor( +fn node_stakes_at_slot_extractor( bank: &Bank, current_slot_height: u64, state_extractor: F, @@ -59,25 +61,23 @@ where .find(|bank| bank.id() <= slot_height) .unwrap_or_else(|| banks.last().unwrap()); - staked_nodes_extractor(bank, state_extractor) + node_stakes_extractor(bank, state_extractor) } /// Collect the node Pubkey and staker account balance for nodes /// that have non-zero balance in their corresponding staker accounts. /// state_extractor takes (stake, vote_state) and maps to an output -fn staked_nodes_extractor(bank: &Bank, state_extractor: F) -> HashMap +fn node_stakes_extractor(bank: &Bank, state_extractor: F) -> HashMap where F: Fn(u64, &VoteState) -> T, { - bank.vote_states(|_| true) + bank.vote_states(|account_id, _| bank.get_balance(&account_id) > 0) .iter() - .filter_map(|state| { - let balance = bank.get_balance(&state.staker_id); - if balance > 0 { - Some((state.node_id, state_extractor(balance, &state))) - } else { - None - } + .map(|(account_id, state)| { + ( + state.delegate_id, + state_extractor(bank.get_balance(&account_id), &state), + ) }) .collect() } @@ -86,7 +86,7 @@ fn epoch_stakes_and_lockouts( bank: &Bank, epoch_height: u64, ) -> HashMap)> { - staked_nodes_at_epoch_extractor(bank, epoch_height, |stake, state| (stake, state.root_slot)) + node_stakes_at_epoch_extractor(bank, epoch_height, |stake, state| (stake, state.root_slot)) } fn find_supermajority_slot<'a, I>(supermajority_stake: u64, stakes_and_lockouts: I) -> Option @@ -146,7 +146,7 @@ mod tests { expected.insert(pubkey, bootstrap_tokens - 1); let bank = Bank::new_from_parent(&Arc::new(bank)); assert_eq!( - staked_nodes_at_slot_extractor(&bank, bank.slot_height(), |s, _| s), + node_stakes_at_slot_extractor(&bank, bank.slot_height(), |s, _| s), expected ); } @@ -154,13 +154,12 @@ mod tests { #[test] fn test_epoch_stakes_and_lockouts() { let validator = Keypair::new(); - let voter = Keypair::new(); let (genesis_block, mint_keypair) = GenesisBlock::new(500); let bank = Bank::new(&genesis_block); let bank_voter = Keypair::new(); - // Give the validator some stake + // Give the validator some stake but don't setup a staking account bank.transfer( 1, &mint_keypair, @@ -169,9 +168,7 @@ mod tests { ) .unwrap(); - voting_keypair_tests::new_vote_account_with_vote(&validator, &voter, &bank, 1, 0); - assert_eq!(bank.get_balance(&validator.pubkey()), 0); - // Validator has zero balance, so they get filtered out. Only the bootstrap leader + // Validator has no token staked, so they get filtered out. Only the bootstrap leader // created by the genesis block will get included let expected: Vec<_> = epoch_stakes_and_lockouts(&bank, 0) .values() @@ -179,11 +176,11 @@ mod tests { .collect(); assert_eq!(expected, vec![(1, None)]); - voting_keypair_tests::new_vote_account_with_vote(&mint_keypair, &bank_voter, &bank, 1, 0); + voting_keypair_tests::new_vote_account_with_vote(&mint_keypair, &bank_voter, &bank, 499, 0); let result: HashSet<_> = HashSet::from_iter(epoch_stakes_and_lockouts(&bank, 0).values().cloned()); - let expected: HashSet<_> = HashSet::from_iter(vec![(1, None), (498, None)]); + let expected: HashSet<_> = HashSet::from_iter(vec![(1, None), (499, None)]); assert_eq!(result, expected); } diff --git a/src/thin_client.rs b/src/thin_client.rs index 8b4e1af818..7d30a8713e 100644 --- a/src/thin_client.rs +++ b/src/thin_client.rs @@ -590,8 +590,13 @@ mod tests { let vote_account_id = validator_vote_account_keypair.pubkey(); let last_id = client.get_last_id(); - let transaction = - VoteTransaction::new_account(&validator_keypair, vote_account_id, last_id, 1, 1); + let transaction = VoteTransaction::fund_staking_account( + &validator_keypair, + vote_account_id, + last_id, + 1, + 1, + ); let signature = client.transfer_signed(&transaction).unwrap(); client.poll_for_signature(&signature).unwrap(); @@ -608,7 +613,8 @@ mod tests { let vote_state = VoteState::deserialize(&account_user_data); - if vote_state.map(|vote_state| vote_state.node_id) == Ok(validator_keypair.pubkey()) { + if vote_state.map(|vote_state| vote_state.delegate_id) == Ok(validator_keypair.pubkey()) + { break; } diff --git a/src/voting_keypair.rs b/src/voting_keypair.rs index 6d06ddbd9a..c839b7f5f3 100644 --- a/src/voting_keypair.rs +++ b/src/voting_keypair.rs @@ -110,7 +110,13 @@ pub mod tests { num_tokens: u64, ) { let last_id = bank.last_id(); - let tx = VoteTransaction::new_account(from_keypair, *voting_pubkey, last_id, num_tokens, 0); + let tx = VoteTransaction::fund_staking_account( + from_keypair, + *voting_pubkey, + last_id, + num_tokens, + 0, + ); bank.process_transaction(&tx).unwrap(); }