From f954a47d78febbe3ec1500cda22c75c15294be00 Mon Sep 17 00:00:00 2001 From: Sean Chen Date: Thu, 7 Jul 2022 04:08:24 -0500 Subject: [PATCH] Add `--upgrade-height` parameter to `upgrade client` command (#2357) Adds a flag --upgrade-height parameter that receives a Height parameter from the user that specifies the application height at which the host chain should halt before performing the client upgrade. --- * Pull in upstream changes * Add required `upgrade-height` flag to `upgrade client` command * Sleep until src application height reaches the target input height * Formatting * Have `upgrade client` command loop until chain reaches target height * Add gm.toml example file to guide and change thread sleep duration * Remove reference to dev-env * Add info's to wait loop and remove dev-env mentions from guide * Saving progress * Slight cleanup * Rename src -> host * Clean up example gm.toml * Fix host -> reference language * Quick fixes to the Hermes guide for testing the 'upgrade client' command * Added new upgrade-height flag to REQUIRED flags in help output. Fixed unit tests and added new test for the new flag * fix upgrade logic * fmt * naming * Updated guide and comment for 'upgrade client' command * Add changelog entry * Updated 'Testing client upgrade' in Hermes guide * naming * implement changes to `upgrade-clients` too * Formatting * pass height by value * error message fix * Change info's to debug's * fix race condition with node * Change info's to debug's * Cargo fmt * Updated ADR 010 to include 'upgrade-height' flag for 'upgrade clients' and added a unit test for the flag Co-authored-by: Romain Ruetschi Co-authored-by: Luca Joss Co-authored-by: Philippe Laferriere --- ...e-height-flag-to-upgrade-client-command.md | 2 + .../adr-010-unified-cli-arguments-hermes.md | 4 +- guide/src/commands/upgrade/index.md | 12 +- guide/src/commands/upgrade/test.md | 290 +++++++----------- relayer-cli/src/commands/tx/client.rs | 225 ++++++++++---- relayer/src/foreign_client.rs | 4 +- 6 files changed, 305 insertions(+), 232 deletions(-) create mode 100644 .changelog/unreleased/ibc-relayer-cli/2300-add-upgrade-height-flag-to-upgrade-client-command.md diff --git a/.changelog/unreleased/ibc-relayer-cli/2300-add-upgrade-height-flag-to-upgrade-client-command.md b/.changelog/unreleased/ibc-relayer-cli/2300-add-upgrade-height-flag-to-upgrade-client-command.md new file mode 100644 index 0000000000..d418522148 --- /dev/null +++ b/.changelog/unreleased/ibc-relayer-cli/2300-add-upgrade-height-flag-to-upgrade-client-command.md @@ -0,0 +1,2 @@ +- Added a required flag `--upgrade-height` that halts the reference chain at the + specified height when performing a client upgrade ([#2300](https://github.com/informalsystems/ibc-rs/issues/2300)) diff --git a/docs/architecture/adr-010-unified-cli-arguments-hermes.md b/docs/architecture/adr-010-unified-cli-arguments-hermes.md index 29f77d14b6..5301b3c106 100644 --- a/docs/architecture/adr-010-unified-cli-arguments-hermes.md +++ b/docs/architecture/adr-010-unified-cli-arguments-hermes.md @@ -35,9 +35,9 @@ The following commands are implemented, with the binary name `hermes` often omit * `update client --host-chain --client ` * Optional: `[--height ] [--trusted-height ]` -* `upgrade client --host-chain --client ` +* `upgrade client --host-chain --client --upgrade-height ` -* `upgrade clients --reference-chain ` +* `upgrade clients --reference-chain --upgrade-height ` * Optional: `[--host-chain ]` ### Create a connection diff --git a/guide/src/commands/upgrade/index.md b/guide/src/commands/upgrade/index.md index 27efdfba22..86274e5b3f 100644 --- a/guide/src/commands/upgrade/index.md +++ b/guide/src/commands/upgrade/index.md @@ -6,14 +6,20 @@ Use the `upgrade client` command to upgrade a client after a chain upgrade. ```shell USAGE: - hermes upgrade client --host-chain --client + hermes upgrade client --host-chain --client --upgrade-height DESCRIPTION: Upgrade an IBC client REQUIRED: - --client Identifier of the client to be upgraded - --host-chain Identifier of the chain that hosts the client + --client + Identifier of the client to be upgraded + + --host-chain + Identifier of the chain that hosts the client + + --upgrade-height + The height at which the reference chain halts for the client upgrade ``` __Example__ diff --git a/guide/src/commands/upgrade/test.md b/guide/src/commands/upgrade/test.md index 51ca667291..6082a7096e 100644 --- a/guide/src/commands/upgrade/test.md +++ b/guide/src/commands/upgrade/test.md @@ -2,35 +2,45 @@ ## Prerequisites -- gaiad `(v4.2.*)`, for example: +- gaiad `(v7.0.*)`, for example: ```shell gaiad version --log_level error --long | head -n4 ``` -```shell -name: gaia -server_name: gaiad -version: v4.2.0 -commit: 535be14a8bdbfeb0d950914b5baa2dc72c6b081c -``` - ## Testing procedure -### Setup - -__With dev-env__ - -```shell -./scripts/dev-env ~/.hermes/config.toml ibc-0 ibc-1 +### Setup using Gaia manager +> Note: The `gm.toml` file that we're using here looks like this: +``` +[global] + add_to_hermes = true + auto_maintain_config = true + extra_wallets = 2 + gaiad_binary = "$GOPATH/bin/gaiad" + hdpath = "" + home_dir = "$HOME/.gm" + ports_start_at = 27040 + validator_mnemonic = "" + wallet_mnemonic = "" + + [global.hermes] + binary = "" + config = "$HOME/.hermes/config.toml" + log_level = "info" + telemetry_enabled = true + telemetry_host = "127.0.0.1" + telemetry_port = 3001 + +[ibc-0] + ports_start_at = 27000 + +[ibc-1] + ports_start_at = 27010 ``` -The `one-chain` script is invoked for each chain and modifies the `genesis.json` file to use a short window for governance proposals (`200s` for `max_deposit_period` and `voting_period`). Therefore, an upgrade proposal can be submitted, voted on and accepted within a short time. - - -__With gm__ * Run the command `gm start` -* Go to the file `$HOME/.gm/ibc-0/config/genesis.json` and change `max_deposit_period` and `voting_period` to a lower value, such as 200s -* Run the commands: `gm reset`, `gm hermes config` and `gm keys` +* Go to the file `$HOME/.gm/ibc-0/config/genesis.json` and change `max_deposit_period` and `voting_period` to a lower value, such as 120s +* Run the commands: `gm reset`, `gm hermes config` and `gm hermes keys` ### Test upgrading chain and client @@ -58,100 +68,95 @@ __With gm__ 2. Create and submit an upgrade plan for chain `ibc-0`: Use the hermes test command to make an upgrade proposal. In the example below a software upgrade proposal is made for `ibc-0`, for the height `300` blocks from latest height. `10000000stake` is deposited. - The proposal includes the upgraded client state constructed from the state of `07-tendermint-0` client on `ibc-1` that was created in the previous step. In addition, the `unbonding_period` of the client is set to some new value (`400h`) + The proposal includes the upgraded client state constructed from the state of `07-tendermint-0` client on `ibc-1` that was created in the previous step. ```shell - hermes tx raw upgrade-chain --dst-chain ibc-0 --src-chain ibc-1 --src-client 07-tendermint-0 --amount 10000000 --height-offset 300 + hermes tx raw upgrade-chain --dst-chain ibc-0 --src-chain ibc-1 --src-client 07-tendermint-0 --amount 10000000 --height-offset 60 ``` ```text Success: transaction::Hash(CE98D8D98091BA8016BD852D18056E54C4CB3C4525E7F40DD3C40B4FD0F2482B) ``` - Note that the height offset should be picked such that the proposal plan height is reached after the `200s` voting period. + 3. Verify that the proposal was accepted by querying the upgrade plan to check that it was submitted correctly. - 3. Verify that the proposal was accepted: - - Query the upgrade plan to check that it was submitted correctly. Note the `height` at which the proposal will take effect (chain halts). Also `status: PROPOSAL_STATUS_VOTING_PERIOD`. - - Setup done with dev-env: + > Note: You can find the RPC port used to query the local node by running + > `gm ports` in order to see a list of the ports being used. ```shell - gaiad query gov proposal 1 --home data/ibc-0/ + gaiad --node tcp://localhost:27000 query gov proposal 1 --home $HOME/.gm/ibc-0/ ``` - Setup done with gm: - - ```shell - gaiad --node tcp://localhost: query gov proposal 1 --home $HOME/.gm/ibc-0/ - ``` + If successful, you should see output like this. Note that the status of the proposal near the bottom of the output should be + `PROPOSAL_STATUS_VOTING_PERIOD` indicating that the voting period is still ongoing. ```text content: - '@type': /cosmos.upgrade.v1beta1.SoftwareUpgradeProposal + '@type': /ibc.core.client.v1.UpgradeProposal description: upgrade the chain software and unbonding period plan: - height: "332" - info: upgrade the chain software and unbonding period - name: test + height: "65" + info: "" + name: plan time: "0001-01-01T00:00:00Z" - upgraded_client_state: - '@type': /ibc.lightclients.tendermint.v1.ClientState - allow_update_after_expiry: true - allow_update_after_misbehaviour: true - chain_id: ibc-0 - frozen_height: - revision_height: "0" - revision_number: "0" - latest_height: - revision_height: "333" - revision_number: "0" - max_clock_drift: 0s - proof_specs: - - inner_spec: - child_order: - - 0 - - 1 - child_size: 33 - empty_child: null - hash: SHA256 - max_prefix_length: 12 - min_prefix_length: 4 - leaf_spec: - hash: SHA256 - length: VAR_PROTO - prefix: AA== - prehash_key: NO_HASH - prehash_value: SHA256 - max_depth: 0 - min_depth: 0 - - inner_spec: - child_order: - - 0 - - 1 - child_size: 32 - empty_child: null - hash: SHA256 - max_prefix_length: 1 - min_prefix_length: 1 - leaf_spec: - hash: SHA256 - length: VAR_PROTO - prefix: AA== - prehash_key: NO_HASH - prehash_value: SHA256 - max_depth: 0 - min_depth: 0 - trust_level: - denominator: "0" - numerator: "0" - trusting_period: 0s - unbonding_period: 1440000s - upgrade_path: - - upgrade - - upgradedIBCState - title: upgrade_ibc_clients - deposit_end_time: "2021-04-12T16:33:37.187389Z" + upgraded_client_state: null + title: proposal 0 + upgraded_client_state: + '@type': /ibc.lightclients.tendermint.v1.ClientState + allow_update_after_expiry: false + allow_update_after_misbehaviour: false + chain_id: ibc-0 + frozen_height: + revision_height: "0" + revision_number: "0" + latest_height: + revision_height: "66" + revision_number: "0" + max_clock_drift: 0s + proof_specs: + - inner_spec: + child_order: + - 0 + - 1 + child_size: 33 + empty_child: null + hash: SHA256 + max_prefix_length: 12 + min_prefix_length: 4 + leaf_spec: + hash: SHA256 + length: VAR_PROTO + prefix: AA== + prehash_key: NO_HASH + prehash_value: SHA256 + max_depth: 0 + min_depth: 0 + - inner_spec: + child_order: + - 0 + - 1 + child_size: 32 + empty_child: null + hash: SHA256 + max_prefix_length: 1 + min_prefix_length: 1 + leaf_spec: + hash: SHA256 + length: VAR_PROTO + prefix: AA== + prehash_key: NO_HASH + prehash_value: SHA256 + max_depth: 0 + min_depth: 0 + trust_level: + denominator: "0" + numerator: "0" + trusting_period: 0s + unbonding_period: 1814400s + upgrade_path: + - upgrade + - upgradedIBCState + deposit_end_time: "2022-07-06T15:14:38.993051Z" final_tally_result: abstain: "0" "no": "0" @@ -159,12 +164,12 @@ __With gm__ "yes": "0" proposal_id: "1" status: PROPOSAL_STATUS_VOTING_PERIOD - submit_time: "2021-04-12T16:30:17.187389Z" + submit_time: "2022-07-06T15:12:38.993051Z" total_deposit: - amount: "10000000" denom: stake - voting_end_time: "2021-04-12T16:33:37.187389Z" - voting_start_time: "2021-04-12T16:30:17.187389Z" + voting_end_time: "2022-07-06T15:14:38.993051Z" + voting_start_time: "2022-07-06T15:12:38.993051Z" ``` 4. Vote on the proposal @@ -172,107 +177,48 @@ __With gm__ The parameter `1` should match the `proposal_id:` from the upgrade proposal submitted at step 3. This command must be issued while the proposal status is `PROPOSAL_STATUS_VOTING_PERIOD`. Confirm transaction when prompted. - Setup done with dev-env: - - ```shell - gaiad tx gov vote 1 yes --home data/ibc-0/data/ --keyring-backend test --keyring-dir data/ibc-0/ --chain-id ibc-0 --from validator - ``` - Setup done with gm: - ```shell - gaiad --node tcp://localhost: tx gov vote 1 yes --home $HOME/.gm/ibc-0/data/ --keyring-backend test --keyring-dir $HOME/.gm/ibc-0/ --chain-id ibc-0 --from validator + gaiad --node tcp://localhost:27000 tx gov vote 1 yes --home $HOME/.gm/ibc-0/data/ --keyring-backend test --keyring-dir $HOME/.gm/ibc-0/ --chain-id ibc-0 --from validator ``` ```text confirm transaction before signing and broadcasting [y/N]: y - {"height":"85","txhash":"AC24D80B1BFE0832769DECFDD3B3DF999A363D5E4390B0B673344FFDED9150B2","codespace":"","code":0,"data":"0A060A04766F7465","raw_log":"[{\"events\":[{\"type\":\"message\",\"attributes\":[{\"key\":\"action\",\"value\":\"vote\"},{\"key\":\"module\",\"value\":\"governance\"},{\"key\":\"sender\",\"value\":\"cosmos1srfzw0jkyyn7wf0ps4zy0tuvdaclfj2ufgp6w3\"}]},{\"type\":\"proposal_vote\",\"attributes\":[{\"key\":\"option\",\"value\":\"VOTE_OPTION_YES\"},{\"key\":\"proposal_id\",\"value\":\"1\"}]}]}]","logs":[{"msg_index":0,"log":"","events":[{"type":"message","attributes":[{"key":"action","value":"vote"},{"key":"module","value":"governance"},{"key":"sender","value":"cosmos1srfzw0jkyyn7wf0ps4zy0tuvdaclfj2ufgp6w3"}]},{"type":"proposal_vote","attributes":[{"key":"option","value":"VOTE_OPTION_YES"},{"key":"proposal_id","value":"1"}]}]}],"info":"","gas_wanted":"200000","gas_used":"43716","tx":null,"timestamp":""} + txhash: 50CC1C39FBB14F99580A916ADE7F02883FFCC35D7862153F16BE86138151E17C ``` - 5. Wait approximately 200 seconds until the proposal changes status to `PROPOSAL_STATUS_PASSED`. - Note the `final tally_result` that includes the vote submitted in the previous step. - Setup done with dev-env +5. Test the `upgrade client` CLI - ```shell - gaiad query gov proposal 1 --home data/ibc-0/ - ``` - Setup done with gm: - - ```shell - gaiad --node tcp://localhost: query gov proposal 1 --home $HOME/.gm/ibc-0/ - ``` - - ```text - content: - '@type': /cosmos.upgrade.v1beta1.SoftwareUpgradeProposal - description: upgrade the chain software and unbonding period - ... - final_tally_result: - abstain: "0" - "no": "0" - no_with_veto: "0" - "yes": "100000000000" - proposal_id: "1" - status: PROPOSAL_STATUS_PASSED - submit_time: "2021-04-12T16:30:17.187389Z" - total_deposit: - - amount: "10000000" - denom: stake - voting_end_time: "2021-04-12T16:33:37.187389Z" - voting_start_time: "2021-04-12T16:30:17.187389Z" - ``` - -6. Test the `upgrade client` CLI - - The following command performs the upgrade for client `07-tendermint-0`. It outputs two events, one for the updated client state, + The following command waits for the reference chain `ibc-0` to halt and then performs the upgrade for client `07-tendermint-0` on `ibc-1`. It outputs two events, one for the updated client state, and another for the upgraded state. + The `--upgrade-height 65` value is taken from the `height` in the upgrade plan output. ```shell - hermes upgrade client --host-chain ibc-1 --client 07-tendermint-0 + hermes upgrade client --host-chain ibc-1 --client 07-tendermint-0 --upgrade-height 65 ``` ```json Success: [ UpdateClient( - UpdateClient { - common: Attributes { - height: Height { revision: 1, height: 438 }, - client_id: ClientId( - "07-tendermint-0", - ), - client_type: Tendermint, - consensus_height: Height { revision: 0, height: 440 }, - }, - header: Some( - Tendermint(..) - ), - }, + h: 1-68, cs_h: 07-tendermint-0(0-65), ), UpgradeClient( UpgradeClient( Attributes { - height: Height { revision: 1, height: 438 }, + height: Height { + revision: 1, + height: 68, + }, client_id: ClientId( "07-tendermint-0", ), client_type: Tendermint, - consensus_height: Height { revision: 0, height: 441 }, + consensus_height: Height { + revision: 0, + height: 66, + }, }, ), ), ] ``` - -If the command fails with the error: - -```shell -Error: failed while trying to upgrade client id 07-tendermint-0 for chain ibc-0: failed while fetching from chain the upgraded client state: conversion from a protobuf `Any` into a domain type failed: conversion from a protobuf `Any` into a domain type failed: error converting message type into domain type: the client state was not found -``` - -It might be due to the chain not being at the height defined for the upgrade. This can be checked with the INFO output of the command: - -```shell -INFO ThreadId(01) [ibc-0 -> ibc-1:07-tendermint-0] upgrade Height: 0-82 -``` - -Where in this case the chain is at height 82. diff --git a/relayer-cli/src/commands/tx/client.rs b/relayer-cli/src/commands/tx/client.rs index e1b396a879..5935f3bbbb 100644 --- a/relayer-cli/src/commands/tx/client.rs +++ b/relayer-cli/src/commands/tx/client.rs @@ -1,4 +1,5 @@ -use core::fmt; +use core::{fmt, time::Duration}; +use std::thread; use abscissa_core::clap::Parser; use abscissa_core::{Command, Runnable}; @@ -14,7 +15,7 @@ use ibc_relayer::chain::requests::{ use ibc_relayer::config::Config; use ibc_relayer::foreign_client::{CreateOptions, ForeignClient}; use tendermint_light_client_verifier::types::TrustThreshold; -use tracing::warn; +use tracing::debug; use crate::application::app_config; use crate::cli_utils::{spawn_chain_runtime, spawn_chain_runtime_generic, ChainHandlePair}; @@ -214,18 +215,27 @@ pub struct TxUpgradeClientCmd { help = "Identifier of the client to be upgraded" )] client_id: ClientId, + + #[clap( + long = "upgrade-height", + required = true, + value_name = "REFERENCE_UPGRADE_HEIGHT", + help_heading = "REQUIRED", + help = "The height at which the reference chain halts for the client upgrade" + )] + reference_upgrade_height: u64, } impl Runnable for TxUpgradeClientCmd { fn run(&self) { let config = app_config(); - let dst_chain = match spawn_chain_runtime(&config, &self.chain_id) { + let host_chain = match spawn_chain_runtime(&config, &self.chain_id) { Ok(handle) => handle, Err(e) => Output::error(format!("{}", e)).exit(), }; - let src_chain_id = match dst_chain.query_client_state( + let reference_chain_id = match host_chain.query_client_state( QueryClientStateRequest { client_id: self.client_id.clone(), height: QueryHeight::Latest, @@ -242,32 +252,60 @@ impl Runnable for TxUpgradeClientCmd { } }; - let src_chain = match spawn_chain_runtime(&config, &src_chain_id) { + let reference_chain = match spawn_chain_runtime(&config, &reference_chain_id) { Ok(handle) => handle, Err(e) => Output::error(format!("{}", e)).exit(), }; - let client = ForeignClient::find(src_chain, dst_chain, &self.client_id) + let client = ForeignClient::find(reference_chain, host_chain, &self.client_id) .unwrap_or_else(exit_with_unrecoverable_error); - // Assumption: this query is run while the chain is halted as a result of an upgrade - let src_upgrade_height = { - let src_application_height = match client.src_chain().query_latest_height() { + // In order to perform the client upgrade, the chain is paused at the height specified by + // the user. When the chain is paused, the application height reports a height of 1 less + // than the height according to Tendermint. As a result, the target height at which the + // upgrade occurs at (the application height) is 1 less than the height specified by + // the user, hence the decrement of the upgrade height. + let reference_upgrade_height = Height::new( + client.src_chain().id().version(), + self.reference_upgrade_height, + ) + .unwrap_or_else(exit_with_unrecoverable_error); + + let target_reference_application_height = reference_upgrade_height + .decrement() + .expect("Upgrade height cannot be 1"); + + let mut reference_application_latest_height = match client.src_chain().query_latest_height() + { + Ok(height) => height, + Err(e) => Output::error(format!("{}", e)).exit(), + }; + + debug!( + "Reference application latest height: {}", + reference_application_latest_height + ); + + while reference_application_latest_height != target_reference_application_height { + thread::sleep(Duration::from_millis(500)); + + reference_application_latest_height = match client.src_chain().query_latest_height() { Ok(height) => height, Err(e) => Output::error(format!("{}", e)).exit(), }; - // When the chain is halted, the application height reports a height - // 1 less than the halted height - src_application_height.increment() - }; + debug!( + "Reference application latest height: {}", + reference_application_latest_height + ); + } - warn!( - "Assuming that chain '{}' is currently halted for upgrade at height {}", - client.src_chain().id(), - src_upgrade_height - ); - let outcome = client.upgrade(src_upgrade_height); + // sdk chains don't immediately update their stores after halting (at + // least, as seen by the query interface). Sleep to avoid a race + // condition with the chain + thread::sleep(Duration::from_millis(6000)); + + let outcome = client.upgrade(reference_upgrade_height); match outcome { Ok(receipt) => Output::success(receipt).exit(), @@ -285,38 +323,75 @@ pub struct TxUpgradeClientsCmd { help_heading = "REQUIRED", help = "Identifier of the chain that underwent an upgrade; all clients targeting this chain will be upgraded" )] - src_chain_id: ChainId, + reference_chain_id: ChainId, + + #[clap( + long = "upgrade-height", + required = true, + value_name = "REFERENCE_UPGRADE_HEIGHT", + help_heading = "REQUIRED", + help = "The height at which the reference chain halts for the client upgrade" + )] + reference_upgrade_height: u64, } impl Runnable for TxUpgradeClientsCmd { fn run(&self) { let config = app_config(); - let src_chain = match spawn_chain_runtime(&config, &self.src_chain_id) { + let reference_chain = match spawn_chain_runtime(&config, &self.reference_chain_id) { Ok(handle) => handle, Err(e) => Output::error(format!("{}", e)).exit(), }; - let src_upgrade_height = { - let src_application_height = match src_chain.query_latest_height() { + let reference_upgrade_height = Height::new( + reference_chain.id().version(), + self.reference_upgrade_height, + ) + .unwrap_or_else(exit_with_unrecoverable_error); + + let target_reference_application_height = reference_upgrade_height + .decrement() + .expect("Upgrade height cannot be 1"); + + let mut reference_application_latest_height = match reference_chain.query_latest_height() { + Ok(height) => height, + Err(e) => Output::error(format!("{}", e)).exit(), + }; + + debug!( + "Reference application latest height: {}", + reference_application_latest_height + ); + + while reference_application_latest_height != target_reference_application_height { + thread::sleep(Duration::from_millis(500)); + + reference_application_latest_height = match reference_chain.query_latest_height() { Ok(height) => height, Err(e) => Output::error(format!("{}", e)).exit(), }; - // When the chain is halted, the application height reports a height - // 1 less than the halted height - src_application_height.increment() - }; + debug!( + "Reference application latest height: {}", + reference_application_latest_height + ); + } + + // sdk chains don't immediately update their stores after halting (at + // least, as seen by the query interface). Sleep to avoid a race + // condition with the chain + thread::sleep(Duration::from_millis(6000)); let results = config .chains .iter() .filter_map(|chain| { - (self.src_chain_id != chain.id).then(|| { + (self.reference_chain_id != chain.id).then(|| { self.upgrade_clients_for_chain( &config, - src_chain.clone(), + reference_chain.clone(), &chain.id, - &src_upgrade_height, + reference_upgrade_height, ) }) }) @@ -334,26 +409,28 @@ impl TxUpgradeClientsCmd { fn upgrade_clients_for_chain( &self, config: &Config, - src_chain: Chain, - dst_chain_id: &ChainId, - src_upgrade_height: &Height, + reference_chain: Chain, + host_chain_id: &ChainId, + reference_upgrade_height: Height, ) -> UpgradeClientsForChainResult { - let dst_chain = spawn_chain_runtime_generic::(config, dst_chain_id)?; + let host_chain = spawn_chain_runtime_generic::(config, host_chain_id)?; let req = QueryClientStatesRequest { pagination: Some(PageRequest::all()), }; - let outputs = dst_chain + let outputs = host_chain .query_clients(req) .map_err(Error::relayer)? .into_iter() - .filter_map(|c| (self.src_chain_id == c.client_state.chain_id()).then(|| c.client_id)) + .filter_map(|c| { + (self.reference_chain_id == c.client_state.chain_id()).then(|| c.client_id) + }) .map(|id| { TxUpgradeClientsCmd::upgrade_client( id, - dst_chain.clone(), - src_chain.clone(), - src_upgrade_height, + host_chain.clone(), + reference_chain.clone(), + reference_upgrade_height, ) }) .collect(); @@ -363,14 +440,14 @@ impl TxUpgradeClientsCmd { fn upgrade_client( client_id: ClientId, - dst_chain: Chain, - src_chain: Chain, - src_upgrade_height: &Height, + host_chain: Chain, + reference_chain: Chain, + reference_upgrade_height: Height, ) -> Result, Error> { - let client = ForeignClient::restore(client_id, dst_chain, src_chain); + let client = ForeignClient::restore(client_id, host_chain, reference_chain); client - .upgrade(*src_upgrade_height) + .upgrade(reference_upgrade_height) .map_err(Error::foreign_client) } } @@ -804,42 +881,84 @@ mod tests { assert_eq!( TxUpgradeClientCmd { chain_id: ChainId::from_string("chain_id"), - client_id: ClientId::from_str("client_to_upgrade").unwrap() + client_id: ClientId::from_str("client_to_upgrade").unwrap(), + reference_upgrade_height: 42, }, TxUpgradeClientCmd::parse_from(&[ "test", "--host-chain", "chain_id", "--client", - "client_to_upgrade" + "client_to_upgrade", + "--upgrade-height", + "42" ]) ) } #[test] fn test_upgrade_client_no_chain() { - assert!( - TxUpgradeClientCmd::try_parse_from(&["test", "--client", "client_to_upgrade"]).is_err() - ) + assert!(TxUpgradeClientCmd::try_parse_from(&[ + "test", + "--client", + "client_to_upgrade", + "--upgrade-height", + "42" + ]) + .is_err()) } #[test] fn test_upgrade_client_no_client() { - assert!(TxUpgradeClientCmd::try_parse_from(&["test", "--host-chain", "chain_id"]).is_err()) + assert!(TxUpgradeClientCmd::try_parse_from(&[ + "test", + "--host-chain", + "chain_id", + "--upgrade-height", + "42" + ]) + .is_err()) + } + + #[test] + fn test_upgrade_client_no_upgrade_height() { + assert!(TxUpgradeClientCmd::try_parse_from(&[ + "test", + "--host-chain", + "chain_id", + "--client", + "client_to_upgrade" + ]) + .is_err()) } #[test] fn test_upgrade_clients_required_only() { assert_eq!( TxUpgradeClientsCmd { - src_chain_id: ChainId::from_string("chain_id") + reference_chain_id: ChainId::from_string("chain_id"), + reference_upgrade_height: 42, }, - TxUpgradeClientsCmd::parse_from(&["test", "--reference-chain", "chain_id"]) + TxUpgradeClientsCmd::parse_from(&[ + "test", + "--reference-chain", + "chain_id", + "--upgrade-height", + "42" + ]) + ) + } + + #[test] + fn test_upgrade_clients_no_upgrade_height() { + assert!( + TxUpgradeClientsCmd::try_parse_from(&["test", "--reference-chain", "chain_id",]) + .is_err() ) } #[test] fn test_upgrade_clients_no_chain() { - assert!(TxUpgradeClientsCmd::try_parse_from(&["test"]).is_err()) + assert!(TxUpgradeClientsCmd::try_parse_from(&["test", "--upgrade-height", "42"]).is_err()) } } diff --git a/relayer/src/foreign_client.rs b/relayer/src/foreign_client.rs index 1805f10ccc..3550594e0d 100644 --- a/relayer/src/foreign_client.rs +++ b/relayer/src/foreign_client.rs @@ -448,7 +448,7 @@ impl ForeignClient ForeignClient