From 07c0635d9a65d78a52d7bfc924f97511e02569f9 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 18 Jan 2023 14:09:45 -0500 Subject: [PATCH 01/15] packet validation scaffoldinig --- crates/ibc/src/core/context.rs | 45 ++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index f0224d9d5..1abd8d48c 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -91,7 +91,7 @@ mod val_exec_ctx { use crate::core::ics04_channel::msgs::chan_open_confirm::MsgChannelOpenConfirm; use crate::core::ics04_channel::msgs::chan_open_init::MsgChannelOpenInit; use crate::core::ics04_channel::msgs::chan_open_try::MsgChannelOpenTry; - use crate::core::ics04_channel::msgs::ChannelMsg; + use crate::core::ics04_channel::msgs::{ChannelMsg, PacketMsg}; use crate::core::ics04_channel::packet::{Receipt, Sequence}; use crate::core::ics04_channel::timeout::TimeoutHeight; use crate::core::ics05_port::error::PortError::UnknownPort; @@ -133,7 +133,7 @@ mod val_exec_ctx { /// Return the module_id associated with a given port_id fn lookup_module_by_port(&self, port_id: &PortId) -> Option; - fn lookup_module(&self, msg: &ChannelMsg) -> Result { + fn lookup_module_channel(&self, msg: &ChannelMsg) -> Result { let port_id = match msg { ChannelMsg::OpenInit(msg) => &msg.port_id_on_a, ChannelMsg::OpenTry(msg) => &msg.port_id_on_b, @@ -149,6 +149,21 @@ mod val_exec_ctx { }))?; Ok(module_id) } + + fn lookup_module_packet(&self, msg: &PacketMsg) -> Result { + let port_id = match msg { + PacketMsg::Recv(msg) => &msg.packet.port_on_b, + PacketMsg::Ack(msg) => &msg.packet.port_on_a, + PacketMsg::Timeout(msg) => &msg.packet.port_on_a, + PacketMsg::TimeoutOnClose(msg) => &msg.packet.port_on_a, + }; + let module_id = self + .lookup_module_by_port(port_id) + .ok_or(ChannelError::Port(UnknownPort { + port_id: port_id.clone(), + }))?; + Ok(module_id) + } } pub trait ValidationContext: Router { @@ -173,7 +188,9 @@ mod val_exec_ctx { } .map_err(RouterError::ContextError), MsgEnvelope::Channel(msg) => { - let module_id = self.lookup_module(&msg).map_err(ContextError::from)?; + let module_id = self + .lookup_module_channel(&msg) + .map_err(ContextError::from)?; if !self.has_route(&module_id) { return Err(ChannelError::RouteNotFound) .map_err(ContextError::ChannelError) @@ -196,7 +213,23 @@ mod val_exec_ctx { } .map_err(RouterError::ContextError) } - MsgEnvelope::Packet(_msg) => todo!(), + MsgEnvelope::Packet(msg) => { + let module_id = self + .lookup_module_packet(&msg) + .map_err(ContextError::from)?; + if !self.has_route(&module_id) { + return Err(ChannelError::RouteNotFound) + .map_err(ContextError::ChannelError) + .map_err(RouterError::ContextError); + } + + match msg { + PacketMsg::Recv(_) => todo!(), + PacketMsg::Ack(_) => todo!(), + PacketMsg::Timeout(_) => todo!(), + PacketMsg::TimeoutOnClose(_) => todo!(), + } + } } } @@ -413,7 +446,9 @@ mod val_exec_ctx { } .map_err(RouterError::ContextError), MsgEnvelope::Channel(msg) => { - let module_id = self.lookup_module(&msg).map_err(ContextError::from)?; + let module_id = self + .lookup_module_channel(&msg) + .map_err(ContextError::from)?; if !self.has_route(&module_id) { return Err(ChannelError::RouteNotFound) .map_err(ContextError::ChannelError) From f4a5bc03f2c6ef2491599e669924400eca08b7ab Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 18 Jan 2023 14:57:05 -0500 Subject: [PATCH 02/15] recv_packet validation --- .../clients/ics07_tendermint/client_state.rs | 74 +++++++++++ crates/ibc/src/core/context.rs | 29 ++++- .../ibc/src/core/ics02_client/client_state.rs | 16 +++ .../core/ics04_channel/handler/recv_packet.rs | 121 ++++++++++++++++++ crates/ibc/src/mock/client_state.rs | 16 +++ 5 files changed, 252 insertions(+), 4 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index 17be438b8..b0aecf3aa 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -966,6 +966,39 @@ impl Ics2ClientState for ClientState { verify_membership(client_state, prefix, proof, root, path, value) } + #[cfg(feature = "val_exec_ctx")] + fn new_verify_packet_data( + &self, + ctx: &dyn ValidationContext, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + commitment: PacketCommitment, + ) -> Result<(), ClientError> { + let client_state = downcast_tm_client_state(self)?; + client_state.verify_height(height)?; + new_verify_delay_passed(ctx, height, connection_end)?; + + let commitment_path = CommitmentsPath { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + sequence, + }; + + verify_membership( + client_state, + connection_end.counterparty().prefix(), + proof, + root, + commitment_path, + commitment.into_vec(), + ) + } + fn verify_packet_data( &self, ctx: &dyn ChannelReader, @@ -1172,6 +1205,47 @@ fn verify_delay_passed( .map_err(|e| e.into()) } +#[cfg(feature = "val_exec_ctx")] +fn new_verify_delay_passed( + ctx: &dyn ValidationContext, + height: Height, + connection_end: &ConnectionEnd, +) -> Result<(), ClientError> { + let current_timestamp = ctx.host_timestamp().map_err(|e| ClientError::Other { + description: e.to_string(), + })?; + let current_height = ctx.host_height().map_err(|e| ClientError::Other { + description: e.to_string(), + })?; + + let client_id = connection_end.client_id(); + let processed_time = + ctx.client_update_time(client_id, &height) + .map_err(|_| Error::ProcessedTimeNotFound { + client_id: client_id.clone(), + height, + })?; + let processed_height = ctx.client_update_height(client_id, &height).map_err(|_| { + Error::ProcessedHeightNotFound { + client_id: client_id.clone(), + height, + } + })?; + + let delay_period_time = connection_end.delay_period(); + let delay_period_height = ctx.block_delay(&delay_period_time); + + ClientState::verify_delay_passed( + current_timestamp, + current_height, + processed_time, + processed_height, + delay_period_time, + delay_period_height, + ) + .map_err(|e| e.into()) +} + fn downcast_tm_client_state(cs: &dyn Ics2ClientState) -> Result<&ClientState, ClientError> { cs.as_any() .downcast_ref::() diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 1abd8d48c..1a419d532 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -82,7 +82,7 @@ mod val_exec_ctx { }; use crate::core::ics04_channel::handler::{ chan_close_confirm, chan_close_init, chan_open_ack, chan_open_confirm, chan_open_init, - chan_open_try, + chan_open_try, recv_packet, }; use crate::core::ics04_channel::msgs::acknowledgement::Acknowledgement; use crate::core::ics04_channel::msgs::chan_close_confirm::MsgChannelCloseConfirm; @@ -91,6 +91,7 @@ mod val_exec_ctx { use crate::core::ics04_channel::msgs::chan_open_confirm::MsgChannelOpenConfirm; use crate::core::ics04_channel::msgs::chan_open_init::MsgChannelOpenInit; use crate::core::ics04_channel::msgs::chan_open_try::MsgChannelOpenTry; + use crate::core::ics04_channel::msgs::recv_packet::MsgRecvPacket; use crate::core::ics04_channel::msgs::{ChannelMsg, PacketMsg}; use crate::core::ics04_channel::packet::{Receipt, Sequence}; use crate::core::ics04_channel::timeout::TimeoutHeight; @@ -223,12 +224,15 @@ mod val_exec_ctx { .map_err(RouterError::ContextError); } - match msg { - PacketMsg::Recv(_) => todo!(), + // TODO: use the return value appropriately + let _ = match msg { + PacketMsg::Recv(msg) => recv_packet_validate(self, module_id, msg), PacketMsg::Ack(_) => todo!(), PacketMsg::Timeout(_) => todo!(), PacketMsg::TimeoutOnClose(_) => todo!(), - } + }; + + todo!() } } } @@ -1149,4 +1153,21 @@ mod val_exec_ctx { Ok(()) } + + fn recv_packet_validate( + ctx_b: &ValCtx, + module_id: ModuleId, + msg: MsgRecvPacket, + ) -> Result<(), ContextError> + where + ValCtx: ValidationContext, + { + recv_packet::validate(ctx_b, &msg)?; + + let _module = ctx_b + .get_route(&module_id) + .ok_or(ChannelError::RouteNotFound)?; + + todo!() + } } diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 3247d13dd..3aecb0c15 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -175,6 +175,22 @@ pub trait ClientState: expected_client_state: Any, ) -> Result<(), ClientError>; + /// Verify a `proof` that a packet has been commited. + #[cfg(feature = "val_exec_ctx")] + #[allow(clippy::too_many_arguments)] + fn new_verify_packet_data( + &self, + ctx: &dyn ValidationContext, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + commitment: PacketCommitment, + ) -> Result<(), ClientError>; + /// Verify a `proof` that a packet has been commited. #[allow(clippy::too_many_arguments)] fn verify_packet_data( diff --git a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs index 36064dd6a..ee3ddf719 100644 --- a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs @@ -12,6 +12,127 @@ use crate::handler::{HandlerOutput, HandlerResult}; use crate::timestamp::Expiry; use alloc::string::ToString; +#[cfg(feature = "val_exec_ctx")] +pub(crate) use val_exec_ctx::*; +#[cfg(feature = "val_exec_ctx")] +pub(crate) mod val_exec_ctx { + use super::*; + use crate::core::{ContextError, ValidationContext}; + + pub fn validate(ctx_b: &Ctx, msg: &MsgRecvPacket) -> Result<(), ContextError> + where + Ctx: ValidationContext, + { + let port_chan_id_on_b = &(msg.packet.port_on_b.clone(), msg.packet.chan_on_b.clone()); + let chan_end_on_b = ctx_b.channel_end(port_chan_id_on_b)?; + + if !chan_end_on_b.state_matches(&State::Open) { + return Err(PacketError::InvalidChannelState { + channel_id: msg.packet.chan_on_a.clone(), + state: chan_end_on_b.state, + } + .into()); + } + + let counterparty = Counterparty::new( + msg.packet.port_on_a.clone(), + Some(msg.packet.chan_on_a.clone()), + ); + + if !chan_end_on_b.counterparty_matches(&counterparty) { + return Err(PacketError::InvalidPacketCounterparty { + port_id: msg.packet.port_on_a.clone(), + channel_id: msg.packet.chan_on_a.clone(), + } + .into()); + } + + let conn_id_on_b = &chan_end_on_b.connection_hops()[0]; + let conn_end_on_b = ctx_b.connection_end(conn_id_on_b)?; + + if !conn_end_on_b.state_matches(&ConnectionState::Open) { + return Err(PacketError::ConnectionNotOpen { + connection_id: chan_end_on_b.connection_hops()[0].clone(), + } + .into()); + } + + let latest_height = ctx_b.host_height()?; + if msg.packet.timeout_height_on_b.has_expired(latest_height) { + return Err(PacketError::LowPacketHeight { + chain_height: latest_height, + timeout_height: msg.packet.timeout_height_on_b, + } + .into()); + } + + let latest_timestamp = ctx_b.host_timestamp()?; + if let Expiry::Expired = latest_timestamp.check_expiry(&msg.packet.timeout_timestamp_on_b) { + return Err(PacketError::LowPacketTimestamp.into()); + } + + // Verify proofs + { + let client_id_on_b = conn_end_on_b.client_id(); + let client_state_of_a_on_b = ctx_b.client_state(client_id_on_b)?; + + // The client must not be frozen. + if client_state_of_a_on_b.is_frozen() { + return Err(PacketError::FrozenClient { + client_id: client_id_on_b.clone(), + } + .into()); + } + + let consensus_state_of_a_on_b = + ctx_b.consensus_state(client_id_on_b, &msg.proof_height_on_a)?; + + let expected_commitment_on_a = ctx_b.packet_commitment( + &msg.packet.data, + &msg.packet.timeout_height_on_b, + &msg.packet.timeout_timestamp_on_b, + ); + // Verify the proof for the packet against the chain store. + client_state_of_a_on_b + .new_verify_packet_data( + ctx_b, + msg.proof_height_on_a, + &conn_end_on_b, + &msg.proof_commitment_on_a, + consensus_state_of_a_on_b.root(), + &msg.packet.port_on_a, + &msg.packet.chan_on_a, + msg.packet.sequence, + expected_commitment_on_a, + ) + .map_err(|e| ChannelError::PacketVerificationFailed { + sequence: msg.packet.sequence, + client_error: e, + }) + .map_err(PacketError::Channel)?; + } + + if chan_end_on_b.order_matches(&Order::Ordered) { + let next_seq_recv = ctx_b.get_next_sequence_recv(port_chan_id_on_b)?; + if msg.packet.sequence > next_seq_recv { + return Err(PacketError::InvalidPacketSequence { + given_sequence: msg.packet.sequence, + next_sequence: next_seq_recv, + } + .into()); + } + } else { + ctx_b.get_packet_receipt(&( + msg.packet.port_on_b.clone(), + msg.packet.chan_on_b.clone(), + msg.packet.sequence, + ))?; + }; + + Ok(()) + } +} + #[derive(Clone, Debug)] pub enum RecvPacketResult { NoOp, diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index c806de31e..06e925175 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -355,6 +355,22 @@ impl ClientState for MockClientState { Ok(()) } + #[cfg(feature = "val_exec_ctx")] + fn new_verify_packet_data( + &self, + _ctx: &dyn ValidationContext, + _height: Height, + _connection_end: &ConnectionEnd, + _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, + _port_id: &PortId, + _channel_id: &ChannelId, + _sequence: Sequence, + _commitment: PacketCommitment, + ) -> Result<(), ClientError> { + Ok(()) + } + fn verify_packet_data( &self, _ctx: &dyn ChannelReader, From eaafb58c95f4598a68f676c09fa11204f8ae459f Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 26 Jan 2023 13:53:50 -0500 Subject: [PATCH 03/15] remove todo --- crates/ibc/src/core/context.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 1a419d532..4b210ec0b 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -224,15 +224,13 @@ mod val_exec_ctx { .map_err(RouterError::ContextError); } - // TODO: use the return value appropriately - let _ = match msg { + match msg { PacketMsg::Recv(msg) => recv_packet_validate(self, module_id, msg), PacketMsg::Ack(_) => todo!(), PacketMsg::Timeout(_) => todo!(), PacketMsg::TimeoutOnClose(_) => todo!(), - }; - - todo!() + } + .map_err(RouterError::ContextError) } } } From 00c18e497dbdd052243514ebf8bd538d81504ae2 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 26 Jan 2023 14:37:28 -0500 Subject: [PATCH 04/15] recv_packet_validate() --- crates/ibc/src/core/context.rs | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 4b210ec0b..7f42e9014 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -225,7 +225,7 @@ mod val_exec_ctx { } match msg { - PacketMsg::Recv(msg) => recv_packet_validate(self, module_id, msg), + PacketMsg::Recv(msg) => recv_packet_validate(self, msg), PacketMsg::Ack(_) => todo!(), PacketMsg::Timeout(_) => todo!(), PacketMsg::TimeoutOnClose(_) => todo!(), @@ -1152,20 +1152,12 @@ mod val_exec_ctx { Ok(()) } - fn recv_packet_validate( - ctx_b: &ValCtx, - module_id: ModuleId, - msg: MsgRecvPacket, - ) -> Result<(), ContextError> + fn recv_packet_validate(ctx_b: &ValCtx, msg: MsgRecvPacket) -> Result<(), ContextError> where ValCtx: ValidationContext, { - recv_packet::validate(ctx_b, &msg)?; - - let _module = ctx_b - .get_route(&module_id) - .ok_or(ChannelError::RouteNotFound)?; + recv_packet::validate(ctx_b, &msg) - todo!() + // nothing to validate with the module, since `onRecvPacket` cannot fail. } } From 69477fe54153f602607baa568928906bb2b36486 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 26 Jan 2023 14:45:00 -0500 Subject: [PATCH 05/15] execute scaffolding --- crates/ibc/src/core/context.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 7f42e9014..e2a7d222f 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -471,7 +471,24 @@ mod val_exec_ctx { } .map_err(RouterError::ContextError) } - MsgEnvelope::Packet(_msg) => todo!(), + MsgEnvelope::Packet(msg) => { + let module_id = self + .lookup_module_packet(&msg) + .map_err(ContextError::from)?; + if !self.has_route(&module_id) { + return Err(ChannelError::RouteNotFound) + .map_err(ContextError::ChannelError) + .map_err(RouterError::ContextError); + } + + match msg { + PacketMsg::Recv(msg) => recv_packet_execute(self, module_id, msg), + PacketMsg::Ack(_) => todo!(), + PacketMsg::Timeout(_) => todo!(), + PacketMsg::TimeoutOnClose(_) => todo!(), + } + .map_err(RouterError::ContextError) + } } } @@ -1160,4 +1177,15 @@ mod val_exec_ctx { // nothing to validate with the module, since `onRecvPacket` cannot fail. } + + fn recv_packet_execute( + _ctx_b: &mut ExecCtx, + _module_id: ModuleId, + _msg: MsgRecvPacket, + ) -> Result<(), ContextError> + where + ExecCtx: ExecutionContext, + { + todo!() + } } From 4c61b34aa8ec977436629cf311fa7562171ca113 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 26 Jan 2023 14:57:45 -0500 Subject: [PATCH 06/15] Module::on_recv_packet_execute --- crates/ibc/src/core/ics26_routing/context.rs | 7 +++++++ crates/ibc/src/mock/context.rs | 20 ++++++++++++++++++++ crates/ibc/src/test_utils.rs | 9 +++++++++ 3 files changed, 36 insertions(+) diff --git a/crates/ibc/src/core/ics26_routing/context.rs b/crates/ibc/src/core/ics26_routing/context.rs index 77df602c3..bb086a06a 100644 --- a/crates/ibc/src/core/ics26_routing/context.rs +++ b/crates/ibc/src/core/ics26_routing/context.rs @@ -270,6 +270,13 @@ pub trait Module: Send + Sync + AsAnyMut + Debug { Ok(ModuleExtras::empty()) } + #[cfg(feature = "val_exec_ctx")] + fn on_recv_packet_execute( + &mut self, + packet: &Packet, + relayer: &Signer, + ) -> (ModuleExtras, Acknowledgement); + fn on_recv_packet( &mut self, _output: &mut ModuleOutputBuilder, diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index b942968a3..1ab418253 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -1966,6 +1966,17 @@ mod tests { Ok((ModuleExtras::empty(), counterparty_version.clone())) } + #[cfg(feature = "val_exec_ctx")] + fn on_recv_packet_execute( + &mut self, + _packet: &Packet, + _relayer: &Signer, + ) -> (ModuleExtras, Acknowledgement) { + self.counter += 1; + + (ModuleExtras::empty(), Acknowledgement::from(vec![1u8])) + } + fn on_recv_packet( &mut self, _output: &mut ModuleOutputBuilder, @@ -2058,6 +2069,15 @@ mod tests { Ok((ModuleExtras::empty(), counterparty_version.clone())) } + #[cfg(feature = "val_exec_ctx")] + fn on_recv_packet_execute( + &mut self, + _packet: &Packet, + _relayer: &Signer, + ) -> (ModuleExtras, Acknowledgement) { + (ModuleExtras::empty(), Acknowledgement::from(vec![1u8])) + } + fn on_recv_packet( &mut self, _output: &mut ModuleOutputBuilder, diff --git a/crates/ibc/src/test_utils.rs b/crates/ibc/src/test_utils.rs index ede647eb3..b563550c2 100644 --- a/crates/ibc/src/test_utils.rs +++ b/crates/ibc/src/test_utils.rs @@ -166,6 +166,15 @@ impl Module for DummyTransferModule { )) } + #[cfg(feature = "val_exec_ctx")] + fn on_recv_packet_execute( + &mut self, + _packet: &Packet, + _relayer: &Signer, + ) -> (ModuleExtras, Acknowledgement) { + (ModuleExtras::empty(), Acknowledgement::from(vec![1u8])) + } + fn on_recv_packet( &mut self, _output: &mut ModuleOutputBuilder, From 110d44d4b690e82540178529e66faf4022dac43d Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 26 Jan 2023 15:21:07 -0500 Subject: [PATCH 07/15] token transfer on_recv_packet (with a FIXME) --- .../ibc/src/applications/transfer/context.rs | 35 ++++++++++ .../transfer/relay/on_recv_packet.rs | 70 +++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/crates/ibc/src/applications/transfer/context.rs b/crates/ibc/src/applications/transfer/context.rs index 17fc782f2..803edef4d 100644 --- a/crates/ibc/src/applications/transfer/context.rs +++ b/crates/ibc/src/applications/transfer/context.rs @@ -339,6 +339,8 @@ pub fn on_timeout_packet( pub use val_exec_ctx::*; #[cfg(feature = "val_exec_ctx")] mod val_exec_ctx { + use crate::applications::transfer::relay::on_recv_packet::process_recv_packet_execute; + pub use super::*; #[allow(clippy::too_many_arguments)] @@ -498,6 +500,39 @@ mod val_exec_ctx { ) -> Result { Ok(ModuleExtras::empty()) } + + pub fn on_recv_packet_execute( + ctx: &mut impl TokenTransferContext, + packet: &Packet, + ) -> (ModuleExtras, Acknowledgement) { + let data = match serde_json::from_slice::(&packet.data) { + Ok(data) => data, + Err(_) => { + let ack = TokenTransferAcknowledgement::Error( + TokenTransferError::PacketDataDeserialization.to_string(), + ); + return (ModuleExtras::empty(), ack.into()); + } + }; + + let (mut extras, ack) = match process_recv_packet_execute(ctx, packet, data.clone()) { + Ok(extras) => (extras, TokenTransferAcknowledgement::success()), + Err(e) => ( + ModuleExtras::empty(), + TokenTransferAcknowledgement::from_error(e), + ), + }; + + let recv_event = RecvEvent { + receiver: data.receiver, + denom: data.token.denom, + amount: data.token.amount, + success: ack.is_successful(), + }; + extras.events.push(recv_event.into()); + + (extras, ack.into()) + } } #[cfg(test)] diff --git a/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs b/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs index c49633f7b..48290a4be 100644 --- a/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs +++ b/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs @@ -60,3 +60,73 @@ pub fn process_recv_packet( Ok(()) } + +#[cfg(feature = "val_exec_ctx")] +pub use val_exec_ctx::*; +#[cfg(feature = "val_exec_ctx")] +mod val_exec_ctx { + pub use super::*; + use crate::core::ics04_channel::handler::ModuleExtras; + + //////////////////////////////////////////////////////////// + // FIXME BEFORE MERGE: events need to be emitted even when there's an error + //////////////////////////////////////////////////////////// + pub fn process_recv_packet_execute( + ctx: &mut Ctx, + packet: &Packet, + data: PacketData, + ) -> Result { + if !ctx.is_receive_enabled() { + return Err(TokenTransferError::ReceiveDisabled); + } + + let receiver_account = data + .receiver + .clone() + .try_into() + .map_err(|_| TokenTransferError::ParseAccountFailure)?; + + let extras = if is_receiver_chain_source( + packet.port_on_a.clone(), + packet.chan_on_a.clone(), + &data.token.denom, + ) { + // sender chain is not the source, unescrow tokens + let prefix = TracePrefix::new(packet.port_on_a.clone(), packet.chan_on_a.clone()); + let coin = { + let mut c = data.token; + c.denom.remove_trace_prefix(&prefix); + c + }; + + let escrow_address = + ctx.get_channel_escrow_address(&packet.port_on_b, &packet.chan_on_b)?; + + ctx.send_coins(&escrow_address, &receiver_account, &coin)?; + + ModuleExtras::empty() + } else { + // sender chain is the source, mint vouchers + let prefix = TracePrefix::new(packet.port_on_b.clone(), packet.chan_on_b.clone()); + let coin = { + let mut c = data.token; + c.denom.add_trace_prefix(prefix); + c + }; + + let denom_trace_event = DenomTraceEvent { + trace_hash: ctx.denom_hash_string(&coin.denom), + denom: coin.denom.clone(), + }; + + ctx.mint_coins(&receiver_account, &coin)?; + + ModuleExtras { + events: vec![denom_trace_event.into()], + log: Vec::new(), + } + }; + + Ok(extras) + } +} From 0129dc8077358d72b0b975406579ce0b5cdc995d Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Thu, 26 Jan 2023 18:01:46 -0500 Subject: [PATCH 08/15] recv_packet_execute --- crates/ibc/src/core/context.rs | 112 +++++++++++++++++- .../core/ics04_channel/handler/recv_packet.rs | 31 +++++ 2 files changed, 137 insertions(+), 6 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index e2a7d222f..24236c493 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -74,11 +74,12 @@ mod val_exec_ctx { use crate::core::ics03_connection::version::{ get_compatible_versions, pick_version, Version as ConnectionVersion, }; - use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, State}; + use crate::core::ics04_channel::channel::{ChannelEnd, Counterparty, Order, State}; use crate::core::ics04_channel::commitment::{AcknowledgementCommitment, PacketCommitment}; use crate::core::ics04_channel::context::calculate_block_delay; use crate::core::ics04_channel::events::{ - CloseConfirm, CloseInit, OpenAck, OpenConfirm, OpenInit, OpenTry, + CloseConfirm, CloseInit, OpenAck, OpenConfirm, OpenInit, OpenTry, ReceivePacket, + WriteAcknowledgement, }; use crate::core::ics04_channel::handler::{ chan_close_confirm, chan_close_init, chan_open_ack, chan_open_confirm, chan_open_init, @@ -1179,13 +1180,112 @@ mod val_exec_ctx { } fn recv_packet_execute( - _ctx_b: &mut ExecCtx, - _module_id: ModuleId, - _msg: MsgRecvPacket, + ctx_b: &mut ExecCtx, + module_id: ModuleId, + msg: MsgRecvPacket, ) -> Result<(), ContextError> where ExecCtx: ExecutionContext, { - todo!() + let chan_port_id_on_b = (msg.packet.port_on_b.clone(), msg.packet.chan_on_b.clone()); + let chan_end_on_b = ctx_b.channel_end(&chan_port_id_on_b)?; + + // check for the no-op cases (i.e. when another relayer already relayed the packet) + // we don't want to fail the transaction in this case + match chan_end_on_b.ordering { + Order::None => (), + Order::Unordered => { + let packet = msg.packet.clone(); + if let Ok(_receipt) = + ctx_b.get_packet_receipt(&(packet.port_on_b, packet.chan_on_b, packet.sequence)) + { + // the receipt exists, so another relayer already relayed + // the packet + return Ok(()); + } + } + Order::Ordered => { + let next_seq_recv = ctx_b.get_next_sequence_recv(&chan_port_id_on_b)?; + + if msg.packet.sequence < next_seq_recv { + // the sequence number has already been incremented, so + // another relayer already relayed the packet + return Ok(()); + } + } + } + + let module = ctx_b + .get_route_mut(&module_id) + .ok_or(ChannelError::RouteNotFound)?; + + let (extras, acknowledgement) = module.on_recv_packet_execute(&msg.packet, &msg.signer); + + // FIXME: Enforce this in the type instead (e.g. constructor revokes empty vecs) + if acknowledgement.is_empty() { + return Err(PacketError::InvalidAcknowledgement.into()); + } + + // state changes + { + // `recvPacket` core handler state changes + match chan_end_on_b.ordering { + Order::Unordered => { + let path = ReceiptsPath { + port_id: msg.packet.port_on_b.clone(), + channel_id: msg.packet.chan_on_b.clone(), + sequence: msg.packet.sequence, + }; + + ctx_b.store_packet_receipt(path, Receipt::Ok)?; + } + Order::Ordered => { + let port_chan_id_on_b = + (msg.packet.port_on_b.clone(), msg.packet.chan_on_b.clone()); + let next_seq_recv = ctx_b.get_next_sequence_recv(&port_chan_id_on_b)?; + + ctx_b.store_next_sequence_recv(port_chan_id_on_b, next_seq_recv.increment())?; + } + _ => {} + } + + // `writeAcknowledgement` handler state changes + ctx_b.store_packet_acknowledgement( + ( + msg.packet.port_on_b.clone(), + msg.packet.chan_on_b.clone(), + msg.packet.sequence, + ), + ctx_b.ack_commitment(&acknowledgement), + )?; + } + + // emit events and logs + { + ctx_b.log_message("success: packet receive".to_string()); + ctx_b.log_message("success: packet write acknowledgement".to_string()); + + let conn_id_on_b = &chan_end_on_b.connection_hops()[0]; + ctx_b.emit_ibc_event(IbcEvent::ReceivePacket(ReceivePacket::new( + msg.packet.clone(), + chan_end_on_b.ordering, + conn_id_on_b.clone(), + ))); + ctx_b.emit_ibc_event(IbcEvent::WriteAcknowledgement(WriteAcknowledgement::new( + msg.packet, + acknowledgement, + conn_id_on_b.clone(), + ))); + + for module_event in extras.events { + ctx_b.emit_ibc_event(IbcEvent::AppModule(module_event)); + } + + for log_message in extras.log { + ctx_b.log_message(log_message); + } + } + + Ok(()) } } diff --git a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs index ee3ddf719..f4312e2c5 100644 --- a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs @@ -121,16 +121,47 @@ pub(crate) mod val_exec_ctx { } .into()); } + + if msg.packet.sequence == next_seq_recv { + // Case where the recvPacket is successful and an + // acknowledgement will be written (not a no-op) + validate_write_acknowledgement(ctx_b, msg)?; + } } else { ctx_b.get_packet_receipt(&( msg.packet.port_on_b.clone(), msg.packet.chan_on_b.clone(), msg.packet.sequence, ))?; + + // Case where the recvPacket is successful and an + // acknowledgement will be written (not a no-op) + validate_write_acknowledgement(ctx_b, msg)?; }; Ok(()) } + + fn validate_write_acknowledgement( + ctx_b: &Ctx, + msg: &MsgRecvPacket, + ) -> Result<(), ContextError> + where + Ctx: ValidationContext, + { + let packet = msg.packet.clone(); + if ctx_b + .get_packet_acknowledgement(&(packet.port_on_b, packet.chan_on_b, packet.sequence)) + .is_ok() + { + return Err(PacketError::AcknowledgementExists { + sequence: msg.packet.sequence, + } + .into()); + } + + Ok(()) + } } #[derive(Clone, Debug)] From afc3ffa0edd706c91004f7407fc8b1c4fb9ce4a0 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 27 Jan 2023 08:51:43 -0500 Subject: [PATCH 09/15] refactor expression for readability --- crates/ibc/src/core/context.rs | 36 ++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 24236c493..c33587aaa 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -1190,28 +1190,30 @@ mod val_exec_ctx { let chan_port_id_on_b = (msg.packet.port_on_b.clone(), msg.packet.chan_on_b.clone()); let chan_end_on_b = ctx_b.channel_end(&chan_port_id_on_b)?; - // check for the no-op cases (i.e. when another relayer already relayed the packet) - // we don't want to fail the transaction in this case - match chan_end_on_b.ordering { - Order::None => (), - Order::Unordered => { - let packet = msg.packet.clone(); - if let Ok(_receipt) = - ctx_b.get_packet_receipt(&(packet.port_on_b, packet.chan_on_b, packet.sequence)) - { - // the receipt exists, so another relayer already relayed - // the packet - return Ok(()); + // Check if another relayer already relayed the packet. + // We don't want to fail the transaction in this case. + { + let packet_already_received = match chan_end_on_b.ordering { + // Note: ibc-go doesn't make the check for `Order::None` channels + Order::None => false, + Order::Unordered => { + let packet = msg.packet.clone(); + + ctx_b + .get_packet_receipt(&(packet.port_on_b, packet.chan_on_b, packet.sequence)) + .is_ok() } - } - Order::Ordered => { - let next_seq_recv = ctx_b.get_next_sequence_recv(&chan_port_id_on_b)?; + Order::Ordered => { + let next_seq_recv = ctx_b.get_next_sequence_recv(&chan_port_id_on_b)?; - if msg.packet.sequence < next_seq_recv { // the sequence number has already been incremented, so // another relayer already relayed the packet - return Ok(()); + msg.packet.sequence < next_seq_recv } + }; + + if packet_already_received { + return Ok(()); } } From 444a159031ed3453b4f9d06dca29dcd9932d009f Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 27 Jan 2023 09:28:16 -0500 Subject: [PATCH 10/15] add comment --- crates/ibc/src/core/context.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index c33587aaa..6e975a010 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -1174,6 +1174,7 @@ mod val_exec_ctx { where ValCtx: ValidationContext, { + // Note: this contains the validation for `write_acknowledgement` as well. recv_packet::validate(ctx_b, &msg) // nothing to validate with the module, since `onRecvPacket` cannot fail. From 69f5aff0414d12359fd119e634252c7b406e9fbf Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 27 Jan 2023 09:53:49 -0500 Subject: [PATCH 11/15] ack cannot be empty --- crates/ibc/src/core/context.rs | 5 ---- .../handler/write_acknowledgement.rs | 6 +---- .../ics04_channel/msgs/acknowledgement.rs | 23 +++++++++++++------ crates/ibc/src/mock/context.rs | 14 +++++++---- crates/ibc/src/test_utils.rs | 7 ++++-- 5 files changed, 32 insertions(+), 23 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 6e975a010..a85a16e39 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -1224,11 +1224,6 @@ mod val_exec_ctx { let (extras, acknowledgement) = module.on_recv_packet_execute(&msg.packet, &msg.signer); - // FIXME: Enforce this in the type instead (e.g. constructor revokes empty vecs) - if acknowledgement.is_empty() { - return Err(PacketError::InvalidAcknowledgement.into()); - } - // state changes { // `recvPacket` core handler state changes diff --git a/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs index a0ef1a50d..c927427b5 100644 --- a/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs @@ -56,10 +56,6 @@ pub fn process( Err(e) => return Err(e), } - if ack.is_empty() { - return Err(PacketError::InvalidAcknowledgement); - } - let result = PacketResult::WriteAck(WriteAckPacketResult { port_id: packet.port_on_b.clone(), channel_id: packet.chan_on_b.clone(), @@ -181,7 +177,7 @@ mod tests { .collect(); for test in tests { - let res = process(&test.ctx, test.packet.clone(), test.ack.into()); + let res = process(&test.ctx, test.packet.clone(), test.ack.try_into().unwrap()); // Additionally check the events and the output objects in the result. match res { Ok(proto_output) => { diff --git a/crates/ibc/src/core/ics04_channel/msgs/acknowledgement.rs b/crates/ibc/src/core/ics04_channel/msgs/acknowledgement.rs index 727867e95..5e49abc43 100644 --- a/crates/ibc/src/core/ics04_channel/msgs/acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/msgs/acknowledgement.rs @@ -1,6 +1,6 @@ use crate::prelude::*; -use derive_more::{From, Into}; +use derive_more::Into; use ibc_proto::ibc::core::channel::v1::MsgAcknowledgement as RawMsgAcknowledgement; use ibc_proto::protobuf::Protobuf; @@ -14,6 +14,7 @@ use crate::Height; pub const TYPE_URL: &str = "/ibc.core.channel.v1.MsgAcknowledgement"; /// A generic Acknowledgement type that modules may interpret as they like. +/// An acknowledgement cannot be empty. #[cfg_attr( feature = "parity-scale-codec", derive( @@ -27,14 +28,10 @@ pub const TYPE_URL: &str = "/ibc.core.channel.v1.MsgAcknowledgement"; derive(borsh::BorshSerialize, borsh::BorshDeserialize) )] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Clone, Debug, PartialEq, Eq, From, Into)] +#[derive(Clone, Debug, PartialEq, Eq, Into)] pub struct Acknowledgement(Vec); impl Acknowledgement { - pub fn is_empty(&self) -> bool { - self.0.is_empty() - } - // Returns the data as a slice of bytes. pub fn as_bytes(&self) -> &[u8] { self.0.as_slice() @@ -47,6 +44,18 @@ impl AsRef<[u8]> for Acknowledgement { } } +impl TryFrom> for Acknowledgement { + type Error = PacketError; + + fn try_from(bytes: Vec) -> Result { + if bytes.is_empty() { + Err(PacketError::InvalidAcknowledgement) + } else { + Ok(Self(bytes)) + } + } +} + /// /// Message definition for packet acknowledgements. /// @@ -80,7 +89,7 @@ impl TryFrom for MsgAcknowledgement { .packet .ok_or(PacketError::MissingPacket)? .try_into()?, - acknowledgement: raw_msg.acknowledgement.into(), + acknowledgement: raw_msg.acknowledgement.try_into()?, proof_acked_on_b: raw_msg .proof_acked .try_into() diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index 1ab418253..edfebe6c6 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -1974,7 +1974,10 @@ mod tests { ) -> (ModuleExtras, Acknowledgement) { self.counter += 1; - (ModuleExtras::empty(), Acknowledgement::from(vec![1u8])) + ( + ModuleExtras::empty(), + Acknowledgement::try_from(vec![1u8]).unwrap(), + ) } fn on_recv_packet( @@ -1985,7 +1988,7 @@ mod tests { ) -> Acknowledgement { self.counter += 1; - Acknowledgement::from(vec![1u8]) + Acknowledgement::try_from(vec![1u8]).unwrap() } } @@ -2075,7 +2078,10 @@ mod tests { _packet: &Packet, _relayer: &Signer, ) -> (ModuleExtras, Acknowledgement) { - (ModuleExtras::empty(), Acknowledgement::from(vec![1u8])) + ( + ModuleExtras::empty(), + Acknowledgement::try_from(vec![1u8]).unwrap(), + ) } fn on_recv_packet( @@ -2084,7 +2090,7 @@ mod tests { _packet: &Packet, _relayer: &Signer, ) -> Acknowledgement { - Acknowledgement::from(vec![1u8]) + Acknowledgement::try_from(vec![1u8]).unwrap() } } diff --git a/crates/ibc/src/test_utils.rs b/crates/ibc/src/test_utils.rs index b563550c2..5174fbad1 100644 --- a/crates/ibc/src/test_utils.rs +++ b/crates/ibc/src/test_utils.rs @@ -172,7 +172,10 @@ impl Module for DummyTransferModule { _packet: &Packet, _relayer: &Signer, ) -> (ModuleExtras, Acknowledgement) { - (ModuleExtras::empty(), Acknowledgement::from(vec![1u8])) + ( + ModuleExtras::empty(), + Acknowledgement::try_from(vec![1u8]).unwrap(), + ) } fn on_recv_packet( @@ -181,7 +184,7 @@ impl Module for DummyTransferModule { _packet: &Packet, _relayer: &Signer, ) -> Acknowledgement { - Acknowledgement::from(vec![1u8]) + Acknowledgement::try_from(vec![1u8]).unwrap() } } From 3bca9debdbf1b2c17ccf589b0ac5d0470267dc25 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 27 Jan 2023 10:12:07 -0500 Subject: [PATCH 12/15] token transfer ack --- .../applications/transfer/acknowledgement.rs | 19 ++++++++++++++----- .../handler/write_acknowledgement.rs | 13 +------------ 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/crates/ibc/src/applications/transfer/acknowledgement.rs b/crates/ibc/src/applications/transfer/acknowledgement.rs index 095fc79fa..87bbc0edd 100644 --- a/crates/ibc/src/applications/transfer/acknowledgement.rs +++ b/crates/ibc/src/applications/transfer/acknowledgement.rs @@ -56,9 +56,10 @@ impl Display for TokenTransferAcknowledgement { impl From for Vec { fn from(ack: TokenTransferAcknowledgement) -> Self { + // WARNING: Make sure all branches always return a non-empty vector. + // Otherwise, the conversion to `Acknowledgement` will panic. match ack { TokenTransferAcknowledgement::Success(_) => r#"{"result":"AQ=="}"#.as_bytes().into(), - // TokenTransferAcknowledgement::Error(s) => s.as_bytes(), TokenTransferAcknowledgement::Error(s) => alloc::format!(r#"{{"error":"{s}"}}"#).into(), } } @@ -66,7 +67,10 @@ impl From for Vec { impl From for Acknowledgement { fn from(ack: TokenTransferAcknowledgement) -> Self { - ack.into() + let v: Vec = ack.into(); + + v.try_into() + .expect("token transfer internal error: ack is never supposed to be empty") } } @@ -98,18 +102,23 @@ mod test { let ack_success: Vec = TokenTransferAcknowledgement::success().into(); // Check that it's the same output as ibc-go + // Note: this also implicitly checks that the ack bytes are non-empty, + // which would make the conversion to `Acknowledgement` panic assert_eq!(ack_success, r#"{"result":"AQ=="}"#.as_bytes()); } #[test] fn test_ack_error_to_vec() { - let ack_error = TokenTransferAcknowledgement::Error( + let ack_error: Vec = TokenTransferAcknowledgement::Error( "cannot unmarshal ICS-20 transfer packet data".to_string(), - ); + ) + .into(); // Check that it's the same output as ibc-go + // Note: this also implicitly checks that the ack bytes are non-empty, + // which would make the conversion to `Acknowledgement` panic assert_eq!( - Vec::::from(ack_error), + ack_error, r#"{"error":"cannot unmarshal ICS-20 transfer packet data"}"#.as_bytes() ); } diff --git a/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs index c927427b5..458dbf36b 100644 --- a/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs @@ -93,7 +93,7 @@ mod tests { use crate::core::ics04_channel::handler::write_acknowledgement::process; use crate::core::ics04_channel::packet::test_utils::get_dummy_raw_packet; 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::mock::context::MockContext; use crate::timestamp::ZERO_DURATION; use crate::{core::ics04_channel::packet::Packet, events::IbcEvent}; @@ -117,7 +117,6 @@ mod tests { packet.data = vec![0]; let ack = vec![0]; - let ack_null = Vec::new(); let dest_channel_end = ChannelEnd::new( State::Open, @@ -162,16 +161,6 @@ mod tests { ack, want_pass: true, }, - Test { - name: "Zero ack".to_string(), - ctx: context - .with_client(&ClientId::default(), Height::new(0, 1).unwrap()) - .with_connection(ConnectionId::default(), connection_end) - .with_channel(PortId::default(), ChannelId::default(), dest_channel_end), - packet, - ack: ack_null, - want_pass: false, - }, ] .into_iter() .collect(); From ace576550fa97e8cf296da5b97e3a16d11bbc440 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 27 Jan 2023 10:17:25 -0500 Subject: [PATCH 13/15] clippy --- .../core/ics04_channel/handler/write_acknowledgement.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs index 458dbf36b..5565fd64f 100644 --- a/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/write_acknowledgement.rs @@ -149,15 +149,14 @@ mod tests { Test { name: "Good parameters".to_string(), ctx: context - .clone() .with_client(&ClientId::default(), client_height) - .with_connection(ConnectionId::default(), connection_end.clone()) + .with_connection(ConnectionId::default(), connection_end) .with_channel( packet.port_on_b.clone(), packet.chan_on_b.clone(), - dest_channel_end.clone(), + dest_channel_end, ), - packet: packet.clone(), + packet, ack, want_pass: true, }, From dc84bf2e8359f4bfcb78e3a8cbb9cad3a1c47002 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Fri, 27 Jan 2023 11:48:46 -0500 Subject: [PATCH 14/15] fix token transfer recv_packet --- .../ibc/src/applications/transfer/context.rs | 5 +- .../transfer/relay/on_recv_packet.rs | 47 ++++++++++--------- crates/ibc/src/core/ics04_channel/handler.rs | 1 + 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/crates/ibc/src/applications/transfer/context.rs b/crates/ibc/src/applications/transfer/context.rs index 803edef4d..c1acae891 100644 --- a/crates/ibc/src/applications/transfer/context.rs +++ b/crates/ibc/src/applications/transfer/context.rs @@ -517,10 +517,7 @@ mod val_exec_ctx { let (mut extras, ack) = match process_recv_packet_execute(ctx, packet, data.clone()) { Ok(extras) => (extras, TokenTransferAcknowledgement::success()), - Err(e) => ( - ModuleExtras::empty(), - TokenTransferAcknowledgement::from_error(e), - ), + Err((extras, error)) => (extras, TokenTransferAcknowledgement::from_error(error)), }; let recv_event = RecvEvent { diff --git a/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs b/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs index 48290a4be..d6c7f7716 100644 --- a/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs +++ b/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs @@ -68,23 +68,21 @@ mod val_exec_ctx { pub use super::*; use crate::core::ics04_channel::handler::ModuleExtras; - //////////////////////////////////////////////////////////// - // FIXME BEFORE MERGE: events need to be emitted even when there's an error - //////////////////////////////////////////////////////////// pub fn process_recv_packet_execute( ctx: &mut Ctx, packet: &Packet, data: PacketData, - ) -> Result { + ) -> Result { if !ctx.is_receive_enabled() { - return Err(TokenTransferError::ReceiveDisabled); + return Err((ModuleExtras::empty(), TokenTransferError::ReceiveDisabled)); } - let receiver_account = data - .receiver - .clone() - .try_into() - .map_err(|_| TokenTransferError::ParseAccountFailure)?; + let receiver_account = data.receiver.clone().try_into().map_err(|_| { + ( + ModuleExtras::empty(), + TokenTransferError::ParseAccountFailure, + ) + })?; let extras = if is_receiver_chain_source( packet.port_on_a.clone(), @@ -99,10 +97,12 @@ mod val_exec_ctx { c }; - let escrow_address = - ctx.get_channel_escrow_address(&packet.port_on_b, &packet.chan_on_b)?; + let escrow_address = ctx + .get_channel_escrow_address(&packet.port_on_b, &packet.chan_on_b) + .map_err(|token_err| (ModuleExtras::empty(), token_err))?; - ctx.send_coins(&escrow_address, &receiver_account, &coin)?; + ctx.send_coins(&escrow_address, &receiver_account, &coin) + .map_err(|token_err| (ModuleExtras::empty(), token_err))?; ModuleExtras::empty() } else { @@ -114,17 +114,22 @@ mod val_exec_ctx { c }; - let denom_trace_event = DenomTraceEvent { - trace_hash: ctx.denom_hash_string(&coin.denom), - denom: coin.denom.clone(), + let extras = { + let denom_trace_event = DenomTraceEvent { + trace_hash: ctx.denom_hash_string(&coin.denom), + denom: coin.denom.clone(), + }; + ModuleExtras { + events: vec![denom_trace_event.into()], + log: Vec::new(), + } }; + let extras_closure = extras.clone(); - ctx.mint_coins(&receiver_account, &coin)?; + ctx.mint_coins(&receiver_account, &coin) + .map_err(|token_err| (extras_closure, token_err))?; - ModuleExtras { - events: vec![denom_trace_event.into()], - log: Vec::new(), - } + extras }; Ok(extras) diff --git a/crates/ibc/src/core/ics04_channel/handler.rs b/crates/ibc/src/core/ics04_channel/handler.rs index fe418481d..576a3cd6d 100644 --- a/crates/ibc/src/core/ics04_channel/handler.rs +++ b/crates/ibc/src/core/ics04_channel/handler.rs @@ -49,6 +49,7 @@ pub struct ChannelResult { pub channel_end: ChannelEnd, } +#[derive(Clone, Debug)] pub struct ModuleExtras { pub events: Vec, pub log: Vec, From 147ec47a78503d30dfb26cdc1cba20766066b9c0 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Mon, 30 Jan 2023 15:02:37 -0500 Subject: [PATCH 15/15] typo --- crates/ibc/src/core/ics02_client/client_state.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 3aecb0c15..b5e75112c 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -175,7 +175,7 @@ pub trait ClientState: expected_client_state: Any, ) -> Result<(), ClientError>; - /// Verify a `proof` that a packet has been commited. + /// Verify a `proof` that a packet has been committed. #[cfg(feature = "val_exec_ctx")] #[allow(clippy::too_many_arguments)] fn new_verify_packet_data( @@ -191,7 +191,7 @@ pub trait ClientState: commitment: PacketCommitment, ) -> Result<(), ClientError>; - /// Verify a `proof` that a packet has been commited. + /// Verify a `proof` that a packet has been committed. #[allow(clippy::too_many_arguments)] fn verify_packet_data( &self, @@ -206,7 +206,7 @@ pub trait ClientState: commitment: PacketCommitment, ) -> Result<(), ClientError>; - /// Verify a `proof` that a packet has been commited. + /// Verify a `proof` that a packet has been committed. #[allow(clippy::too_many_arguments)] fn verify_packet_acknowledgement( &self,