From 96a61cc4e47f7cecdd3cb023fa0830186277a8a1 Mon Sep 17 00:00:00 2001 From: Sunny Gleason Date: Fri, 13 Mar 2020 16:30:04 -0400 Subject: [PATCH] Cli: add subcommand to withdraw from vote account (#8550) * feat: cli command for vote account withdraw * Rework names * Update to flexible signer, and make consistent with other cli apis * Add integration test * Clean up default help msg Co-authored-by: Michael Vines Co-authored-by: Tyera Eulberg --- cli/src/cli.rs | 24 ++++++- cli/src/nonce.rs | 31 +-------- cli/src/stake.rs | 12 ++-- cli/src/vote.rs | 168 ++++++++++++++++++++++++++++++++++++++++++++-- cli/tests/vote.rs | 109 ++++++++++++++++++++++++++++++ 5 files changed, 305 insertions(+), 39 deletions(-) create mode 100644 cli/tests/vote.rs diff --git a/cli/src/cli.rs b/cli/src/cli.rs index 8358d4e9a..4d2662cae 100644 --- a/cli/src/cli.rs +++ b/cli/src/cli.rs @@ -364,6 +364,12 @@ pub enum CliCommand { use_lamports_unit: bool, commitment_config: CommitmentConfig, }, + WithdrawFromVoteAccount { + vote_account_pubkey: Pubkey, + destination_account_pubkey: Pubkey, + withdraw_authority: SignerIndex, + lamports: u64, + }, VoteAuthorize { vote_account_pubkey: Pubkey, new_authorized_pubkey: Pubkey, @@ -696,6 +702,9 @@ pub fn parse_command( VoteAuthorize::Withdrawer, ), ("vote-account", Some(matches)) => parse_vote_get_account_command(matches), + ("withdraw-from-vote-account", Some(matches)) => { + parse_withdraw_from_vote_account(matches, default_signer_path, wallet_manager) + } // Wallet Commands ("address", Some(matches)) => Ok(CliCommandInfo { command: CliCommand::Address, @@ -1924,6 +1933,19 @@ pub fn process_command(config: &CliConfig) -> ProcessResult { *use_lamports_unit, *commitment_config, ), + CliCommand::WithdrawFromVoteAccount { + vote_account_pubkey, + withdraw_authority, + lamports, + destination_account_pubkey, + } => process_withdraw_from_vote_account( + &rpc_client, + config, + vote_account_pubkey, + *withdraw_authority, + *lamports, + destination_account_pubkey, + ), CliCommand::VoteAuthorize { vote_account_pubkey, new_authorized_pubkey, @@ -2288,7 +2310,7 @@ pub fn app<'ab, 'v>(name: &str, about: &'ab str, version: &'v str) -> App<'ab, ' .takes_value(true) .required(false) .validator(is_valid_signer) - .help("From (base) key, defaults to client keypair."), + .help("From (base) key, [default: cli config keypair]"), ), ) .subcommand( diff --git a/cli/src/nonce.rs b/cli/src/nonce.rs index f6e744e48..b12399bcc 100644 --- a/cli/src/nonce.rs +++ b/cli/src/nonce.rs @@ -199,7 +199,7 @@ impl NonceSubCommands for App<'_, '_> { ) .subcommand( SubCommand::with_name("withdraw-from-nonce-account") - .about("Withdraw lamports from the nonce account") + .about("Withdraw SOL from the nonce account") .arg( Arg::with_name("nonce_account_keypair") .index(1) @@ -207,7 +207,7 @@ impl NonceSubCommands for App<'_, '_> { .takes_value(true) .required(true) .validator(is_keypair_or_ask_keyword) - .help("Nonce account from to withdraw from"), + .help("Nonce account to withdraw from"), ) .arg( Arg::with_name("destination_account_pubkey") @@ -216,7 +216,7 @@ impl NonceSubCommands for App<'_, '_> { .takes_value(true) .required(true) .validator(is_pubkey_or_keypair) - .help("The account to which the lamports should be transferred"), + .help("The account to which the SOL should be transferred"), ) .arg( Arg::with_name("amount") @@ -869,31 +869,6 @@ mod tests { } ); - let test_withdraw_from_nonce_account = test_commands.clone().get_matches_from(vec![ - "test", - "withdraw-from-nonce-account", - &keypair_file, - &nonce_account_string, - "42", - ]); - assert_eq!( - parse_command( - &test_withdraw_from_nonce_account, - &default_keypair_file, - None - ) - .unwrap(), - CliCommandInfo { - command: CliCommand::WithdrawFromNonceAccount { - nonce_account: read_keypair_file(&keypair_file).unwrap().pubkey(), - nonce_authority: 0, - destination_account_pubkey: nonce_account_pubkey, - lamports: 42000000000 - }, - signers: vec![read_keypair_file(&default_keypair_file).unwrap().into()], - } - ); - // Test WithdrawFromNonceAccount Subcommand with authority let test_withdraw_from_nonce_account = test_commands.clone().get_matches_from(vec![ "test", diff --git a/cli/src/stake.rs b/cli/src/stake.rs index 90242eb0d..73c4ee4be 100644 --- a/cli/src/stake.rs +++ b/cli/src/stake.rs @@ -34,13 +34,13 @@ use std::{ops::Deref, sync::Arc}; pub const STAKE_AUTHORITY_ARG: ArgConstant<'static> = ArgConstant { name: "stake_authority", long: "stake-authority", - help: "Public key of authorized staker (defaults to cli config pubkey)", + help: "Authorized staker [default: cli config keypair]", }; pub const WITHDRAW_AUTHORITY_ARG: ArgConstant<'static> = ArgConstant { name: "withdraw_authority", long: "withdraw-authority", - help: "Public key of authorized withdrawer (defaults to cli config pubkey)", + help: "Authorized withdrawer [default: cli config keypair]", }; fn stake_authority_arg<'a, 'b>() -> Arg<'a, 'b> { @@ -279,7 +279,7 @@ impl StakeSubCommands for App<'_, '_> { .takes_value(true) .validator(is_amount) .required(true) - .help("The amount to move into the new stake account, in unit SOL") + .help("The amount to move into the new stake account, in SOL") ) .arg( Arg::with_name("seed") @@ -296,7 +296,7 @@ impl StakeSubCommands for App<'_, '_> { ) .subcommand( SubCommand::with_name("withdraw-stake") - .about("Withdraw the unstaked lamports from the stake account") + .about("Withdraw the unstaked SOL from the stake account") .arg( Arg::with_name("stake_account_pubkey") .index(1) @@ -313,7 +313,7 @@ impl StakeSubCommands for App<'_, '_> { .takes_value(true) .required(true) .validator(is_pubkey_or_keypair) - .help("The account to which the lamports should be transferred") + .help("The account to which the SOL should be transferred") ) .arg( Arg::with_name("amount") @@ -374,7 +374,7 @@ impl StakeSubCommands for App<'_, '_> { .takes_value(true) .value_name("KEYPAIR or PUBKEY or REMOTE WALLET PATH") .validator(is_valid_signer) - .help("Public key of signing custodian (defaults to cli config pubkey)") + .help("Public key of signing custodian [default: cli config pubkey]") ) .offline_args() .arg(nonce_arg()) diff --git a/cli/src/vote.rs b/cli/src/vote.rs index 90eaf7b2c..5300aacfe 100644 --- a/cli/src/vote.rs +++ b/cli/src/vote.rs @@ -1,7 +1,7 @@ use crate::cli::{ build_balance_message, check_account_for_fee, check_unique_pubkeys, generate_unique_signers, log_instruction_custom_error, CliCommand, CliCommandInfo, CliConfig, CliError, CliSignerInfo, - ProcessResult, + ProcessResult, SignerIndex, }; use clap::{value_t_or_exit, App, Arg, ArgMatches, SubCommand}; use solana_clap_utils::{input_parsers::*, input_validators::*}; @@ -16,7 +16,7 @@ use solana_sdk::{ transaction::Transaction, }; use solana_vote_program::{ - vote_instruction::{self, VoteError}, + vote_instruction::{self, withdraw, VoteError}, vote_state::{VoteAuthorize, VoteInit, VoteState}, }; use std::sync::Arc; @@ -62,7 +62,7 @@ impl VoteSubCommands for App<'_, '_> { .value_name("PUBKEY") .takes_value(true) .validator(is_pubkey_or_keypair) - .help("Public key of the authorized voter (defaults to vote account)"), + .help("Public key of the authorized voter [default: vote account]"), ) .arg( Arg::with_name("authorized_withdrawer") @@ -70,7 +70,7 @@ impl VoteSubCommands for App<'_, '_> { .value_name("PUBKEY") .takes_value(true) .validator(is_pubkey_or_keypair) - .help("Public key of the authorized withdrawer (defaults to cli config pubkey)"), + .help("Public key of the authorized withdrawer [default: cli config pubkey]"), ) .arg( Arg::with_name("seed") @@ -183,6 +183,45 @@ impl VoteSubCommands for App<'_, '_> { .help("Display balance in lamports instead of SOL"), ), ) + .subcommand( + SubCommand::with_name("withdraw-from-vote-account") + .about("Withdraw lamports from a vote account into a specified account") + .arg( + Arg::with_name("vote_account_pubkey") + .index(1) + .value_name("VOTE ACCOUNT PUBKEY") + .takes_value(true) + .required(true) + .validator(is_pubkey_or_keypair) + .help("Vote account from which to withdraw"), + ) + .arg( + Arg::with_name("destination_account_pubkey") + .index(2) + .value_name("DESTINATION ACCOUNT") + .takes_value(true) + .required(true) + .validator(is_pubkey_or_keypair) + .help("The account to which the SOL should be transferred"), + ) + .arg( + Arg::with_name("amount") + .index(3) + .value_name("AMOUNT") + .takes_value(true) + .required(true) + .validator(is_amount) + .help("The amount to withdraw, in SOL"), + ) + .arg( + Arg::with_name("authorized_withdrawer") + .long("authorized-withdrawer") + .value_name("KEYPAIR or PUBKEY or REMOTE WALLET PATH") + .takes_value(true) + .validator(is_valid_signer) + .help("Authorized withdrawer [default: cli config keypair]"), + ) + ) } } @@ -291,6 +330,36 @@ pub fn parse_vote_get_account_command( }) } +pub fn parse_withdraw_from_vote_account( + matches: &ArgMatches<'_>, + default_signer_path: &str, + wallet_manager: Option<&Arc>, +) -> Result { + let vote_account_pubkey = pubkey_of(matches, "vote_account_pubkey").unwrap(); + let destination_account_pubkey = pubkey_of(matches, "destination_account_pubkey").unwrap(); + let lamports = lamports_of_sol(matches, "amount").unwrap(); + let (withdraw_authority, withdraw_authority_pubkey) = + signer_of(matches, "authorized_withdrawer", wallet_manager)?; + + let payer_provided = None; + let signer_info = generate_unique_signers( + vec![payer_provided, withdraw_authority], + matches, + default_signer_path, + wallet_manager, + )?; + + Ok(CliCommandInfo { + command: CliCommand::WithdrawFromVoteAccount { + vote_account_pubkey, + destination_account_pubkey, + withdraw_authority: signer_info.index_of(withdraw_authority_pubkey).unwrap(), + lamports, + }, + signers: signer_info.signers, + }) +} + pub fn process_create_vote_account( rpc_client: &RpcClient, config: &CliConfig, @@ -517,6 +586,37 @@ pub fn process_show_vote_account( Ok("".to_string()) } +pub fn process_withdraw_from_vote_account( + rpc_client: &RpcClient, + config: &CliConfig, + vote_account_pubkey: &Pubkey, + withdraw_authority: SignerIndex, + lamports: u64, + destination_account_pubkey: &Pubkey, +) -> ProcessResult { + let (recent_blockhash, fee_calculator) = rpc_client.get_recent_blockhash()?; + let withdraw_authority = config.signers[withdraw_authority]; + + let ix = withdraw( + vote_account_pubkey, + &withdraw_authority.pubkey(), + lamports, + destination_account_pubkey, + ); + + let message = Message::new_with_payer(&[ix], Some(&config.signers[0].pubkey())); + let mut transaction = Transaction::new_unsigned(message); + transaction.try_sign(&config.signers, recent_blockhash)?; + check_account_for_fee( + rpc_client, + &config.signers[0].pubkey(), + &fee_calculator, + &transaction.message, + )?; + let result = rpc_client.send_and_confirm_transaction(&mut transaction, &config.signers); + log_instruction_custom_error::(result) +} + #[cfg(test)] mod tests { use super::*; @@ -699,5 +799,65 @@ mod tests { ], } ); + + // Test WithdrawFromVoteAccount subcommand + let test_withdraw_from_vote_account = test_commands.clone().get_matches_from(vec![ + "test", + "withdraw-from-vote-account", + &keypair_file, + &pubkey_string, + "42", + ]); + assert_eq!( + parse_command( + &test_withdraw_from_vote_account, + &default_keypair_file, + None + ) + .unwrap(), + CliCommandInfo { + command: CliCommand::WithdrawFromVoteAccount { + vote_account_pubkey: read_keypair_file(&keypair_file).unwrap().pubkey(), + destination_account_pubkey: pubkey, + withdraw_authority: 0, + lamports: 42_000_000_000 + }, + signers: vec![read_keypair_file(&default_keypair_file).unwrap().into()], + } + ); + + // Test WithdrawFromVoteAccount subcommand with authority + let withdraw_authority = Keypair::new(); + let (withdraw_authority_file, mut tmp_file) = make_tmp_file(); + write_keypair(&withdraw_authority, tmp_file.as_file_mut()).unwrap(); + let test_withdraw_from_vote_account = test_commands.clone().get_matches_from(vec![ + "test", + "withdraw-from-vote-account", + &keypair_file, + &pubkey_string, + "42", + "--authorized-withdrawer", + &withdraw_authority_file, + ]); + assert_eq!( + parse_command( + &test_withdraw_from_vote_account, + &default_keypair_file, + None + ) + .unwrap(), + CliCommandInfo { + command: CliCommand::WithdrawFromVoteAccount { + vote_account_pubkey: read_keypair_file(&keypair_file).unwrap().pubkey(), + destination_account_pubkey: pubkey, + withdraw_authority: 1, + lamports: 42_000_000_000 + }, + signers: vec![ + read_keypair_file(&default_keypair_file).unwrap().into(), + read_keypair_file(&withdraw_authority_file).unwrap().into() + ], + } + ); } } diff --git a/cli/tests/vote.rs b/cli/tests/vote.rs new file mode 100644 index 000000000..cbe27c70d --- /dev/null +++ b/cli/tests/vote.rs @@ -0,0 +1,109 @@ +use solana_cli::cli::{process_command, request_and_confirm_airdrop, CliCommand, CliConfig}; +use solana_client::rpc_client::RpcClient; +use solana_core::validator::TestValidator; +use solana_faucet::faucet::run_local_faucet; +use solana_sdk::{ + account_utils::StateMut, + pubkey::Pubkey, + signature::{Keypair, Signer}, +}; +use solana_vote_program::vote_state::{VoteAuthorize, VoteState, VoteStateVersions}; +use std::{fs::remove_dir_all, sync::mpsc::channel, thread::sleep, time::Duration}; + +fn check_balance(expected_balance: u64, client: &RpcClient, pubkey: &Pubkey) { + (0..5).for_each(|tries| { + let balance = client.retry_get_balance(pubkey, 1).unwrap().unwrap(); + if balance == expected_balance { + return; + } + if tries == 4 { + assert_eq!(balance, expected_balance); + } + sleep(Duration::from_millis(500)); + }); +} + +#[test] +fn test_vote_authorize_and_withdraw() { + let TestValidator { + server, + leader_data, + alice, + ledger_path, + .. + } = TestValidator::run(); + let (sender, receiver) = channel(); + run_local_faucet(alice, sender, None); + let faucet_addr = receiver.recv().unwrap(); + + let rpc_client = RpcClient::new_socket(leader_data.rpc); + let default_signer = Keypair::new(); + + let mut config = CliConfig::default(); + config.json_rpc_url = format!("http://{}:{}", leader_data.rpc.ip(), leader_data.rpc.port()); + config.signers = vec![&default_signer]; + + request_and_confirm_airdrop( + &rpc_client, + &faucet_addr, + &config.signers[0].pubkey(), + 100_000, + ) + .unwrap(); + + // Create vote account + let vote_account_keypair = Keypair::new(); + let vote_account_pubkey = vote_account_keypair.pubkey(); + config.signers = vec![&default_signer, &vote_account_keypair]; + config.command = CliCommand::CreateVoteAccount { + seed: None, + node_pubkey: config.signers[0].pubkey(), + authorized_voter: None, + authorized_withdrawer: Some(config.signers[0].pubkey()), + commission: 0, + }; + process_command(&config).unwrap(); + let vote_account = rpc_client + .get_account(&vote_account_keypair.pubkey()) + .unwrap(); + let vote_state: VoteStateVersions = vote_account.state().unwrap(); + let authorized_withdrawer = vote_state.convert_to_current().authorized_withdrawer; + assert_eq!(authorized_withdrawer, config.signers[0].pubkey()); + let expected_balance = rpc_client + .get_minimum_balance_for_rent_exemption(VoteState::size_of()) + .unwrap() + .max(1); + check_balance(expected_balance, &rpc_client, &vote_account_pubkey); + + // Authorize vote account withdrawal to another signer + let withdraw_authority = Keypair::new(); + config.signers = vec![&default_signer]; + config.command = CliCommand::VoteAuthorize { + vote_account_pubkey, + new_authorized_pubkey: withdraw_authority.pubkey(), + vote_authorize: VoteAuthorize::Withdrawer, + }; + process_command(&config).unwrap(); + let vote_account = rpc_client + .get_account(&vote_account_keypair.pubkey()) + .unwrap(); + let vote_state: VoteStateVersions = vote_account.state().unwrap(); + let authorized_withdrawer = vote_state.convert_to_current().authorized_withdrawer; + assert_eq!(authorized_withdrawer, withdraw_authority.pubkey()); + + // Withdraw from vote account + let destination_account = Pubkey::new_rand(); // Send withdrawal to new account to make balance check easy + config.signers = vec![&default_signer, &withdraw_authority]; + config.command = CliCommand::WithdrawFromVoteAccount { + vote_account_pubkey, + withdraw_authority: 1, + lamports: 100, + destination_account_pubkey: destination_account, + }; + process_command(&config).unwrap(); + check_balance(expected_balance - 100, &rpc_client, &vote_account_pubkey); + check_balance(100, &rpc_client, &destination_account); + + server.close().unwrap(); + remove_dir_all(ledger_path).unwrap(); +}