From f6a3357bcb436f5501ae5d588630159b289a31af Mon Sep 17 00:00:00 2001 From: Gianmarco Fraccaroli Date: Tue, 9 Apr 2024 15:18:39 +0200 Subject: [PATCH 1/2] sdk: refactor change_consensus_key to be part of the sdk --- crates/apps/src/lib/cli/client.rs | 21 +------- crates/apps/src/lib/client/tx.rs | 85 ++++++----------------------- crates/sdk/src/args.rs | 89 +++++++++++++++---------------- crates/sdk/src/lib.rs | 3 +- crates/sdk/src/tx.rs | 66 +++++++++++++++++++++++ 5 files changed, 130 insertions(+), 134 deletions(-) diff --git a/crates/apps/src/lib/cli/client.rs b/crates/apps/src/lib/cli/client.rs index a344c41b21..fcfa078644 100644 --- a/crates/apps/src/lib/cli/client.rs +++ b/crates/apps/src/lib/cli/client.rs @@ -285,25 +285,8 @@ impl CliApi { }); client.wait_until_node_is_synced(&io).await?; let args = args.to_sdk(&mut ctx); - let cli::context::ChainContext { - wallet, - mut config, - shielded, - native_token, - } = ctx.take_chain_or_exit(); - let namada = NamadaImpl::native_new( - client, - wallet, - shielded, - io, - native_token, - ); - tx::submit_change_consensus_key( - &namada, - &mut config, - args, - ) - .await?; + let namada = ctx.to_sdk(client, io); + tx::submit_change_consensus_key(&namada, args).await?; } Sub::TxMetadataChange(TxMetadataChange(args)) => { let chain_ctx = ctx.borrow_mut_chain_or_exit(); diff --git a/crates/apps/src/lib/client/tx.rs b/crates/apps/src/lib/client/tx.rs index 30d41f17c7..f76b702f46 100644 --- a/crates/apps/src/lib/client/tx.rs +++ b/crates/apps/src/lib/client/tx.rs @@ -20,7 +20,7 @@ use namada::governance::ProposalVote; use namada::ibc::apps::transfer::types::Memo; use namada::io::Io; use namada::state::EPOCH_SWITCH_BLOCKS_DELAY; -use namada::tx::data::pos::{BecomeValidator, ConsensusKeyChange}; +use namada::tx::data::pos::BecomeValidator; use namada::tx::{CompressedSignature, Section, Signer, Tx}; use namada_sdk::rpc::{InnerTxResult, TxBroadcastData, TxResponse}; use namada_sdk::signing::validate_fee_and_gen_unshield; @@ -320,22 +320,10 @@ where pub async fn submit_change_consensus_key( namada: &impl Namada, - config: &mut crate::config::Config, - args::ConsensusKeyChange { - tx: tx_args, - validator, - consensus_key, - unsafe_dont_encrypt, - tx_code_path: _, - }: args::ConsensusKeyChange, + args: args::ConsensusKeyChange, ) -> Result<(), error::Error> { - let tx_args = args::Tx { - chain_id: tx_args - .clone() - .chain_id - .or_else(|| Some(config.ledger.chain_id.clone())), - ..tx_args.clone() - }; + let validator = args.validator; + let consensus_key = args.consensus_key; // Determine the alias for the new key let mut wallet = namada.wallet_mut().await; @@ -369,13 +357,13 @@ pub async fn submit_change_consensus_key( .unwrap_or_else(|| { display_line!(namada.io(), "Generating new consensus key..."); let password = - read_and_confirm_encryption_password(unsafe_dont_encrypt); + read_and_confirm_encryption_password(args.unsafe_dont_encrypt); wallet .gen_store_secret_key( // Note that TM only allows ed25519 for consensus key SchemeType::Ed25519, Some(consensus_key_alias.clone()), - tx_args.wallet_alias_force, + args.tx.wallet_alias_force, password, &mut OsRng, ) @@ -383,66 +371,25 @@ pub async fn submit_change_consensus_key( .1 .ref_to() }); + // To avoid wallet deadlocks in following operations drop(wallet); - // Check that the new consensus key is unique - let consensus_keys = rpc::query_consensus_keys(namada.client()).await; - - if consensus_keys.contains(&new_key) { - edisplay_line!(namada.io(), "The consensus key is already being used."); - safe_exit(1) - } - - let tx_code_hash = - query_wasm_code_hash(namada, args::TX_CHANGE_CONSENSUS_KEY_WASM) - .await - .unwrap(); - - let chain_id = tx_args.chain_id.clone().unwrap(); - let mut tx = Tx::new(chain_id, tx_args.expiration); - - let data = ConsensusKeyChange { + let args = namada::sdk::args::ConsensusKeyChange { validator: validator.clone(), - consensus_key: new_key.clone(), + consensus_key: Some(new_key.clone()), + ..args }; - tx.add_code_from_hash( - tx_code_hash, - Some(args::TX_CHANGE_CONSENSUS_KEY_WASM.to_string()), - ) - .add_data(data); - - if let Some(memo) = &tx_args.memo { - tx.add_memo(memo); - }; - - let signing_data = - init_validator_signing_data(namada, &tx_args, vec![new_key]).await?; - let (fee_amount, _, unshield) = validate_fee_and_gen_unshield( - namada, - &tx_args, - &signing_data.fee_payer, - ) - .await?; - - tx::prepare_tx( - namada.client(), - &tx_args, - &mut tx, - unshield, - fee_amount, - signing_data.fee_payer.clone(), - ) - .await?; + let (mut tx, signing_data) = args.build(namada).await?; - if tx_args.dump_tx { - tx::dump_tx(namada.io(), &tx_args, tx); + if args.tx.dump_tx { + tx::dump_tx(namada.io(), &args.tx, tx); } else { - sign(namada, &mut tx, &tx_args, signing_data).await?; - let resp = namada.submit(tx, &tx_args).await?; + sign(namada, &mut tx, &args.tx, signing_data).await?; + let resp = namada.submit(tx, &args.tx).await?; - if !tx_args.dry_run { + if !args.tx.dry_run { if resp.is_applied_and_valid().is_some() { namada.wallet_mut().await.save().unwrap_or_else(|err| { edisplay_line!(namada.io(), "{}", err) diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index b14d0240bf..0380506c8a 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -1397,51 +1397,50 @@ pub struct ConsensusKeyChange { pub tx_code_path: PathBuf, } -// impl TxBuilder for ConsensusKeyChange { -// fn tx(self, func: F) -> Self -// where -// F: FnOnce(Tx) -> Tx, -// { -// ConsensusKeyChange { -// tx: func(self.tx), -// ..self -// } -// } -// } - -// impl ConsensusKeyChange { -// /// Validator address (should be self) -// pub fn validator(self, validator: C::Address) -> Self { -// Self { validator, ..self } -// } - -// /// Value to which the tx changes the commission rate -// pub fn consensus_key(self, consensus_key: C::Keypair) -> Self { -// Self { -// consensus_key: Some(consensus_key), -// ..self -// } -// } - -// /// Path to the TX WASM code file -// pub fn tx_code_path(self, tx_code_path: PathBuf) -> Self { -// Self { -// tx_code_path, -// ..self -// } -// } -// } - -// impl ConsensusKeyChange { -// /// Build a transaction from this builder -// pub async fn build( -// &self, -// context: &impl Namada, -// ) -> crate::error::Result<(namada_tx::Tx, SigningTxData, -// Option)> { -// tx::build_change_consensus_key(context, self).await -// } -// } +impl TxBuilder for ConsensusKeyChange { + fn tx(self, func: F) -> Self + where + F: FnOnce(Tx) -> Tx, + { + ConsensusKeyChange { + tx: func(self.tx), + ..self + } + } +} + +impl ConsensusKeyChange { + /// Validator address (should be self) + pub fn validator(self, validator: C::Address) -> Self { + Self { validator, ..self } + } + + /// Value to which the tx changes the commission rate + pub fn consensus_key(self, consensus_key: C::PublicKey) -> Self { + Self { + consensus_key: Some(consensus_key), + ..self + } + } + + /// Path to the TX WASM code file + pub fn tx_code_path(self, tx_code_path: PathBuf) -> Self { + Self { + tx_code_path, + ..self + } + } +} + +impl ConsensusKeyChange { + /// Build a transaction from this builder + pub async fn build( + &self, + context: &impl Namada, + ) -> crate::error::Result<(namada_tx::Tx, SigningTxData)> { + tx::build_change_consensus_key(context, self).await + } +} #[derive(Clone, Debug)] /// Commission rate change args diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 081865d26e..beb46098c0 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -329,10 +329,11 @@ pub trait Namada: Sized + MaybeSync + MaybeSend { fn new_change_consensus_key( &self, validator: Address, + consensus_key: common::PublicKey, ) -> args::ConsensusKeyChange { args::ConsensusKeyChange { validator, - consensus_key: None, + consensus_key: Some(consensus_key), tx_code_path: PathBuf::from(TX_CHANGE_CONSENSUS_KEY_WASM), unsafe_dont_encrypt: false, tx: self.tx_builder(), diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 694936e98b..382fcaeb33 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -49,6 +49,7 @@ use namada_proof_of_stake::types::{CommissionPair, ValidatorState}; use namada_token::storage_key::balance_key; use namada_token::DenominatedAmount; use namada_tx::data::pgf::UpdateStewardCommission; +use namada_tx::data::pos::ConsensusKeyChange; use namada_tx::data::{pos, ResultCode, TxResult}; pub use namada_tx::{Signature, *}; @@ -562,6 +563,71 @@ pub async fn save_initialized_accounts( } } +/// Submit validator commission rate change +pub async fn build_change_consensus_key( + context: &impl Namada, + args::ConsensusKeyChange { + tx: tx_args, + validator, + consensus_key, + tx_code_path, + unsafe_dont_encrypt: _, + }: &args::ConsensusKeyChange, +) -> Result<(Tx, SigningTxData)> { + let consensus_key = if let Some(consensus_key) = consensus_key { + consensus_key + } else { + edisplay_line!(context.io(), "Consensus key must must be present."); + return Err(Error::from(TxSubmitError::Other( + "Consensus key must must be present.".to_string(), + ))); + }; + + // Check that the new consensus key is unique + let consensus_keys = rpc::get_consensus_keys(context.client()).await?; + + if consensus_keys.contains(consensus_key) { + edisplay_line!( + context.io(), + "The consensus key is already being used." + ); + return Err(Error::from(TxSubmitError::ConsensusKeyNotUnique)); + } + + let data = ConsensusKeyChange { + validator: validator.clone(), + consensus_key: consensus_key.clone(), + }; + + let signing_data = signing::init_validator_signing_data( + context, + tx_args, + vec![consensus_key.clone()], + ) + .await?; + + let (fee_amount, _updated_balance, unshield) = + validate_fee_and_gen_unshield( + context, + tx_args, + &signing_data.fee_payer, + ) + .await?; + + build( + context, + tx_args, + tx_code_path.clone(), + data, + do_nothing, + unshield, + fee_amount, + &signing_data.fee_payer, + ) + .await + .map(|tx| (tx, signing_data)) +} + /// Submit validator commission rate change pub async fn build_validator_commission_change( context: &impl Namada, From 896117784aa248bb699a7d0e0f505d6b162ed7b9 Mon Sep 17 00:00:00 2001 From: Gianmarco Fraccaroli Date: Tue, 9 Apr 2024 15:53:06 +0200 Subject: [PATCH 2/2] added changelog --- .changelog/unreleased/sdk/3037-sdk-change-consensus-keys.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/sdk/3037-sdk-change-consensus-keys.md diff --git a/.changelog/unreleased/sdk/3037-sdk-change-consensus-keys.md b/.changelog/unreleased/sdk/3037-sdk-change-consensus-keys.md new file mode 100644 index 0000000000..49740212d2 --- /dev/null +++ b/.changelog/unreleased/sdk/3037-sdk-change-consensus-keys.md @@ -0,0 +1,2 @@ +- Add a new method to the sdk to change a validator consensus key. + ([\#3037](https://github.com/anoma/namada/pull/3037)) \ No newline at end of file