Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cli] Add ability to authorize Voter or Withdrawer authorities on vote accounts using derived key #25948

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,8 @@ pub enum CliCommand {
fee_payer: SignerIndex,
authorized: SignerIndex,
new_authorized: Option<SignerIndex>,
derived_address_seed: Option<String>, // Seed of current authority's derived key
derived_address_program_id: Option<Pubkey>, // Owner of current authority's derived key
},
VoteUpdateValidator {
vote_account_pubkey: Pubkey,
Expand Down Expand Up @@ -1418,6 +1420,8 @@ pub fn process_command(config: &CliConfig) -> ProcessResult {
fee_payer,
authorized,
new_authorized,
derived_address_seed,
derived_address_program_id,
} => process_vote_authorize(
&rpc_client,
config,
Expand All @@ -1433,6 +1437,8 @@ pub fn process_command(config: &CliConfig) -> ProcessResult {
*nonce_authority,
memo.as_ref(),
*fee_payer,
derived_address_seed.as_ref(),
*derived_address_program_id,
),
CliCommand::VoteUpdateValidator {
vote_account_pubkey,
Expand Down Expand Up @@ -2032,6 +2038,8 @@ mod tests {
fee_payer: 0,
authorized: 0,
new_authorized: None,
derived_address_seed: None,
derived_address_program_id: None,
};
let result = process_command(&vote_config);
assert!(result.is_ok());
Expand Down Expand Up @@ -2249,6 +2257,8 @@ mod tests {
fee_payer: 0,
authorized: 0,
new_authorized: None,
derived_address_seed: None,
derived_address_program_id: None,
};
assert!(process_command(&config).is_err());

Expand Down
91 changes: 88 additions & 3 deletions cli/src/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl VoteSubCommands for App<'_, '_> {
.value_name("AUTHORIZED_KEYPAIR")
.required(true)
.validator(is_valid_signer)
.help("Current authorized vote signer."),
.help("Current authorized vote signer or withdrawer."),
)
.arg(
pubkey!(Arg::with_name("new_authorized_pubkey")
Expand All @@ -137,6 +137,21 @@ impl VoteSubCommands for App<'_, '_> {
.required(true),
"New authorized vote signer. "),
)
.arg(
pubkey!(Arg::with_name("authorized_owner")
.value_name("AUTHORIZED_OWNER")
.required(false)
.requires("authorized_seed"),
"Owner of current authorized vote signer or withdrawer's derived key. "),
)
.arg(
Arg::with_name("authorized_seed")
.value_name("AUTHORIZED_SEED")
.required(false)
.requires("authorized_owner")
.validator(is_derived_address_seed)
.help("Seed of current authorized vote signer or withdrawer's derived key."),
)
.offline_args()
.nonce_args(false)
.arg(fee_payer_arg())
Expand Down Expand Up @@ -167,6 +182,21 @@ impl VoteSubCommands for App<'_, '_> {
.required(true),
"New authorized withdrawer. "),
)
.arg(
pubkey!(Arg::with_name("authorized_owner")
.value_name("AUTHORIZED_OWNER")
.required(false)
.requires("authorized_seed"),
"Owner of current authorized withdrawer's derived key. "),
)
.arg(
Arg::with_name("authorized_seed")
.value_name("AUTHORIZED_SEED")
.required(false)
.requires("authorized_owner")
.validator(is_derived_address_seed)
.help("Seed of current authorized withdrawer's derived key."),
)
.offline_args()
.nonce_args(false)
.arg(fee_payer_arg())
Expand Down Expand Up @@ -511,6 +541,19 @@ pub fn parse_vote_authorize(
let signer_info =
default_signer.generate_unique_signers(bulk_signers, matches, wallet_manager)?;

let authorized_owner = pubkey_of(matches, "authorized_owner");
let authorized_seed = matches
.value_of("authorized_seed")
.map(|seed| seed.to_string());
if authorized_owner.is_some() != authorized_seed.is_some() {
// This should never happen, because the CLI config is already
// supposed to enforce that either both or neither of these exist.
return Err(CliError::BadParameter(
"Supply either both an `--authorized_seed` and an `--authorized_owner`, or neither"
.to_string(),
));
}

Ok(CliCommandInfo {
command: CliCommand::VoteAuthorize {
vote_account_pubkey,
Expand All @@ -529,6 +572,8 @@ pub fn parse_vote_authorize(
} else {
None
},
derived_address_seed: authorized_seed,
derived_address_program_id: authorized_owner,
},
signers: signer_info.signers,
})
Expand Down Expand Up @@ -889,8 +934,17 @@ pub fn process_vote_authorize(
nonce_authority: SignerIndex,
memo: Option<&String>,
fee_payer: SignerIndex,
derived_address_seed: Option<&String>,
derived_address_program_id: Option<Pubkey>,
) -> ProcessResult {
let authorized = config.signers[authorized];
let provided_current_authority = match (derived_address_seed, derived_address_program_id) {
(Some(seed), Some(owner)) => {
Pubkey::create_with_seed(&authorized.pubkey(), seed.as_str(), &owner)
.unwrap_or_else(|_| authorized.pubkey())
}
_ => authorized.pubkey(),
};
let new_authorized_signer = new_authorized.map(|index| config.signers[index]);

let vote_state = if !sign_only {
Expand All @@ -912,7 +966,7 @@ pub fn process_vote_authorize(
})?;
check_current_authority(
&[current_authorized_voter, vote_state.authorized_withdrawer],
&authorized.pubkey(),
&provided_current_authority,
)?;
if let Some(signer) = new_authorized_signer {
if signer.is_interactive() {
Expand All @@ -930,18 +984,37 @@ pub fn process_vote_authorize(
(new_authorized_pubkey, "new_authorized_pubkey".to_string()),
)?;
if let Some(vote_state) = vote_state {
check_current_authority(&[vote_state.authorized_withdrawer], &authorized.pubkey())?
check_current_authority(
&[vote_state.authorized_withdrawer],
&provided_current_authority,
)?
}
}
}

let vote_ix = if new_authorized_signer.is_some() {
if derived_address_seed.is_some() && derived_address_program_id.is_some() {
return Err(
CliError::BadParameter(
String::from("It is not possible to authorize a new authority in checked mode when you supply a seed. Derived keys can't sign transactions."),
).into(),
);
}
vote_instruction::authorize_checked(
vote_account_pubkey, // vote account to update
&authorized.pubkey(), // current authorized
new_authorized_pubkey, // new vote signer/withdrawer
vote_authorize, // vote or withdraw
)
} else if derived_address_seed.is_some() && derived_address_program_id.is_some() {
vote_instruction::authorize_with_seed(
vote_account_pubkey, // vote account to update
&authorized.pubkey(), // Base key of current authority's derived key
&derived_address_program_id.unwrap(), // Owner of current authority's derived key
derived_address_seed.unwrap().as_str(), // Seed of current authority's derived key
new_authorized_pubkey, // new vote signer/withdrawer
vote_authorize, // vote or withdraw
)
} else {
vote_instruction::authorize(
vote_account_pubkey, // vote account to update
Expand Down Expand Up @@ -1450,6 +1523,8 @@ mod tests {
fee_payer: 0,
authorized: 0,
new_authorized: None,
derived_address_seed: None,
derived_address_program_id: None,
},
signers: vec![read_keypair_file(&default_keypair_file).unwrap().into()],
}
Expand Down Expand Up @@ -1482,6 +1557,8 @@ mod tests {
fee_payer: 0,
authorized: 1,
new_authorized: None,
derived_address_seed: None,
derived_address_program_id: None,
},
signers: vec![
read_keypair_file(&default_keypair_file).unwrap().into(),
Expand Down Expand Up @@ -1516,6 +1593,8 @@ mod tests {
fee_payer: 0,
authorized: 1,
new_authorized: None,
derived_address_seed: None,
derived_address_program_id: None,
},
signers: vec![
read_keypair_file(&default_keypair_file).unwrap().into(),
Expand Down Expand Up @@ -1564,6 +1643,8 @@ mod tests {
fee_payer: 0,
authorized: 1,
new_authorized: None,
derived_address_seed: None,
derived_address_program_id: None,
},
signers: vec![
Presigner::new(&pubkey2, &sig2).into(),
Expand Down Expand Up @@ -1600,6 +1681,8 @@ mod tests {
fee_payer: 0,
authorized: 0,
new_authorized: Some(1),
derived_address_seed: None,
derived_address_program_id: None,
},
signers: vec![
read_keypair_file(&default_keypair_file).unwrap().into(),
Expand Down Expand Up @@ -1631,6 +1714,8 @@ mod tests {
fee_payer: 0,
authorized: 1,
new_authorized: Some(2),
derived_address_seed: None,
derived_address_program_id: None,
},
signers: vec![
read_keypair_file(&default_keypair_file).unwrap().into(),
Expand Down
21 changes: 19 additions & 2 deletions cli/tests/vote.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![allow(clippy::integer_arithmetic)]

use {
solana_cli::{
check_balance,
Expand All @@ -14,6 +15,7 @@ use {
solana_sdk::{
account_utils::StateMut,
commitment_config::CommitmentConfig,
pubkey::Pubkey,
signature::{Keypair, NullSigner, Signer},
},
solana_streamer::socket::SocketAddrSpace,
Expand Down Expand Up @@ -43,13 +45,18 @@ fn test_vote_authorize_and_withdraw() {
// Create vote account
let vote_account_keypair = Keypair::new();
let vote_account_pubkey = vote_account_keypair.pubkey();
let authority_seed = "AUTHORITY_SEED";
let authority_owner = Pubkey::new_unique();
let initial_authorized_withdrawer =
Pubkey::create_with_seed(&default_signer.pubkey(), authority_seed, &authority_owner)
.unwrap();
config.signers = vec![&default_signer, &vote_account_keypair];
config.command = CliCommand::CreateVoteAccount {
vote_account: 1,
seed: None,
identity_account: 0,
authorized_voter: None,
authorized_withdrawer: config.signers[0].pubkey(),
authorized_withdrawer: initial_authorized_withdrawer,
commission: 0,
sign_only: false,
dump_transaction_message: false,
Expand All @@ -65,7 +72,7 @@ fn test_vote_authorize_and_withdraw() {
.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());
assert_eq!(authorized_withdrawer, initial_authorized_withdrawer);
let expected_balance = rpc_client
.get_minimum_balance_for_rent_exemption(VoteState::size_of())
.unwrap()
Expand Down Expand Up @@ -110,6 +117,8 @@ fn test_vote_authorize_and_withdraw() {
fee_payer: 0,
authorized: 0,
new_authorized: None,
derived_address_seed: Some(authority_seed.to_string()),
derived_address_program_id: Some(authority_owner),
};
process_command(&config).unwrap();
let vote_account = rpc_client
Expand All @@ -135,6 +144,8 @@ fn test_vote_authorize_and_withdraw() {
fee_payer: 0,
authorized: 1,
new_authorized: Some(1),
derived_address_seed: None,
derived_address_program_id: None,
};
process_command(&config).unwrap_err(); // unsigned by new authority should fail
config.signers = vec![
Expand All @@ -155,6 +166,8 @@ fn test_vote_authorize_and_withdraw() {
fee_payer: 0,
authorized: 1,
new_authorized: Some(2),
derived_address_seed: None,
derived_address_program_id: None,
};
process_command(&config).unwrap();
let vote_account = rpc_client
Expand Down Expand Up @@ -329,6 +342,8 @@ fn test_offline_vote_authorize_and_withdraw() {
fee_payer: 0,
authorized: 0,
new_authorized: None,
derived_address_seed: None,
derived_address_program_id: None,
};
config_offline.output_format = OutputFormat::JsonCompact;
let sig_response = process_command(&config_offline).unwrap();
Expand All @@ -351,6 +366,8 @@ fn test_offline_vote_authorize_and_withdraw() {
fee_payer: 0,
authorized: 0,
new_authorized: None,
derived_address_seed: None,
derived_address_program_id: None,
};
process_command(&config_payer).unwrap();
let vote_account = rpc_client
Expand Down