UX improvement in relayer (#541)

* Avoid unwraps in client txs. Better LightClient errors.

* Avoided unwrap calls in tx raw commands

* Tagged relayer RPC error with the culprit IP address.

* Replaced start with v0

* Introduced logging levels. WIP: remove status_ calls

* Updated the sample config file

* Adapted keys commands to JSON output

* Transition to tracing macros & JSON outputs

* Changelog

* Added operation instructions.

* Simplified Output usage; h/t Romain's suggestions.

* Better err message for client consensus query

CHANGELOG.md

@@ -18,6 +18,7 @@
 - [relayer-cli]
   - Replace `ChannelConfig` in `Channel::new` ([#511])
   - Add `packet-send` CLI ([#470])
+  - UX improvements for relayer txs ([#536, 540])
 
 - [relayer]
   - Performance improvements ([#514], [#537])
@@ -44,7 +45,9 @@
 [#517]: https://github.com/informalsystems/ibc-rs/issues/517
 [#525]: https://github.com/informalsystems/ibc-rs/issues/525
 [#527]: https://github.com/informalsystems/ibc-rs/issues/527
+[#536]: https://github.com/informalsystems/ibc-rs/issues/536
 [#537]: https://github.com/informalsystems/ibc-rs/issues/537
+[#540]: https://github.com/informalsystems/ibc-rs/issues/540
 
 
 ## v0.0.6

modules/src/ics02_client/client_def.rs

@@ -195,7 +195,6 @@ impl Protobuf<Any> for AnyClientState {}
 impl TryFrom<Any> for AnyClientState {
     type Error = Error;
 
-    // TODO Fix type urls: avoid having hardcoded values sprinkled around the whole codebase.
     fn try_from(raw: Any) -> Result<Self, Self::Error> {
         match raw.type_url.as_str() {
             "" => Err(Kind::EmptyClientState.into()),
@@ -286,6 +285,8 @@ impl TryFrom<Any> for AnyConsensusState {
 
     fn try_from(value: Any) -> Result<Self, Self::Error> {
         match value.type_url.as_str() {
+            "" => Err(Kind::EmptyConsensusState.into()),
+
             TENDERMINT_CONSENSUS_STATE_TYPE_URL => Ok(AnyConsensusState::Tendermint(
                 TendermintConsensusState::decode_vec(&value.value)
                     .map_err(|e| Kind::InvalidRawConsensusState.context(e))?,

modules/src/ics02_client/error.rs

@@ -36,6 +36,9 @@ pub enum Kind {
     #[error("unknown client consensus state type: {0}")]
     UnknownConsensusStateType(String),
 
+    #[error("empty client consensus state")]
+    EmptyConsensusState,
+
     #[error("unknown header type: {0}")]
     UnknownHeaderType(String),
 

relayer-cli/relayer_operation_instructions.md

@@ -279,3 +279,44 @@ Jan 20 11:28:47.842  INFO relayer::macros::profiling:    ⏳ inner operation - s
 Jan 20 11:28:49.846  INFO relayer::macros::profiling:    ⏳ inner operation - elapsed: 2004ms
 Jan 20 11:28:49.847  INFO relayer::macros::profiling: ⏳ myfunction: x=42 - elapsed: 3005ms
 ```
+
+## Parametrizing the log output level
+
+The relayer configuration file, called `loop_config.toml` in the examples above
+permits parametrization of output verbosity via the knob called `log_level`.
+Relevant snippet: 
+
+```toml
+[global]
+timeout = '10s'
+strategy = 'naive'
+log_level = 'error'
+```
+
+Valid options for `log_level` are: 'error', 'warn', 'info', 'debug', 'trace'.
+These levels correspond to the tracing sub-component of the relayer-cli, [see 
+here](https://docs.rs/tracing-core/0.1.17/tracing_core/struct.Level.html).
+
+The relayer will _always_ print a last line summarizing the result of its 
+operation for queries of transactions. In addition to  this last line, 
+arbitrary debug, info, or other outputs may be produced.  Example, with 
+`log_level = 'debug'`:
+
+```bash
+Running `target/debug/relayer -c loop_config.toml query client consensus ibc-0 07-tendermint-X 0 1`
+{"timestamp":"Jan 20 19:21:52.070","level":"DEBUG","fields":{"message":"registered component: abscissa_core::terminal::component::Terminal (v0.5.2)"},"target":"abscissa_core::component::registry"}
+{"timestamp":"Jan 20 19:21:52.071","level":"DEBUG","fields":{"message":"registered component: relayer_cli::components::Tracing (v0.0.6)"},"target":"abscissa_core::component::registry"}
+{"timestamp":"Jan 20 19:21:52.078","level":"INFO","fields":{"message":"Options QueryClientConsensusOptions { client_id: ClientId(\"07-tendermint-X\"), revision_number: 0, revision_height: 1, height: 0, proof: true }"},"target":"relayer_cli::commands::query::client"}
+{"timestamp":"Jan 20 19:21:52.080","level":"DEBUG","fields":{"message":"resolving host=\"localhost\""},"target":"hyper::client::connect::dns"}
+{"timestamp":"Jan 20 19:21:52.083","level":"DEBUG","fields":{"message":"connecting to [::1]:26657"},"target":"hyper::client::connect::http"}
+{"timestamp":"Jan 20 19:21:52.083","level":"DEBUG","fields":{"message":"connecting to 127.0.0.1:26657"},"target":"hyper::client::connect::http"}
+{"status":"error","result":["query error: RPC error to endpoint tcp://localhost:26657: error trying to connect: tcp connect error: Connection refused (os error 61) (code: 0)"]}
+```
+
+For the same command, with `log_level = 'error'`, just the last line will be 
+produced:
+
+```bash
+   Running `target/debug/relayer -c loop_config.toml query client consensus ibc-0 07-tendermint-X 0 1`
+{"status":"error","result":["query error: RPC error to endpoint tcp://localhost:26657: error trying to connect: tcp connect error: Connection refused (os error 61) (code: 0)"]}
+```

relayer-cli/src/application.rs

@@ -1,9 +1,12 @@
 //! Cli Abscissa Application
 
+use crate::components::Tracing;
 use crate::{commands::CliCmd, config::Config};
+use abscissa_core::terminal::component::Terminal;
 use abscissa_core::{
     application::{self, AppCell},
-    config, trace, Application, EntryPoint, FrameworkError, StandardPaths,
+    component::Component,
+    config, trace, Application, Configurable, EntryPoint, FrameworkError, StandardPaths,
 };
 
 /// Application state
@@ -98,6 +101,24 @@ impl Application for CliApp {
         Ok(())
     }
 
+    /// Overrides the default abscissa components, so that we can setup tracing on our own. See
+    /// also `register_components`.
+    fn framework_components(
+        &mut self,
+        command: &Self::Cmd,
+    ) -> Result<Vec<Box<dyn Component<Self>>>, FrameworkError> {
+        let terminal = Terminal::new(self.term_colors(command));
+
+        let config = command
+            .config_path()
+            .map(|path| self.load_config(&path))
+            .transpose()?
+            .unwrap_or_default();
+        let tracing = Tracing::new(config.global)?;
+
+        Ok(vec![Box::new(terminal), Box::new(tracing)])
+    }
+
     /// Get tracing configuration from command-line options
     fn tracing_config(&self, command: &EntryPoint<CliCmd>) -> trace::Config {
         if command.verbose {

relayer-cli/src/commands.rs

@@ -12,12 +12,11 @@ mod listen;
 mod query;
 mod start;
 mod tx;
-mod v0;
 mod version;
 
 use self::{
     config::ConfigCmd, keys::KeysCmd, light::LightCmd, listen::ListenCmd, query::QueryCmd,
-    start::StartCmd, tx::TxCmd, v0::V0Cmd, version::VersionCmd,
+    start::StartCmd, tx::TxCmd, version::VersionCmd,
 };
 
 use crate::config::Config;
@@ -34,12 +33,8 @@ pub enum CliCmd {
     #[options(help = "get usage information")]
     Help(Help<Self>),
 
-    /// The `v0` subcommand
-    #[options(help = "start the v0 relayer")]
-    V0(V0Cmd),
-
     /// The `start` subcommand
-    #[options(help = "start the relayer")]
+    #[options(help = "start the relayer (currently this refers to the v0 relayer)")]
     Start(StartCmd),
 
     /// The `listen` subcommand

relayer-cli/src/commands/config/validate.rs

@@ -1,16 +1,19 @@
-use crate::prelude::*;
-
 use abscissa_core::{Command, Options, Runnable};
 
+use crate::conclude::Output;
+use crate::prelude::*;
+
 #[derive(Command, Debug, Options)]
 pub struct ValidateCmd {}
 
 impl Runnable for ValidateCmd {
     /// Validate the loaded configuration.
     fn run(&self) {
         let config = app_config();
-        status_ok!("Loaded configuration:", "{:#?}", *config);
+        info!("Loaded configuration: {:?}", *config);
 
         // TODO: Validate configuration
+
+        Output::with_success().exit();
     }
 }

relayer-cli/src/commands/keys/add.rs

@@ -1,10 +1,11 @@
-use crate::application::app_config;
 use abscissa_core::{Command, Options, Runnable};
+
 use relayer::config::Config;
+use relayer::keys::add::{add_key, KeysAddOptions};
 
+use crate::application::app_config;
+use crate::conclude::Output;
 use crate::error::{Error, Kind};
-use crate::prelude::*;
-use relayer::keys::add::{add_key, KeysAddOptions};
 
 #[derive(Clone, Command, Debug, Options)]
 pub struct KeysAddCmd {
@@ -47,17 +48,16 @@ impl Runnable for KeysAddCmd {
 
         let opts = match self.validate_options(&config) {
             Err(err) => {
-                status_err!("invalid options: {}", err);
-                return;
+                return Output::error(err).exit();
             }
             Ok(result) => result,
         };
 
         let res: Result<String, Error> = add_key(opts).map_err(|e| Kind::Keys.context(e).into());
 
         match res {
-            Ok(r) => status_info!("key add result: ", "{:?}", r),
-            Err(e) => status_info!("key add failed: ", "{}", e),
+            Ok(r) => Output::success(r).exit(),
+            Err(e) => Output::error(format!("{}", e)).exit(),
         }
     }
 }

relayer-cli/src/commands/keys/list.rs

@@ -1,10 +1,11 @@
-use crate::application::app_config;
 use abscissa_core::{Command, Options, Runnable};
+
 use relayer::config::Config;
+use relayer::keys::list::{list_keys, KeysListOptions};
 
+use crate::application::app_config;
+use crate::conclude::Output;
 use crate::error::{Error, Kind};
-use crate::prelude::*;
-use relayer::keys::list::{list_keys, KeysListOptions};
 
 #[derive(Clone, Command, Debug, Options)]
 pub struct KeysListCmd {
@@ -37,17 +38,16 @@ impl Runnable for KeysListCmd {
 
         let opts = match self.validate_options(&config) {
             Err(err) => {
-                status_err!("invalid options: {}", err);
-                return;
+                return Output::error(err).exit();
             }
             Ok(result) => result,
         };
 
         let res: Result<String, Error> = list_keys(opts).map_err(|e| Kind::Keys.context(e).into());
 
         match res {
-            Ok(r) => status_info!("keys list result: ", "{:?}", r),
-            Err(e) => status_info!("keys list failed: ", "{}", e),
+            Ok(r) => Output::success(r).exit(),
+            Err(e) => Output::error(format!("{}", e)).exit(),
         }
     }
 }

relayer-cli/src/commands/keys/restore.rs

@@ -1,10 +1,11 @@
-use crate::application::app_config;
 use abscissa_core::{Command, Options, Runnable};
+
 use relayer::config::Config;
+use relayer::keys::restore::{restore_key, KeysRestoreOptions};
 
+use crate::application::app_config;
+use crate::conclude::Output;
 use crate::error::{Error, Kind};
-use crate::prelude::*;
-use relayer::keys::restore::{restore_key, KeysRestoreOptions};
 
 #[derive(Clone, Command, Debug, Options)]
 pub struct KeyRestoreCmd {
@@ -55,8 +56,7 @@ impl Runnable for KeyRestoreCmd {
 
         let opts = match self.validate_options(&config) {
             Err(err) => {
-                status_err!("invalid options: {}", err);
-                return;
+                return Output::error(err).exit();
             }
             Ok(result) => result,
         };
@@ -65,8 +65,8 @@ impl Runnable for KeyRestoreCmd {
             restore_key(opts).map_err(|e| Kind::Keys.context(e).into());
 
         match res {
-            Ok(r) => status_info!("key restore result: ", "{:?}", hex::encode(r)),
-            Err(e) => status_info!("key restore failed: ", "{}", e),
+            Ok(r) => Output::success(r).exit(),
+            Err(e) => Output::error(format!("{}", e)).exit(),
         }
     }
 }

relayer-cli/src/commands/light/rm.rs

@@ -1,12 +1,13 @@
 use std::{io, io::Write, ops::Deref};
 
-use crate::prelude::*;
-
 use abscissa_core::{application::fatal_error, error::BoxError, Command, Options, Runnable};
+use tendermint_light_client::types::PeerId;
 
 use ibc::ics24_host::identifier::ChainId;
 use relayer::config::PeersConfig;
-use tendermint_light_client::types::PeerId;
+
+use crate::conclude::Output;
+use crate::prelude::*;
 
 #[derive(Command, Debug, Options)]
 pub struct RmCmd {
@@ -88,20 +89,26 @@ impl RmCmd {
             .as_mut()
             .ok_or_else(|| format!("no peers configured for chain: {}", options.chain_id))?;
 
-        if options.all && (options.yes || confirm(&options.chain_id)?) {
+        let rmd_peers = if options.all && (options.yes || confirm(&options.chain_id)?) {
             let removed_peers = get_all_peer_ids(&peers_config);
             chain_config.peers = None;
-            status_ok!("Removed", "light client peers {:?}", removed_peers);
+
+            removed_peers
         } else {
+            let mut res: Vec<String> = vec![];
             for peer_id in options.peer_ids {
                 let removed_peer = remove_peer(&mut peers_config, peer_id, options.force)?;
-                status_ok!("Removed", "light client peer '{}'", removed_peer);
+                res.push(removed_peer.to_string());
             }
+
+            res
         };
 
         let config_path = crate::config::config_path()?;
         relayer::config::store(&config, config_path)?;
 
+        Output::success(format!("Removed light client peer(s) '{:?}'", rmd_peers)).exit();
+
         Ok(())
     }
 }

relayer-cli/src/commands/query/channel.rs

@@ -1,7 +1,6 @@
 use std::sync::Arc;
 
 use abscissa_core::{Command, Options, Runnable};
-use serde_json::json;
 use tendermint_proto::Protobuf;
 use tokio::runtime::Runtime as TokioRuntime;
 
@@ -86,11 +85,11 @@ impl Runnable for QueryChannelEndCmd {
 
         let (chain_config, opts) = match self.validate_options(&config) {
             Err(err) => {
-                return Output::with_error().with_result(json!(err)).exit();
+                return Output::error(err).exit();
             }
             Ok(result) => result,
         };
-        status_info!("Options", "{:?}", opts);
+        info!("Options {:?}", opts);
 
         // run without proof:
         // cargo run --bin relayer -- -c relayer/tests/config/fixtures/simple_config.toml query channel end ibc-test firstport firstchannel --height 3 -p false
@@ -111,10 +110,8 @@ impl Runnable for QueryChannelEndCmd {
             });
 
         match res {
-            Ok(ce) => Output::with_success().with_result(json!(ce)).exit(),
-            Err(e) => Output::with_error()
-                .with_result(json!(format!("{}", e)))
-                .exit(),
+            Ok(ce) => Output::success(ce).exit(),
+            Err(e) => Output::error(format!("{}", e)).exit(),
         }
     }
 }