diff --git a/.changelog/unreleased/improvements/relayer-cli/1421-create-channel-cli.md b/.changelog/unreleased/improvements/relayer-cli/1421-create-channel-cli.md new file mode 100644 index 0000000000..a64398d712 --- /dev/null +++ b/.changelog/unreleased/improvements/relayer-cli/1421-create-channel-cli.md @@ -0,0 +1,2 @@ +- Change `create channel` CLI command such that it is more difficult to create + clients / connections using it ([#1421](https://github.com/informalsystems/ibc-rs/issues/1421)) diff --git a/guide/src/commands/path-setup/index.md b/guide/src/commands/path-setup/index.md index 60c5243407..620a356860 100644 --- a/guide/src/commands/path-setup/index.md +++ b/guide/src/commands/path-setup/index.md @@ -40,4 +40,4 @@ DESCRIPTION: SUBCOMMANDS: help Get usage information client Update an IBC client -``` \ No newline at end of file +``` diff --git a/relayer-cli/Cargo.toml b/relayer-cli/Cargo.toml index 7317f1c8cf..16c2aabd3d 100644 --- a/relayer-cli/Cargo.toml +++ b/relayer-cli/Cargo.toml @@ -56,6 +56,8 @@ itertools = "0.10.3" atty = "0.2.14" flex-error = { version = "0.4.4", default-features = false, features = ["std", "eyre_tracer"] } signal-hook = "0.3.13" +dialoguer = "0.10.0" +console = "0.15.0" [dependencies.tendermint-proto] version = "=0.23.5" diff --git a/relayer-cli/src/commands/create.rs b/relayer-cli/src/commands/create.rs index 46b04e4c51..f2e9ea3a95 100644 --- a/relayer-cli/src/commands/create.rs +++ b/relayer-cli/src/commands/create.rs @@ -18,6 +18,8 @@ pub enum CreateCmds { /// Create a new connection between two chains Connection(CreateConnectionCommand), - /// Create a new channel between two chains + /// Create a new channel between two chains using a pre-existing connection. + /// Alternatively, create a new client and a new connection underlying + /// the new channel if a pre-existing connection is not provided. Channel(CreateChannelCommand), } diff --git a/relayer-cli/src/commands/create/channel.rs b/relayer-cli/src/commands/create/channel.rs index c8fd42f9bb..5118ae1a79 100644 --- a/relayer-cli/src/commands/create/channel.rs +++ b/relayer-cli/src/commands/create/channel.rs @@ -1,6 +1,9 @@ use abscissa_core::clap::Parser; use abscissa_core::{Command, Runnable}; +use console::style; +use dialoguer::Confirm; + use ibc::core::ics02_client::client_state::ClientState; use ibc::core::ics03_connection::connection::IdentifiedConnectionEnd; use ibc::core::ics04_channel::channel::Order; @@ -17,43 +20,63 @@ use crate::conclude::{exit_with_unrecoverable_error, Output}; use crate::prelude::*; use ibc_relayer::config::default::connection_delay; +static PROMPT: &str = "Are you sure you want a new connection & clients to be created? Hermes will use default security parameters."; +static HINT: &str = "Consider using the default invocation\n\nhermes create channel --port-a --port-b \n\nto re-use a pre-existing connection."; + +/// The data structure that represents all the possible options when invoking +/// the `create channel` CLI command. +/// +/// There are two possible ways to invoke this command: +/// +/// `create channel --port-a --port-b ` is the default +/// way in which this command should be used, specifying a `Connection-ID` for this new channel +/// to re-use. The command expects that `Connection-ID` is associated with chain A. +/// +/// `create channel --port-a --port-b --new-client-connection` +/// to indicate that a new connection/client pair is being created as part of this new channel. +/// This brings up an interactive yes/no prompt to ensure that the operator at least +/// considers the fact that they're initializing a new connection with the channel. +/// +/// Note that `Connection-ID`s have to be considered based off of the chain's perspective. Although +/// chain A and chain B might refer to the connection with different names, they are actually referring +/// to the same connection. #[derive(Clone, Command, Debug, Parser)] #[clap(disable_version_flag = true)] pub struct CreateChannelCommand { #[clap( required = true, - help = "identifier of the side `a` chain for the new channel" + help = "Identifier of the side `a` chain for the new channel" )] - chain_a_id: ChainId, - - #[clap(help = "identifier of the side `b` chain for the new channel (optional)")] - chain_b_id: Option, + chain_a: ChainId, #[clap( short, long, - help = "identifier of the connection on chain `a` to use in creating the new channel" + help = "Identifier of the side `b` chain for the new channel" )] + chain_b: Option, + + /// Identifier of the connection on chain `a` to use in creating the new channel. connection_a: Option, #[clap( long, required = true, - help = "identifier of the side `a` port for the new channel" + help = "Identifier of the side `a` port for the new channel" )] port_a: PortId, #[clap( long, required = true, - help = "identifier of the side `b` port for the new channel" + help = "Identifier of the side `b` port for the new channel" )] port_b: PortId, #[clap( short, long, - help = "the channel ordering, valid options 'unordered' (default) and 'ordered'", + help = "The channel ordering, valid options 'unordered' (default) and 'ordered'", default_value_t )] order: Order, @@ -62,34 +85,68 @@ pub struct CreateChannelCommand { short, long = "channel-version", alias = "version", - help = "the version for the new channel" + help = "The version for the new channel" )] version: Option, + + #[clap( + long, + help = "Indicates that a new client and connection will be created underlying the new channel" + )] + new_client_connection: bool, } impl Runnable for CreateChannelCommand { fn run(&self) { - match &self.chain_b_id { - None => self.run_reusing_connection(), - Some(chain_b) => self.run_using_new_connection(chain_b), + match &self.connection_a { + Some(conn) => self.run_reusing_connection(conn), + None => match &self.chain_b { + Some(chain_b) => { + if self.new_client_connection { + match Confirm::new() + .with_prompt(format!( + "{}: {}\n{}: {}", + style("WARN").yellow(), + PROMPT, + style("Hint").cyan(), + HINT + )) + .interact() + { + Ok(confirm) => { + if confirm { + self.run_using_new_connection(chain_b); + } else { + Output::error("You elected not to create new clients and connections. Please re-invoke `create channel` with a pre-existing connection ID".to_string()).exit(); + } + } + Err(e) => { + Output::error(format!( + "An error occurred while waiting for user input: {}", + e + )); + } + } + } else { + Output::error( + "The `--new-client-connection` flag is required if invoking with `--chain-b`".to_string() + ) + .exit(); + } + } + None => Output::error("Missing one of `` or ``".to_string()) + .exit(), + }, } } } impl CreateChannelCommand { - // Creates a new channel, as well as a new underlying connection and clients. - fn run_using_new_connection(&self, chain_b_id: &ChainId) { + /// Creates a new channel, as well as a new underlying connection and clients. + fn run_using_new_connection(&self, chain_b: &ChainId) { let config = app_config(); - // Bail with an explicit error. The user might be expecting to use this connection. - if self.connection_a.is_some() { - Output::error( - "Option `` is incompatible with ``".to_string(), - ) - .exit(); - } - - let chains = ChainHandlePair::spawn(&config, &self.chain_a_id, chain_b_id) + let chains = ChainHandlePair::spawn(&config, &self.chain_a, chain_b) .unwrap_or_else(exit_with_unrecoverable_error); info!( @@ -119,39 +176,29 @@ impl CreateChannelCommand { Output::success(channel).exit(); } - // Creates a new channel, reusing an already existing connection and its clients. - fn run_reusing_connection(&self) { + /// Creates a new channel, reusing an already existing connection and its clients. + fn run_reusing_connection(&self, connection_a: &ConnectionId) { let config = app_config(); // Validate & spawn runtime for side a. - let chain_a = spawn_chain_runtime(&config, &self.chain_a_id) + let chain_a = spawn_chain_runtime(&config, &self.chain_a) .unwrap_or_else(exit_with_unrecoverable_error); - // Unwrap the identifier of the connection on side a. - let connection_a_id = match &self.connection_a { - Some(c) => c, - None => Output::error( - "Option `--connection-a` is necessary when argument is missing" - .to_string(), - ) - .exit(), - }; - // Query the connection end. let height = Height::new(chain_a.id().version(), 0); let conn_end = chain_a - .query_connection(connection_a_id, height) + .query_connection(connection_a, height) .unwrap_or_else(exit_with_unrecoverable_error); // Query the client state, obtain the identifier of chain b. - let chain_b_id = chain_a + let chain_b = chain_a .query_client_state(conn_end.client_id(), height) .map(|cs| cs.chain_id()) .unwrap_or_else(exit_with_unrecoverable_error); // Spawn the runtime for side b. let chain_b = - spawn_chain_runtime(&config, &chain_b_id).unwrap_or_else(exit_with_unrecoverable_error); + spawn_chain_runtime(&config, &chain_b).unwrap_or_else(exit_with_unrecoverable_error); // Create the foreign client handles. let client_a = ForeignClient::find(chain_b.clone(), chain_a.clone(), conn_end.client_id()) @@ -159,7 +206,7 @@ impl CreateChannelCommand { let client_b = ForeignClient::find(chain_a, chain_b, conn_end.counterparty().client_id()) .unwrap_or_else(exit_with_unrecoverable_error); - let identified_end = IdentifiedConnectionEnd::new(connection_a_id.clone(), conn_end); + let identified_end = IdentifiedConnectionEnd::new(connection_a.clone(), conn_end); let connection = Connection::find(client_a, client_b, &identified_end) .unwrap_or_else(exit_with_unrecoverable_error);