Remove the OCap system (#2186)

* Remove capabilities * changelog * Start documenting where we diverge from spec * Update modules/README.md Co-authored-by: Shoaib Ahmed <[email protected]> * remove MockContext::with_port * cippy tests Co-authored-by: Shoaib Ahmed <[email protected]>
informalsystems · May 11, 2022 · 3aed2d9 · 3aed2d9
1 parent bbd5ed8
commit 3aed2d9
Show file tree

Hide file tree

Showing 25 changed files with 81 additions and 478 deletions.
diff --git a/.changelog/unreleased/improvements/ibc/2159-remove-ocaps.md b/.changelog/unreleased/improvements/ibc/2159-remove-ocaps.md
@@ -0,0 +1,2 @@
+- Remove object capabilities from the modules
+  ([#2159](https://github.com/informalsystems/ibc-rs/issues/2159))
diff --git a/modules/README.md b/modules/README.md
@@ -18,6 +18,31 @@ Implementation of the Inter-Blockchain Communication Protocol ([IBC]) module.
 
 See documentation on [docs.rs][docs-link].
 
+## Divergence from the Interchain Standards (ICS)
+This crate diverges from the [ICS specification](https://github.com/cosmos/ibc) in a number of ways. See below for more details.
+
+### Module system: no support for untrusted modules
+ICS 24 (Host Requirements) gives the [following requirement](https://github.com/cosmos/ibc/blob/master/spec/core/ics-024-host-requirements/README.md#module-system) about the module system that the host state machine must support:
+
+> The host state machine must support a module system, whereby self-contained, potentially mutually distrusted packages of code can safely execute on the same ledger [...].
+
+**This crate currently does not support mutually distrusted packages**. That is, modules on the host state machine are assumed to be fully trusted. In practice, this means that every module has either been written by the host state machine developers, or fully vetted by them.
+
+### Port system: No object capability system
+ICS 5 (Port Allocation) requires the host system to support either object-capability reference or source authentication for modules.
+
+> In the former object-capability case, the IBC handler must have the ability to generate object-capabilities, unique, opaque references which can be passed to a module and will not be duplicable by other modules. [...]
+> In the latter source authentication case, the IBC handler must have the ability to securely read the source identifier of the calling module, a unique string for each module in the host state machine, which cannot be altered by the module or faked by another module.
+
+**This crate currently requires neither of the host system**. Since modules are assumed to be trusted, there is no need for this object capability system that protects resources for potentially malicious modules.
+
+For more background on this, see [this issue](https://github.com/informalsystems/ibc-rs/issues/2159).
+
+### Port system: transferring and releasing a port
+ICS 5 (Port Allocation) requires the IBC handler to permit [transferring ownership of a port](https://github.com/cosmos/ibc/tree/master/spec/core/ics-005-port-allocation#transferring-ownership-of-a-port) and [releasing a port](https://github.com/cosmos/ibc/tree/master/spec/core/ics-005-port-allocation#releasing-a-port).
+
+We currently support neither.
+
 ## License
 
 Copyright © 2021 Informal Systems Inc. and ibc-rs authors.

diff --git a/modules/src/core/ics04_channel/context.rs b/modules/src/core/ics04_channel/context.rs
@@ -13,18 +13,15 @@ use crate::core::ics04_channel::handler::recv_packet::RecvPacketResult;
 use crate::core::ics04_channel::handler::{ChannelIdState, ChannelResult};
 use crate::core::ics04_channel::msgs::acknowledgement::Acknowledgement;
 use crate::core::ics04_channel::{error::Error, packet::Receipt};
-use crate::core::ics05_port::capabilities::ChannelCapability;
-use crate::core::ics05_port::context::CapabilityReader;
 use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId};
-use crate::core::ics26_routing::context::ModuleId;
 use crate::prelude::*;
 use crate::timestamp::Timestamp;
 use crate::Height;
 
 use super::packet::{PacketResult, Sequence};
 
 /// A context supplying all the necessary read-only dependencies for processing any `ChannelMsg`.
-pub trait ChannelReader: CapabilityReader {
+pub trait ChannelReader {
     /// Returns the ChannelEnd for the given `port_id` and `chan_id`.
     fn channel_end(&self, port_channel_id: &(PortId, ChannelId)) -> Result<ChannelEnd, Error>;
 
@@ -43,8 +40,6 @@ pub trait ChannelReader: CapabilityReader {
         height: Height,
     ) -> Result<AnyConsensusState, Error>;
 
-    fn authenticated_capability(&self, port_id: &PortId) -> Result<ChannelCapability, Error>;
-
     fn get_next_sequence_send(
         &self,
         port_channel_id: &(PortId, ChannelId),
@@ -131,13 +126,6 @@ pub trait ChannelReader: CapabilityReader {
     fn block_delay(&self, delay_period_time: Duration) -> u64 {
         calculate_block_delay(delay_period_time, self.max_expected_time_per_block())
     }
-
-    /// Return the module_id along with the capability associated with a given (channel-id, port_id)
-    fn lookup_module_by_channel(
-        &self,
-        channel_id: &ChannelId,
-        port_id: &PortId,
-    ) -> Result<(ModuleId, ChannelCapability), Error>;
 }
 
 /// A context supplying all the necessary write-only dependencies (i.e., storage writing facility)

diff --git a/modules/src/core/ics04_channel/error.rs b/modules/src/core/ics04_channel/error.rs
@@ -104,17 +104,6 @@ define_error! {
         MissingChannel
             | _ | { "missing channel end" },
 
-        NoPortCapability
-            { port_id: PortId }
-            | e | {
-                format_args!(
-                    "the port {0} has no capability associated",
-                    e.port_id)
-            },
-
-        InvalidPortCapability
-            | _ | { "the module associated with the port does not have the capability it needs" },
-
         InvalidVersionLengthConnection
             | _ | { "single version must be negociated on connection before opening channel" },
 

diff --git a/modules/src/core/ics04_channel/handler.rs b/modules/src/core/ics04_channel/handler.rs
@@ -5,7 +5,6 @@ use crate::core::ics04_channel::context::ChannelReader;
 use crate::core::ics04_channel::error::Error;
 use crate::core::ics04_channel::msgs::ChannelMsg;
 use crate::core::ics04_channel::{msgs::PacketMsg, packet::PacketResult};
-use crate::core::ics05_port::capabilities::ChannelCapability;
 use crate::core::ics24_host::identifier::{ChannelId, PortId};
 use crate::core::ics26_routing::context::{
     Ics26Context, ModuleId, ModuleOutput, OnRecvPacketAck, Router,
@@ -42,7 +41,6 @@ pub struct ChannelResult {
     pub port_id: PortId,
     pub channel_id: ChannelId,
     pub channel_id_state: ChannelIdState,
-    pub channel_cap: ChannelCapability,
     pub channel_end: ChannelEnd,
 }
 
@@ -106,7 +104,6 @@ where
             &msg.channel.connection_hops,
             &msg.port_id,
             &result.channel_id,
-            &result.channel_cap,
             msg.channel.counterparty(),
             &msg.channel.version,
         )?,
@@ -117,7 +114,6 @@ where
                 &msg.channel.connection_hops,
                 &msg.port_id,
                 &result.channel_id,
-                &result.channel_cap,
                 msg.channel.counterparty(),
                 &msg.counterparty_version,
             )?;
@@ -142,30 +138,23 @@ where
     Ok(result)
 }
 
-pub fn packet_validate<Ctx>(ctx: &Ctx, msg: &PacketMsg) -> Result<ModuleId, Error>
+pub fn get_module_for_packet_msg<Ctx>(ctx: &Ctx, msg: &PacketMsg) -> Result<ModuleId, Error>
 where
     Ctx: Ics26Context,
 {
     let module_id = match msg {
-        PacketMsg::RecvPacket(msg) => {
-            ctx.lookup_module_by_channel(
-                &msg.packet.destination_channel,
-                &msg.packet.destination_port,
-            )?
-            .0
-        }
-        PacketMsg::AckPacket(msg) => {
-            ctx.lookup_module_by_channel(&msg.packet.source_channel, &msg.packet.source_port)?
-                .0
-        }
-        PacketMsg::ToPacket(msg) => {
-            ctx.lookup_module_by_channel(&msg.packet.source_channel, &msg.packet.source_port)?
-                .0
-        }
-        PacketMsg::ToClosePacket(msg) => {
-            ctx.lookup_module_by_channel(&msg.packet.source_channel, &msg.packet.source_port)?
-                .0
-        }
+        PacketMsg::RecvPacket(msg) => ctx
+            .lookup_module_by_port(&msg.packet.destination_port)
+            .map_err(Error::ics05_port)?,
+        PacketMsg::AckPacket(msg) => ctx
+            .lookup_module_by_port(&msg.packet.source_port)
+            .map_err(Error::ics05_port)?,
+        PacketMsg::ToPacket(msg) => ctx
+            .lookup_module_by_port(&msg.packet.source_port)
+            .map_err(Error::ics05_port)?,
+        PacketMsg::ToClosePacket(msg) => ctx
+            .lookup_module_by_port(&msg.packet.source_port)
+            .map_err(Error::ics05_port)?,
     };
 
     if ctx.router().has_route(&module_id) {

diff --git a/modules/src/core/ics04_channel/handler/acknowledgement.rs b/modules/src/core/ics04_channel/handler/acknowledgement.rs
@@ -34,8 +34,6 @@ pub fn process(
         return Err(Error::channel_closed(packet.source_channel));
     }
 
-    let _channel_cap = ctx.authenticated_capability(&packet.source_port)?;
-
     let counterparty = Counterparty::new(
         packet.destination_port.clone(),
         Some(packet.destination_channel),
@@ -134,7 +132,7 @@ mod tests {
     use crate::core::ics04_channel::msgs::acknowledgement::test_util::get_dummy_raw_msg_acknowledgement;
     use crate::core::ics04_channel::msgs::acknowledgement::MsgAcknowledgement;
     use crate::core::ics04_channel::Version;
-    use crate::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId};
+    use crate::core::ics24_host::identifier::{ClientId, ConnectionId};
     use crate::events::IbcEvent;
     use crate::mock::context::MockContext;
     use crate::prelude::*;
@@ -195,23 +193,11 @@ mod tests {
                 msg: msg.clone(),
                 want_pass: false,
             },
-            Test {
-                name: "Processing fails because the port does not have a capability associated"
-                    .to_string(),
-                ctx: context.clone().with_channel(
-                    PortId::default(),
-                    ChannelId::default(),
-                    source_channel_end.clone(),
-                ),
-                msg: msg.clone(),
-                want_pass: false,
-            },
             Test {
                 name: "Good parameters".to_string(),
                 ctx: context
                     .with_client(&ClientId::default(), client_height)
                     .with_connection(ConnectionId::default(), connection_end)
-                    .with_port_capability(packet.destination_port.clone())
                     .with_channel(
                         packet.source_port.clone(),
                         packet.source_channel,

diff --git a/modules/src/core/ics04_channel/handler/chan_close_confirm.rs b/modules/src/core/ics04_channel/handler/chan_close_confirm.rs
@@ -25,9 +25,6 @@ pub(crate) fn process(
         return Err(Error::channel_closed(msg.channel_id));
     }
 
-    // Channel capabilities
-    let channel_cap = ctx.authenticated_capability(&msg.port_id)?;
-
     // An OPEN IBC connection running on the local (host) chain should exist.
     if channel_end.connection_hops().len() != 1 {
         return Err(Error::invalid_connection_hops_length(
@@ -82,7 +79,6 @@ pub(crate) fn process(
         port_id: msg.port_id.clone(),
         channel_id: msg.channel_id,
         channel_id_state: ChannelIdState::Reused,
-        channel_cap,
         channel_end,
     };
 
@@ -159,7 +155,6 @@ mod tests {
         let context = default_context
             .with_client(&client_id, client_consensus_state_height)
             .with_connection(conn_id, conn_end)
-            .with_port_capability(msg_chan_close_confirm.port_id.clone())
             .with_channel(
                 msg_chan_close_confirm.port_id.clone(),
                 msg_chan_close_confirm.channel_id,

diff --git a/modules/src/core/ics04_channel/handler/chan_close_init.rs b/modules/src/core/ics04_channel/handler/chan_close_init.rs
@@ -26,10 +26,7 @@ pub(crate) fn process(
         ));
     }
 
-    // Channel capabilities
-    let channel_cap = ctx.authenticated_capability(&msg.port_id)?;
     // An OPEN IBC connection running on the local (host) chain should exist.
-
     if channel_end.connection_hops().len() != 1 {
         return Err(Error::invalid_connection_hops_length(
             1,
@@ -54,7 +51,6 @@ pub(crate) fn process(
         port_id: msg.port_id.clone(),
         channel_id: msg.channel_id,
         channel_id_state: ChannelIdState::Reused,
-        channel_cap,
         channel_end,
     };
 
@@ -131,7 +127,6 @@ mod tests {
             default_context
                 .with_client(&client_id, client_consensus_state_height)
                 .with_connection(conn_id, conn_end)
-                .with_port_capability(msg_chan_close_init.port_id.clone())
                 .with_channel(
                     msg_chan_close_init.port_id.clone(),
                     msg_chan_close_init.channel_id,

diff --git a/modules/src/core/ics04_channel/handler/chan_open_ack.rs b/modules/src/core/ics04_channel/handler/chan_open_ack.rs
@@ -28,9 +28,6 @@ pub(crate) fn process(
         ));
     }
 
-    // Channel capabilities
-    let channel_cap = ctx.authenticated_capability(&msg.port_id)?;
-
     // An OPEN IBC connection running on the local (host) chain should exist.
 
     if channel_end.connection_hops().len() != 1 {
@@ -91,7 +88,6 @@ pub(crate) fn process(
         port_id: msg.port_id.clone(),
         channel_id: msg.channel_id,
         channel_id_state: ChannelIdState::Reused,
-        channel_cap,
         channel_end,
     };
 
@@ -226,7 +222,6 @@ mod tests {
                         &msg_conn_try.client_id,
                         Height::new(0, client_consensus_state_height),
                     )
-                    .with_port_capability(msg_chan_ack.port_id.clone())
                     .with_channel(
                         msg_chan_ack.port_id.clone(),
                         msg_chan_ack.channel_id,
@@ -235,24 +230,6 @@ mod tests {
                 msg: ChannelMsg::ChannelOpenAck(msg_chan_ack.clone()),
                 want_pass: false,
             },
-            Test {
-                name: "Processing fails because port does not have a capability associate"
-                    .to_string(),
-                ctx: context
-                    .clone()
-                    .with_client(
-                        &msg_conn_try.client_id,
-                        Height::new(0, client_consensus_state_height),
-                    )
-                    .with_connection(cid.clone(), conn_end.clone())
-                    .with_channel(
-                        msg_chan_ack.port_id.clone(),
-                        msg_chan_ack.channel_id,
-                        chan_end.clone(),
-                    ),
-                msg: ChannelMsg::ChannelOpenAck(msg_chan_ack.clone()),
-                want_pass: false,
-            },
             Test {
                 name: "Processing fails because a connection does exist".to_string(),
                 ctx: context
@@ -261,7 +238,6 @@ mod tests {
                         &msg_conn_try.client_id,
                         Height::new(0, client_consensus_state_height),
                     )
-                    .with_port_capability(msg_chan_ack.port_id.clone())
                     .with_channel(
                         msg_chan_ack.port_id.clone(),
                         msg_chan_ack.channel_id,
@@ -275,7 +251,6 @@ mod tests {
                 ctx: context
                     .clone()
                     .with_connection(cid.clone(), conn_end.clone())
-                    .with_port_capability(msg_chan_ack.port_id.clone())
                     .with_channel(
                         msg_chan_ack.port_id.clone(),
                         msg_chan_ack.channel_id,
@@ -292,7 +267,6 @@ mod tests {
                         Height::new(0, client_consensus_state_height),
                     )
                     .with_connection(cid, conn_end)
-                    .with_port_capability(msg_chan_ack.port_id.clone())
                     .with_channel(
                         msg_chan_ack.port_id.clone(),
                         msg_chan_ack.channel_id,

diff --git a/modules/src/core/ics04_channel/handler/chan_open_confirm.rs b/modules/src/core/ics04_channel/handler/chan_open_confirm.rs
@@ -28,9 +28,6 @@ pub(crate) fn process(
         ));
     }
 
-    // Channel capabilities
-    let channel_cap = ctx.authenticated_capability(&msg.port_id)?;
-
     // An OPEN IBC connection running on the local (host) chain should exist.
     if channel_end.connection_hops().len() != 1 {
         return Err(Error::invalid_connection_hops_length(
@@ -86,7 +83,6 @@ pub(crate) fn process(
         port_id: msg.port_id.clone(),
         channel_id: msg.channel_id,
         channel_id_state: ChannelIdState::Reused,
-        channel_cap,
         channel_end,
     };
 
@@ -174,7 +170,6 @@ mod tests {
             ctx: context
                 .with_client(&client_id, Height::new(0, client_consensus_state_height))
                 .with_connection(conn_id, conn_end)
-                .with_port_capability(msg_chan_confirm.port_id.clone())
                 .with_channel(
                     msg_chan_confirm.port_id.clone(),
                     msg_chan_confirm.channel_id,