From 48f23b7824282bacefc78dcf68e51db66851f4c3 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 31 Jan 2023 15:42:09 -0500 Subject: [PATCH 01/17] timeout validate scaffolding --- .../clients/ics07_tendermint/client_state.rs | 65 +++++++++ crates/ibc/src/core/context.rs | 16 ++- .../ibc/src/core/ics02_client/client_state.rs | 30 ++++ .../src/core/ics04_channel/handler/timeout.rs | 132 ++++++++++++++++++ crates/ibc/src/mock/client_state.rs | 30 ++++ 5 files changed, 271 insertions(+), 2 deletions(-) diff --git a/crates/ibc/src/clients/ics07_tendermint/client_state.rs b/crates/ibc/src/clients/ics07_tendermint/client_state.rs index b0aecf3aa..4f1047bd5 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -1062,6 +1062,40 @@ impl Ics2ClientState for ClientState { ) } + #[cfg(feature = "val_exec_ctx")] + #[allow(clippy::too_many_arguments)] + fn new_verify_next_sequence_recv( + &self, + ctx: &dyn ValidationContext, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + ) -> Result<(), ClientError> { + let client_state = downcast_tm_client_state(self)?; + client_state.verify_height(height)?; + new_verify_delay_passed(ctx, height, connection_end)?; + + let mut seq_bytes = Vec::new(); + u64::from(sequence) + .encode(&mut seq_bytes) + .expect("buffer size too small"); + + let seq_path = SeqRecvsPath(port_id.clone(), channel_id.clone()); + + verify_membership( + client_state, + connection_end.counterparty().prefix(), + proof, + root, + seq_path, + seq_bytes, + ) + } + fn verify_next_sequence_recv( &self, ctx: &dyn ChannelReader, @@ -1094,6 +1128,37 @@ impl Ics2ClientState for ClientState { ) } + #[cfg(feature = "val_exec_ctx")] + #[allow(clippy::too_many_arguments)] + fn new_verify_packet_receipt_absence( + &self, + ctx: &dyn ValidationContext, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + ) -> Result<(), ClientError> { + let client_state = downcast_tm_client_state(self)?; + client_state.verify_height(height)?; + new_verify_delay_passed(ctx, height, connection_end)?; + + let receipt_path = ReceiptsPath { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + sequence, + }; + verify_non_membership( + client_state, + connection_end.counterparty().prefix(), + proof, + root, + receipt_path, + ) + } + fn verify_packet_receipt_absence( &self, ctx: &dyn ChannelReader, diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index a85a16e39..350141d19 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -83,7 +83,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, recv_packet, + chan_open_try, recv_packet, timeout, }; use crate::core::ics04_channel::msgs::acknowledgement::Acknowledgement; use crate::core::ics04_channel::msgs::chan_close_confirm::MsgChannelCloseConfirm; @@ -93,6 +93,7 @@ mod val_exec_ctx { 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::timeout::MsgTimeout; use crate::core::ics04_channel::msgs::{ChannelMsg, PacketMsg}; use crate::core::ics04_channel::packet::{Receipt, Sequence}; use crate::core::ics04_channel::timeout::TimeoutHeight; @@ -228,7 +229,7 @@ mod val_exec_ctx { match msg { PacketMsg::Recv(msg) => recv_packet_validate(self, msg), PacketMsg::Ack(_) => todo!(), - PacketMsg::Timeout(_) => todo!(), + PacketMsg::Timeout(msg) => timeout_packet_validate(self, msg), PacketMsg::TimeoutOnClose(_) => todo!(), } .map_err(RouterError::ContextError) @@ -1286,4 +1287,15 @@ mod val_exec_ctx { Ok(()) } + + fn timeout_packet_validate(ctx_a: &ValCtx, msg: MsgTimeout) -> Result<(), ContextError> + where + ValCtx: ValidationContext, + { + timeout::validate(ctx_a, &msg)?; + + // TODO: cb validate + + Ok(()) + } } diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index b5e75112c..f6de52313 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -221,6 +221,21 @@ pub trait ClientState: ack: AcknowledgementCommitment, ) -> Result<(), ClientError>; + /// Verify a `proof` that of the next_seq_received. + #[cfg(feature = "val_exec_ctx")] + #[allow(clippy::too_many_arguments)] + fn new_verify_next_sequence_recv( + &self, + ctx: &dyn ValidationContext, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + ) -> Result<(), ClientError>; + /// Verify a `proof` that of the next_seq_received. #[allow(clippy::too_many_arguments)] fn verify_next_sequence_recv( @@ -235,6 +250,21 @@ pub trait ClientState: sequence: Sequence, ) -> Result<(), ClientError>; + /// Verify a `proof` that a packet has not been received. + #[cfg(feature = "val_exec_ctx")] + #[allow(clippy::too_many_arguments)] + fn new_verify_packet_receipt_absence( + &self, + ctx: &dyn ValidationContext, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + ) -> Result<(), ClientError>; + /// Verify a `proof` that a packet has not been received. #[allow(clippy::too_many_arguments)] fn verify_packet_receipt_absence( diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout.rs b/crates/ibc/src/core/ics04_channel/handler/timeout.rs index a811fdce7..f43ec1715 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout.rs @@ -11,6 +11,138 @@ use crate::handler::{HandlerOutput, HandlerResult}; use crate::prelude::*; use crate::timestamp::Expiry; +#[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_a: &Ctx, msg: &MsgTimeout) -> Result<(), ContextError> + where + Ctx: ValidationContext, + { + let port_chan_id_on_a = &(msg.packet.port_on_a.clone(), msg.packet.chan_on_a.clone()); + let chan_end_on_a = ctx_a.channel_end(port_chan_id_on_a)?; + + if !chan_end_on_a.state_matches(&State::Open) { + return Err(PacketError::ChannelClosed { + channel_id: msg.packet.chan_on_a.clone(), + } + .into()); + } + + let counterparty = Counterparty::new( + msg.packet.port_on_b.clone(), + Some(msg.packet.chan_on_b.clone()), + ); + + if !chan_end_on_a.counterparty_matches(&counterparty) { + return Err(PacketError::InvalidPacketCounterparty { + port_id: msg.packet.port_on_b.clone(), + channel_id: msg.packet.chan_on_b.clone(), + } + .into()); + } + + let conn_id_on_a = chan_end_on_a.connection_hops()[0].clone(); + let conn_end_on_a = ctx_a.connection_end(&conn_id_on_a)?; + + //verify packet commitment + let commitment_on_a = ctx_a.get_packet_commitment(&( + msg.packet.port_on_a.clone(), + msg.packet.chan_on_a.clone(), + msg.packet.sequence, + ))?; + + let expected_commitment_on_a = ctx_a.packet_commitment( + &msg.packet.data, + &msg.packet.timeout_height_on_b, + &msg.packet.timeout_timestamp_on_b, + ); + if commitment_on_a != expected_commitment_on_a { + return Err(PacketError::IncorrectPacketCommitment { + sequence: msg.packet.sequence, + } + .into()); + } + + // Verify proofs + { + let client_id_on_a = conn_end_on_a.client_id(); + let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?; + + // check that timeout height or timeout timestamp has passed on the other end + if msg + .packet + .timeout_height_on_b + .has_expired(msg.proof_height_on_b) + { + return Err(PacketError::PacketTimeoutHeightNotReached { + timeout_height: msg.packet.timeout_height_on_b, + chain_height: msg.proof_height_on_b, + } + .into()); + } + + let consensus_state_of_b_on_a = + ctx_a.consensus_state(client_id_on_a, &msg.proof_height_on_b)?; + let timestamp_of_b = consensus_state_of_b_on_a.timestamp(); + + if let Expiry::Expired = msg + .packet + .timeout_timestamp_on_b + .check_expiry(×tamp_of_b) + { + return Err(PacketError::PacketTimeoutTimestampNotReached { + timeout_timestamp: msg.packet.timeout_timestamp_on_b, + chain_timestamp: timestamp_of_b, + } + .into()); + } + let next_seq_recv_verification_result = if chan_end_on_a.order_matches(&Order::Ordered) + { + if msg.packet.sequence < msg.next_seq_recv_on_b { + return Err(PacketError::InvalidPacketSequence { + given_sequence: msg.packet.sequence, + next_sequence: msg.next_seq_recv_on_b, + } + .into()); + } + client_state_of_b_on_a.new_verify_next_sequence_recv( + ctx_a, + msg.proof_height_on_b, + &conn_end_on_a, + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + &msg.packet.port_on_b, + &msg.packet.chan_on_b, + msg.packet.sequence, + ) + } else { + client_state_of_b_on_a.new_verify_packet_receipt_absence( + ctx_a, + msg.proof_height_on_b, + &conn_end_on_a, + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + &msg.packet.port_on_b, + &msg.packet.chan_on_b, + msg.packet.sequence, + ) + }; + next_seq_recv_verification_result + .map_err(|e| ChannelError::PacketVerificationFailed { + sequence: msg.next_seq_recv_on_b, + client_error: e, + }) + .map_err(PacketError::Channel)?; + } + + Ok(()) + } +} + #[derive(Clone, Debug)] pub struct TimeoutPacketResult { pub port_id: PortId, diff --git a/crates/ibc/src/mock/client_state.rs b/crates/ibc/src/mock/client_state.rs index 06e925175..e27667ff4 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -401,6 +401,21 @@ impl ClientState for MockClientState { Ok(()) } + #[cfg(feature = "val_exec_ctx")] + fn new_verify_next_sequence_recv( + &self, + _ctx: &dyn ValidationContext, + _height: Height, + _connection_end: &ConnectionEnd, + _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, + _port_id: &PortId, + _channel_id: &ChannelId, + _sequence: Sequence, + ) -> Result<(), ClientError> { + Ok(()) + } + fn verify_next_sequence_recv( &self, _ctx: &dyn ChannelReader, @@ -415,6 +430,21 @@ impl ClientState for MockClientState { Ok(()) } + #[cfg(feature = "val_exec_ctx")] + fn new_verify_packet_receipt_absence( + &self, + _ctx: &dyn ValidationContext, + _height: Height, + _connection_end: &ConnectionEnd, + _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, + _port_id: &PortId, + _channel_id: &ChannelId, + _sequence: Sequence, + ) -> Result<(), ClientError> { + Ok(()) + } + fn verify_packet_receipt_absence( &self, _ctx: &dyn ChannelReader, From 81d0adcd1321ba61df6f3bb91f5a1d1d6ea4a424 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Tue, 31 Jan 2023 17:41:26 -0500 Subject: [PATCH 02/17] validate still not done --- crates/ibc/src/core/context.rs | 8 ++++++-- crates/ibc/src/core/ics26_routing/context.rs | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 350141d19..dc08ffd3d 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -229,7 +229,7 @@ mod val_exec_ctx { match msg { PacketMsg::Recv(msg) => recv_packet_validate(self, msg), PacketMsg::Ack(_) => todo!(), - PacketMsg::Timeout(msg) => timeout_packet_validate(self, msg), + PacketMsg::Timeout(msg) => timeout_packet_validate(self, module_id, msg), PacketMsg::TimeoutOnClose(_) => todo!(), } .map_err(RouterError::ContextError) @@ -1288,7 +1288,11 @@ mod val_exec_ctx { Ok(()) } - fn timeout_packet_validate(ctx_a: &ValCtx, msg: MsgTimeout) -> Result<(), ContextError> + fn timeout_packet_validate( + ctx_a: &ValCtx, + module_id: ModuleId, + msg: MsgTimeout, + ) -> Result<(), ContextError> where ValCtx: ValidationContext, { diff --git a/crates/ibc/src/core/ics26_routing/context.rs b/crates/ibc/src/core/ics26_routing/context.rs index bb086a06a..fab9166e4 100644 --- a/crates/ibc/src/core/ics26_routing/context.rs +++ b/crates/ibc/src/core/ics26_routing/context.rs @@ -294,6 +294,20 @@ pub trait Module: Send + Sync + AsAnyMut + Debug { Ok(()) } + #[cfg(feature = "val_exec_ctx")] + fn on_timeout_packet_validate( + &self, + packet: &Packet, + relayer: &Signer, + ) -> (ModuleExtras, Result<(), PacketError>); + + #[cfg(feature = "val_exec_ctx")] + fn on_timeout_packet_execute( + &mut self, + packet: &Packet, + relayer: &Signer, + ) -> (ModuleExtras, Result<(), PacketError>); + fn on_timeout_packet( &mut self, _output: &mut ModuleOutputBuilder, From e330a2008299d3637ec52368c3f59d46d0e07d83 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 1 Feb 2023 13:59:51 -0500 Subject: [PATCH 03/17] complete timeout validate --- crates/ibc/src/core/context.rs | 8 ++++-- crates/ibc/src/mock/context.rs | 47 ++++++++++++++++++++++++++++++++++ crates/ibc/src/test_utils.rs | 18 +++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index dc08ffd3d..d2dcfe1fc 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -1298,8 +1298,12 @@ mod val_exec_ctx { { timeout::validate(ctx_a, &msg)?; - // TODO: cb validate + let module = ctx_a + .get_route(&module_id) + .ok_or(ChannelError::RouteNotFound)?; - Ok(()) + let (_, cb_result) = module.on_timeout_packet_validate(&msg.packet, &msg.signer); + + cb_result.map_err(ContextError::PacketError) } } diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index edfebe6c6..c3def2f60 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -1990,6 +1990,30 @@ mod tests { Acknowledgement::try_from(vec![1u8]).unwrap() } + + #[cfg(feature = "val_exec_ctx")] + fn on_timeout_packet_validate( + &self, + _packet: &Packet, + _relayer: &Signer, + ) -> ( + ModuleExtras, + Result<(), crate::core::ics04_channel::error::PacketError>, + ) { + (ModuleExtras::empty(), Ok(())) + } + + #[cfg(feature = "val_exec_ctx")] + fn on_timeout_packet_execute( + &mut self, + _packet: &Packet, + _relayer: &Signer, + ) -> ( + ModuleExtras, + Result<(), crate::core::ics04_channel::error::PacketError>, + ) { + (ModuleExtras::empty(), Ok(())) + } } #[derive(Debug, Default)] @@ -2092,6 +2116,29 @@ mod tests { ) -> Acknowledgement { Acknowledgement::try_from(vec![1u8]).unwrap() } + #[cfg(feature = "val_exec_ctx")] + fn on_timeout_packet_validate( + &self, + _packet: &Packet, + _relayer: &Signer, + ) -> ( + ModuleExtras, + Result<(), crate::core::ics04_channel::error::PacketError>, + ) { + (ModuleExtras::empty(), Ok(())) + } + + #[cfg(feature = "val_exec_ctx")] + fn on_timeout_packet_execute( + &mut self, + _packet: &Packet, + _relayer: &Signer, + ) -> ( + ModuleExtras, + Result<(), crate::core::ics04_channel::error::PacketError>, + ) { + (ModuleExtras::empty(), Ok(())) + } } let r = MockRouterBuilder::default() diff --git a/crates/ibc/src/test_utils.rs b/crates/ibc/src/test_utils.rs index 5174fbad1..56b94b1c8 100644 --- a/crates/ibc/src/test_utils.rs +++ b/crates/ibc/src/test_utils.rs @@ -186,6 +186,24 @@ impl Module for DummyTransferModule { ) -> Acknowledgement { Acknowledgement::try_from(vec![1u8]).unwrap() } + + #[cfg(feature = "val_exec_ctx")] + fn on_timeout_packet_validate( + &self, + _packet: &Packet, + _relayer: &Signer, + ) -> (ModuleExtras, Result<(), PacketError>) { + (ModuleExtras::empty(), Ok(())) + } + + #[cfg(feature = "val_exec_ctx")] + fn on_timeout_packet_execute( + &mut self, + _packet: &Packet, + _relayer: &Signer, + ) -> (ModuleExtras, Result<(), PacketError>) { + (ModuleExtras::empty(), Ok(())) + } } impl TokenTransferKeeper for DummyTransferModule { From 2424af9bd1b946c1e5bc48157c5cd19701d0e5ba Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 1 Feb 2023 15:12:55 -0500 Subject: [PATCH 04/17] timeout validate: accept noop --- crates/ibc/src/core/ics04_channel/handler/timeout.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout.rs b/crates/ibc/src/core/ics04_channel/handler/timeout.rs index f43ec1715..3c918a76d 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout.rs @@ -49,11 +49,19 @@ pub(crate) mod val_exec_ctx { let conn_end_on_a = ctx_a.connection_end(&conn_id_on_a)?; //verify packet commitment - let commitment_on_a = ctx_a.get_packet_commitment(&( + let commitment_on_a = match ctx_a.get_packet_commitment(&( msg.packet.port_on_a.clone(), msg.packet.chan_on_a.clone(), msg.packet.sequence, - ))?; + )) { + Ok(commitment_on_a) => commitment_on_a, + + // This error indicates that the timeout has already been relayed + // or there is a misconfigured relayer attempting to prove a timeout + // for a packet never sent. Core IBC will treat this error as a no-op in order to + // prevent an entire relay transaction from failing and consuming unnecessary fees. + Err(_) => return Ok(()), + }; let expected_commitment_on_a = ctx_a.packet_commitment( &msg.packet.data, From 098cb602cf2a15230787747e51dc462ffb104fd1 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 1 Feb 2023 15:48:00 -0500 Subject: [PATCH 05/17] timeout packet execute --- crates/ibc/src/core/context.rs | 96 ++++++++++++++++++++++++++++++++-- 1 file changed, 93 insertions(+), 3 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index d2dcfe1fc..3ce52c394 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -78,8 +78,8 @@ mod val_exec_ctx { 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, ReceivePacket, - WriteAcknowledgement, + ChannelClosed, CloseConfirm, CloseInit, OpenAck, OpenConfirm, OpenInit, OpenTry, + ReceivePacket, TimeoutPacket, WriteAcknowledgement, }; use crate::core::ics04_channel::handler::{ chan_close_confirm, chan_close_init, chan_open_ack, chan_open_confirm, chan_open_init, @@ -486,7 +486,7 @@ mod val_exec_ctx { match msg { PacketMsg::Recv(msg) => recv_packet_execute(self, module_id, msg), PacketMsg::Ack(_) => todo!(), - PacketMsg::Timeout(_) => todo!(), + PacketMsg::Timeout(msg) => timeout_packet_execute(self, module_id, msg), PacketMsg::TimeoutOnClose(_) => todo!(), } .map_err(RouterError::ContextError) @@ -1306,4 +1306,94 @@ mod val_exec_ctx { cb_result.map_err(ContextError::PacketError) } + + fn timeout_packet_execute( + ctx_a: &mut ExecCtx, + module_id: ModuleId, + msg: MsgTimeout, + ) -> Result<(), ContextError> + where + ExecCtx: ExecutionContext, + { + let port_chan_id_on_a = (msg.packet.port_on_a.clone(), msg.packet.chan_on_a.clone()); + let chan_end_on_a = ctx_a.channel_end(&port_chan_id_on_a)?; + + // In all cases, this event is emitted + ctx_a.emit_ibc_event(IbcEvent::TimeoutPacket(TimeoutPacket::new( + msg.packet.clone(), + chan_end_on_a.ordering, + ))); + + // check if we're in the NO-OP case + if ctx_a + .get_packet_commitment(&( + msg.packet.port_on_a.clone(), + msg.packet.chan_on_a.clone(), + msg.packet.sequence, + )) + .is_err() + { + // This error indicates that the timeout has already been relayed + // or there is a misconfigured relayer attempting to prove a timeout + // for a packet never sent. Core IBC will treat this error as a no-op in order to + // prevent an entire relay transaction from failing and consuming unnecessary fees. + return Ok(()); + }; + + let module = ctx_a + .get_route_mut(&module_id) + .ok_or(ChannelError::RouteNotFound)?; + + let (extras, cb_result) = module.on_timeout_packet_execute(&msg.packet, &msg.signer); + + cb_result?; + + // apply state changes + let chan_end_on_a = { + let commitment_path = CommitmentsPath { + port_id: msg.packet.port_on_a.clone(), + channel_id: msg.packet.chan_on_a.clone(), + sequence: msg.packet.sequence, + }; + ctx_a.delete_packet_commitment(commitment_path)?; + + if let Order::Ordered = chan_end_on_a.ordering { + let mut chan_end_on_a = chan_end_on_a; + chan_end_on_a.state = State::Closed; + ctx_a.store_channel(port_chan_id_on_a, chan_end_on_a.clone())?; + + chan_end_on_a + } else { + chan_end_on_a + } + }; + + // emit events and logs + { + ctx_a.log_message("success: packet timeout".to_string()); + + if let Order::Ordered = chan_end_on_a.ordering { + let conn_id_on_a = chan_end_on_a.connection_hops()[0].clone(); + + ctx_a.emit_ibc_event(IbcEvent::ChannelClosed(ChannelClosed::new( + msg.packet.port_on_a.clone(), + msg.packet.chan_on_a.clone(), + chan_end_on_a.counterparty().port_id.clone(), + chan_end_on_a.counterparty().channel_id.clone(), + conn_id_on_a, + chan_end_on_a.ordering, + ))); + } + + for module_event in extras.events { + ctx_a.emit_ibc_event(IbcEvent::AppModule(module_event)); + } + + for log_message in extras.log { + ctx_a.log_message(log_message); + } + } + + Ok(()) + } } From d5b6f5b7fdc60d92faa204f0fac47ce55de5f3b1 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 1 Feb 2023 15:57:38 -0500 Subject: [PATCH 06/17] timeout_on_close validation --- crates/ibc/src/core/context.rs | 26 ++- .../ics04_channel/handler/timeout_on_close.rs | 154 ++++++++++++++++++ 2 files changed, 178 insertions(+), 2 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 3ce52c394..60044f74a 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -83,7 +83,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, recv_packet, timeout, + chan_open_try, recv_packet, timeout, timeout_on_close, }; use crate::core::ics04_channel::msgs::acknowledgement::Acknowledgement; use crate::core::ics04_channel::msgs::chan_close_confirm::MsgChannelCloseConfirm; @@ -94,6 +94,7 @@ mod val_exec_ctx { 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::timeout::MsgTimeout; + use crate::core::ics04_channel::msgs::timeout_on_close::MsgTimeoutOnClose; use crate::core::ics04_channel::msgs::{ChannelMsg, PacketMsg}; use crate::core::ics04_channel::packet::{Receipt, Sequence}; use crate::core::ics04_channel::timeout::TimeoutHeight; @@ -230,7 +231,9 @@ mod val_exec_ctx { PacketMsg::Recv(msg) => recv_packet_validate(self, msg), PacketMsg::Ack(_) => todo!(), PacketMsg::Timeout(msg) => timeout_packet_validate(self, module_id, msg), - PacketMsg::TimeoutOnClose(_) => todo!(), + PacketMsg::TimeoutOnClose(msg) => { + timeout_on_close_packet_validate(self, module_id, msg) + } } .map_err(RouterError::ContextError) } @@ -1396,4 +1399,23 @@ mod val_exec_ctx { Ok(()) } + + fn timeout_on_close_packet_validate( + ctx_a: &ValCtx, + module_id: ModuleId, + msg: MsgTimeoutOnClose, + ) -> Result<(), ContextError> + where + ValCtx: ValidationContext, + { + timeout_on_close::validate(ctx_a, &msg)?; + + let module = ctx_a + .get_route(&module_id) + .ok_or(ChannelError::RouteNotFound)?; + + let (_, cb_result) = module.on_timeout_on_close_packet_validate(&msg.packet, &msg.signer); + + cb_result.map_err(ContextError::PacketError) + } } diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs index bbb98444f..c73f80738 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs @@ -11,6 +11,160 @@ use crate::events::IbcEvent; use crate::handler::{HandlerOutput, HandlerResult}; use crate::prelude::*; +#[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_a: &Ctx, msg: &MsgTimeoutOnClose) -> Result<(), ContextError> + where + Ctx: ValidationContext, + { + let packet = &msg.packet; + + let port_chan_id_on_a = &(msg.packet.port_on_a.clone(), msg.packet.chan_on_a.clone()); + let chan_end_on_a = ctx_a.channel_end(port_chan_id_on_a)?; + + let counterparty = + Counterparty::new(packet.port_on_b.clone(), Some(packet.chan_on_b.clone())); + + if !chan_end_on_a.counterparty_matches(&counterparty) { + return Err(PacketError::InvalidPacketCounterparty { + port_id: packet.port_on_b.clone(), + channel_id: packet.chan_on_b.clone(), + } + .into()); + } + + //verify the packet was sent, check the store + let commitment_on_a = match ctx_a.get_packet_commitment(&( + msg.packet.port_on_a.clone(), + msg.packet.chan_on_a.clone(), + msg.packet.sequence, + )) { + Ok(commitment_on_a) => commitment_on_a, + + // This error indicates that the timeout has already been relayed + // or there is a misconfigured relayer attempting to prove a timeout + // for a packet never sent. Core IBC will treat this error as a no-op in order to + // prevent an entire relay transaction from failing and consuming unnecessary fees. + Err(_) => return Ok(()), + }; + + let expected_commitment_on_a = ctx_a.packet_commitment( + &packet.data, + &packet.timeout_height_on_b, + &packet.timeout_timestamp_on_b, + ); + if commitment_on_a != expected_commitment_on_a { + return Err(PacketError::IncorrectPacketCommitment { + sequence: packet.sequence, + } + .into()); + } + + let conn_id_on_a = chan_end_on_a.connection_hops()[0].clone(); + let conn_end_on_a = ctx_a.connection_end(&conn_id_on_a)?; + + // Verify proofs + { + let client_id_on_a = conn_end_on_a.client_id(); + let client_state_of_b_on_a = ctx_a.client_state(client_id_on_a)?; + + // The client must not be frozen. + if client_state_of_b_on_a.is_frozen() { + return Err(PacketError::FrozenClient { + client_id: client_id_on_a.clone(), + } + .into()); + } + + let consensus_state_of_b_on_a = + ctx_a.consensus_state(client_id_on_a, &msg.proof_height_on_b)?; + let prefix_on_b = conn_end_on_a.counterparty().prefix(); + let port_id_on_b = &chan_end_on_a.counterparty().port_id; + let chan_id_on_b = + chan_end_on_a + .counterparty() + .channel_id() + .ok_or(PacketError::Channel( + ChannelError::InvalidCounterpartyChannelId, + ))?; + let conn_id_on_b = conn_end_on_a.counterparty().connection_id().ok_or( + PacketError::UndefinedConnectionCounterparty { + connection_id: chan_end_on_a.connection_hops()[0].clone(), + }, + )?; + let expected_conn_hops_on_b = vec![conn_id_on_b.clone()]; + let expected_counterparty = + Counterparty::new(packet.port_on_a.clone(), Some(packet.chan_on_a.clone())); + let expected_chan_end_on_b = ChannelEnd::new( + State::Closed, + *chan_end_on_a.ordering(), + expected_counterparty, + expected_conn_hops_on_b, + chan_end_on_a.version().clone(), + ); + + // Verify the proof for the channel state against the expected channel end. + // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. + client_state_of_b_on_a + .verify_channel_state( + msg.proof_height_on_b, + prefix_on_b, + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + port_id_on_b, + chan_id_on_b, + &expected_chan_end_on_b, + ) + .map_err(ChannelError::VerifyChannelFailed) + .map_err(PacketError::Channel)?; + + let next_seq_recv_verification_result = if chan_end_on_a.order_matches(&Order::Ordered) + { + if packet.sequence < msg.next_seq_recv_on_b { + return Err(PacketError::InvalidPacketSequence { + given_sequence: packet.sequence, + next_sequence: msg.next_seq_recv_on_b, + }.into()); + } + client_state_of_b_on_a.new_verify_next_sequence_recv( + ctx_a, + msg.proof_height_on_b, + &conn_end_on_a, + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + &packet.port_on_b, + &packet.chan_on_b, + packet.sequence, + ) + } else { + client_state_of_b_on_a.new_verify_packet_receipt_absence( + ctx_a, + msg.proof_height_on_b, + &conn_end_on_a, + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + &packet.port_on_b, + &packet.chan_on_b, + packet.sequence, + ) + }; + next_seq_recv_verification_result + .map_err(|e| ChannelError::PacketVerificationFailed { + sequence: msg.next_seq_recv_on_b, + client_error: e, + }) + .map_err(PacketError::Channel)?; + }; + + Ok(()) + } +} + /// Per our convention, this message is processed on chain A. pub(crate) fn process( ctx_a: &Ctx, From 5f2bc505aa3d7355e2e0261231a4503d56af6e31 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 1 Feb 2023 16:13:18 -0500 Subject: [PATCH 07/17] timeout_on_close done --- crates/ibc/src/core/context.rs | 87 ++++++++++--------- .../ics04_channel/handler/timeout_on_close.rs | 3 +- crates/ibc/src/core/ics26_routing/context.rs | 2 + 3 files changed, 52 insertions(+), 40 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 60044f74a..7545b4542 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -230,10 +230,14 @@ mod val_exec_ctx { match msg { PacketMsg::Recv(msg) => recv_packet_validate(self, msg), PacketMsg::Ack(_) => todo!(), - PacketMsg::Timeout(msg) => timeout_packet_validate(self, module_id, msg), - PacketMsg::TimeoutOnClose(msg) => { - timeout_on_close_packet_validate(self, module_id, msg) + PacketMsg::Timeout(msg) => { + timeout_packet_validate(self, module_id, TimeoutMsgType::Timeout(msg)) } + PacketMsg::TimeoutOnClose(msg) => timeout_packet_validate( + self, + module_id, + TimeoutMsgType::TimeoutOnClose(msg), + ), } .map_err(RouterError::ContextError) } @@ -489,8 +493,14 @@ mod val_exec_ctx { match msg { PacketMsg::Recv(msg) => recv_packet_execute(self, module_id, msg), PacketMsg::Ack(_) => todo!(), - PacketMsg::Timeout(msg) => timeout_packet_execute(self, module_id, msg), - PacketMsg::TimeoutOnClose(_) => todo!(), + PacketMsg::Timeout(msg) => { + timeout_packet_execute(self, module_id, TimeoutMsgType::Timeout(msg)) + } + PacketMsg::TimeoutOnClose(msg) => timeout_packet_execute( + self, + module_id, + TimeoutMsgType::TimeoutOnClose(msg), + ), } .map_err(RouterError::ContextError) } @@ -1291,21 +1301,34 @@ mod val_exec_ctx { Ok(()) } + enum TimeoutMsgType { + Timeout(MsgTimeout), + TimeoutOnClose(MsgTimeoutOnClose), + } + fn timeout_packet_validate( ctx_a: &ValCtx, module_id: ModuleId, - msg: MsgTimeout, + timeout_msg_type: TimeoutMsgType, ) -> Result<(), ContextError> where ValCtx: ValidationContext, { - timeout::validate(ctx_a, &msg)?; + match &timeout_msg_type { + TimeoutMsgType::Timeout(msg) => timeout::validate(ctx_a, msg), + TimeoutMsgType::TimeoutOnClose(msg) => timeout_on_close::validate(ctx_a, msg), + }?; let module = ctx_a .get_route(&module_id) .ok_or(ChannelError::RouteNotFound)?; - let (_, cb_result) = module.on_timeout_packet_validate(&msg.packet, &msg.signer); + let (packet, signer) = match timeout_msg_type { + TimeoutMsgType::Timeout(msg) => (msg.packet, msg.signer), + TimeoutMsgType::TimeoutOnClose(msg) => (msg.packet, msg.signer), + }; + + let (_, cb_result) = module.on_timeout_packet_validate(&packet, &signer); cb_result.map_err(ContextError::PacketError) } @@ -1313,26 +1336,31 @@ mod val_exec_ctx { fn timeout_packet_execute( ctx_a: &mut ExecCtx, module_id: ModuleId, - msg: MsgTimeout, + timeout_msg_type: TimeoutMsgType, ) -> Result<(), ContextError> where ExecCtx: ExecutionContext, { - let port_chan_id_on_a = (msg.packet.port_on_a.clone(), msg.packet.chan_on_a.clone()); + let (packet, signer) = match timeout_msg_type { + TimeoutMsgType::Timeout(msg) => (msg.packet, msg.signer), + TimeoutMsgType::TimeoutOnClose(msg) => (msg.packet, msg.signer), + }; + + let port_chan_id_on_a = (packet.port_on_a.clone(), packet.chan_on_a.clone()); let chan_end_on_a = ctx_a.channel_end(&port_chan_id_on_a)?; // In all cases, this event is emitted ctx_a.emit_ibc_event(IbcEvent::TimeoutPacket(TimeoutPacket::new( - msg.packet.clone(), + packet.clone(), chan_end_on_a.ordering, ))); // check if we're in the NO-OP case if ctx_a .get_packet_commitment(&( - msg.packet.port_on_a.clone(), - msg.packet.chan_on_a.clone(), - msg.packet.sequence, + packet.port_on_a.clone(), + packet.chan_on_a.clone(), + packet.sequence, )) .is_err() { @@ -1347,16 +1375,16 @@ mod val_exec_ctx { .get_route_mut(&module_id) .ok_or(ChannelError::RouteNotFound)?; - let (extras, cb_result) = module.on_timeout_packet_execute(&msg.packet, &msg.signer); + let (extras, cb_result) = module.on_timeout_packet_execute(&packet, &signer); cb_result?; // apply state changes let chan_end_on_a = { let commitment_path = CommitmentsPath { - port_id: msg.packet.port_on_a.clone(), - channel_id: msg.packet.chan_on_a.clone(), - sequence: msg.packet.sequence, + port_id: packet.port_on_a.clone(), + channel_id: packet.chan_on_a.clone(), + sequence: packet.sequence, }; ctx_a.delete_packet_commitment(commitment_path)?; @@ -1379,8 +1407,8 @@ mod val_exec_ctx { let conn_id_on_a = chan_end_on_a.connection_hops()[0].clone(); ctx_a.emit_ibc_event(IbcEvent::ChannelClosed(ChannelClosed::new( - msg.packet.port_on_a.clone(), - msg.packet.chan_on_a.clone(), + packet.port_on_a.clone(), + packet.chan_on_a.clone(), chan_end_on_a.counterparty().port_id.clone(), chan_end_on_a.counterparty().channel_id.clone(), conn_id_on_a, @@ -1399,23 +1427,4 @@ mod val_exec_ctx { Ok(()) } - - fn timeout_on_close_packet_validate( - ctx_a: &ValCtx, - module_id: ModuleId, - msg: MsgTimeoutOnClose, - ) -> Result<(), ContextError> - where - ValCtx: ValidationContext, - { - timeout_on_close::validate(ctx_a, &msg)?; - - let module = ctx_a - .get_route(&module_id) - .ok_or(ChannelError::RouteNotFound)?; - - let (_, cb_result) = module.on_timeout_on_close_packet_validate(&msg.packet, &msg.signer); - - cb_result.map_err(ContextError::PacketError) - } } diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs index c73f80738..82474a51d 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs @@ -129,7 +129,8 @@ pub(crate) mod val_exec_ctx { return Err(PacketError::InvalidPacketSequence { given_sequence: packet.sequence, next_sequence: msg.next_seq_recv_on_b, - }.into()); + } + .into()); } client_state_of_b_on_a.new_verify_next_sequence_recv( ctx_a, diff --git a/crates/ibc/src/core/ics26_routing/context.rs b/crates/ibc/src/core/ics26_routing/context.rs index fab9166e4..d3f608bfd 100644 --- a/crates/ibc/src/core/ics26_routing/context.rs +++ b/crates/ibc/src/core/ics26_routing/context.rs @@ -294,6 +294,7 @@ pub trait Module: Send + Sync + AsAnyMut + Debug { Ok(()) } + /// Note: `MsgTimeout` and `MsgTimeoutOnClose` use the same callback #[cfg(feature = "val_exec_ctx")] fn on_timeout_packet_validate( &self, @@ -301,6 +302,7 @@ pub trait Module: Send + Sync + AsAnyMut + Debug { relayer: &Signer, ) -> (ModuleExtras, Result<(), PacketError>); + /// Note: `MsgTimeout` and `MsgTimeoutOnClose` use the same callback #[cfg(feature = "val_exec_ctx")] fn on_timeout_packet_execute( &mut self, From 93897d6cd2b6e8a068632a3b600785652be2c0b2 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 1 Feb 2023 17:33:01 -0500 Subject: [PATCH 08/17] ack validate --- .../clients/ics07_tendermint/client_state.rs | 32 +++++ crates/ibc/src/core/context.rs | 26 +++- .../ibc/src/core/ics02_client/client_state.rs | 16 +++ .../ics04_channel/handler/acknowledgement.rs | 124 ++++++++++++++++++ crates/ibc/src/core/ics26_routing/context.rs | 16 +++ crates/ibc/src/mock/client_state.rs | 16 +++ crates/ibc/src/mock/context.rs | 46 +++++++ crates/ibc/src/test_utils.rs | 20 +++ 8 files changed, 292 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 4c0d82e95..f46a19cf0 100644 --- a/crates/ibc/src/clients/ics07_tendermint/client_state.rs +++ b/crates/ibc/src/clients/ics07_tendermint/client_state.rs @@ -1150,6 +1150,38 @@ impl Ics2ClientState for ClientState { ) } + #[cfg(feature = "val_exec_ctx")] + fn new_verify_packet_acknowledgement( + &self, + ctx: &dyn ValidationContext, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + ack: AcknowledgementCommitment, + ) -> Result<(), ClientError> { + let client_state = downcast_tm_client_state(self)?; + client_state.verify_height(height)?; + new_verify_delay_passed(ctx, height, connection_end)?; + + let ack_path = AcksPath { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + sequence, + }; + verify_membership( + client_state, + connection_end.counterparty().prefix(), + proof, + root, + ack_path, + ack.into_vec(), + ) + } + fn verify_packet_acknowledgement( &self, ctx: &dyn ChannelReader, diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 7545b4542..e84f3966b 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -82,10 +82,10 @@ mod val_exec_ctx { ReceivePacket, TimeoutPacket, 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, recv_packet, timeout, timeout_on_close, + acknowledgement, chan_close_confirm, chan_close_init, chan_open_ack, chan_open_confirm, + chan_open_init, chan_open_try, recv_packet, timeout, timeout_on_close, }; - use crate::core::ics04_channel::msgs::acknowledgement::Acknowledgement; + use crate::core::ics04_channel::msgs::acknowledgement::{Acknowledgement, MsgAcknowledgement}; use crate::core::ics04_channel::msgs::chan_close_confirm::MsgChannelCloseConfirm; use crate::core::ics04_channel::msgs::chan_close_init::MsgChannelCloseInit; use crate::core::ics04_channel::msgs::chan_open_ack::MsgChannelOpenAck; @@ -229,7 +229,7 @@ mod val_exec_ctx { match msg { PacketMsg::Recv(msg) => recv_packet_validate(self, msg), - PacketMsg::Ack(_) => todo!(), + PacketMsg::Ack(msg) => acknowledgement_validate(self, module_id, msg), PacketMsg::Timeout(msg) => { timeout_packet_validate(self, module_id, TimeoutMsgType::Timeout(msg)) } @@ -1300,6 +1300,24 @@ mod val_exec_ctx { Ok(()) } + fn acknowledgement_validate( + ctx_a: &ValCtx, + module_id: ModuleId, + msg: MsgAcknowledgement, + ) -> Result<(), ContextError> + where + ValCtx: ValidationContext, + { + acknowledgement::validate(ctx_a, &msg)?; + + let module = ctx_a + .get_route(&module_id) + .ok_or(ChannelError::RouteNotFound)?; + + module + .on_acknowledgement_packet_validate(&msg.packet, &msg.acknowledgement, &msg.signer) + .map_err(ContextError::PacketError) + } enum TimeoutMsgType { Timeout(MsgTimeout), diff --git a/crates/ibc/src/core/ics02_client/client_state.rs b/crates/ibc/src/core/ics02_client/client_state.rs index 151da09b5..b12d5a6aa 100644 --- a/crates/ibc/src/core/ics02_client/client_state.rs +++ b/crates/ibc/src/core/ics02_client/client_state.rs @@ -220,6 +220,22 @@ pub trait ClientState: commitment: PacketCommitment, ) -> Result<(), ClientError>; + /// Verify a `proof` that a packet has been committed. + #[cfg(feature = "val_exec_ctx")] + #[allow(clippy::too_many_arguments)] + fn new_verify_packet_acknowledgement( + &self, + ctx: &dyn ValidationContext, + height: Height, + connection_end: &ConnectionEnd, + proof: &CommitmentProofBytes, + root: &CommitmentRoot, + port_id: &PortId, + channel_id: &ChannelId, + sequence: Sequence, + ack: AcknowledgementCommitment, + ) -> Result<(), ClientError>; + /// Verify a `proof` that a packet has been committed. #[allow(clippy::too_many_arguments)] fn verify_packet_acknowledgement( diff --git a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs index 14e98c811..847c2c0dc 100644 --- a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs @@ -11,6 +11,130 @@ use crate::events::IbcEvent; use crate::handler::{HandlerOutput, HandlerResult}; use crate::prelude::*; +#[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_a: &Ctx, msg: &MsgAcknowledgement) -> Result<(), ContextError> + where + Ctx: ValidationContext, + { + let packet = &msg.packet; + + let port_chan_id_on_a = &(msg.packet.port_on_a.clone(), msg.packet.chan_on_a.clone()); + let chan_end_on_a = ctx_a.channel_end(port_chan_id_on_a)?; + + if !chan_end_on_a.state_matches(&State::Open) { + return Err(PacketError::ChannelClosed { + channel_id: packet.chan_on_a.clone(), + } + .into()); + } + + let counterparty = + Counterparty::new(packet.port_on_b.clone(), Some(packet.chan_on_b.clone())); + + if !chan_end_on_a.counterparty_matches(&counterparty) { + return Err(PacketError::InvalidPacketCounterparty { + port_id: packet.port_on_b.clone(), + channel_id: packet.chan_on_b.clone(), + } + .into()); + } + + let conn_id_on_a = &chan_end_on_a.connection_hops()[0]; + let conn_end_on_a = ctx_a.connection_end(conn_id_on_a)?; + + if !conn_end_on_a.state_matches(&ConnectionState::Open) { + return Err(PacketError::ConnectionNotOpen { + connection_id: chan_end_on_a.connection_hops()[0].clone(), + } + .into()); + } + + // Verify packet commitment + let commitment_on_a = match ctx_a.get_packet_commitment(&( + msg.packet.port_on_a.clone(), + msg.packet.chan_on_a.clone(), + msg.packet.sequence, + )) { + Ok(commitment_on_a) => commitment_on_a, + + // This error indicates that the timeout has already been relayed + // or there is a misconfigured relayer attempting to prove a timeout + // for a packet never sent. Core IBC will treat this error as a no-op in order to + // prevent an entire relay transaction from failing and consuming unnecessary fees. + Err(_) => return Ok(()), + }; + + if commitment_on_a + != ctx_a.packet_commitment( + &packet.data, + &packet.timeout_height_on_b, + &packet.timeout_timestamp_on_b, + ) + { + return Err(PacketError::IncorrectPacketCommitment { + sequence: packet.sequence, + } + .into()); + } + + // Verify proofs + { + let client_id_on_a = conn_end_on_a.client_id(); + let client_state_on_a = ctx_a.client_state(client_id_on_a)?; + + // The client must not be frozen. + if client_state_on_a.is_frozen() { + return Err(PacketError::FrozenClient { + client_id: client_id_on_a.clone(), + } + .into()); + } + + let consensus_state = ctx_a.consensus_state(client_id_on_a, &msg.proof_height_on_b)?; + + let ack_commitment = ctx_a.ack_commitment(&msg.acknowledgement); + + // Verify the proof for the packet against the chain store. + client_state_on_a + .new_verify_packet_acknowledgement( + ctx_a, + msg.proof_height_on_b, + &conn_end_on_a, + &msg.proof_acked_on_b, + consensus_state.root(), + &packet.port_on_b, + &packet.chan_on_b, + packet.sequence, + ack_commitment, + ) + .map_err(|e| ChannelError::PacketVerificationFailed { + sequence: packet.sequence, + client_error: e, + }) + .map_err(PacketError::Channel)?; + } + + if let Order::Ordered = chan_end_on_a.ordering { + let next_seq_ack = ctx_a.get_next_sequence_ack(port_chan_id_on_a)?; + + if packet.sequence != next_seq_ack { + return Err(PacketError::InvalidPacketSequence { + given_sequence: packet.sequence, + next_sequence: next_seq_ack, + } + .into()); + } + } + + Ok(()) + } +} #[derive(Clone, Debug)] pub struct AckPacketResult { pub port_id: PortId, diff --git a/crates/ibc/src/core/ics26_routing/context.rs b/crates/ibc/src/core/ics26_routing/context.rs index d3f608bfd..711ee3452 100644 --- a/crates/ibc/src/core/ics26_routing/context.rs +++ b/crates/ibc/src/core/ics26_routing/context.rs @@ -284,6 +284,22 @@ pub trait Module: Send + Sync + AsAnyMut + Debug { _relayer: &Signer, ) -> Acknowledgement; + #[cfg(feature = "val_exec_ctx")] + fn on_acknowledgement_packet_validate( + &self, + _packet: &Packet, + _acknowledgement: &Acknowledgement, + _relayer: &Signer, + ) -> Result<(), PacketError>; + + #[cfg(feature = "val_exec_ctx")] + fn on_acknowledgement_packet_execute( + &mut self, + _packet: &Packet, + _acknowledgement: &Acknowledgement, + _relayer: &Signer, + ) -> (ModuleExtras, Result<(), PacketError>); + fn on_acknowledgement_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 3cca27116..414b64ff8 100644 --- a/crates/ibc/src/mock/client_state.rs +++ b/crates/ibc/src/mock/client_state.rs @@ -471,6 +471,22 @@ impl ClientState for MockClientState { ) -> Result<(), ClientError> { Ok(()) } + + #[cfg(feature = "val_exec_ctx")] + fn new_verify_packet_acknowledgement( + &self, + _ctx: &dyn ValidationContext, + _height: Height, + _connection_end: &ConnectionEnd, + _proof: &CommitmentProofBytes, + _root: &CommitmentRoot, + _port_id: &PortId, + _channel_id: &ChannelId, + _sequence: Sequence, + _ack: AcknowledgementCommitment, + ) -> Result<(), ClientError> { + Ok(()) + } } impl From for MockClientState { diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index c3def2f60..ebf47a8e1 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -2014,6 +2014,29 @@ mod tests { ) { (ModuleExtras::empty(), Ok(())) } + + #[cfg(feature = "val_exec_ctx")] + fn on_acknowledgement_packet_validate( + &self, + _packet: &Packet, + _acknowledgement: &Acknowledgement, + _relayer: &Signer, + ) -> Result<(), crate::core::ics04_channel::error::PacketError> { + Ok(()) + } + + #[cfg(feature = "val_exec_ctx")] + fn on_acknowledgement_packet_execute( + &mut self, + _packet: &Packet, + _acknowledgement: &Acknowledgement, + _relayer: &Signer, + ) -> ( + ModuleExtras, + Result<(), crate::core::ics04_channel::error::PacketError>, + ) { + (ModuleExtras::empty(), Ok(())) + } } #[derive(Debug, Default)] @@ -2139,6 +2162,29 @@ mod tests { ) { (ModuleExtras::empty(), Ok(())) } + + #[cfg(feature = "val_exec_ctx")] + fn on_acknowledgement_packet_validate( + &self, + _packet: &Packet, + _acknowledgement: &Acknowledgement, + _relayer: &Signer, + ) -> Result<(), crate::core::ics04_channel::error::PacketError> { + Ok(()) + } + + #[cfg(feature = "val_exec_ctx")] + fn on_acknowledgement_packet_execute( + &mut self, + _packet: &Packet, + _acknowledgement: &Acknowledgement, + _relayer: &Signer, + ) -> ( + ModuleExtras, + Result<(), crate::core::ics04_channel::error::PacketError>, + ) { + (ModuleExtras::empty(), Ok(())) + } } let r = MockRouterBuilder::default() diff --git a/crates/ibc/src/test_utils.rs b/crates/ibc/src/test_utils.rs index 56b94b1c8..f0658ffaa 100644 --- a/crates/ibc/src/test_utils.rs +++ b/crates/ibc/src/test_utils.rs @@ -204,6 +204,26 @@ impl Module for DummyTransferModule { ) -> (ModuleExtras, Result<(), PacketError>) { (ModuleExtras::empty(), Ok(())) } + + #[cfg(feature = "val_exec_ctx")] + fn on_acknowledgement_packet_validate( + &self, + _packet: &Packet, + _acknowledgement: &Acknowledgement, + _relayer: &Signer, + ) -> Result<(), PacketError> { + Ok(()) + } + + #[cfg(feature = "val_exec_ctx")] + fn on_acknowledgement_packet_execute( + &mut self, + _packet: &Packet, + _acknowledgement: &Acknowledgement, + _relayer: &Signer, + ) -> (ModuleExtras, Result<(), PacketError>) { + (ModuleExtras::empty(), Ok(())) + } } impl TokenTransferKeeper for DummyTransferModule { From 313aea157f50d8c289ca2afbea7e612bd3782859 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 1 Feb 2023 17:52:28 -0500 Subject: [PATCH 09/17] Acknowledgement execute --- crates/ibc/src/core/context.rs | 95 ++++++++++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 5 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index e84f3966b..5a1395fd0 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -78,8 +78,8 @@ mod val_exec_ctx { use crate::core::ics04_channel::commitment::{AcknowledgementCommitment, PacketCommitment}; use crate::core::ics04_channel::context::calculate_block_delay; use crate::core::ics04_channel::events::{ - ChannelClosed, CloseConfirm, CloseInit, OpenAck, OpenConfirm, OpenInit, OpenTry, - ReceivePacket, TimeoutPacket, WriteAcknowledgement, + AcknowledgePacket, ChannelClosed, CloseConfirm, CloseInit, OpenAck, OpenConfirm, OpenInit, + OpenTry, ReceivePacket, TimeoutPacket, WriteAcknowledgement, }; use crate::core::ics04_channel::handler::{ acknowledgement, chan_close_confirm, chan_close_init, chan_open_ack, chan_open_confirm, @@ -229,7 +229,9 @@ mod val_exec_ctx { match msg { PacketMsg::Recv(msg) => recv_packet_validate(self, msg), - PacketMsg::Ack(msg) => acknowledgement_validate(self, module_id, msg), + PacketMsg::Ack(msg) => { + acknowledgement_packet_validate(self, module_id, msg) + } PacketMsg::Timeout(msg) => { timeout_packet_validate(self, module_id, TimeoutMsgType::Timeout(msg)) } @@ -492,7 +494,7 @@ mod val_exec_ctx { match msg { PacketMsg::Recv(msg) => recv_packet_execute(self, module_id, msg), - PacketMsg::Ack(_) => todo!(), + PacketMsg::Ack(msg) => acknowledgement_packet_execute(self, module_id, msg), PacketMsg::Timeout(msg) => { timeout_packet_execute(self, module_id, TimeoutMsgType::Timeout(msg)) } @@ -1300,7 +1302,8 @@ mod val_exec_ctx { Ok(()) } - fn acknowledgement_validate( + + fn acknowledgement_packet_validate( ctx_a: &ValCtx, module_id: ModuleId, msg: MsgAcknowledgement, @@ -1319,6 +1322,88 @@ mod val_exec_ctx { .map_err(ContextError::PacketError) } + fn acknowledgement_packet_execute( + ctx_a: &mut ExecCtx, + module_id: ModuleId, + msg: MsgAcknowledgement, + ) -> Result<(), ContextError> + where + ExecCtx: ExecutionContext, + { + let port_chan_id_on_a = (msg.packet.port_on_a.clone(), msg.packet.chan_on_a.clone()); + let chan_end_on_a = ctx_a.channel_end(&port_chan_id_on_a)?; + let conn_id_on_a = &chan_end_on_a.connection_hops()[0]; + + // In all cases, this event is emitted + ctx_a.emit_ibc_event(IbcEvent::AcknowledgePacket(AcknowledgePacket::new( + msg.packet.clone(), + chan_end_on_a.ordering, + conn_id_on_a.clone(), + ))); + + // check if we're in the NO-OP case + if ctx_a + .get_packet_commitment(&( + msg.packet.port_on_a.clone(), + msg.packet.chan_on_a.clone(), + msg.packet.sequence, + )) + .is_err() + { + // This error indicates that the timeout has already been relayed + // or there is a misconfigured relayer attempting to prove a timeout + // for a packet never sent. Core IBC will treat this error as a no-op in order to + // prevent an entire relay transaction from failing and consuming unnecessary fees. + return Ok(()); + }; + + let module = ctx_a + .get_route_mut(&module_id) + .ok_or(ChannelError::RouteNotFound)?; + + let (extras, cb_result) = module.on_acknowledgement_packet_execute( + &msg.packet, + &msg.acknowledgement, + &msg.signer, + ); + + cb_result?; + + // apply state changes + { + let commitment_path = CommitmentsPath { + port_id: msg.packet.port_on_a.clone(), + channel_id: msg.packet.chan_on_a.clone(), + sequence: msg.packet.sequence, + }; + ctx_a.delete_packet_commitment(commitment_path)?; + + if let Order::Ordered = chan_end_on_a.ordering { + // Note: in validation, we verified that `msg.packet.sequence == nextSeqRecv` + // (where `nextSeqRecv` is the value in the store) + ctx_a + .store_next_sequence_ack(port_chan_id_on_a, msg.packet.sequence.increment())?; + } + } + + // emit events and logs + { + ctx_a.log_message("success: packet acknowledgement".to_string()); + + // Note: Acknowledgement event was emitted at the beginning + + for module_event in extras.events { + ctx_a.emit_ibc_event(IbcEvent::AppModule(module_event)); + } + + for log_message in extras.log { + ctx_a.log_message(log_message); + } + } + + Ok(()) + } + enum TimeoutMsgType { Timeout(MsgTimeout), TimeoutOnClose(MsgTimeoutOnClose), From e1beb4c6c2ceda869d5a268ca292698e5c7af01c Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 1 Feb 2023 17:54:49 -0500 Subject: [PATCH 10/17] fix on_timeout_validate --- crates/ibc/src/core/context.rs | 6 +++--- crates/ibc/src/core/ics26_routing/context.rs | 2 +- crates/ibc/src/mock/context.rs | 14 ++++---------- crates/ibc/src/test_utils.rs | 4 ++-- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/crates/ibc/src/core/context.rs b/crates/ibc/src/core/context.rs index 5a1395fd0..ffde7e8fb 100644 --- a/crates/ibc/src/core/context.rs +++ b/crates/ibc/src/core/context.rs @@ -1431,9 +1431,9 @@ mod val_exec_ctx { TimeoutMsgType::TimeoutOnClose(msg) => (msg.packet, msg.signer), }; - let (_, cb_result) = module.on_timeout_packet_validate(&packet, &signer); - - cb_result.map_err(ContextError::PacketError) + module + .on_timeout_packet_validate(&packet, &signer) + .map_err(ContextError::PacketError) } fn timeout_packet_execute( diff --git a/crates/ibc/src/core/ics26_routing/context.rs b/crates/ibc/src/core/ics26_routing/context.rs index 711ee3452..7a085e81e 100644 --- a/crates/ibc/src/core/ics26_routing/context.rs +++ b/crates/ibc/src/core/ics26_routing/context.rs @@ -316,7 +316,7 @@ pub trait Module: Send + Sync + AsAnyMut + Debug { &self, packet: &Packet, relayer: &Signer, - ) -> (ModuleExtras, Result<(), PacketError>); + ) -> Result<(), PacketError>; /// Note: `MsgTimeout` and `MsgTimeoutOnClose` use the same callback #[cfg(feature = "val_exec_ctx")] diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index ebf47a8e1..681d15571 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -1996,11 +1996,8 @@ mod tests { &self, _packet: &Packet, _relayer: &Signer, - ) -> ( - ModuleExtras, - Result<(), crate::core::ics04_channel::error::PacketError>, - ) { - (ModuleExtras::empty(), Ok(())) + ) -> Result<(), crate::core::ics04_channel::error::PacketError> { + Ok(()) } #[cfg(feature = "val_exec_ctx")] @@ -2144,11 +2141,8 @@ mod tests { &self, _packet: &Packet, _relayer: &Signer, - ) -> ( - ModuleExtras, - Result<(), crate::core::ics04_channel::error::PacketError>, - ) { - (ModuleExtras::empty(), Ok(())) + ) -> Result<(), crate::core::ics04_channel::error::PacketError> { + Ok(()) } #[cfg(feature = "val_exec_ctx")] diff --git a/crates/ibc/src/test_utils.rs b/crates/ibc/src/test_utils.rs index f0658ffaa..022ad0ab0 100644 --- a/crates/ibc/src/test_utils.rs +++ b/crates/ibc/src/test_utils.rs @@ -192,8 +192,8 @@ impl Module for DummyTransferModule { &self, _packet: &Packet, _relayer: &Signer, - ) -> (ModuleExtras, Result<(), PacketError>) { - (ModuleExtras::empty(), Ok(())) + ) -> Result<(), PacketError> { + Ok(()) } #[cfg(feature = "val_exec_ctx")] From ed71e759010c80320e16ed9a66e7fd109cfecb39 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 1 Feb 2023 18:08:05 -0500 Subject: [PATCH 11/17] ics20 new ack cbs --- .../ibc/src/applications/transfer/context.rs | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/crates/ibc/src/applications/transfer/context.rs b/crates/ibc/src/applications/transfer/context.rs index c1acae891..0bed77666 100644 --- a/crates/ibc/src/applications/transfer/context.rs +++ b/crates/ibc/src/applications/transfer/context.rs @@ -286,6 +286,70 @@ pub fn on_recv_packet( ack.into() } +#[cfg(feature = "val_exec_ctx")] +pub fn on_acknowledgement_packet_validate( + _ctx: &impl TokenTransferContext, + packet: &Packet, + acknowledgement: &Acknowledgement, + _relayer: &Signer, +) -> Result<(), TokenTransferError> { + let _data = serde_json::from_slice::(&packet.data) + .map_err(|_| TokenTransferError::PacketDataDeserialization)?; + + let _acknowledgement = + serde_json::from_slice::(acknowledgement.as_ref()) + .map_err(|_| TokenTransferError::AckDeserialization)?; + + Ok(()) +} + +#[cfg(feature = "val_exec_ctx")] +pub fn on_acknowledgement_packet_execute( + ctx: &mut impl TokenTransferContext, + packet: &Packet, + acknowledgement: &Acknowledgement, + _relayer: &Signer, +) -> (ModuleExtras, Result<(), TokenTransferError>) { + let data = match serde_json::from_slice::(&packet.data) { + Ok(data) => data, + Err(_) => { + return ( + ModuleExtras::empty(), + Err(TokenTransferError::PacketDataDeserialization), + ); + } + }; + + let acknowledgement = + match serde_json::from_slice::(acknowledgement.as_ref()) { + Ok(ack) => ack, + Err(_) => { + return ( + ModuleExtras::empty(), + Err(TokenTransferError::AckDeserialization), + ); + } + }; + + if let Err(err) = process_ack_packet(ctx, packet, &data, &acknowledgement) { + return (ModuleExtras::empty(), Err(err)); + } + + let ack_event = AckEvent { + receiver: data.receiver, + denom: data.token.denom, + amount: data.token.amount, + acknowledgement: acknowledgement.clone(), + }; + + let extras = ModuleExtras { + events: vec![ack_event.into(), AckStatusEvent { acknowledgement }.into()], + log: Vec::new(), + }; + + (extras, Ok(())) +} + pub fn on_acknowledgement_packet( ctx: &mut impl TokenTransferContext, output: &mut ModuleOutputBuilder, From 3e139844c587ec96a7ca74f24128258b9880f2fb Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 1 Feb 2023 18:16:46 -0500 Subject: [PATCH 12/17] ics-20 new timeout cb --- .../ibc/src/applications/transfer/context.rs | 175 +++++++++++------- 1 file changed, 111 insertions(+), 64 deletions(-) diff --git a/crates/ibc/src/applications/transfer/context.rs b/crates/ibc/src/applications/transfer/context.rs index 0bed77666..a0dfa4cbc 100644 --- a/crates/ibc/src/applications/transfer/context.rs +++ b/crates/ibc/src/applications/transfer/context.rs @@ -286,70 +286,6 @@ pub fn on_recv_packet( ack.into() } -#[cfg(feature = "val_exec_ctx")] -pub fn on_acknowledgement_packet_validate( - _ctx: &impl TokenTransferContext, - packet: &Packet, - acknowledgement: &Acknowledgement, - _relayer: &Signer, -) -> Result<(), TokenTransferError> { - let _data = serde_json::from_slice::(&packet.data) - .map_err(|_| TokenTransferError::PacketDataDeserialization)?; - - let _acknowledgement = - serde_json::from_slice::(acknowledgement.as_ref()) - .map_err(|_| TokenTransferError::AckDeserialization)?; - - Ok(()) -} - -#[cfg(feature = "val_exec_ctx")] -pub fn on_acknowledgement_packet_execute( - ctx: &mut impl TokenTransferContext, - packet: &Packet, - acknowledgement: &Acknowledgement, - _relayer: &Signer, -) -> (ModuleExtras, Result<(), TokenTransferError>) { - let data = match serde_json::from_slice::(&packet.data) { - Ok(data) => data, - Err(_) => { - return ( - ModuleExtras::empty(), - Err(TokenTransferError::PacketDataDeserialization), - ); - } - }; - - let acknowledgement = - match serde_json::from_slice::(acknowledgement.as_ref()) { - Ok(ack) => ack, - Err(_) => { - return ( - ModuleExtras::empty(), - Err(TokenTransferError::AckDeserialization), - ); - } - }; - - if let Err(err) = process_ack_packet(ctx, packet, &data, &acknowledgement) { - return (ModuleExtras::empty(), Err(err)); - } - - let ack_event = AckEvent { - receiver: data.receiver, - denom: data.token.denom, - amount: data.token.amount, - acknowledgement: acknowledgement.clone(), - }; - - let extras = ModuleExtras { - events: vec![ack_event.into(), AckStatusEvent { acknowledgement }.into()], - log: Vec::new(), - }; - - (extras, Ok(())) -} - pub fn on_acknowledgement_packet( ctx: &mut impl TokenTransferContext, output: &mut ModuleOutputBuilder, @@ -594,6 +530,117 @@ mod val_exec_ctx { (extras, ack.into()) } + + pub fn on_acknowledgement_packet_validate( + _ctx: &impl TokenTransferContext, + packet: &Packet, + acknowledgement: &Acknowledgement, + _relayer: &Signer, + ) -> Result<(), TokenTransferError> { + let _data = serde_json::from_slice::(&packet.data) + .map_err(|_| TokenTransferError::PacketDataDeserialization)?; + + let _acknowledgement = + serde_json::from_slice::(acknowledgement.as_ref()) + .map_err(|_| TokenTransferError::AckDeserialization)?; + + // TODO: validate `refund_packet_token` + + Ok(()) + } + + pub fn on_acknowledgement_packet_execute( + ctx: &mut impl TokenTransferContext, + packet: &Packet, + acknowledgement: &Acknowledgement, + _relayer: &Signer, + ) -> (ModuleExtras, Result<(), TokenTransferError>) { + let data = match serde_json::from_slice::(&packet.data) { + Ok(data) => data, + Err(_) => { + return ( + ModuleExtras::empty(), + Err(TokenTransferError::PacketDataDeserialization), + ); + } + }; + + let acknowledgement = match serde_json::from_slice::( + acknowledgement.as_ref(), + ) { + Ok(ack) => ack, + Err(_) => { + return ( + ModuleExtras::empty(), + Err(TokenTransferError::AckDeserialization), + ); + } + }; + + if let Err(err) = process_ack_packet(ctx, packet, &data, &acknowledgement) { + return (ModuleExtras::empty(), Err(err)); + } + + let ack_event = AckEvent { + receiver: data.receiver, + denom: data.token.denom, + amount: data.token.amount, + acknowledgement: acknowledgement.clone(), + }; + + let extras = ModuleExtras { + events: vec![ack_event.into(), AckStatusEvent { acknowledgement }.into()], + log: Vec::new(), + }; + + (extras, Ok(())) + } + + pub fn on_timeout_packet_validate( + _ctx: &impl TokenTransferContext, + packet: &Packet, + _relayer: &Signer, + ) -> Result<(), TokenTransferError> { + let _data = serde_json::from_slice::(&packet.data) + .map_err(|_| TokenTransferError::PacketDataDeserialization)?; + + // TODO: validate `refund_packet_token` + + Ok(()) + } + + pub fn on_timeout_packet_execute( + ctx: &mut impl TokenTransferContext, + packet: &Packet, + _relayer: &Signer, + ) -> (ModuleExtras, Result<(), TokenTransferError>) { + let data = match serde_json::from_slice::(&packet.data) { + Ok(data) => data, + Err(_) => { + return ( + ModuleExtras::empty(), + Err(TokenTransferError::PacketDataDeserialization), + ); + } + }; + + if let Err(err) = process_timeout_packet(ctx, packet, &data) { + return (ModuleExtras::empty(), Err(err)); + } + + let timeout_event = TimeoutEvent { + refund_receiver: data.sender, + refund_denom: data.token.denom, + refund_amount: data.token.amount, + }; + + let extras = ModuleExtras { + events: vec![timeout_event.into()], + log: Vec::new(), + }; + + (extras, Ok(())) + } } #[cfg(test)] From 69d14e52857e6a23bda46a677bf9c660f80940ec Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 8 Feb 2023 11:14:31 -0500 Subject: [PATCH 13/17] PacketError --- crates/ibc/src/mock/context.rs | 30 +++++++++--------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index 681d15571..988f4850e 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -1720,6 +1720,7 @@ mod val_exec_ctx { #[cfg(test)] mod tests { + use super::*; use test_log::test; use alloc::str::FromStr; @@ -1738,7 +1739,6 @@ mod tests { use crate::mock::context::MockContext; use crate::mock::context::MockRouterBuilder; use crate::mock::host::HostType; - use crate::prelude::*; use crate::signer::Signer; use crate::test_utils::get_dummy_bech32_account; use crate::Height; @@ -1996,7 +1996,7 @@ mod tests { &self, _packet: &Packet, _relayer: &Signer, - ) -> Result<(), crate::core::ics04_channel::error::PacketError> { + ) -> Result<(), PacketError> { Ok(()) } @@ -2005,10 +2005,7 @@ mod tests { &mut self, _packet: &Packet, _relayer: &Signer, - ) -> ( - ModuleExtras, - Result<(), crate::core::ics04_channel::error::PacketError>, - ) { + ) -> (ModuleExtras, Result<(), PacketError>) { (ModuleExtras::empty(), Ok(())) } @@ -2018,7 +2015,7 @@ mod tests { _packet: &Packet, _acknowledgement: &Acknowledgement, _relayer: &Signer, - ) -> Result<(), crate::core::ics04_channel::error::PacketError> { + ) -> Result<(), PacketError> { Ok(()) } @@ -2028,10 +2025,7 @@ mod tests { _packet: &Packet, _acknowledgement: &Acknowledgement, _relayer: &Signer, - ) -> ( - ModuleExtras, - Result<(), crate::core::ics04_channel::error::PacketError>, - ) { + ) -> (ModuleExtras, Result<(), PacketError>) { (ModuleExtras::empty(), Ok(())) } } @@ -2141,7 +2135,7 @@ mod tests { &self, _packet: &Packet, _relayer: &Signer, - ) -> Result<(), crate::core::ics04_channel::error::PacketError> { + ) -> Result<(), PacketError> { Ok(()) } @@ -2150,10 +2144,7 @@ mod tests { &mut self, _packet: &Packet, _relayer: &Signer, - ) -> ( - ModuleExtras, - Result<(), crate::core::ics04_channel::error::PacketError>, - ) { + ) -> (ModuleExtras, Result<(), PacketError>) { (ModuleExtras::empty(), Ok(())) } @@ -2163,7 +2154,7 @@ mod tests { _packet: &Packet, _acknowledgement: &Acknowledgement, _relayer: &Signer, - ) -> Result<(), crate::core::ics04_channel::error::PacketError> { + ) -> Result<(), PacketError> { Ok(()) } @@ -2173,10 +2164,7 @@ mod tests { _packet: &Packet, _acknowledgement: &Acknowledgement, _relayer: &Signer, - ) -> ( - ModuleExtras, - Result<(), crate::core::ics04_channel::error::PacketError>, - ) { + ) -> (ModuleExtras, Result<(), PacketError>) { (ModuleExtras::empty(), Ok(())) } } From 51ddc415db14241790459208f74451628491ee6c Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 8 Feb 2023 11:18:48 -0500 Subject: [PATCH 14/17] ack: move ordered check --- .../ics04_channel/handler/acknowledgement.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs index 847c2c0dc..9f919c53a 100644 --- a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs @@ -83,6 +83,18 @@ pub(crate) mod val_exec_ctx { .into()); } + if let Order::Ordered = chan_end_on_a.ordering { + let next_seq_ack = ctx_a.get_next_sequence_ack(port_chan_id_on_a)?; + + if packet.sequence != next_seq_ack { + return Err(PacketError::InvalidPacketSequence { + given_sequence: packet.sequence, + next_sequence: next_seq_ack, + } + .into()); + } + } + // Verify proofs { let client_id_on_a = conn_end_on_a.client_id(); @@ -120,18 +132,6 @@ pub(crate) mod val_exec_ctx { .map_err(PacketError::Channel)?; } - if let Order::Ordered = chan_end_on_a.ordering { - let next_seq_ack = ctx_a.get_next_sequence_ack(port_chan_id_on_a)?; - - if packet.sequence != next_seq_ack { - return Err(PacketError::InvalidPacketSequence { - given_sequence: packet.sequence, - next_sequence: next_seq_ack, - } - .into()); - } - } - Ok(()) } } From 8915502598f65dba07dabfa5ba1508e2a5b1e94f Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 8 Feb 2023 13:40:17 -0500 Subject: [PATCH 15/17] ics-20 ack packet validate --- .../ibc/src/applications/transfer/context.rs | 33 ++++++++++++------- crates/ibc/src/applications/transfer/relay.rs | 22 +++++++++++-- .../transfer/relay/on_ack_packet.rs | 19 ----------- 3 files changed, 42 insertions(+), 32 deletions(-) delete mode 100644 crates/ibc/src/applications/transfer/relay/on_ack_packet.rs diff --git a/crates/ibc/src/applications/transfer/context.rs b/crates/ibc/src/applications/transfer/context.rs index a0dfa4cbc..fb6d1383a 100644 --- a/crates/ibc/src/applications/transfer/context.rs +++ b/crates/ibc/src/applications/transfer/context.rs @@ -4,9 +4,9 @@ use super::error::TokenTransferError; use crate::applications::transfer::acknowledgement::TokenTransferAcknowledgement; use crate::applications::transfer::events::{AckEvent, AckStatusEvent, RecvEvent, TimeoutEvent}; use crate::applications::transfer::packet::PacketData; -use crate::applications::transfer::relay::on_ack_packet::process_ack_packet; 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::relay::refund_packet_token; use crate::applications::transfer::{PrefixedCoin, PrefixedDenom, VERSION}; use crate::core::ics04_channel::channel::{Counterparty, Order}; use crate::core::ics04_channel::commitment::PacketCommitment; @@ -300,7 +300,9 @@ pub fn on_acknowledgement_packet( serde_json::from_slice::(acknowledgement.as_ref()) .map_err(|_| TokenTransferError::AckDeserialization)?; - process_ack_packet(ctx, packet, &data, &acknowledgement)?; + if !acknowledgement.is_successful() { + refund_packet_token(ctx, packet, &data)?; + } let ack_event = AckEvent { receiver: data.receiver, @@ -339,7 +341,9 @@ 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; + use crate::applications::transfer::relay::{ + on_recv_packet::process_recv_packet_execute, refund_packet_token_validate, + }; pub use super::*; @@ -531,20 +535,25 @@ mod val_exec_ctx { (extras, ack.into()) } - pub fn on_acknowledgement_packet_validate( - _ctx: &impl TokenTransferContext, + pub fn on_acknowledgement_packet_validate( + _ctx: &Ctx, packet: &Packet, acknowledgement: &Acknowledgement, _relayer: &Signer, - ) -> Result<(), TokenTransferError> { - let _data = serde_json::from_slice::(&packet.data) + ) -> Result<(), TokenTransferError> + where + Ctx: TokenTransferContext, + { + let data = serde_json::from_slice::(&packet.data) .map_err(|_| TokenTransferError::PacketDataDeserialization)?; - let _acknowledgement = + let acknowledgement = serde_json::from_slice::(acknowledgement.as_ref()) .map_err(|_| TokenTransferError::AckDeserialization)?; - // TODO: validate `refund_packet_token` + if !acknowledgement.is_successful() { + refund_packet_token_validate::(&data)?; + } Ok(()) } @@ -577,8 +586,10 @@ mod val_exec_ctx { } }; - if let Err(err) = process_ack_packet(ctx, packet, &data, &acknowledgement) { - return (ModuleExtras::empty(), Err(err)); + if !acknowledgement.is_successful() { + if let Err(err) = refund_packet_token(ctx, packet, &data) { + return (ModuleExtras::empty(), Err(err)); + } } let ack_event = AckEvent { diff --git a/crates/ibc/src/applications/transfer/relay.rs b/crates/ibc/src/applications/transfer/relay.rs index d27a2fec9..6aadc5f3b 100644 --- a/crates/ibc/src/applications/transfer/relay.rs +++ b/crates/ibc/src/applications/transfer/relay.rs @@ -6,12 +6,11 @@ use crate::applications::transfer::packet::PacketData; use crate::core::ics04_channel::packet::Packet; use crate::prelude::*; -pub mod on_ack_packet; pub mod on_recv_packet; pub mod on_timeout_packet; pub mod send_transfer; -fn refund_packet_token( +pub fn refund_packet_token( ctx: &mut impl TokenTransferContext, packet: &Packet, data: &PacketData, @@ -38,3 +37,22 @@ fn refund_packet_token( ctx.mint_coins(&sender, &data.token) } } + +#[cfg(feature = "val_exec_ctx")] +pub use val_exec_ctx::*; +#[cfg(feature = "val_exec_ctx")] +mod val_exec_ctx { + use super::*; + + pub fn refund_packet_token_validate( + data: &PacketData, + ) -> Result<(), TokenTransferError> { + let _sender: ::AccountId = data + .sender + .clone() + .try_into() + .map_err(|_| TokenTransferError::ParseAccountFailure)?; + + Ok(()) + } +} diff --git a/crates/ibc/src/applications/transfer/relay/on_ack_packet.rs b/crates/ibc/src/applications/transfer/relay/on_ack_packet.rs deleted file mode 100644 index 32779a871..000000000 --- a/crates/ibc/src/applications/transfer/relay/on_ack_packet.rs +++ /dev/null @@ -1,19 +0,0 @@ -use crate::applications::transfer::acknowledgement::TokenTransferAcknowledgement; -use crate::applications::transfer::context::TokenTransferContext; -use crate::applications::transfer::error::TokenTransferError; -use crate::applications::transfer::packet::PacketData; -use crate::applications::transfer::relay::refund_packet_token; -use crate::core::ics04_channel::packet::Packet; - -pub fn process_ack_packet( - ctx: &mut impl TokenTransferContext, - packet: &Packet, - data: &PacketData, - ack: &TokenTransferAcknowledgement, -) -> Result<(), TokenTransferError> { - if !ack.is_successful() { - refund_packet_token(ctx, packet, data)?; - } - - Ok(()) -} From e39aaa9054040bc9701a8480a960c15df7888a26 Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 8 Feb 2023 13:44:44 -0500 Subject: [PATCH 16/17] ics-20 refund packet validate --- .../ibc/src/applications/transfer/context.rs | 18 ++++++++++-------- crates/ibc/src/applications/transfer/relay.rs | 1 - .../transfer/relay/on_timeout_packet.rs | 13 ------------- 3 files changed, 10 insertions(+), 22 deletions(-) delete mode 100644 crates/ibc/src/applications/transfer/relay/on_timeout_packet.rs diff --git a/crates/ibc/src/applications/transfer/context.rs b/crates/ibc/src/applications/transfer/context.rs index fb6d1383a..d26667964 100644 --- a/crates/ibc/src/applications/transfer/context.rs +++ b/crates/ibc/src/applications/transfer/context.rs @@ -5,7 +5,6 @@ use crate::applications::transfer::acknowledgement::TokenTransferAcknowledgement use crate::applications::transfer::events::{AckEvent, AckStatusEvent, RecvEvent, TimeoutEvent}; use crate::applications::transfer::packet::PacketData; 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::relay::refund_packet_token; use crate::applications::transfer::{PrefixedCoin, PrefixedDenom, VERSION}; use crate::core::ics04_channel::channel::{Counterparty, Order}; @@ -325,7 +324,7 @@ pub fn on_timeout_packet( let data = serde_json::from_slice::(&packet.data) .map_err(|_| TokenTransferError::PacketDataDeserialization)?; - process_timeout_packet(ctx, packet, &data)?; + refund_packet_token(ctx, packet, &data)?; let timeout_event = TimeoutEvent { refund_receiver: data.sender, @@ -607,15 +606,18 @@ mod val_exec_ctx { (extras, Ok(())) } - pub fn on_timeout_packet_validate( - _ctx: &impl TokenTransferContext, + pub fn on_timeout_packet_validate( + _ctx: &Ctx, packet: &Packet, _relayer: &Signer, - ) -> Result<(), TokenTransferError> { - let _data = serde_json::from_slice::(&packet.data) + ) -> Result<(), TokenTransferError> + where + Ctx: TokenTransferContext, + { + let data = serde_json::from_slice::(&packet.data) .map_err(|_| TokenTransferError::PacketDataDeserialization)?; - // TODO: validate `refund_packet_token` + refund_packet_token_validate::(&data)?; Ok(()) } @@ -635,7 +637,7 @@ mod val_exec_ctx { } }; - if let Err(err) = process_timeout_packet(ctx, packet, &data) { + if let Err(err) = refund_packet_token(ctx, packet, &data) { return (ModuleExtras::empty(), Err(err)); } diff --git a/crates/ibc/src/applications/transfer/relay.rs b/crates/ibc/src/applications/transfer/relay.rs index 6aadc5f3b..10d3d027f 100644 --- a/crates/ibc/src/applications/transfer/relay.rs +++ b/crates/ibc/src/applications/transfer/relay.rs @@ -7,7 +7,6 @@ use crate::core::ics04_channel::packet::Packet; use crate::prelude::*; pub mod on_recv_packet; -pub mod on_timeout_packet; pub mod send_transfer; pub fn refund_packet_token( diff --git a/crates/ibc/src/applications/transfer/relay/on_timeout_packet.rs b/crates/ibc/src/applications/transfer/relay/on_timeout_packet.rs deleted file mode 100644 index 0f54016a4..000000000 --- a/crates/ibc/src/applications/transfer/relay/on_timeout_packet.rs +++ /dev/null @@ -1,13 +0,0 @@ -use crate::applications::transfer::context::TokenTransferContext; -use crate::applications::transfer::error::TokenTransferError; -use crate::applications::transfer::packet::PacketData; -use crate::applications::transfer::relay::refund_packet_token; -use crate::core::ics04_channel::packet::Packet; - -pub fn process_timeout_packet( - ctx: &mut impl TokenTransferContext, - packet: &Packet, - data: &PacketData, -) -> Result<(), TokenTransferError> { - refund_packet_token(ctx, packet, data) -} From 6ab6f1cc7f6880f7b33f3efec1c49433139dd84b Mon Sep 17 00:00:00 2001 From: Philippe Laferriere Date: Wed, 8 Feb 2023 13:51:09 -0500 Subject: [PATCH 17/17] changelog --- .../feature/393-finish-validation-execution-context.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/feature/393-finish-validation-execution-context.md diff --git a/.changelog/unreleased/feature/393-finish-validation-execution-context.md b/.changelog/unreleased/feature/393-finish-validation-execution-context.md new file mode 100644 index 000000000..a9981e46b --- /dev/null +++ b/.changelog/unreleased/feature/393-finish-validation-execution-context.md @@ -0,0 +1,3 @@ +- Finish implementing ValidationContext::validate() and + ExecutionContext::execute() ([#393](https://github.com/cosmos/ibc- + rs/issues/393)) \ No newline at end of file