Implement Capability{Reader,Keeper} traits (#1826)

* Impl Display for Path types instead of enum variants

* Fix no-std build

* Add CapabilityKeeper, CapabilityReader and PortKeeper traits

* Remove Ics26Context clone

* Fix build

* Add unclog entry

* Add comments

* Update Mock impl

* Apply suggestions from code review

Co-authored-by: Romain Ruetschi <romain@informal.systems>

* Use .to_string() instead of format!()

* Add CapabilityName newtype

* Simplify mock PortReader impl

* Fix validation check for CapabilityName

Co-authored-by: Romain Ruetschi <romain@informal.systems>

.changelog/unreleased/improvements/ibc/1769-cap-reader-keeper.md

@@ -0,0 +1,2 @@
+- Define CapabilityReader and CapabilityKeeper traits
+  ([#1769](https://github.com/informalsystems/ibc-rs/issues/1769))

modules/src/applications/ics20_fungible_token_transfer/context.rs

@@ -2,4 +2,4 @@ use crate::core::ics04_channel::context::{ChannelKeeper, ChannelReader};
 
 /// Captures all the dependencies which the ICS20 module requires to be able to dispatch and
 /// process IBC messages.
-pub trait Ics20Context: ChannelReader + ChannelKeeper + Clone {}
+pub trait Ics20Context: ChannelReader + ChannelKeeper {}

modules/src/clients/ics07_tendermint/client_def.rs

@@ -286,13 +286,19 @@ impl ClientDef for TendermintClient {
             channel_id: channel_id.clone(),
             sequence,
         };
+
+        let mut commitment_bytes = Vec::new();
+        commitment
+            .encode(&mut commitment_bytes)
+            .expect("buffer size too small");
+
         verify_membership(
             client_state,
             connection_end.counterparty().prefix(),
             proof,
             root,
             commitment_path,
-            encode_to_vec(&commitment),
+            commitment_bytes,
         )
     }
 
@@ -342,14 +348,19 @@ impl ClientDef for TendermintClient {
         client_state.verify_height(height)?;
         verify_delay_passed(ctx, height, connection_end)?;
 
+        let mut seq_bytes = Vec::new();
+        u64::from(sequence)
+            .encode(&mut seq_bytes)
+            .expect("buffer size too small");
+
         let seq_path = SeqRecvsPath(port_id.clone(), channel_id.clone());
         verify_membership(
             client_state,
             connection_end.counterparty().prefix(),
             proof,
             root,
             seq_path,
-            encode_to_vec(&u64::from(sequence)),
+            seq_bytes,
         )
     }
 
@@ -470,11 +481,3 @@ fn downcast_consensus_state(cs: AnyConsensusState) -> Result<ConsensusState, Ics
     )
     .ok_or_else(|| Ics02Error::client_args_type_mismatch(ClientType::Tendermint))
 }
-
-// A copy of `prost::Message::encode_to_vec`, as it is currently
-// feature gated behind `std`, even though it could be used with `alloc`.
-fn encode_to_vec(message: &impl Message) -> Vec<u8> {
-    let mut buf = Vec::with_capacity(message.encoded_len());
-    message.encode_raw(&mut buf);
-    buf
-}

modules/src/core/ics05_port/capabilities.rs

@@ -1,5 +1,8 @@
 //! Capabilities: this is a placeholder.
 
+use crate::prelude::*;
+use core::{fmt, str::FromStr};
+
 #[derive(Clone, Debug, PartialEq)]
 pub struct Capability {
     index: u64,
@@ -26,3 +29,34 @@ impl From<u64> for Capability {
         Self { index }
     }
 }
+
+#[derive(Debug, PartialEq)]
+pub struct InvalidCapabilityName;
+
+#[derive(Clone, Debug, PartialEq)]
+pub struct CapabilityName(String);
+
+impl CapabilityName {
+    pub fn new(s: impl AsRef<str>) -> Result<Self, InvalidCapabilityName> {
+        let s = s.as_ref().trim();
+        if !s.is_empty() {
+            Ok(Self(s.to_owned()))
+        } else {
+            Err(InvalidCapabilityName)
+        }
+    }
+}
+
+impl fmt::Display for CapabilityName {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.0)
+    }
+}
+
+impl FromStr for CapabilityName {
+    type Err = InvalidCapabilityName;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Self::new(s)
+    }
+}

modules/src/core/ics05_port/context.rs

@@ -1,11 +1,74 @@
-use crate::core::ics05_port::capabilities::Capability;
+use crate::core::ics05_port::capabilities::{Capability, CapabilityName};
 use crate::core::ics05_port::error::Error;
 use crate::core::ics24_host::identifier::PortId;
+use crate::core::ics24_host::path::PortsPath;
+use crate::prelude::*;
 
