Replace positional arguments with flags in CLI (#2275)

* Updated CLI commands to take flags everywhere and updated e2e tests accordingly

* Updated gm to use flags when calling Hermes

* Added missing flags to e2e test for 'query client state' command

* Fixed flag errors in e2e tests and removed conflicting short flag.

* Removed all short flags and updated CLI commands comments

* Removed forgotten short flags.

* Updated Hermes guide with flags instead of positional arguments

* Updated script and comment with new long flags for Hermes

* Completed 'tx raw upgrade-' commands guide page. Updated Testing client upgrade guide page

* Added changelog entry

* Added example unit-tests to the 'keys add' command

* Added value names to parameters and removed cli parsing unit-tests

* Cargo fmt changes

* Updated flags in order to reflect ADR 010

* Updated guide to reflect flag changes from ADR 010

* Updated gm script and e2e tests to match flag changes from ADR 010

* Fixed ADR 010 typo

* Remove short flags from `key` error messages

* Fix inconsistent error messaging in channel.rs

* Fix more inconsistent error messaging

* Run cargo fmt

* Fix commands that use deprecated `-c` flag

* Added the query transfer subcommand in docs.

* Scrub query clients

* Scrub query client state

* Removed excessive debug line

* Disabled filtering for query channels CLI

* Added aliases for 'connection', 'channel' and 'sequence' flags

* Fixed error in CLI replacing HeightQuery with QueryHeight

* Fix cargo fmt

* Changed 'version' to 'channel-version' for 'hermes create channel' command

* Separated 'hermes create channel' help into short and long message

* Updated 'hermes listen' so that '--events' flag can take multiple values

* Updated guide with new '--events' flag for 'hermes listen'

* Start all arguments help text with an uppercase letter, to match Clap's help for autogenerated arguments (eg. --help)

* Update guide to account for argument help text starting with an uppercase letter

* Add shortened aliases for `tx raw` commands

Co-authored-by: Sean Chen <seanchen11235@gmail.com>
Co-authored-by: Adi Seredinschi <adi@informal.systems>
Co-authored-by: Romain Ruetschi <romain@informal.systems>

.changelog/unreleased/breaking-changes/ibc-relayer-cli/2239-changed-args-to-flags.md

@@ -0,0 +1,2 @@
+- Updated all CLI commands to take flags instead of positional arguments.
+  ([#2239](https://github.com/informalsystems/ibc-rs/issues/2239))

ci/e2e.sh

@@ -16,7 +16,7 @@ echo "--------------------------------------------------------------------------
 echo "Show relayer version"
 echo "-----------------------------------------------------------------------------------------------------------------"
 echo Config: "$CONFIG_PATH"
-$RELAYER_CMD -c "$CONFIG_PATH" version
+$RELAYER_CMD --config "$CONFIG_PATH" version
 echo "-----------------------------------------------------------------------------------------------------------------"
 echo "Setting up chains"
 echo "-----------------------------------------------------------------------------------------------------------------"
@@ -34,10 +34,10 @@ echo "==========================================================================
 echo "-----------------------------------------------------------------------------------------------------------------"
 echo "Add keys for chains"
 echo "-----------------------------------------------------------------------------------------------------------------"
-hermes -c "$CONFIG_PATH" keys add "$CHAIN_A" -f user_seed_"$CHAIN_A".json
-hermes -c "$CONFIG_PATH" keys add "$CHAIN_B" -f user_seed_"$CHAIN_B".json
-hermes -c "$CONFIG_PATH" keys add "$CHAIN_A" -f user2_seed_"$CHAIN_A".json  -k user2
-hermes -c "$CONFIG_PATH" keys add "$CHAIN_B" -f user2_seed_"$CHAIN_B".json  -k user2
+hermes --config "$CONFIG_PATH" keys add --chain "$CHAIN_A" --key-file user_seed_"$CHAIN_A".json
+hermes --config "$CONFIG_PATH" keys add --chain "$CHAIN_B" --key-file user_seed_"$CHAIN_B".json
+hermes --config "$CONFIG_PATH" keys add --chain "$CHAIN_A" --key-file user2_seed_"$CHAIN_A".json  --key-name user2
+hermes --config "$CONFIG_PATH" keys add --chain "$CHAIN_B" --key-file user2_seed_"$CHAIN_B".json  --key-name user2
 
 echo "================================================================================================================="
 echo "                                             END-TO-END TESTS                                                    "

docs/architecture/adr-010-unified-cli-arguments-hermes.md

@@ -15,7 +15,7 @@ To avoid confusion, all the parameters should take long flags. The following app
 
 * Only long flags are used in order to avoid having nonintuitive flags or conflicting flags.
 * Any parameter ending with `_id` should have the `_id` removed from the flag to shorten it. For example the flag for `chain_id` should only be `chain`.
-* Flags which can be shortened and still be meaningful should be shortened. This is done for `connection`, `channel` and `sequence`, which become respectively `conn`, `chan` and `seq`.
+* Flags which can be shortened and still be meaningful should have a shortened alias. This is done for `connection`, `channel` and `sequence`, which have respectively `conn`, `chan` and `seq` aliases.
 * For the channel and connection creation CLIs, the objects at the two ends are prefixed by `--a-` and `--b-`. Example `--a-chain` and `--b-chain`.
 * Whenever `chain`, `conn`, `chan` and `port` flags have to be disambiguated with a specifier, the specifier will be a prefix. Example of specifiers we currently use are `host`, `reference`, `a`, `b` and `counterparty`.
 
@@ -50,7 +50,7 @@ The following commands are implemented, with the binary name `hermes` often omit
 
 ### Create a channel
 
-* `create channel --a-chain <A_CHAIN_ID> --a-conn <A_CONNECTION_ID> --a-port <A_PORT_ID> --b-port <B_PORT_ID>`
+* `create channel --a-chain <A_CHAIN_ID> --a-connection <A_CONNECTION_ID> --a-port <A_PORT_ID> --b-port <B_PORT_ID>`
     * Optional: `[--chan-version <VERSION>] [--order <ORDER>]`
 
 * `create channel --a-chain <A_CHAIN_ID> --b-chain <B_CHAIN_ID> --a-port <A_PORT_ID> --b-port <B_PORT_ID> --new-client-conn`
@@ -73,7 +73,7 @@ The following commands are implemented, with the binary name `hermes` often omit
 ### Listen
 
 * `listen --chain <CHAIN_ID>`
-    * Optional: `[--event <EVENT>]`
+    * Optional: `[--events <EVENT>...]`
 
 ### Misbehaviour
 
@@ -86,7 +86,7 @@ The following commands are implemented, with the binary name `hermes` often omit
 
 ### Clear packets
 
-* `clear packets --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID>`
+* `clear packets --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID>`
 
 ### Queries
 
@@ -109,44 +109,48 @@ __Client__
 
 __Connection__
 
-* `query connection channels --chain <CHAIN_ID> --conn <CONNECTION_ID>`
+* `query connection channels --chain <CHAIN_ID> --connection <CONNECTION_ID>`
 
-* `query connection end --chain <CHAIN_ID> --conn <CONNECTION_ID>`
+* `query connection end --chain <CHAIN_ID> --connection <CONNECTION_ID>`
     * Optional: `[--height <HEIGHT>]`
 
 * `query connections --chain <CHAIN_ID>`
-    * Optional: `[--chain-counterparty <CHAIN_COUNTERPARTY_ID>] [--verbose]`
+    * Optional: `[--counterparty-chain <COUNTERPARTY_CHAIN_ID>] [--verbose]`
 
 __Channel__
 
-* `query channel client --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID>`
+* `query channel client --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID>`
 
-* `query channel end --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID>`
+* `query channel end --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID>`
     * Optional: `[--height <HEIGHT>]`
 
-* `query channel full --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID>`
+* `query channel full --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID>`
     * Optional: `[--height <HEIGHT>] [--verbose]`
 
 * `query channels --chain <CHAIN_ID>`
     * Optional: `[--counterparty-chain <COUNTERPARTY_CHAIN_ID>] [--verbose]`
 
 __Packet__
 
-* `query packet ack --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID> --seq <SEQUENCE>`
+* `query packet ack --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID> --sequence <SEQUENCE>`
     * Optional: `[--height <HEIGHT>]`
 
-* `query packet acks --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID>`
+* `query packet acks --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID>`
 
-* `query packet commitment --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID> --seq <SEQUENCE>`
+* `query packet commitment --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID> --sequence <SEQUENCE>`
     * Optional: `[--height <HEIGHT>]`
 
-* `query packet commitments --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID>`
+* `query packet commitments --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID>`
+
+* `query packet pending --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID>`
+
+* `query packet unreceived-acks --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID>`
 
-* `query packet pending --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID>`
+* `query packet unreceived-packets --chain <CHAIN_ID> --port <PORT_ID> --channel <CHANNEL_ID>`
 
-* `query packet unreceived-acks --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID>`
+__Transfer__
 
-* `query packet unreceived-packets --chain <CHAIN_ID> --port <PORT_ID> --chan <CHANNEL_ID>`
+* `query transfer denom-trace --chain <CHAIN_ID> --hash <HASH>`
 
 __Tx__
 

e2e/e2e/channel.py

@@ -27,9 +27,9 @@ class TxChanOpenInit(Cmd[TxChanOpenInitRes]):
     ordering: Optional[Ordering] = None
 
     def args(self) -> List[str]:
-        args = [self.dst_chain_id, self.src_chain_id,
-                self.connection_id,
-                self.dst_port_id, self.src_port_id]
+        args = ["--dst-chain", self.dst_chain_id, "--src-chain", self.src_chain_id,
+                "--dst-conn", self.connection_id,
+                "--dst-port", self.dst_port_id, "--src-port", self.src_port_id]
 
         if self.ordering is not None:
             args.extend(['--ordering', str(self.ordering)])
@@ -65,10 +65,10 @@ class TxChanOpenTry(Cmd[TxChanOpenTryRes]):
     ordering: Optional[Ordering] = None
 
     def args(self) -> List[str]:
-        args = [self.dst_chain_id, self.src_chain_id,
-                self.connection_id,
-                self.dst_port_id, self.src_port_id,
-                "-s", self.src_channel_id]
+        args = ["--dst-chain", self.dst_chain_id, "--src-chain", self.src_chain_id,
+                "--dst-conn", self.connection_id,
+                "--dst-port", self.dst_port_id, "--src-port", self.src_port_id,
+                "--src-chan", self.src_channel_id]
 
         if self.ordering is not None:
             args.extend(['--ordering', str(self.ordering)])
@@ -104,11 +104,11 @@ class TxChanOpenAck(Cmd[TxChanOpenAckRes]):
     src_channel_id: ChannelId
 
     def args(self) -> List[str]:
-        args = [self.dst_chain_id, self.src_chain_id,
-                self.connection_id,
-                self.dst_port_id, self.src_port_id,
-                "-d", self.dst_channel_id,
-                "-s", self.src_channel_id]
+        args = ["--dst-chain", self.dst_chain_id, "--src-chain", self.src_chain_id,
+                "--dst-conn", self.connection_id,
+                "--dst-port", self.dst_port_id, "--src-port", self.src_port_id,
+                "--dst-chan", self.dst_channel_id,
+                "--src-chan", self.src_channel_id]
 
         return args
 
@@ -141,11 +141,11 @@ class TxChanOpenConfirm(Cmd[TxChanOpenConfirmRes]):
     src_channel_id: ChannelId
 
     def args(self) -> List[str]:
-        args = [self.dst_chain_id, self.src_chain_id,
-                self.connection_id,
-                self.dst_port_id, self.src_port_id,
-                "-d", self.dst_channel_id,
-                "-s", self.src_channel_id]
+        args = ["--dst-chain", self.dst_chain_id, "--src-chain", self.src_chain_id,
+                "--dst-conn", self.connection_id,
+                "--dst-port", self.dst_port_id, "--src-port", self.src_port_id,
+                "--dst-chan", self.dst_channel_id,
+                "--src-chan", self.src_channel_id]
 
         return args
 
@@ -177,11 +177,11 @@ class TxChanCloseInit(Cmd[TxChanCloseInitRes]):
     src_chan_id: ChannelId
 
     def args(self) -> List[str]:
-        args = [self.dst_chain_id, self.src_chain_id,
-                self.dst_conn_id,
-                self.dst_port_id, self.src_port_id,
-                "-d", self.dst_chan_id,
-                "-s", self.src_chan_id]
+        args = ["--dst-chain", self.dst_chain_id, "--src-chain", self.src_chain_id,
+                "--dst-conn", self.dst_conn_id,
+                "--dst-port", self.dst_port_id, "--src-port", self.src_port_id,
+                "--dst-chan", self.dst_chan_id,
+                "--src-chan", self.src_chan_id]
 
         return args
 
@@ -214,11 +214,11 @@ class TxChanCloseConfirm(Cmd[TxChanCloseConfirmRes]):
     src_chan_id: ChannelId
 
     def args(self) -> List[str]:
-        args = [self.dst_chain_id, self.src_chain_id,
-                self.dst_conn_id,
-                self.dst_port_id, self.src_port_id,
-                "-d", self.dst_chan_id,
-                "-s", self.src_chan_id]
+        args = ["--dst-chain", self.dst_chain_id, "--src-chain", self.src_chain_id,
+                "--dst-conn", self.dst_conn_id,
+                "--dst-port", self.dst_port_id, "--src-port", self.src_port_id,
+                "--dst-chan", self.dst_chan_id,
+                "--src-chan", self.src_chan_id]
 
         return args
 
@@ -267,7 +267,7 @@ class QueryChannelEnd(Cmd[ChannelEnd]):
     channel_id: ChannelId
 
     def args(self) -> List[str]:
-        return [self.chain_id, self.port_id, self.channel_id]
+        return ["--chain", self.chain_id, "--port", self.port_id, "--chan", self.channel_id]
 
     def process(self, result: Any) -> ChannelEnd:
         return from_dict(ChannelEnd, result)
@@ -280,7 +280,7 @@ class QueryChannelEnds(Cmd[ChannelEnds]):
     channel_id: ChannelId
 
     def args(self) -> List[str]:
-        return [self.chain_id, self.port_id, self.channel_id]
+        return ["--chain", self.chain_id, "--port", self.port_id, "--chan", self.channel_id]
 
     def process(self, result: Any) -> ChannelEnds:
         return from_dict(ChannelEnds, result)

e2e/e2e/client.py

@@ -19,7 +19,7 @@ class TxCreateClient(Cmd[ClientCreated]):
     src_chain_id: ChainId
 
     def args(self) -> List[str]:
-        return [self.dst_chain_id, self.src_chain_id]
+        return ["--host-chain", self.dst_chain_id, "--reference-chain", self.src_chain_id]
 
     def process(self, result: Any) -> ClientCreated:
         return from_dict(ClientCreated, result['CreateClient'])
@@ -43,7 +43,7 @@ class TxUpdateClient(Cmd[ClientUpdated]):
     dst_client_id: ClientId
 
     def args(self) -> List[str]:
-        return [self.dst_chain_id, self.dst_client_id]
+        return ["--host-chain", self.dst_chain_id, "--client", self.dst_client_id]
 
     def process(self, result: Any) -> ClientUpdated:
         return from_dict(ClientUpdated, result[-1]['UpdateClient']['common'])
@@ -86,7 +86,7 @@ def args(self) -> List[str]:
         if self.proof:
             args.append('--proof')
 
-        args.extend([self.chain_id, self.client_id])
+        args.extend(["--chain", self.chain_id, "--client", self.client_id])
 
         return args
 

e2e/e2e/cmd.py

@@ -55,7 +55,7 @@ def to_cmd(self) -> str:
         return f"{self.name} {' '.join(self.args())}"
 
     def run(self, config: Config, retries: int = 0) -> CmdResult[T]:
-        full_cmd = f'{config.relayer_cmd} -c {config.config_file} --json'.split(' ')
+        full_cmd = f'{config.relayer_cmd} --config {config.config_file} --json'.split(' ')
         full_cmd.extend(self.name.split(' '))
         full_cmd.extend(self.args())
         l.debug(' '.join(full_cmd))

e2e/e2e/connection.py

@@ -22,8 +22,8 @@ class TxConnInit(Cmd[TxConnInitRes]):
     src_client_id: ClientId
 
     def args(self) -> List[str]:
-        return [self.dst_chain_id, self.src_chain_id,
-                self.dst_client_id, self.src_client_id]
+        return ["--dst-chain", self.dst_chain_id, "--src-chain", self.src_chain_id,
+                "--dst-client", self.dst_client_id, "--src-client", self.src_client_id]
 
     def process(self, result: Any) -> TxConnInitRes:
         return from_dict(TxConnInitRes, result['OpenInitConnection'])
@@ -46,9 +46,9 @@ class TxConnTry(Cmd[TxConnTryRes]):
     src_conn_id: ConnectionId
 
     def args(self) -> List[str]:
-        return [self.dst_chain_id, self.src_chain_id,
-                self.dst_client_id, self.src_client_id,
-                "-s", self.src_conn_id]
+        return ["--dst-chain", self.dst_chain_id, "--src-chain", self.src_chain_id,
+                "--dst-client", self.dst_client_id, "--src-client", self.src_client_id,
+                "--src-conn", self.src_conn_id]
 
     def process(self, result: Any) -> TxConnTryRes:
         return from_dict(TxConnTryRes, result['OpenTryConnection'])
@@ -72,10 +72,10 @@ class TxConnAck(Cmd[TxConnAckRes]):
     src_conn_id: ConnectionId
 
     def args(self) -> List[str]:
-        return [self.dst_chain_id, self.src_chain_id,
-                self.dst_client_id, self.src_client_id,
-                "-d", self.dst_conn_id,
-                "-s", self.src_conn_id]
+        return ["--dst-chain", self.dst_chain_id, "--src-chain", self.src_chain_id,
+                "--dst-client", self.dst_client_id, "--src-client", self.src_client_id,
+                "--dst-conn", self.dst_conn_id,
+                "--src-conn", self.src_conn_id]
 
     def process(self, result: Any) -> TxConnAckRes:
         return from_dict(TxConnAckRes, result['OpenAckConnection'])
@@ -99,10 +99,10 @@ class TxConnConfirm(Cmd[TxConnConfirmRes]):
     src_conn_id: ConnectionId
 
     def args(self) -> List[str]:
-        return [self.dst_chain_id, self.src_chain_id,
-                self.dst_client_id, self.src_client_id,
-                "-d", self.dst_conn_id,
-                "-s", self.src_conn_id]
+        return ["--dst-chain", self.dst_chain_id, "--src-chain", self.src_chain_id,
+                "--dst-client", self.dst_client_id, "--src-client", self.src_client_id,
+                "--dst-conn", self.dst_conn_id,
+                "--src-conn", self.src_conn_id]
 
     def process(self, result: Any) -> TxConnConfirmRes:
         return from_dict(TxConnConfirmRes, result['OpenConfirmConnection'])
@@ -139,7 +139,7 @@ class QueryConnectionEnd(Cmd[ConnectionEnd]):
     connection_id: ConnectionId
 
     def args(self) -> List[str]:
-        return [self.chain_id, self.connection_id]
+        return ["--chain", self.chain_id, "--conn", self.connection_id]
 
     def process(self, result: Any) -> ConnectionEnd:
         return from_dict(ConnectionEnd, result)