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

UX improvement in relayer #541

Merged
merged 13 commits into from
Jan 21, 2021
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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
Expand Down
23 changes: 22 additions & 1 deletion relayer-cli/src/application.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 {
Expand Down
9 changes: 2 additions & 7 deletions relayer-cli/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
9 changes: 6 additions & 3 deletions relayer-cli/src/commands/config/validate.rs
Original file line number Diff line number Diff line change
@@ -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();
}
}
17 changes: 10 additions & 7 deletions relayer-cli/src/commands/keys/add.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use crate::application::app_config;
use abscissa_core::{Command, Options, Runnable};
use serde_json::json;

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 {
Expand Down Expand Up @@ -47,17 +49,18 @@ impl Runnable for KeysAddCmd {

let opts = match self.validate_options(&config) {
Err(err) => {
status_err!("invalid options: {}", err);
return;
return Output::with_error().with_result(json!(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::with_success().with_result(json!(r)).exit(),
Err(e) => Output::with_error()
.with_result(json!(format!("{}", e)))
.exit(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could introduce a couple helpers to streamline such code a bit, eg.

Suggested change
Ok(r) => Output::with_success().with_result(json!(r)).exit(),
Err(e) => Output::with_error()
.with_result(json!(format!("{}", e)))
.exit(),
Ok(r) => Output::success(json!(r)).exit(),
Err(e) => Output::error(json!(format!("{}", e))).exit(),

Copy link
Member

@romac romac Jan 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, what do you think about changing the type of success/error/with_result to

pub fn with_result(mut self, res: impl Serialize) -> Self {
        self.result.push(serde_json::to_value(res).unwrap()); // same unwrap as in `json!` macro
        self
    }

which would result into

Suggested change
Ok(r) => Output::with_success().with_result(json!(r)).exit(),
Err(e) => Output::with_error()
.with_result(json!(format!("{}", e)))
.exit(),
Ok(r) => Output::success(r).exit(),
Err(e) => Output::error(format!("{}", e)).exit(),

Having to call unwrap on the result of serde_json::to_value is unfortunate, but that's actually what the json! macro does under the hood so perhaps it's okay?

Also note that this would still allow passing a custom JSON object with the json! macro:

Output::success(json!({ "hello": "world" })).exit()

Reason is that json! returns a Value whose Serialize instance produces the same Value when given to to_value.

}
}
}
17 changes: 10 additions & 7 deletions relayer-cli/src/commands/keys/list.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use crate::application::app_config;
use abscissa_core::{Command, Options, Runnable};
use serde_json::json;

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 {
Expand Down Expand Up @@ -37,17 +39,18 @@ impl Runnable for KeysListCmd {

let opts = match self.validate_options(&config) {
Err(err) => {
status_err!("invalid options: {}", err);
return;
return Output::with_error().with_result(json!(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::with_success().with_result(json!(r)).exit(),
Err(e) => Output::with_error()
.with_result(json!(format!("{}", e)))
.exit(),
}
}
}
17 changes: 10 additions & 7 deletions relayer-cli/src/commands/keys/restore.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use crate::application::app_config;
use abscissa_core::{Command, Options, Runnable};
use serde_json::json;

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 {
Expand Down Expand Up @@ -55,8 +57,7 @@ impl Runnable for KeyRestoreCmd {

let opts = match self.validate_options(&config) {
Err(err) => {
status_err!("invalid options: {}", err);
return;
return Output::with_error().with_result(json!(err)).exit();
}
Ok(result) => result,
};
Expand All @@ -65,8 +66,10 @@ 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::with_success().with_result(json!(r)).exit(),
Err(e) => Output::with_error()
.with_result(json!(format!("{}", e)))
.exit(),
}
}
}
25 changes: 19 additions & 6 deletions relayer-cli/src/commands/light/rm.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use std::{io, io::Write, ops::Deref};

use crate::prelude::*;

use abscissa_core::{application::fatal_error, error::BoxError, Command, Options, Runnable};
use serde_json::json;
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 {
Expand Down Expand Up @@ -88,20 +90,31 @@ 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::with_success()
.with_result(json!(format!(
"Removed light client peer(s) '{:?}'",
rmd_peers
)))
.exit();

Ok(())
}
}
Expand Down
2 changes: 1 addition & 1 deletion relayer-cli/src/commands/query/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl Runnable for QueryChannelEndCmd {
}
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
Expand Down
7 changes: 4 additions & 3 deletions relayer-cli/src/commands/query/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use abscissa_core::{Command, Options, Runnable};
use serde_json::json;
use tendermint_proto::Protobuf;
use tokio::runtime::Runtime as TokioRuntime;
use tracing::info;

use ibc::ics02_client::client_def::{AnyClientState, AnyConsensusState};
use ibc::ics02_client::raw::ConnectionIds as ConnectionIDs;
Expand Down Expand Up @@ -75,7 +76,7 @@ impl Runnable for QueryClientStateCmd {
}
Ok(result) => result,
};
status_info!("Options", "{:?}", opts);
info!("Options {:?}", opts);

let rt = Arc::new(TokioRuntime::new().unwrap());
let chain = CosmosSDKChain::bootstrap(chain_config, rt).unwrap();
Expand Down Expand Up @@ -169,7 +170,7 @@ impl Runnable for QueryClientConsensusCmd {
}
Ok(result) => result,
};
status_info!("Options", "{:?}", opts);
info!("Options {:?}", opts);

let rt = Arc::new(TokioRuntime::new().unwrap());
let chain = CosmosSDKChain::bootstrap(chain_config, rt).unwrap();
Expand Down Expand Up @@ -280,7 +281,7 @@ impl Runnable for QueryClientConnectionsCmd {
}
Ok(result) => result,
};
status_info!("Options", "{:?}", opts);
info!("Options {:?}", opts);

let rt = Arc::new(TokioRuntime::new().unwrap());
let chain = CosmosSDKChain::bootstrap(chain_config, rt).unwrap();
Expand Down
4 changes: 2 additions & 2 deletions relayer-cli/src/commands/query/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl Runnable for QueryConnectionEndCmd {
}
Ok(result) => result,
};
status_info!("Options", "{:?}", opts);
info!("Options {:?}", opts);

let rt = Arc::new(TokioRuntime::new().unwrap());
let chain = CosmosSDKChain::bootstrap(chain_config, rt).unwrap();
Expand Down Expand Up @@ -152,7 +152,7 @@ impl Runnable for QueryConnectionChannelsCmd {
}
Ok(result) => result,
};
status_info!("Options", "{:?}", opts);
info!("Options {:?}", opts);

