Merge pull request #4049 from anoma/mergify/bp/maint-0.45/pr-4047

Fix client missing help messages (backport #4047)

.changelog/unreleased/improvements/4047-fix-help-msgs.md

@@ -0,0 +1,2 @@
+- Client now prints help messages on missing arguments.
+  ([\#4047](https://github.com/anoma/namada/pull/4047))

crates/apps_lib/src/cli.rs

@@ -1319,6 +1319,7 @@ pub mod cmds {
         fn def() -> App {
             App::new(Self::CMD)
                 .about(wrap!("Send a transaction with custom WASM code."))
+                .arg_required_else_help(true)
                 .add_args::<args::TxCustom<args::CliTypes>>()
         }
     }
@@ -1470,6 +1471,7 @@ pub mod cmds {
                     "Send a signed transaction to create a new established \
                      account."
                 ))
+                .arg_required_else_help(true)
                 .add_args::<args::TxInitAccount<args::CliTypes>>()
         }
     }
@@ -2167,6 +2169,7 @@ pub mod cmds {
                     "Find a PoS validator and its consensus key by its native \
                      address or Tendermint address."
                 ))
+                .arg_required_else_help(true)
                 .add_args::<args::QueryFindValidator<args::CliTypes>>()
         }
     }
@@ -3328,6 +3331,7 @@ pub mod args {
     use std::str::FromStr;
 
     use data_encoding::HEXUPPER;
+    use either::Either;
     use namada_core::masp::{MaspEpoch, PaymentAddress};
     use namada_sdk::address::{Address, EstablishedAddress};
     pub use namada_sdk::args::*;
@@ -3605,7 +3609,7 @@ pub mod args {
     pub const TEMPLATES_PATH: Arg<PathBuf> = arg("templates-path");
     pub const TIMEOUT_HEIGHT: ArgOpt<u64> = arg_opt("timeout-height");
     pub const TIMEOUT_SEC_OFFSET: ArgOpt<u64> = arg_opt("timeout-sec-offset");
-    pub const TM_ADDRESS: ArgOpt<String> = arg_opt("tm-address");
+    pub const TM_ADDRESS_OPT: ArgOpt<String> = arg_opt("tm-address");
     pub const TOKEN_OPT: ArgOpt<WalletAddress> = TOKEN.opt();
     pub const TOKEN_STR_OPT: ArgOpt<String> = TOKEN_STR.opt();
     pub const TOKEN: Arg<WalletAddress> = arg("token");
@@ -7241,26 +7245,34 @@ pub mod args {
     impl Args for QueryFindValidator<CliTypes> {
         fn parse(matches: &ArgMatches) -> Self {
             let query = Query::parse(matches);
-            let tm_addr = TM_ADDRESS.parse(matches);
+            let tm_addr = TM_ADDRESS_OPT.parse(matches);
             let validator_addr = VALIDATOR_OPT.parse(matches);
-            Self {
-                query,
-                tm_addr,
-                validator_addr,
-            }
+
+            let addr = match (tm_addr, validator_addr) {
+                (Some(tm_addr), None) => Either::Left(tm_addr),
+                (None, Some(validator_addr)) => Either::Right(validator_addr),
+                _ => unreachable!(
+                    "Wrong arguments supplied, CLI should prevent this"
+                ),
+            };
+            Self { query, addr }
         }
 
         fn def(app: App) -> App {
             app.add_args::<Query<CliTypes>>()
                 .arg(
-                    TM_ADDRESS.def().help(wrap!(
-                        "The address of the validator in Tendermint."
-                    )),
+                    TM_ADDRESS_OPT
+                        .def()
+                        .help(wrap!(
+                            "The address of the validator in Tendermint."
+                        ))
+                        .conflicts_with(VALIDATOR_OPT.name),
                 )
                 .arg(
                     VALIDATOR_OPT
                         .def()
-                        .help(wrap!("The native address of the validator.")),
+                        .help(wrap!("The native address of the validator."))
+                        .conflicts_with(TM_ADDRESS_OPT.name),
                 )
         }
     }
@@ -7274,10 +7286,9 @@ pub mod args {
         ) -> Result<QueryFindValidator<SdkTypes>, Self::Error> {
             Ok(QueryFindValidator::<SdkTypes> {
                 query: self.query.to_sdk(ctx)?,
-                tm_addr: self.tm_addr,
-                validator_addr: self
-                    .validator_addr
-                    .map(|x| ctx.borrow_chain_or_exit().get(&x)),
+                addr: self.addr.map_right(|validator_addr| {
+                    ctx.borrow_chain_or_exit().get(&validator_addr)
+                }),
             })
         }
     }

crates/apps_lib/src/client/rpc.rs

@@ -5,6 +5,7 @@ use std::io;
 
 use borsh::BorshDeserialize;
 use data_encoding::HEXLOWER;
+use either::Either;
 use masp_primitives::asset_type::AssetType;
 use masp_primitives::merkle_tree::MerklePath;
 use masp_primitives::sapling::Node;
@@ -1777,67 +1778,64 @@ pub async fn query_find_validator<N: Namada>(
     context: &N,
     args: args::QueryFindValidator,
 ) {
-    let args::QueryFindValidator {
-        query: _,
-        tm_addr,
-        mut validator_addr,
-    } = args;
-    if let Some(tm_addr) = tm_addr {
-        if tm_addr.len() != 40 {
-            edisplay_line!(
-                context.io(),
-                "Expected 40 characters in Tendermint address, got {}",
-                tm_addr.len()
-            );
-            cli::safe_exit(1);
-        }
-        let tm_addr = tm_addr.to_ascii_uppercase();
-        let validator = unwrap_client_response::<N::Client, _>(
-            RPC.vp()
-                .pos()
-                .validator_by_tm_addr(context.client(), &tm_addr)
-                .await,
-        );
-        match validator {
-            Some(address) => {
-                display_line!(
+    let args::QueryFindValidator { query: _, addr } = args;
+    let validator_addr = match addr {
+        Either::Left(comet_addr) => {
+            // Retrieve the native address from the Comet one
+            if comet_addr.len() != 40 {
+                edisplay_line!(
                     context.io(),
-                    "Found validator address \"{address}\"."
+                    "Expected 40 characters in Tendermint address, got {}",
+                    comet_addr.len()
                 );
-                if validator_addr.is_none() {
-                    validator_addr = Some(address);
-                }
-            }
-            None => {
-                display_line!(
-                    context.io(),
-                    "No validator with Tendermint address {tm_addr} found."
-                )
+                cli::safe_exit(1);
             }
-        }
-    }
-    if let Some(validator_addr) = validator_addr {
-        if let Some(consensus_key) = unwrap_client_response::<N::Client, _>(
-            RPC.vp()
-                .pos()
-                .consensus_key(context.client(), &validator_addr)
-                .await,
-        ) {
-            let pkh: PublicKeyHash = (&consensus_key).into();
-            display_line!(context.io(), "Consensus key: {consensus_key}");
-            display_line!(
-                context.io(),
-                "Tendermint key: {}",
-                tm_consensus_key_raw_hash(&consensus_key)
+            let tm_addr = comet_addr.to_ascii_uppercase();
+            let validator = unwrap_client_response::<N::Client, _>(
+                RPC.vp()
+                    .pos()
+                    .validator_by_tm_addr(context.client(), &tm_addr)
+                    .await,
             );
-            display_line!(context.io(), "Consensus key hash: {}", pkh);
-        } else {
-            display_line!(
-                context.io(),
-                "Consensus key for validator {validator_addr} could not be \
-                 found."
-            )
+            match validator {
+                Some(address) => {
+                    display_line!(
+                        context.io(),
+                        "Found validator address \"{address}\"."
+                    );
+                    address
+                }
+                None => {
+                    edisplay_line!(
+                        context.io(),
+                        "No validator with Tendermint address {tm_addr} found."
+                    );
+                    cli::safe_exit(1);
+                }
+            }
         }
+        Either::Right(validator_addr) => validator_addr,
+    };
+
+    if let Some(consensus_key) = unwrap_client_response::<N::Client, _>(
+        RPC.vp()
+            .pos()
+            .consensus_key(context.client(), &validator_addr)
+            .await,
+    ) {
+        let pkh: PublicKeyHash = (&consensus_key).into();
+        display_line!(context.io(), "Consensus key: {consensus_key}");
+        display_line!(
+            context.io(),
+            "Tendermint key: {}",
+            tm_consensus_key_raw_hash(&consensus_key)
+        );
+        display_line!(context.io(), "Consensus key hash: {}", pkh);
+    } else {
+        display_line!(
+            context.io(),
+            "Consensus key for validator {validator_addr} could not be found."
+        )
     }
 }
 

crates/sdk/src/args.rs

@@ -5,6 +5,7 @@ use std::path::PathBuf;
 use std::str::FromStr;
 use std::time::Duration as StdDuration;
 
+use either::Either;
 use namada_core::address::Address;
 use namada_core::chain::{BlockHeight, ChainId, Epoch};
 use namada_core::collections::HashMap;
@@ -2242,10 +2243,8 @@ pub struct QueryStakingRewardsRate<C: NamadaTypes = SdkTypes> {
 pub struct QueryFindValidator<C: NamadaTypes = SdkTypes> {
     /// Common query args
     pub query: Query<C>,
-    /// Tendermint address
-    pub tm_addr: Option<String>,
-    /// Native validator address
-    pub validator_addr: Option<C::Address>,
+    /// Validator address, either Comet or native
+    pub addr: Either<String, C::Address>,
 }
 
 /// Query the raw bytes of given storage key