From 4779625f231c1f08938d6333fd582cf12148b39e Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 2 Jul 2019 14:52:53 -0700 Subject: [PATCH] change vote commission to u8 (from u32) (#4887) (#4918) automerge --- core/src/rpc.rs | 4 +- multinode-demo/fullnode.sh | 2 +- programs/stake_api/src/stake_state.rs | 2 +- .../stake_tests/tests/stake_instruction.rs | 251 ++++++++++++++++++ programs/vote_api/src/vote_instruction.rs | 6 +- programs/vote_api/src/vote_state.rs | 20 +- wallet/src/wallet.rs | 6 +- 7 files changed, 271 insertions(+), 20 deletions(-) create mode 100644 programs/stake_tests/tests/stake_instruction.rs diff --git a/core/src/rpc.rs b/core/src/rpc.rs index 727854af84..ed28435852 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -205,8 +205,8 @@ pub struct RpcVoteAccountInfo { /// The current stake, in lamports, delegated to this vote account pub stake: u64, - /// A 32-bit integer used as a fraction (commission/MAX_U32) for rewards payout - pub commission: u32, + /// An 8-bit integer used as a fraction (commission/MAX_U8) for rewards payout + pub commission: u8, } #[rpc(server)] diff --git a/multinode-demo/fullnode.sh b/multinode-demo/fullnode.sh index c4afc32fb1..863969b772 100755 --- a/multinode-demo/fullnode.sh +++ b/multinode-demo/fullnode.sh @@ -94,7 +94,7 @@ setup_validator_accounts() { # Fund the vote account from the node, with the node as the identity_pubkey $solana_wallet --keypair "$identity_keypair_path" --url "http://$entrypoint_ip:8899" \ - create-vote-account "$vote_pubkey" "$identity_pubkey" 1 --commission 65535 || return $? + create-vote-account "$vote_pubkey" "$identity_pubkey" 1 --commission 255 || return $? # Fund the stake account from the node, with the node as the identity_pubkey $solana_wallet --keypair "$identity_keypair_path" --url "http://$entrypoint_ip:8899" \ diff --git a/programs/stake_api/src/stake_state.rs b/programs/stake_api/src/stake_state.rs index b2c02c1271..8b7d713878 100644 --- a/programs/stake_api/src/stake_state.rs +++ b/programs/stake_api/src/stake_state.rs @@ -707,7 +707,7 @@ mod tests { None, // would be Some((0, 2 * 1 + 1 * 2, 3)), stake.calculate_rewards(1.0, &vote_state) ); - vote_state.commission = std::u32::MAX - 1; + vote_state.commission = std::u8::MAX - 1; assert_eq!( None, // would be pSome((0, 2 * 1 + 1 * 2, 3)), stake.calculate_rewards(1.0, &vote_state) diff --git a/programs/stake_tests/tests/stake_instruction.rs b/programs/stake_tests/tests/stake_instruction.rs new file mode 100644 index 0000000000..e69bc7fa8d --- /dev/null +++ b/programs/stake_tests/tests/stake_instruction.rs @@ -0,0 +1,251 @@ +use assert_matches::assert_matches; +use solana_runtime::bank::Bank; +use solana_runtime::bank_client::BankClient; +use solana_runtime::genesis_utils::{create_genesis_block, GenesisBlockInfo}; +use solana_sdk::account_utils::State; +use solana_sdk::client::SyncClient; +use solana_sdk::message::Message; +use solana_sdk::pubkey::Pubkey; +use solana_sdk::signature::{Keypair, KeypairUtil}; +use solana_sdk::syscall; +use solana_sdk::syscall::rewards::Rewards; +use solana_stake_api::id; +use solana_stake_api::stake_instruction; +use solana_stake_api::stake_instruction::process_instruction; +use solana_stake_api::stake_state::StakeState; +use solana_vote_api::vote_instruction; +use solana_vote_api::vote_state::{Vote, VoteState}; +use std::sync::Arc; + +fn fill_epoch_with_votes( + bank: &Arc, + vote_keypair: &Keypair, + mint_keypair: &Keypair, +) -> Arc { + let mint_pubkey = mint_keypair.pubkey(); + let vote_pubkey = vote_keypair.pubkey(); + let old_epoch = bank.epoch(); + let mut bank = bank.clone(); + while bank.epoch() != old_epoch + 1 { + bank = Arc::new(Bank::new_from_parent( + &bank, + &Pubkey::default(), + 1 + bank.slot(), + )); + + let bank_client = BankClient::new_shared(&bank); + let parent = bank.parent().unwrap(); + + let message = Message::new_with_payer( + vec![vote_instruction::vote( + &vote_pubkey, + &vote_pubkey, + vec![Vote::new(parent.slot() as u64, parent.hash())], + )], + Some(&mint_pubkey), + ); + assert!(bank_client + .send_message(&[&mint_keypair, &vote_keypair], message) + .is_ok()); + } + bank +} + +#[test] +fn test_stake_account_delegate() { + let staker_keypair = Keypair::new(); + let staker_pubkey = staker_keypair.pubkey(); + let vote_keypair = Keypair::new(); + let vote_pubkey = vote_keypair.pubkey(); + let node_pubkey = Pubkey::new_rand(); + + let GenesisBlockInfo { + mut genesis_block, + mint_keypair, + .. + } = create_genesis_block(100_000_000_000); + genesis_block + .native_instruction_processors + .push(solana_stake_program::solana_stake_program!()); + let bank = Bank::new(&genesis_block); + let mint_pubkey = mint_keypair.pubkey(); + let mut bank = Arc::new(bank); + let bank_client = BankClient::new_shared(&bank); + + // Create Vote Account + let message = Message::new(vote_instruction::create_account( + &mint_pubkey, + &vote_pubkey, + &node_pubkey, + std::u8::MAX / 2, + 10, + )); + bank_client + .send_message(&[&mint_keypair], message) + .expect("failed to create vote account"); + + // Create stake account and delegate to vote account + let message = Message::new(stake_instruction::create_stake_account_and_delegate_stake( + &mint_pubkey, + &staker_pubkey, + &vote_pubkey, + 20000, + )); + bank_client + .send_message(&[&mint_keypair, &staker_keypair], message) + .expect("failed to create and delegate stake account"); + + // Test that correct lamports are staked + let account = bank.get_account(&staker_pubkey).expect("account not found"); + let stake_state = account.state().expect("couldn't unpack account data"); + if let StakeState::Stake(stake) = stake_state { + assert_eq!(stake.stake, 20000); + } else { + assert!(false, "wrong account type found") + } + + // Test that we cannot withdraw staked lamports + let message = Message::new_with_payer( + vec![stake_instruction::withdraw( + &staker_pubkey, + &Pubkey::new_rand(), + 20000, + )], + Some(&mint_pubkey), + ); + assert!(bank_client + .send_message(&[&mint_keypair, &staker_keypair], message) + .is_err()); + + // Test that lamports are still staked + let account = bank.get_account(&staker_pubkey).expect("account not found"); + let stake_state = account.state().expect("couldn't unpack account data"); + if let StakeState::Stake(stake) = stake_state { + assert_eq!(stake.stake, 20000); + } else { + assert!(false, "wrong account type found") + } + + // Reward redemption + // Submit enough votes to generate rewards + let old_epoch = bank.epoch(); + bank = fill_epoch_with_votes(&bank, &vote_keypair, &mint_keypair); + + // Test that votes and credits are there + let account = bank.get_account(&vote_pubkey).expect("account not found"); + let vote_state: VoteState = account.state().expect("couldn't unpack account data"); + + // 1 less vote, as the first vote should have cleared the lockout + assert_eq!(vote_state.votes.len(), 31); + assert_eq!(vote_state.credits(), 1); + assert_ne!(old_epoch, bank.epoch()); + + // Cycle thru banks until we reach next epoch + bank = fill_epoch_with_votes(&bank, &vote_keypair, &mint_keypair); + + // Test that rewards are there + let rewards_account = bank + .get_account(&syscall::rewards::id()) + .expect("account not found"); + assert_matches!(Rewards::from(&rewards_account), Some(_)); + + // Redeem the credit + let bank_client = BankClient::new_shared(&bank); + let message = Message::new_with_payer( + vec![stake_instruction::redeem_vote_credits( + &staker_pubkey, + &vote_pubkey, + )], + Some(&mint_pubkey), + ); + assert_matches!(bank_client.send_message(&[&mint_keypair], message), Ok(_)); + + // Test that balance increased, and calculate the rewards + let rewards; + let account = bank.get_account(&staker_pubkey).expect("account not found"); + let stake_state = account.state().expect("couldn't unpack account data"); + if let StakeState::Stake(stake) = stake_state { + assert!(account.lamports > 20000); + assert_eq!(stake.stake, 20000); + rewards = account.lamports - 20000; + } else { + rewards = 0; + assert!(false, "wrong account type found") + } + + // Deactivate the stake + let message = Message::new_with_payer( + vec![stake_instruction::deactivate_stake(&staker_pubkey)], + Some(&mint_pubkey), + ); + assert!(bank_client + .send_message(&[&mint_keypair, &staker_keypair], message) + .is_ok()); + + // Test that we cannot withdraw staked lamports due to cooldown period + let message = Message::new_with_payer( + vec![stake_instruction::withdraw( + &staker_pubkey, + &Pubkey::new_rand(), + 20000, + )], + Some(&mint_pubkey), + ); + assert!(bank_client + .send_message(&[&mint_keypair, &staker_keypair], message) + .is_err()); + + let old_epoch = bank.epoch(); + let slots = bank.get_slots_in_epoch(old_epoch); + + // Create a new bank at later epoch (within cooldown period) + let bank = Bank::new_from_parent(&bank, &Pubkey::default(), slots + bank.slot()); + assert_ne!(old_epoch, bank.epoch()); + let bank = Arc::new(bank); + let bank_client = BankClient::new_shared(&bank); + + let message = Message::new_with_payer( + vec![stake_instruction::withdraw( + &staker_pubkey, + &Pubkey::new_rand(), + 20000, + )], + Some(&mint_pubkey), + ); + assert!(bank_client + .send_message(&[&mint_keypair, &staker_keypair], message) + .is_err()); + + // Create a new bank at later epoch (to account for cooldown of stake) + let mut bank = Bank::new_from_parent( + &bank, + &Pubkey::default(), + genesis_block.slots_per_epoch * 4 + bank.slot(), + ); + bank.add_instruction_processor(id(), process_instruction); + let bank = Arc::new(bank); + let bank_client = BankClient::new_shared(&bank); + + // Test that we can withdraw now + let message = Message::new_with_payer( + vec![stake_instruction::withdraw( + &staker_pubkey, + &Pubkey::new_rand(), + 20000, + )], + Some(&mint_pubkey), + ); + assert!(bank_client + .send_message(&[&mint_keypair, &staker_keypair], message) + .is_ok()); + + // Test that balance and stake is updated correctly (we have withdrawn all lamports except rewards) + let account = bank.get_account(&staker_pubkey).expect("account not found"); + let stake_state = account.state().expect("couldn't unpack account data"); + if let StakeState::Stake(stake) = stake_state { + assert_eq!(account.lamports, rewards); + assert_eq!(stake.stake, rewards); + } else { + assert!(false, "wrong account type found") + } +} diff --git a/programs/vote_api/src/vote_instruction.rs b/programs/vote_api/src/vote_instruction.rs index b98aeebaf2..76662ebf10 100644 --- a/programs/vote_api/src/vote_instruction.rs +++ b/programs/vote_api/src/vote_instruction.rs @@ -17,7 +17,7 @@ use solana_sdk::system_instruction; pub enum VoteInstruction { /// Initialize the VoteState for this `vote account` /// takes a node_pubkey and commission - InitializeAccount(Pubkey, u32), + InitializeAccount(Pubkey, u8), /// Authorize a voter to send signed votes. AuthorizeVoter(Pubkey), @@ -26,7 +26,7 @@ pub enum VoteInstruction { Vote(Vec), } -fn initialize_account(vote_pubkey: &Pubkey, node_pubkey: &Pubkey, commission: u32) -> Instruction { +fn initialize_account(vote_pubkey: &Pubkey, node_pubkey: &Pubkey, commission: u8) -> Instruction { let account_metas = vec![AccountMeta::new(*vote_pubkey, false)]; Instruction::new( id(), @@ -39,7 +39,7 @@ pub fn create_account( from_pubkey: &Pubkey, vote_pubkey: &Pubkey, node_pubkey: &Pubkey, - commission: u32, + commission: u8, lamports: u64, ) -> Vec { let space = VoteState::size_of() as u64; diff --git a/programs/vote_api/src/vote_state.rs b/programs/vote_api/src/vote_state.rs index a47dc1b964..76333e6ebe 100644 --- a/programs/vote_api/src/vote_state.rs +++ b/programs/vote_api/src/vote_state.rs @@ -69,9 +69,9 @@ pub struct VoteState { pub votes: VecDeque, pub node_pubkey: Pubkey, pub authorized_voter_pubkey: Pubkey, - /// fraction of std::u32::MAX that represents what part of a rewards + /// fraction of std::u8::MAX that represents what part of a rewards /// payout should be given to this VoteAccount - pub commission: u32, + pub commission: u8, pub root_slot: Option, /// current epoch @@ -88,7 +88,7 @@ pub struct VoteState { } impl VoteState { - pub fn new(vote_pubkey: &Pubkey, node_pubkey: &Pubkey, commission: u32) -> Self { + pub fn new(vote_pubkey: &Pubkey, node_pubkey: &Pubkey, commission: u8) -> Self { Self { node_pubkey: *node_pubkey, authorized_voter_pubkey: *vote_pubkey, @@ -140,9 +140,9 @@ impl VoteState { pub fn commission_split(&self, on: f64) -> (f64, f64, bool) { match self.commission { 0 => (0.0, on, false), - std::u32::MAX => (on, 0.0, false), + std::u8::MAX => (on, 0.0, false), split => { - let mine = on * f64::from(split) / f64::from(std::u32::MAX); + let mine = on * f64::from(split) / f64::from(std::u8::MAX); (mine, on - mine, true) } } @@ -306,7 +306,7 @@ pub fn authorize_voter( pub fn initialize_account( vote_account: &mut KeyedAccount, node_pubkey: &Pubkey, - commission: u32, + commission: u8, ) -> Result<(), InstructionError> { let vote_state: VoteState = vote_account.state()?; @@ -351,7 +351,7 @@ pub fn process_votes( pub fn create_account( vote_pubkey: &Pubkey, node_pubkey: &Pubkey, - commission: u32, + commission: u8, lamports: u64, ) -> Account { let mut vote_account = Account::new(lamports, VoteState::size_of(), &id()); @@ -367,7 +367,7 @@ pub fn create_account( pub fn create_bootstrap_leader_account( vote_pubkey: &Pubkey, node_pubkey: &Pubkey, - commission: u32, + commission: u8, vote_lamports: u64, ) -> (Account, VoteState) { // Construct a vote account for the bootstrap_leader such that the leader_scheduler @@ -799,10 +799,10 @@ mod tests { assert_eq!(vote_state.commission_split(1.0), (0.0, 1.0, false)); - let vote_state = VoteState::new(&Pubkey::default(), &Pubkey::default(), std::u32::MAX); + let vote_state = VoteState::new(&Pubkey::default(), &Pubkey::default(), std::u8::MAX); assert_eq!(vote_state.commission_split(1.0), (1.0, 0.0, false)); - let vote_state = VoteState::new(&Pubkey::default(), &Pubkey::default(), std::u32::MAX / 2); + let vote_state = VoteState::new(&Pubkey::default(), &Pubkey::default(), std::u8::MAX / 2); let (voter_portion, staker_portion, was_split) = vote_state.commission_split(10.0); assert_eq!( diff --git a/wallet/src/wallet.rs b/wallet/src/wallet.rs index 5797208f9e..cf7cbceb59 100644 --- a/wallet/src/wallet.rs +++ b/wallet/src/wallet.rs @@ -48,7 +48,7 @@ pub enum WalletCommand { Cancel(Pubkey), Confirm(Signature), AuthorizeVoter(Pubkey, Keypair, Pubkey), - CreateVoteAccount(Pubkey, Pubkey, u32, u64), + CreateVoteAccount(Pubkey, Pubkey, u8, u64), ShowVoteAccount(Pubkey), CreateStakeAccount(Pubkey, u64), DelegateStake(Keypair, Pubkey, u64), @@ -466,7 +466,7 @@ fn process_create_vote_account( config: &WalletConfig, voting_account_pubkey: &Pubkey, node_pubkey: &Pubkey, - commission: u32, + commission: u8, lamports: u64, ) -> ProcessResult { let ixs = vote_instruction::create_account( @@ -1378,7 +1378,7 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, ' .long("commission") .value_name("NUM") .takes_value(true) - .help("The commission taken on reward redemption, default: 0"), + .help("The commission taken on reward redemption (0-255), default: 0"), ), ) .subcommand(