Don't require ICS20 module to implement Channel{Reader,Keeper} (#244)

* Add SendPacketReader trait with blanket impl for all types implementing ChannelReader

* Have SendPacket handler depend on SendPacketReader and return SendPacketResult

* Define Ics20ChannelKeeper and depend on that instead of Channel{Reader,Keeper}

* Update tests

* Add blanket impl for Ics20ChannelKeeper

* Add changelog entry

* Use where clause for trait bounds

* Avoid trait object param in send_packet handler

* Apply suggestion to merge Ics20ChannelKeeper into Ics20Keeper

* Rename Ics20Keeper -> TokenTransferKeeper

* Rename Ics20Reader -> TokenTransferReader

Author: hu55a1n1

.changelog/unreleased/breaking-changes/182-ics20-channel-reader-keeper.md

@@ -0,0 +1,4 @@
+- `Ics20Context` no longer requires types implementing it to implement `ChannelReader` and `ChannelKeeper`, and instead depends on the `Ics20ChannelKeeper`
+  and `SendPacketReader` traits. Additionally, the `send_packet` handler now requires the calling context to implement `SendPacketReader` and returns
+  a `SendPacketResult`.
+  ([#182](https://github.com/cosmos/ibc-rs/issues/182))

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

@@ -9,23 +9,53 @@ use crate::applications::transfer::relay::on_recv_packet::process_recv_packet;
 use crate::applications::transfer::relay::on_timeout_packet::process_timeout_packet;
 use crate::applications::transfer::{PrefixedCoin, PrefixedDenom, VERSION};
 use crate::core::ics04_channel::channel::{Counterparty, Order};
-use crate::core::ics04_channel::context::{ChannelKeeper, ChannelReader};
+use crate::core::ics04_channel::commitment::PacketCommitment;
+use crate::core::ics04_channel::context::{ChannelKeeper, SendPacketReader};
+use crate::core::ics04_channel::error::Error as Ics04Error;
+use crate::core::ics04_channel::handler::send_packet::SendPacketResult;
 use crate::core::ics04_channel::handler::ModuleExtras;
 use crate::core::ics04_channel::msgs::acknowledgement::Acknowledgement as GenericAcknowledgement;
-use crate::core::ics04_channel::packet::Packet;
+use crate::core::ics04_channel::packet::{Packet, Sequence};
 use crate::core::ics04_channel::Version;
 use crate::core::ics24_host::identifier::{ChannelId, ConnectionId, PortId};
 use crate::core::ics26_routing::context::{ModuleOutputBuilder, OnRecvPacketAck};
 use crate::prelude::*;
 use crate::signer::Signer;
 
-pub trait Ics20Keeper:
-    ChannelKeeper + BankKeeper<AccountId = <Self as Ics20Keeper>::AccountId>
-{
-    type AccountId;
+pub trait TokenTransferKeeper: BankKeeper {
+    fn store_send_packet_result(&mut self, result: SendPacketResult) -> Result<(), Ics04Error> {
+        self.store_next_sequence_send(
+            result.port_id.clone(),
+            result.channel_id.clone(),
+            result.seq_number,
+        )?;
+
+        self.store_packet_commitment(
+            result.port_id,
+            result.channel_id,
+            result.seq,
+            result.commitment,
+        )?;
+        Ok(())
+    }
+
+    fn store_packet_commitment(
+        &mut self,
+        port_id: PortId,
+        channel_id: ChannelId,
+        sequence: Sequence,
+        commitment: PacketCommitment,
+    ) -> Result<(), Ics04Error>;
+
+    fn store_next_sequence_send(
+        &mut self,
+        port_id: PortId,
+        channel_id: ChannelId,
+        seq: Sequence,
+    ) -> Result<(), Ics04Error>;
 }
 
-pub trait Ics20Reader: ChannelReader {
+pub trait TokenTransferReader: SendPacketReader {
     type AccountId: TryFrom<Signer>;
 
     /// get_port returns the portID for the transfer module.
@@ -36,7 +66,7 @@ pub trait Ics20Reader: ChannelReader {
         &self,
         port_id: &PortId,
         channel_id: &ChannelId,
-    ) -> Result<<Self as Ics20Reader>::AccountId, Ics20Error>;
+    ) -> Result<<Self as TokenTransferReader>::AccountId, Ics20Error>;
 
     /// Returns true iff send is enabled.
     fn is_send_enabled(&self) -> bool;
@@ -51,6 +81,30 @@ pub trait Ics20Reader: ChannelReader {
     }
 }
 
+impl<T> TokenTransferKeeper for T
+where
+    T: ChannelKeeper + BankKeeper,
+{
+    fn store_packet_commitment(
+        &mut self,
+        port_id: PortId,
+        channel_id: ChannelId,
+        sequence: Sequence,
+        commitment: PacketCommitment,
+    ) -> Result<(), Ics04Error> {
+        ChannelKeeper::store_packet_commitment(self, port_id, channel_id, sequence, commitment)
+    }
+
+    fn store_next_sequence_send(
+        &mut self,
+        port_id: PortId,
+        channel_id: ChannelId,
+        seq: Sequence,
+    ) -> Result<(), Ics04Error> {
+        ChannelKeeper::store_next_sequence_send(self, port_id, channel_id, seq)
+    }
+}
+
 // https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-028-public-key-addresses.md
 pub fn cosmos_adr028_escrow_address(port_id: &PortId, channel_id: &ChannelId) -> Vec<u8> {
     let contents = format!("{}/{}", port_id, channel_id);
@@ -94,8 +148,8 @@ pub trait BankKeeper {
 /// Captures all the dependencies which the ICS20 module requires to be able to dispatch and
 /// process IBC messages.
 pub trait Ics20Context:
-    Ics20Keeper<AccountId = <Self as Ics20Context>::AccountId>
-    + Ics20Reader<AccountId = <Self as Ics20Context>::AccountId>
+    TokenTransferKeeper<AccountId = <Self as Ics20Context>::AccountId>
+    + TokenTransferReader<AccountId = <Self as Ics20Context>::AccountId>
 {
     type AccountId: TryFrom<Signer>;
 }
@@ -197,7 +251,7 @@ pub fn on_recv_packet<Ctx: 'static + Ics20Context>(
         Err(_) => {
             return OnRecvPacketAck::Failed(Box::new(Acknowledgement::Error(
                 Ics20Error::packet_data_deserialization().to_string(),
-            )))
+            )));
         }
     };
 

crates/ibc/src/applications/transfer/relay/send_transfer.rs

@@ -94,7 +94,7 @@ where
         events,
     } = send_packet(ctx, packet).map_err(Error::ics04_channel)?;
 
