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/applications/transfer/context.rs b/crates/ibc/src/applications/transfer/context.rs index 17fc782f2..c1acae891 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,36 @@ 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((extras, error)) => (extras, TokenTransferAcknowledgement::from_error(error)), + }; + + 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..d6c7f7716 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,78 @@ 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; + + pub fn process_recv_packet_execute( + ctx: &mut Ctx, + packet: &Packet, + data: PacketData, + ) -> Result { + if !ctx.is_receive_enabled() { + return Err((ModuleExtras::empty(), TokenTransferError::ReceiveDisabled)); + } + + 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(), + 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) + .map_err(|token_err| (ModuleExtras::empty(), token_err))?; + + ctx.send_coins(&escrow_address, &receiver_account, &coin) + .map_err(|token_err| (ModuleExtras::empty(), token_err))?; + + 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 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) + .map_err(|token_err| (extras_closure, token_err))?; + + extras + }; + + Ok(extras) + } +} 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 f0224d9d5..a85a16e39 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -74,15 +74,16 @@ 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, - 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,7 +92,8 @@ 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::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; use crate::core::ics05_port::error::PortError::UnknownPort; @@ -133,7 +135,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 +151,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 +190,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 +215,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_validate(self, msg), + PacketMsg::Ack(_) => todo!(), + PacketMsg::Timeout(_) => todo!(), + PacketMsg::TimeoutOnClose(_) => todo!(), + } + .map_err(RouterError::ContextError) + } } } @@ -413,7 +449,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) @@ -434,7 +472,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) + } } } @@ -1114,4 +1169,121 @@ mod val_exec_ctx { Ok(()) } + + fn recv_packet_validate(ctx_b: &ValCtx, msg: MsgRecvPacket) -> Result<(), ContextError> + 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. + } + + fn recv_packet_execute( + ctx_b: &mut ExecCtx, + module_id: ModuleId, + msg: MsgRecvPacket, + ) -> Result<(), ContextError> + where + ExecCtx: ExecutionContext, + { + 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 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)?; + + // the sequence number has already been incremented, so + // another relayer already relayed the packet + msg.packet.sequence < next_seq_recv + } + }; + + if packet_already_received { + 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); + + // 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/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 3247d13dd..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,23 @@ 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( + &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 committed. #[allow(clippy::too_many_arguments)] fn verify_packet_data( &self, @@ -190,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, 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, 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..f4312e2c5 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,158 @@ 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()); + } + + 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)] pub enum RecvPacketResult { NoOp, 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..5565fd64f 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(), @@ -97,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}; @@ -121,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, @@ -154,34 +149,23 @@ 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, }, - 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(); 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/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/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, diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index b942968a3..edfebe6c6 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -1966,6 +1966,20 @@ 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::try_from(vec![1u8]).unwrap(), + ) + } + fn on_recv_packet( &mut self, _output: &mut ModuleOutputBuilder, @@ -1974,7 +1988,7 @@ mod tests { ) -> Acknowledgement { self.counter += 1; - Acknowledgement::from(vec![1u8]) + Acknowledgement::try_from(vec![1u8]).unwrap() } } @@ -2058,13 +2072,25 @@ 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::try_from(vec![1u8]).unwrap(), + ) + } + fn on_recv_packet( &mut self, _output: &mut ModuleOutputBuilder, _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 ede647eb3..5174fbad1 100644 --- a/crates/ibc/src/test_utils.rs +++ b/crates/ibc/src/test_utils.rs @@ -166,13 +166,25 @@ 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::try_from(vec![1u8]).unwrap(), + ) + } + fn on_recv_packet( &mut self, _output: &mut ModuleOutputBuilder, _packet: &Packet, _relayer: &Signer, ) -> Acknowledgement { - Acknowledgement::from(vec![1u8]) + Acknowledgement::try_from(vec![1u8]).unwrap() } }