let rt = Arc::new(TokioRuntime::new().unwrap());
let chain = CosmosSDKChain::bootstrap(chain_config, rt).unwrap();
Expand Down
12 changes: 6 additions & 6 deletions relayer-cli/src/commands/query/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl Runnable for QueryPacketCommitmentsCmd {
}
Ok(result) => result,
};
status_info!("Options", "{:?}", opts);
info!("Options {:?}", opts);

let rt = Arc::new(TokioRuntime::new().unwrap());
let chain = CosmosSDKChain::bootstrap(chain_config, rt).unwrap();
Expand Down Expand Up @@ -142,7 +142,7 @@ impl Runnable for QueryPacketCommitmentCmd {
}
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 packet commitment ibc-0 transfer ibconexfer 3 --height 3
Expand Down Expand Up @@ -224,7 +224,7 @@ impl Runnable for QueryUnreceivedPacketsCmd {
}
Ok(result) => result,
};
status_info!("Options", "{:?}", opts);
info!("Options {:?}", opts);

let rt = Arc::new(TokioRuntime::new().unwrap());
let src_chain = CosmosSDKChain::bootstrap(src_chain_config, rt.clone()).unwrap();
Expand Down Expand Up @@ -313,7 +313,7 @@ impl Runnable for QueryPacketAcknowledgementsCmd {
}
Ok(result) => result,
};
status_info!("Options", "{:?}", opts);
info!("Options {:?}", opts);

let rt = Arc::new(TokioRuntime::new().unwrap());
let chain = CosmosSDKChain::bootstrap(chain_config, rt).unwrap();
Expand Down Expand Up @@ -392,7 +392,7 @@ impl Runnable for QueryPacketAcknowledgmentCmd {
}
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 packet acknowledgment ibc-0 transfer ibconexfer --height 3
Expand Down Expand Up @@ -471,7 +471,7 @@ impl Runnable for QueryUnreceivedAcknowledgementCmd {
}
Ok(result) => result,
};
status_info!("Options", "{:?}", opts);
info!("Options {:?}", opts);

let rt = Arc::new(TokioRuntime::new().unwrap());
let src_chain = CosmosSDKChain::bootstrap(src_chain_config, rt.clone()).unwrap();
Expand Down
Loading