-    ctx.store_packet_result(result)
+    ctx.store_send_packet_result(result)
         .map_err(Error::ics04_channel)?;
 
     output.merge_output(

crates/ibc/src/core/ics04_channel/context.rs

@@ -150,6 +150,89 @@ pub trait ChannelReader {
     }
 }
 
+pub trait SendPacketReader {
+    /// Returns the ChannelEnd for the given `port_id` and `chan_id`.
+    fn channel_end(&self, port_id: &PortId, channel_id: &ChannelId) -> Result<ChannelEnd, Error>;
+
+    /// Returns the ConnectionState for the given identifier `connection_id`.
+    fn connection_end(&self, connection_id: &ConnectionId) -> Result<ConnectionEnd, Error>;
+
+    /// Returns the ClientState for the given identifier `client_id`. Necessary dependency towards
+    /// proof verification.
+    fn client_state(&self, client_id: &ClientId) -> Result<Box<dyn ClientState>, Error>;
+
+    fn client_consensus_state(
+        &self,
+        client_id: &ClientId,
+        height: Height,
+    ) -> Result<Box<dyn ConsensusState>, Error>;
+
+    fn get_next_sequence_send(
+        &self,
+        port_id: &PortId,
+        channel_id: &ChannelId,
+    ) -> Result<Sequence, Error>;
+
+    fn hash(&self, value: Vec<u8>) -> Vec<u8>;
+
+    fn packet_commitment(
+        &self,
+        packet_data: Vec<u8>,
+        timeout_height: TimeoutHeight,
+        timeout_timestamp: Timestamp,
+    ) -> PacketCommitment {
+        let mut hash_input = timeout_timestamp.nanoseconds().to_be_bytes().to_vec();
+
+        let revision_number = timeout_height.commitment_revision_number().to_be_bytes();
+        hash_input.append(&mut revision_number.to_vec());
+
+        let revision_height = timeout_height.commitment_revision_height().to_be_bytes();
+        hash_input.append(&mut revision_height.to_vec());
+
+        let packet_data_hash = self.hash(packet_data);
+        hash_input.append(&mut packet_data_hash.to_vec());
+
+        self.hash(hash_input).into()
+    }
+}
+
+impl<T> SendPacketReader for T
+where
+    T: ChannelReader,
+{
+    fn channel_end(&self, port_id: &PortId, channel_id: &ChannelId) -> Result<ChannelEnd, Error> {
+        ChannelReader::channel_end(self, port_id, channel_id)
+    }
+
+    fn connection_end(&self, connection_id: &ConnectionId) -> Result<ConnectionEnd, Error> {
+        ChannelReader::connection_end(self, connection_id)
+    }
+
+    fn client_state(&self, client_id: &ClientId) -> Result<Box<dyn ClientState>, Error> {
+        ChannelReader::client_state(self, client_id)
+    }
+
+    fn client_consensus_state(
+        &self,
+        client_id: &ClientId,
+        height: Height,
+    ) -> Result<Box<dyn ConsensusState>, Error> {
+        ChannelReader::client_consensus_state(self, client_id, height)
+    }
+
+    fn get_next_sequence_send(
+        &self,
+        port_id: &PortId,
+        channel_id: &ChannelId,
+    ) -> Result<Sequence, Error> {
+        ChannelReader::get_next_sequence_send(self, port_id, channel_id)
+    }
+
+    fn hash(&self, value: Vec<u8>) -> Vec<u8> {
+        ChannelReader::hash(self, value)
+    }
+}
+
 /// A context supplying all the necessary write-only dependencies (i.e., storage writing facility)
 /// for processing any `ChannelMsg`.
 pub trait ChannelKeeper {

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

@@ -2,8 +2,8 @@ use crate::core::ics04_channel::channel::Counterparty;
 use crate::core::ics04_channel::channel::State;
 use crate::core::ics04_channel::commitment::PacketCommitment;
 use crate::core::ics04_channel::events::SendPacket;
-use crate::core::ics04_channel::packet::{PacketResult, Sequence};
-use crate::core::ics04_channel::{context::ChannelReader, error::Error, packet::Packet};
+use crate::core::ics04_channel::packet::Sequence;
+use crate::core::ics04_channel::{context::SendPacketReader, error::Error, packet::Packet};
 use crate::core::ics24_host::identifier::{ChannelId, PortId};
 use crate::events::IbcEvent;
 use crate::handler::{HandlerOutput, HandlerResult};
@@ -19,7 +19,10 @@ pub struct SendPacketResult {
     pub commitment: PacketCommitment,
 }
 
-pub fn send_packet(ctx: &dyn ChannelReader, packet: Packet) -> HandlerResult<PacketResult, Error> {
+pub fn send_packet(
+    ctx: &impl SendPacketReader,
+    packet: Packet,
+) -> HandlerResult<SendPacketResult, Error> {
     let mut output = HandlerOutput::builder();
 
     let source_channel_end = ctx.channel_end(&packet.source_port, &packet.source_channel)?;
@@ -78,7 +81,7 @@ pub fn send_packet(ctx: &dyn ChannelReader, packet: Packet) -> HandlerResult<Pac
 
     output.log("success: packet send ");
 
-    let result = PacketResult::Send(SendPacketResult {
+    let result = SendPacketResult {
         port_id: packet.source_port.clone(),
         channel_id: packet.source_channel.clone(),
         seq: packet.sequence,
@@ -88,7 +91,7 @@ pub fn send_packet(ctx: &dyn ChannelReader, packet: Packet) -> HandlerResult<Pac
             packet.timeout_height,
             packet.timeout_timestamp,
         ),
-    });
+    };
 
     output.emit(IbcEvent::SendPacket(SendPacket::new(
         packet,