Implement upgrade client proposal handler (#680)

* Abstract upgrade client proposal handler

* Replace all upgrade-related abstractions under tendermint host

* Adjust encapsulation

* Move handler & helper functions to basecoin

* Add unclog

* Refine upgrade client errors

* Reorganize some types

* Add docstring for upgrade client types

* Re-add upgrade_client_proposal_handler

* Make zero_custom_fields public

* upgrade_client_proposal_handler not needed to return Vec<Event>

* upgrade_proposal module dosctring

---------

Co-authored-by: Philippe Laferriere <[email protected]>

.changelog/unreleased/breaking-changes/420-abstract-upgrade-proposal.md

@@ -0,0 +1,2 @@
+- Support for upgrade client proposal by featuring helper contexts and domain types 
+  ([#420](https://github.com/cosmos/ibc-rs/issues/420))

.changelog/unreleased/improvements/141-clarify-upgrade-path-usage.md

@@ -0,0 +1,2 @@
+- Clarify usage of `upgrade_path` for handling upgrade proposals
+  ([#141](https://github.com/cosmos/ibc-rs/issues/141))

crates/ibc/src/applications/transfer/context.rs

@@ -1,7 +1,4 @@
 //! Defines the main context traits and IBC module callbacks
-
-use crate::core::router::ModuleExtras;
-use crate::core::ContextError;
 use crate::prelude::*;
 
 use sha2::{Digest, Sha256};
@@ -22,6 +19,8 @@ use crate::core::ics04_channel::context::{
 use crate::core::ics04_channel::packet::{Acknowledgement, Packet};
 use crate::core::ics04_channel::Version;
 use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId};
+use crate::core::router::ModuleExtras;
+use crate::core::ContextError;
 use crate::signer::Signer;
 
 /// Methods required in token transfer validation, to be implemented by the host

crates/ibc/src/clients/ics07_tendermint/client_state.rs

@@ -32,7 +32,7 @@ use crate::core::ics02_client::client_state::{
 };
 use crate::core::ics02_client::client_type::ClientType;
 use crate::core::ics02_client::consensus_state::ConsensusState;
-use crate::core::ics02_client::error::ClientError;
+use crate::core::ics02_client::error::{ClientError, UpgradeClientError};
 use crate::core::ics23_commitment::commitment::{
     CommitmentPrefix, CommitmentProofBytes, CommitmentRoot,
 };
@@ -251,7 +251,7 @@ impl ClientState {
     }
 
     // Resets custom fields to zero values (used in `update_client`)
-    fn zero_custom_fields(&mut self) {
+    pub fn zero_custom_fields(&mut self) {
         self.trusting_period = ZERO_DURATION;
         self.trust_level = TrustThreshold::ZERO;
         self.allow_update.after_expiry = false;
@@ -436,10 +436,10 @@ impl Ics2ClientState for ClientState {
         // the upgrade height This condition checks both the revision number and
         // the height
         if self.latest_height() >= upgraded_tm_client_state.latest_height() {
-            return Err(ClientError::LowUpgradeHeight {
+            return Err(UpgradeClientError::LowUpgradeHeight {
                 upgraded_height: self.latest_height(),
                 client_height: upgraded_tm_client_state.latest_height(),
-            });
+            })?;
         }
 
         // Check to see if the upgrade path is set

crates/ibc/src/core/context.rs

@@ -1,10 +1,11 @@
+use crate::prelude::*;
+
+use crate::signer::Signer;
+use alloc::string::String;
 use core::time::Duration;
 use displaydoc::Display;
 use ibc_proto::google::protobuf::Any;
 
-use crate::prelude::*;
-use crate::signer::Signer;
-
 use crate::core::events::IbcEvent;
 use crate::core::ics02_client::client_state::ClientState;
 use crate::core::ics02_client::consensus_state::ConsensusState;
@@ -17,7 +18,8 @@ use crate::core::ics03_connection::version::{
 use crate::core::ics04_channel::channel::ChannelEnd;
 use crate::core::ics04_channel::commitment::{AcknowledgementCommitment, PacketCommitment};
 use crate::core::ics04_channel::context::calculate_block_delay;
-use crate::core::ics04_channel::error::{ChannelError, PacketError};
+use crate::core::ics04_channel::error::ChannelError;
+use crate::core::ics04_channel::error::PacketError;
 use crate::core::ics04_channel::packet::{Receipt, Sequence};
 use crate::core::ics23_commitment::commitment::CommitmentPrefix;
 use crate::core::ics24_host::identifier::ClientId;

crates/ibc/src/core/handler.rs

@@ -1,3 +1,4 @@
+use super::context::RouterError;
 use super::ics02_client::handler::{create_client, update_client, upgrade_client};
 use super::ics02_client::msgs::{ClientMsg, MsgUpdateOrMisbehaviour};
 use super::ics03_connection::handler::{
@@ -28,7 +29,7 @@ use super::ics04_channel::handler::timeout::{
 };
 use super::ics04_channel::msgs::{ChannelMsg, PacketMsg};
 use super::ContextError;
-use super::{msgs::MsgEnvelope, ExecutionContext, RouterError, ValidationContext};
+use super::{msgs::MsgEnvelope, ExecutionContext, ValidationContext};
 
 /// Entrypoint which performs both validation and message execution
 pub fn dispatch(ctx: &mut impl ExecutionContext, msg: MsgEnvelope) -> Result<(), RouterError> {

crates/ibc/src/core/ics02_client/error.rs

@@ -1,6 +1,5 @@
 //! Defines the client error type
 
-use crate::core::ContextError;
 use crate::prelude::*;
 
 use displaydoc::Display;
@@ -10,11 +9,14 @@ use crate::core::ics02_client::client_type::ClientType;
 use crate::core::ics23_commitment::error::CommitmentError;
 use crate::core::ics24_host::identifier::{ClientId, IdentifierError};
 use crate::core::timestamp::Timestamp;
+use crate::core::ContextError;
 use crate::Height;
 
 /// Encodes all the possible client errors
 #[derive(Debug, Display)]
 pub enum ClientError {
+    /// upgrade client error: `{0}`
+    Upgrade(UpgradeClientError),
     /// Client identifier constructor failed for type `{client_type}` with counter `{counter}`, validation error: `{validation_error}`
     ClientIdentifierConstructor {
         client_type: ClientType,
@@ -76,10 +78,6 @@ pub enum ClientError {
         latest_height: Height,
         proof_height: Height,
     },
-    /// invalid proof for the upgraded client state error: `{0}`
-    InvalidUpgradeClientProof(CommitmentError),
-    /// invalid proof for the upgraded consensus state error: `{0}`
-    InvalidUpgradeConsensusStateProof(CommitmentError),
     /// invalid commitment proof bytes error: `{0}`
     InvalidCommitmentProof(CommitmentError),
     /// invalid packet timeout timestamp value error: `{0}`
@@ -91,11 +89,6 @@ pub enum ClientError {
         header_height: Height,
         latest_height: Height,
     },
-    /// upgraded client height `{upgraded_height}` must be at greater than current client height `{client_height}`
-    LowUpgradeHeight {
-        upgraded_height: Height,
-        client_height: Height,
-    },
     /// timestamp is invalid or missing, timestamp=`{time1}`,  now=`{time2}`
     InvalidConsensusStateTimestamp { time1: Timestamp, time2: Timestamp },
     /// header not within trusting period: expires_at=`{latest_time}` now=`{update_time}`
@@ -140,12 +133,47 @@ impl std::error::Error for ClientError {
             Self::InvalidClientIdentifier(e) => Some(e),
             Self::InvalidRawHeader(e) => Some(e),
             Self::InvalidRawMisbehaviour(e) => Some(e),
-            Self::InvalidUpgradeClientProof(e) => Some(e),
-            Self::InvalidUpgradeConsensusStateProof(e) => Some(e),
             Self::InvalidCommitmentProof(e) => Some(e),
             Self::InvalidPacketTimestamp(e) => Some(e),
             Self::Ics23Verification(e) => Some(e),
             _ => None,
         }
     }
 }
+
+/// Encodes all the possible upgrade client errors
+#[derive(Debug, Display)]
+pub enum UpgradeClientError {
+    /// invalid proof for the upgraded client state error: `{0}`
+    InvalidUpgradeClientProof(CommitmentError),
+    /// invalid proof for the upgraded consensus state error: `{0}`
+    InvalidUpgradeConsensusStateProof(CommitmentError),
+    /// upgraded client height `{upgraded_height}` must be at greater than current client height `{client_height}`
+    LowUpgradeHeight {
+        upgraded_height: Height,
+        client_height: Height,
+    },
+    /// invalid upgrade proposal: `{reason}`
+    InvalidUpgradeProposal { reason: String },
+    /// invalid upgrade plan: `{reason}`
+    InvalidUpgradePlan { reason: String },
+    /// other upgrade client error: `{reason}`
+    Other { reason: String },
+}
+
+impl From<UpgradeClientError> for ClientError {
+    fn from(e: UpgradeClientError) -> Self {
+        ClientError::Upgrade(e)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for UpgradeClientError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match &self {
+            Self::InvalidUpgradeClientProof(e) => Some(e),
+            Self::InvalidUpgradeConsensusStateProof(e) => Some(e),
+            _ => None,
+        }
+    }
+}

crates/ibc/src/core/ics02_client/handler/upgrade_client.rs

@@ -106,6 +106,7 @@ mod tests {
     use crate::clients::ics07_tendermint::client_type;
     use crate::clients::ics07_tendermint::header::test_util::get_dummy_tendermint_header;
 
+    use crate::core::ics02_client::error::UpgradeClientError;
     use crate::core::ics03_connection::handler::test_util::{Expect, Fixture};
     use crate::core::ics24_host::identifier::ClientId;
     use crate::downcast;
@@ -233,12 +234,14 @@ mod tests {
 
     #[test]
     fn upgrade_client_fail_low_upgrade_height() {
-        let fxt = msg_upgrade_client_fixture(Ctx::WithClient, Msg::LowUpgradeHeight);
-        let expected_err = ContextError::ClientError(ClientError::LowUpgradeHeight {
+        let fxt: Fixture<MsgUpgradeClient> =
+            msg_upgrade_client_fixture(Ctx::WithClient, Msg::LowUpgradeHeight);
+        let expected_err: ClientError = UpgradeClientError::LowUpgradeHeight {
             upgraded_height: Height::new(0, 26).unwrap(),
             client_height: fxt.ctx.latest_height(),
-        });
-        upgrade_client_validate(&fxt, Expect::Failure(Some(expected_err)));
+        }
+        .into();
+        upgrade_client_validate(&fxt, Expect::Failure(Some(expected_err.into())));
     }
 
     #[test]

crates/ibc/src/core/ics02_client/msgs/upgrade_client.rs

@@ -9,7 +9,7 @@ use ibc_proto::ibc::core::client::v1::MsgUpgradeClient as RawMsgUpgradeClient;
 use ibc_proto::ibc::core::commitment::v1::MerkleProof as RawMerkleProof;
 use ibc_proto::protobuf::Protobuf;
 
-use crate::core::ics02_client::error::ClientError;
+use crate::core::ics02_client::error::{ClientError, UpgradeClientError};
 use crate::core::ics23_commitment::commitment::CommitmentProofBytes;
 use crate::core::ics23_commitment::error::CommitmentError;
 use crate::core::ics24_host::identifier::ClientId;
@@ -78,11 +78,13 @@ impl TryFrom<RawMsgUpgradeClient> for MsgUpgradeClient {
 
         let c_bytes =
             CommitmentProofBytes::try_from(proto_msg.proof_upgrade_client).map_err(|_| {
-                ClientError::InvalidUpgradeClientProof(CommitmentError::EmptyMerkleProof)
+                UpgradeClientError::InvalidUpgradeClientProof(CommitmentError::EmptyMerkleProof)
             })?;
         let cs_bytes = CommitmentProofBytes::try_from(proto_msg.proof_upgrade_consensus_state)
             .map_err(|_| {
-                ClientError::InvalidUpgradeConsensusStateProof(CommitmentError::EmptyMerkleProof)
+                UpgradeClientError::InvalidUpgradeConsensusStateProof(
+                    CommitmentError::EmptyMerkleProof,
+                )
             })?;
 
         Ok(MsgUpgradeClient {
@@ -91,9 +93,9 @@ impl TryFrom<RawMsgUpgradeClient> for MsgUpgradeClient {
             client_state: raw_client_state,
             consensus_state: raw_consensus_state,
             proof_upgrade_client: RawMerkleProof::try_from(c_bytes)
-                .map_err(ClientError::InvalidUpgradeClientProof)?,
+                .map_err(UpgradeClientError::InvalidUpgradeClientProof)?,
             proof_upgrade_consensus_state: RawMerkleProof::try_from(cs_bytes)
-                .map_err(ClientError::InvalidUpgradeConsensusStateProof)?,
+                .map_err(UpgradeClientError::InvalidUpgradeConsensusStateProof)?,
             signer: proto_msg.signer.into(),
         })
     }

crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs

@@ -12,7 +12,8 @@ use crate::core::ics24_host::path::Path;
 use crate::core::ics24_host::path::{
     AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, SeqAckPath,
 };
-use crate::core::{events::IbcEvent, ics04_channel::events::AcknowledgePacket, router::ModuleId};
+use crate::core::router::ModuleId;
+use crate::core::{events::IbcEvent, ics04_channel::events::AcknowledgePacket};
 use crate::core::{ContextError, ExecutionContext, ValidationContext};
 
 pub(crate) fn acknowledgement_packet_validate<ValCtx>(

crates/ibc/src/core/ics04_channel/handler/timeout.rs

@@ -1,6 +1,7 @@
 use crate::prelude::*;
 use prost::Message;
 
+use crate::core::events::IbcEvent;
 use crate::core::events::MessageEvent;
 use crate::core::ics03_connection::delay::verify_conn_delay_passed;
 use crate::core::ics04_channel::channel::State;
@@ -11,15 +12,12 @@ use crate::core::ics04_channel::error::PacketError;
 use crate::core::ics04_channel::events::ChannelClosed;
 use crate::core::ics04_channel::msgs::timeout::MsgTimeout;
 use crate::core::ics04_channel::msgs::timeout_on_close::MsgTimeoutOnClose;
+use crate::core::ics04_channel::{events::TimeoutPacket, handler::timeout_on_close};
 use crate::core::ics24_host::path::Path;
 use crate::core::ics24_host::path::{
     ChannelEndPath, ClientConsensusStatePath, CommitmentPath, ReceiptPath, SeqRecvPath,
 };
-use crate::core::{
-    events::IbcEvent,
-    ics04_channel::{events::TimeoutPacket, handler::timeout_on_close},
-    router::ModuleId,
-};
+use crate::core::router::ModuleId;
 use crate::core::{ContextError, ExecutionContext, ValidationContext};
 
 pub(crate) enum TimeoutMsgType {

crates/ibc/src/core/msgs.rs

@@ -1,8 +1,8 @@
-use crate::core::RouterError;
 use crate::prelude::*;
 
 use ibc_proto::google::protobuf::Any;
 
+use crate::core::context::RouterError;
 use crate::core::ics02_client::msgs::{
     create_client, misbehaviour, update_client, upgrade_client, ClientMsg,
 };

crates/ibc/src/core/router.rs

@@ -1,20 +1,21 @@
 //! Defines the `Router`, which binds modules to ports
 
-use crate::{core::events::ModuleEvent, prelude::*};
-
+use crate::prelude::*;
 use alloc::borrow::Borrow;
 use core::fmt::{Debug, Display, Error as FmtError, Formatter};
 
+use crate::core::events::ModuleEvent;
 use crate::core::ics04_channel::channel::{Counterparty, Order};
-use crate::core::ics04_channel::error::{ChannelError, PacketError, PortError::UnknownPort};
+use crate::core::ics04_channel::error::PortError::UnknownPort;
+use crate::core::ics04_channel::error::{ChannelError, PacketError};
 use crate::core::ics04_channel::msgs::ChannelMsg;
+use crate::core::ics04_channel::msgs::PacketMsg;
 use crate::core::ics04_channel::packet::{Acknowledgement, Packet};
 use crate::core::ics04_channel::Version;
-use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId};
+use crate::core::ics24_host::identifier::PortId;
+use crate::core::ics24_host::identifier::{ChannelId, ConnectionId};
 use crate::signer::Signer;
 
-use super::ics04_channel::msgs::PacketMsg;
-
 /// Router as defined in ICS-26, which binds modules to ports.
 pub trait Router {
     /// Returns a reference to a `Module` registered against the specified `ModuleId`

crates/ibc/src/hosts/mod.rs

@@ -1,3 +1,2 @@
 //! Provides convenience implementations for various hosts
-
 pub mod tendermint;

crates/ibc/src/hosts/tendermint/mod.rs

@@ -0,0 +1,12 @@
+//! Provides convenience implementations for Tendermint-based hosts
+
+pub mod upgrade_proposal;
+
+mod validate_self_client;
+pub use validate_self_client::ValidateSelfClientContext;
+
+/// ABCI store/query path for the IBC sub-store
+pub const IBC_QUERY_PATH: &str = "store/ibc/key";
+
+/// ABCI store/query path for the upgrade sub-store
+pub const SDK_UPGRADE_QUERY_PATH: &str = "store/upgrade/key";