Merge branch 'brent/upgrade-consensus-key-change' (anoma#2218)

* origin/brent/upgrade-consensus-key-change:
  benches/tx: fix change_consensus_key by adding the extra req sig
  changelog: add anoma#2218
  client/tx: tidy up change-consensus-key
  github/workflows: add e2e test
  test/e2e: make `Who` copy-able
  test/e2e/ledger: finish consensus key change test
  need `unsafe-dont-encrypt` arg for testing
  fix bug
  WIP

.changelog/unreleased/testing/2218-update-consensus-key-change.md

@@ -0,0 +1,2 @@
+- Added e2e test for change-consensus-key command.
+  ([\#2218](https://github.com/anoma/namada/pull/2218))

.github/workflows/scripts/e2e.json

@@ -27,6 +27,7 @@
     "e2e::ledger_tests::test_bond_queries": 95,
     "e2e::ledger_tests::suspend_ledger": 30,
     "e2e::ledger_tests::stop_ledger_at_height": 18,
+    "e2e::ledger_tests::change_consensus_key": 91,
     "e2e::wallet_tests::wallet_address_cmds": 1,
     "e2e::wallet_tests::wallet_encrypted_key_cmds": 1,
     "e2e::wallet_tests::wallet_encrypted_key_cmds_env_var": 1,

apps/src/lib/cli.rs

@@ -5416,6 +5416,7 @@ pub mod args {
                 tx,
                 validator: chain_ctx.get(&self.validator),
                 consensus_key: self.consensus_key.map(|x| chain_ctx.get(&x)),
+                unsafe_dont_encrypt: self.unsafe_dont_encrypt,
                 tx_code_path: self.tx_code_path.to_path_buf(),
             }
         }
@@ -5426,11 +5427,13 @@ pub mod args {
             let tx = Tx::parse(matches);
             let validator = VALIDATOR.parse(matches);
             let consensus_key = VALIDATOR_CONSENSUS_KEY.parse(matches);
+            let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches);
             let tx_code_path = PathBuf::from(TX_CHANGE_CONSENSUS_KEY_WASM);
             Self {
                 tx,
                 validator,
                 consensus_key,
+                unsafe_dont_encrypt,
                 tx_code_path,
             }
         }
@@ -5444,6 +5447,10 @@ pub mod args {
                     "The desired new consensus key. A new one will be \
                      generated if none given. Note this key must be ed25519.",
                 ))
+                .arg(UNSAFE_DONT_ENCRYPT.def().help(
+                    "UNSAFE: Do not encrypt the generated keypairs. Do not \
+                     use this for keys used in a live network.",
+                ))
         }
     }
 

apps/src/lib/client/tx.rs