-// A context supplying all the necessary read-only dependencies for processing any information regarding a port.
-pub trait PortReader {
-    fn lookup_module_by_port(&self, port_id: &PortId) -> Result<Capability, Error>;
-    fn authenticate(&self, key: &Capability, port_id: &PortId) -> bool;
+/// A context supplying all the necessary read-only dependencies for processing any information regarding a port.
+pub trait PortReader: CapabilityReader {
+    /// Module Id type that can be mapped to an ICS26 router callback module
+    type ModuleId;
+
+    /// Return the module_id along with the capability associated with a given port_id
+    fn lookup_module_by_port(
+        &self,
+        port_id: &PortId,
+    ) -> Result<(Self::ModuleId, Capability), Error>;
+
+    /// Check if the specified port_id is already bounded
+    fn is_bound(&self, port_id: PortId) -> bool {
+        self.get_capability(&Self::port_capability_name(port_id))
+            .is_ok()
+    }
+
+    /// Authenticate a capability key against a port_id by checking if the capability was previously
+    /// generated and bound to the specified port
+    fn authenticate(&self, port_id: PortId, capability: &Capability) -> bool {
+        self.authenticate_capability(&Self::port_capability_name(port_id), capability)
+            .is_ok()
+    }
+
+    fn port_capability_name(port_id: PortId) -> CapabilityName {
+        PortsPath(port_id)
+            .to_string()
+            .parse()
+            .expect("PortsPath cannot be empty string")
+    }
+}
+
+pub trait PortKeeper: CapabilityKeeper + PortReader {
+    /// Binds to a port and returns the associated capability
+    fn bind_port(&mut self, port_id: PortId) -> Result<Capability, Error> {
+        if self.is_bound(port_id.clone()) {
+            Err(Error::port_already_bound(port_id))
+        } else {
+            self.new_capability(Self::port_capability_name(port_id))
+        }
+    }
 }
 
-//  Result<Capability, Error>//return Ok(Capability::new());
+pub trait CapabilityKeeper {
+    /// Create a new capability with the given name.
+    /// Return an error if the capability was already taken.
+    fn new_capability(&mut self, name: CapabilityName) -> Result<Capability, Error>;
+
+    /// Claim the specified capability using the specified name.
+    /// Return an error if the capability was already taken.
+    fn claim_capability(&mut self, name: CapabilityName, capability: Capability);
+
+    /// Release a previously claimed or created capability
+    fn release_capability(&mut self, name: CapabilityName, capability: Capability);
+}
+
+pub trait CapabilityReader {
+    /// Fetch a capability which was previously claimed by specified name
+    fn get_capability(&self, name: &CapabilityName) -> Result<Capability, Error>;
+
+    /// Authenticate a given capability and name. Lookup the capability from the internal store and
+    /// check against the provided name.
+    fn authenticate_capability(
+        &self,
+        name: &CapabilityName,
+        capability: &Capability,
+    ) -> Result<(), Error>;
+}

modules/src/core/ics05_port/error.rs

@@ -6,7 +6,11 @@ define_error! {
     Error {
         UnknownPort
             { port_id: PortId }
-            | e | { format_args!("Port {0} is unknown", e.port_id) },
+            | e | { format_args!("port '{0}' is unknown", e.port_id) },
+
+        PortAlreadyBound
+            { port_id: PortId }
+            | e | { format_args!("port '{0}' is already bound", e.port_id) },
 
         ImplementationSpecific
             | _ | { "implementation specific error" },

modules/src/core/ics24_host/path.rs

@@ -30,104 +30,104 @@ const UPGRADED_CLIENT_CONSENSUS_STATE: &str = "upgradedConsState";
 /// The Path enum abstracts out the different sub-paths.
 #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, From, Display)]
 pub enum Path {
-    #[display(fmt = "clients/{}/clientType", "_0.0")]
     ClientType(ClientTypePath),
-    #[display(fmt = "clients/{}/clientState", "_0.0")]
     ClientState(ClientStatePath),
-    #[display(
-        fmt = "clients/{}/consensusStates/{}-{}",
-        "_0.client_id",
-        "_0.epoch",
-        "_0.height"
-    )]
     ClientConsensusState(ClientConsensusStatePath),
-    #[display(fmt = "clients/{}/connections", "_0.0")]
     ClientConnections(ClientConnectionsPath),
-    #[display(fmt = "connections/{}", "_0.0")]
     Connections(ConnectionsPath),
-    #[display(fmt = "ports/{}", "_0.0")]
     Ports(PortsPath),
-    #[display(fmt = "channelEnds/ports/{}/channels/{}", "_0.0", "_0.1")]
     ChannelEnds(ChannelEndsPath),
-    #[display(fmt = "nextSequenceSend/ports/{}/channels/{}", "_0.0", "_0.1")]
     SeqSends(SeqSendsPath),
-    #[display(fmt = "nextSequenceRecv/ports/{}/channels/{}", "_0.0", "_0.1")]
     SeqRecvs(SeqRecvsPath),
-    #[display(fmt = "nextSequenceAck/ports/{}/channels/{}", "_0.0", "_0.1")]
     SeqAcks(SeqAcksPath),
-    #[display(
-        fmt = "commitments/ports/{}/channels/{}/sequences/{}",
-        "_0.port_id",
-        "_0.channel_id",
-        "_0.sequence"
-    )]
     Commitments(CommitmentsPath),
