Hermes CLI for upgrading client (#723)

* Added domain type def. Also cleaned-up the documentation, which looked very sloppy. https://docs.rs/ibc/0.1.1/ibc/ics02_client/msgs/index.html * Added partial handler & command * Added upgrade proto files. Added Cargo.lock for proto-compiler * Prep for query_upgraded_client_state * Method & support for querying ugpraded client state * Support for querying the upgraded consensus state * Minor dev scripts enhancements & bugs * Added guide. Uses patched Go relayer * Proto conversion for upgrade msg. Refactored upgrade() impl * Fix missing signer bug * Update msg bf. upgrade. Event parsing * Changelog. Revised guide * Aesthetic nits based on file review * Clarifications in the test instructions * Documented Go relayer version in testing instructions * Possible fix for #734 * changelog & method documentation * Apply suggestions from code review Co-authored-by: Romain Ruetschi <romain@informal.systems> * Added more derived trait bounds on module events * Adapt to newer Ics02 structure. * Added Protobuf impl for MsgUpgradeAnyClient * FMT * Added upgrade-chain CLI and updated instructions * Remove a clone and turn zero_custom_fields into a static method * Whitespace and nitpick * Remove obsolete TODO Co-authored-by: Romain Ruetschi <romain@informal.systems> Co-authored-by: Anca Zamfir <zamfiranca@gmail.com>
informalsystems · Mar 23, 2021 · 5b1f409 · 5b1f409
1 parent c2ba84b
commit 5b1f409
Show file tree

Hide file tree

Showing 44 changed files with 1,472 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,7 @@
 - [ibc-relayer-cli]
   - Added `create connection` CLI ([#630])
   - Proposed ADR 006 to describe Hermes v0.2.0 use-cases ([#637])
+  - Added `client-upgrade` CLI ([#357])
   - Update gaia to version 4.1.0 for e2e tests on CI ([#702])
 
 ### IMPROVEMENTS
@@ -33,7 +34,7 @@
   - [nothing yet]
 
 - [ibc-relayer-cli]
-  - [nothing yet]
+  - Clarified success path for updating a client that is already up-to-date ([#734])
 
 ### BUG FIXES
 
@@ -61,6 +62,7 @@
   - [nothing yet]
 
 [#352]: https://github.com/informalsystems/ibc-rs/issues/352
+[#357]: https://github.com/informalsystems/ibc-rs/issues/357
 [#416]: https://github.com/informalsystems/ibc-rs/issues/416
 [#561]: https://github.com/informalsystems/ibc-rs/issues/561
 [#599]: https://github.com/informalsystems/ibc-rs/issues/599
@@ -72,6 +74,7 @@
 [#699]: https://github.com/informalsystems/ibc-rs/issues/699
 [#700]: https://github.com/informalsystems/ibc-rs/pull/700
 [#702]: https://github.com/informalsystems/ibc-rs/issues/702
+[#734]: https://github.com/informalsystems/ibc-rs/issues/734
 [#736]: https://github.com/informalsystems/ibc-rs/issues/736
 [#740]: https://github.com/informalsystems/ibc-rs/issues/740
 [#752]: https://github.com/informalsystems/ibc-rs/issues/752

diff --git a/guide/src/upgrade_test.md b/guide/src/upgrade_test.md
@@ -0,0 +1,166 @@
+## Prerequisites
+
+- gaiad `(v4.1.*)`, for example:
+
+```shell
+$ gaiad version --long | head -n4
+name: gaia
+server_name: gaiad
+version: 4.1.2
+commit: 95b07e641d1f69ee12dd911e92b1679f2c64d385
+```
+
+## Testing procedure
+
+1. Start two gaia instances and initialize hermes:
+
+    ```shell
+    $ ./scripts/dev-env ~/.hermes/config.toml ibc-0 ibc-1
+    ```
+    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.
+
+2. Create one client on `ibc-1` for `ibc-0`:
+
+    ```shell
+    $ hermes tx raw create-client ibc-1 ibc-0
+    ```
+
+3. 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`)
+
+    ```shell
+    $ hermes tx raw upgrade-chain ibc-0 ibc-1 07-tendermint-0 10000000 300
+    ```
+
+    Note that the height offset should be picked such that the proposal plan height is reached after the `200s` voting period.
+
+ 4. 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`.
+
+    ```shell
+    $ gaiad query gov proposal 1 --home data/ibc-0/
+
+    content:
+      '@type': /cosmos.upgrade.v1beta1.SoftwareUpgradeProposal
+      description: upgrade the chain software and unbonding period
+      plan:
+        height: "382"
+        info: upgrade the chain software and unbonding period
+        name: test
+        time: "0001-01-01T00:00:00Z"
+        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: "383"
+            revision_number: "0"
+          max_clock_drift: 0s
+          proof_specs:
+    ...
+          trust_level:
+            denominator: "0"
+            numerator: "0"
+          trusting_period: 0s
+          unbonding_period: 1440000s
+          upgrade_path:
+          - upgrade
+          - upgradedIBCState
+      title: upgrade_ibc_clients
+    deposit_end_time: "2021-03-23T17:25:42.543572Z"
+    final_tally_result:
+      abstain: "0"
+      "no": "0"
+      no_with_veto: "0"
+      "yes": "0"
+    proposal_id: "1"
+    status: PROPOSAL_STATUS_VOTING_PERIOD
+    submit_time: "2021-03-23T17:22:22.543572Z"
+    total_deposit:
+    - amount: "10000000"
+      denom: stake
+    voting_end_time: "2021-03-23T17:25:42.543572Z"
+    voting_start_time: "2021-03-23T17:22:22.543572Z"
+    ```
+
+ 5. Vote on the proposal
+
+    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`.
+
+    ```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
+    ```
+
+    Wait approximately 200 seconds until the proposal changes status to `PROPOSAL_STATUS_PASSED`. Note the `final tally_result` that includes the vote submitted in previous step.
+
+    ```shell
+    $ gaiad query gov proposal 1 --home data/ibc-0/
+
+    content:
+      '@type': /cosmos.upgrade.v1beta1.SoftwareUpgradeProposal
+      description: upgrade the chain software and unbonding period
+      plan:
+    ...
+    final_tally_result:
+      abstain: "0"
+      "no": "0"
+      no_with_veto: "0"
+      "yes": "100000000000"
+    proposal_id: "1"
+    status: PROPOSAL_STATUS_PASSED
+    submit_time: "2021-03-23T17:22:22.543572Z"
+    total_deposit:
+    - amount: "10000000"
+      denom: stake
+    voting_end_time: "2021-03-23T17:25:42.543572Z"
+    voting_start_time: "2021-03-23T17:22:22.543572Z"
+    ```
+
+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, and another for the upgraded state.
+
+    ```shell
+    $ hermes tx raw upgrade-client ibc-1 ibc-0 07-tendermint-0
+
+    {
+      "status": "success",
+      "result": [
+        {
+          "UpdateClient": {
+            "client_id": "07-tendermint-0",
+            "client_type": "Tendermint",
+            "consensus_height": {
+              "revision_height": 332,
+              "revision_number": 0
+            },
+            "height": {
+              "revision_height": 404,
+              "revision_number": 1
+            }
+          }
+        },
+        {
+          "UpgradeClient": {
+            "client_id": "07-tendermint-0",
+            "client_type": "Tendermint",
+            "consensus_height": {
+              "revision_height": 333,
+              "revision_number": 0
+            },
+            "height": {
+              "revision_height": 404,
+              "revision_number": 1
+            }
+          }
+        }
+      ]
+    }
+    ```
diff --git a/modules/src/events.rs b/modules/src/events.rs
@@ -34,6 +34,7 @@ pub enum IbcEvent {
 
     CreateClient(ClientEvents::CreateClient),
     UpdateClient(ClientEvents::UpdateClient),
+    UpgradeClient(ClientEvents::UpgradeClient),
     ClientMisbehavior(ClientEvents::ClientMisbehavior),
 
     OpenInitConnection(ConnectionEvents::OpenInit),
@@ -111,6 +112,7 @@ impl IbcEvent {
             IbcEvent::NewBlock(ev) => ev.set_height(height),
             IbcEvent::CreateClient(ev) => ev.set_height(height),
             IbcEvent::UpdateClient(ev) => ev.set_height(height),
+            IbcEvent::UpgradeClient(ev) => ev.set_height(height),
             IbcEvent::ClientMisbehavior(ev) => ev.set_height(height),
             IbcEvent::OpenInitConnection(ev) => ev.set_height(height),
             IbcEvent::OpenTryConnection(ev) => ev.set_height(height),

diff --git a/modules/src/ics02_client/context.rs b/modules/src/ics02_client/context.rs
@@ -6,7 +6,7 @@ use crate::ics02_client::client_consensus::AnyConsensusState;
 use crate::ics02_client::client_state::AnyClientState;
 use crate::ics02_client::client_type::ClientType;
 use crate::ics02_client::error::Error;
-use crate::ics02_client::handler::ClientResult::{self, Create, Update};
+use crate::ics02_client::handler::ClientResult::{self, Create, Update, Upgrade};
 use crate::ics24_host::identifier::ClientId;
 use crate::Height;
 
@@ -47,6 +47,9 @@ pub trait ClientKeeper {
                 )?;
                 Ok(())
             }
+            Upgrade(_) => {
+                unimplemented!()
+            }
         }
     }
 

diff --git a/modules/src/ics02_client/error.rs b/modules/src/ics02_client/error.rs
@@ -2,6 +2,8 @@ use anomaly::{BoxError, Context};
 use thiserror::Error;
 
 use crate::ics02_client::client_type::ClientType;
+use crate::ics23_commitment::error::Kind as Ics23Kind;
+use crate::ics24_host::error::ValidationKind;
 use crate::ics24_host::identifier::ClientId;
 use crate::Height;
 
@@ -21,6 +23,9 @@ pub enum Kind {
     #[error("client not found: {0}")]
     ClientNotFound(ClientId),
 
+    #[error("client is frozen: {0}")]
+    ClientFrozen(ClientId),
+
     #[error("consensus state not found at: {0} at height {1}")]
     ConsensusStateNotFound(ClientId, Height),
 
@@ -54,8 +59,8 @@ pub enum Kind {
     #[error("invalid raw client consensus state")]
     InvalidRawConsensusState,
 
-    #[error("invalid identifer")]
-    InvalidIdentifier,
+    #[error("invalid client identifier: validation error: {0}")]
+    InvalidClientIdentifier(ValidationKind),
 
     #[error("invalid raw header")]
     InvalidRawHeader,
@@ -66,6 +71,12 @@ pub enum Kind {
     #[error("invalid address")]
     InvalidAddress,
 
+    #[error("invalid proof for the upgraded client state")]
+    InvalidUpgradeClientProof(Ics23Kind),
+
+    #[error("invalid proof for the upgraded consensus state")]
+    InvalidUpgradeConsensusStateProof(Ics23Kind),
+
     #[error("mismatch between client and arguments types, expected: {0:?}")]
     ClientArgsTypeMismatch(ClientType),
 

diff --git a/modules/src/ics02_client/events.rs b/modules/src/ics02_client/events.rs
@@ -12,6 +12,7 @@ use std::convert::{TryFrom, TryInto};
 /// The content of the `type` field for the event that a chain produces upon executing the create client transaction.
 const CREATE_EVENT_TYPE: &str = "create_client";
 const UPDATE_EVENT_TYPE: &str = "update_client";
+const UPGRADE_EVENT_TYPE: &str = "upgrade_client";
 
 /// The content of the `key` field for the attribute containing the client identifier.
 const CLIENT_ID_ATTRIBUTE_KEY: &str = "client_id";
@@ -30,6 +31,9 @@ pub fn try_from_tx(event: &tendermint::abci::Event) -> Option<IbcEvent> {
         UPDATE_EVENT_TYPE => Some(IbcEvent::UpdateClient(UpdateClient(
             extract_attributes_from_tx(event),
         ))),
+        UPGRADE_EVENT_TYPE => Some(IbcEvent::UpgradeClient(UpgradeClient(
+            extract_attributes_from_tx(event),
+        ))),
         _ => None,
     }
 }
@@ -77,7 +81,7 @@ impl From<NewBlock> for IbcEvent {
     }
 }
 
-#[derive(Debug, Deserialize, Serialize, Clone)]
+#[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct Attributes {
     pub height: Height,
     pub client_id: ClientId,
@@ -214,3 +218,19 @@ impl From<ClientMisbehavior> for IbcEvent {
         IbcEvent::ClientMisbehavior(v)
     }
 }
+
+/// Signals a recent upgrade of an on-chain client (IBC Client).
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Deserialize, Serialize)]
+pub struct UpgradeClient(Attributes);
+
+impl UpgradeClient {
+    pub fn set_height(&mut self, height: Height) {
+        self.0.height = height;
+    }
+}
+
+impl From<Attributes> for UpgradeClient {
+    fn from(attrs: Attributes) -> Self {
+        UpgradeClient(attrs)
+    }
+}
diff --git a/modules/src/ics02_client/handler.rs b/modules/src/ics02_client/handler.rs
@@ -7,11 +7,13 @@ use crate::ics02_client::msgs::ClientMsg;
 
 pub mod create_client;
 pub mod update_client;
+pub mod upgrade_client;
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum ClientResult {
     Create(create_client::Result),
     Update(update_client::Result),
+    Upgrade(upgrade_client::Result),
 }
 
 /// General entry point for processing any message related to ICS2 (client functions) protocols.
@@ -22,5 +24,6 @@ where
     match msg {
         ClientMsg::CreateClient(msg) => create_client::process(ctx, msg),
         ClientMsg::UpdateClient(msg) => update_client::process(ctx, msg),
+        ClientMsg::UpgradeClient(msg) => upgrade_client::process(ctx, msg),
     }
 }
diff --git a/modules/src/ics02_client/handler/update_client.rs b/modules/src/ics02_client/handler/update_client.rs
@@ -80,7 +80,7 @@ mod tests {
     use crate::ics02_client::client_state::AnyClientState;
     use crate::ics02_client::error::Kind;
     use crate::ics02_client::handler::dispatch;
-    use crate::ics02_client::handler::ClientResult::{Create, Update};
+    use crate::ics02_client::handler::ClientResult::Update;
     use crate::ics02_client::header::Header;
     use crate::ics02_client::msgs::update_client::MsgUpdateAnyClient;
     use crate::ics02_client::msgs::ClientMsg;
@@ -128,7 +128,7 @@ mod tests {
                             )))
                         )
                     }
-                    Create(_) => panic!("update handler result has type CreateResult"),
+                    _ => panic!("update handler result has incorrect type"),
                 }
             }
             Err(err) => {