@@ -13,6 +13,7 @@ use namada::core::ledger::governance::cli::offline::{
 use namada::core::ledger::governance::cli::onchain::{
     DefaultProposal, PgfFundingProposal, PgfStewardProposal, ProposalVote,
 };
+use namada::core::ledger::storage::EPOCH_SWITCH_BLOCKS_DELAY;
 use namada::ibc::apps::transfer::types::Memo;
 use namada::proto::{CompressedSignature, Section, Signer, Tx};
 use namada::types::address::{Address, ImplicitAddress};
@@ -323,6 +324,7 @@ pub async fn submit_change_consensus_key(
         tx: tx_args,
         validator,
         consensus_key,
+        unsafe_dont_encrypt,
         tx_code_path: _,
     }: args::ConsensusKeyChange,
 ) -> Result<(), error::Error> {
@@ -334,12 +336,8 @@ pub async fn submit_change_consensus_key(
         ..tx_args.clone()
     };
 
-    // TODO: do I need to get the validator alias from somewhere, if it exists?
-    // // Don't think I should generate a new one... Should get the alias
-    // for the consensus key though...
-
-    let wallet = namada.wallet().await;
-
+    // Determine the alias for the new key
+    let mut wallet = namada.wallet_mut().await;
     let alias = wallet.find_alias(&validator).cloned();
     let base_consensus_key_alias = alias
         .map(|al| validator_consensus_key(&al))
@@ -355,8 +353,8 @@ pub async fn submit_change_consensus_key(
             format!("{base_consensus_key_alias}-{key_counter}");
     }
 
-    let mut wallet = namada.wallet_mut().await;
-    let consensus_key = consensus_key
+    // Check the given key or generate a new one
+    let new_key = consensus_key
         .map(|key| match key {
             common::PublicKey::Ed25519(_) => key,
             common::PublicKey::Secp256k1(_) => {
@@ -369,7 +367,8 @@ 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(false);
+            let password =
+                read_and_confirm_encryption_password(unsafe_dont_encrypt);
             wallet
                 .gen_store_secret_key(
                     // Note that TM only allows ed25519 for consensus key
@@ -389,9 +388,8 @@ pub async fn submit_change_consensus_key(
     // Check that the new consensus key is unique
     let consensus_keys = rpc::query_consensus_keys(namada.client()).await;
 
-    let new_ck = consensus_key;
-    if consensus_keys.contains(&new_ck) {
-        edisplay_line!(namada.io(), "Consensus key can only be ed25519");
+    if consensus_keys.contains(&new_key) {
+        edisplay_line!(namada.io(), "The consensus key is already being used.");
         safe_exit(1)
     }
 
@@ -405,15 +403,17 @@ pub async fn submit_change_consensus_key(
 
     let data = ConsensusKeyChange {
         validator: validator.clone(),
-        consensus_key: new_ck,
+        consensus_key: new_key.clone(),
     };
 
     tx.add_code_from_hash(
         tx_code_hash,
         Some(args::TX_CHANGE_CONSENSUS_KEY_WASM.to_string()),
     )
     .add_data(data);
-    let signing_data = aux_signing_data(namada, &tx_args, None, None).await?;
+
+    let signing_data =
+        init_validator_signing_data(namada, &tx_args, vec![new_key]).await?;
 
     tx::prepare_tx(
         namada,
@@ -437,17 +437,14 @@ pub async fn submit_change_consensus_key(
                 .save()
                 .unwrap_or_else(|err| edisplay_line!(namada.io(), "{}", err));
 
-            // let tendermint_home = config.ledger.cometbft_dir();
-            // tendermint_node::write_validator_key(
-            //     &tendermint_home,
-            //     &consensus_key,
-            // );
-            // tendermint_node::write_validator_state(tendermint_home);
-
             display_line!(
                 namada.io(),
-                "  Consensus key \"{}\"",
-                consensus_key_alias
+                "New consensus key stored with alias \
+                 \"{consensus_key_alias}\". It will become active \
+                 {EPOCH_SWITCH_BLOCKS_DELAY} blocks before pipeline offset \
+                 from the current epoch, at which point you'll need to give \
+                 the new key to CometBFT in order to be able to sign with it \
+                 in consensus.",
             );
         } else {
             display_line!(

benches/txs.rs

@@ -686,21 +686,21 @@ fn change_validator_commission(c: &mut Criterion) {
 
 fn change_consensus_key(c: &mut Criterion) {
     let mut csprng = rand::rngs::OsRng {};
-    let consensus_key = ed25519::SigScheme::generate(&mut csprng)
+    let consensus_sk = ed25519::SigScheme::generate(&mut csprng)
         .try_to_sk::<common::SecretKey>()
-        .unwrap()
-        .to_public();
+        .unwrap();
+    let consensus_pk = consensus_sk.to_public();
 
     let shell = BenchShell::default();
     let signed_tx = shell.generate_tx(
         TX_CHANGE_CONSENSUS_KEY_WASM,
         ConsensusKeyChange {
             validator: defaults::validator_address(),
-            consensus_key,
+            consensus_key: consensus_pk,
         },
         None,
         None,
-        vec![&defaults::validator_keypair()],
+        vec![&defaults::validator_keypair(), &consensus_sk],
     );
 
     c.bench_function("change_consensus_key", |b| {

sdk/src/args.rs

@@ -1400,6 +1400,8 @@ pub struct ConsensusKeyChange<C: NamadaTypes = SdkTypes> {
     pub validator: C::Address,
     /// New consensus key
     pub consensus_key: Option<C::PublicKey>,
+    /// Don't encrypt the keypair
+    pub unsafe_dont_encrypt: bool,
     /// Path to the TX WASM code file
     pub tx_code_path: PathBuf,
 }

sdk/src/lib.rs

@@ -331,6 +331,7 @@ pub trait Namada: Sized + MaybeSync + MaybeSend {
             validator,
             consensus_key: None,
             tx_code_path: PathBuf::from(TX_CHANGE_CONSENSUS_KEY_WASM),
+            unsafe_dont_encrypt: false,
             tx: self.tx_builder(),
         }
     }