-    #[display(
-        fmt = "acks/ports/{}/channels/{}/sequences/{}",
-        "_0.port_id",
-        "_0.channel_id",
-        "_0.sequence"
-    )]
     Acks(AcksPath),
-    #[display(
-        fmt = "receipts/ports/{}/channels/{}/sequences/{}",
-        "_0.port_id",
-        "_0.channel_id",
-        "_0.sequence"
-    )]
     Receipts(ReceiptsPath),
     Upgrade(ClientUpgradePath),
 }
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(fmt = "clients/{}/clientType", _0)]
 pub struct ClientTypePath(pub ClientId);
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(fmt = "clients/{}/clientState", _0)]
 pub struct ClientStatePath(pub ClientId);
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(
+    fmt = "clients/{}/consensusStates/{}-{}",
+    "client_id",
+    "epoch",
+    "height"
+)]
 pub struct ClientConsensusStatePath {
     pub client_id: ClientId,
     pub epoch: u64,
     pub height: u64,
 }
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(fmt = "clients/{}/connections", _0)]
 pub struct ClientConnectionsPath(pub ClientId);
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(fmt = "connections/{}", _0)]
 pub struct ConnectionsPath(pub ConnectionId);
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(fmt = "ports/{}", _0)]
 pub struct PortsPath(pub PortId);
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(fmt = "channelEnds/ports/{}/channels/{}", _0, _1)]
 pub struct ChannelEndsPath(pub PortId, pub ChannelId);
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(fmt = "nextSequenceSend/ports/{}/channels/{}", _0, _1)]
 pub struct SeqSendsPath(pub PortId, pub ChannelId);
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(fmt = "nextSequenceRecv/ports/{}/channels/{}", _0, _1)]
 pub struct SeqRecvsPath(pub PortId, pub ChannelId);
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(fmt = "nextSequenceAck/ports/{}/channels/{}", _0, _1)]
 pub struct SeqAcksPath(pub PortId, pub ChannelId);
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(
+    fmt = "commitments/ports/{}/channels/{}/sequences/{}",
+    "port_id",
+    "channel_id",
+    "sequence"
+)]
 pub struct CommitmentsPath {
     pub port_id: PortId,
     pub channel_id: ChannelId,
     pub sequence: Sequence,
 }
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(
+    fmt = "acks/ports/{}/channels/{}/sequences/{}",
+    "port_id",
+    "channel_id",
+    "sequence"
+)]
 pub struct AcksPath {
     pub port_id: PortId,
     pub channel_id: ChannelId,
     pub sequence: Sequence,
 }
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Display)]
+#[display(
+    fmt = "receipts/ports/{}/channels/{}/sequences/{}",
+    "port_id",
+    "channel_id",
+    "sequence"
+)]
 pub struct ReceiptsPath {
     pub port_id: PortId,
     pub channel_id: ChannelId,

modules/src/core/ics26_routing/context.rs

@@ -16,6 +16,5 @@ pub trait Ics26Context:
     + ChannelReader
     + PortReader
     + Ics20Context
-    + Clone
 {
 }

modules/src/core/ics26_routing/handler.rs

@@ -21,9 +21,6 @@ pub fn deliver<Ctx>(ctx: &mut Ctx, messages: Vec<Any>) -> Result<Vec<IbcEvent>,
 where
     Ctx: Ics26Context,
 {
-    // Create a clone, which will store each intermediary stage of applying txs.
-    let mut ctx_interim = ctx.clone();
-
     // A buffer for all the events, to be used as return value.
     let mut res: Vec<IbcEvent> = Vec::new();
 
@@ -32,13 +29,12 @@ where
         let envelope = decode(any_msg)?;
 
         // Process the envelope, and accumulate any events that were generated.
-        let mut output = dispatch(&mut ctx_interim, envelope)?;
+        let mut output = dispatch(ctx, envelope)?;
+
         // TODO: output.log and output.result are discarded
         res.append(&mut output.events);
     }
 
-    // No error has surfaced, so we now apply the changes permanently to the original context.
-    *ctx = ctx_interim;
     Ok(res)
 }