From 05fe2221688dceea2b69ebd8fa6b1e3bf6d7c53f Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Mon, 8 Jan 2024 14:24:36 +0100 Subject: [PATCH 01/25] New configuration for max memo and receiver size --- crates/relayer/src/config.rs | 16 ++++++++++++++++ crates/relayer/src/config/types.rs | 4 ++++ 2 files changed, 20 insertions(+) diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index 74f1e491d3..b75585d0e5 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -230,6 +230,14 @@ pub mod default { buckets: 10, } } + + pub fn max_memo_size() -> MaxTxSize { + MaxTxSize::unsafe_new(180000) + } + + pub fn max_receiver_size() -> MaxTxSize { + MaxTxSize::unsafe_new(180000) + } } #[derive(Clone, Debug, Default, Deserialize, Serialize)] @@ -400,6 +408,10 @@ pub struct Packets { pub tx_confirmation: bool, #[serde(default = "default::auto_register_counterparty_payee")] pub auto_register_counterparty_payee: bool, + #[serde(default = "default::max_memo_size")] + pub max_memo_size: MaxTxSize, + #[serde(default = "default::max_receiver_size")] + pub max_receiver_size: MaxTxSize, } impl Default for Packets { @@ -410,6 +422,8 @@ impl Default for Packets { clear_on_start: default::clear_on_start(), tx_confirmation: default::tx_confirmation(), auto_register_counterparty_payee: default::auto_register_counterparty_payee(), + max_memo_size: default::max_memo_size(), + max_receiver_size: default::max_receiver_size(), } } } @@ -742,6 +756,8 @@ impl> From> for Diagnostic { } use crate::chain::cosmos::config::error::Error as CosmosConfigError; + +use self::types::MaxTxSize; impl From for Error { fn from(error: CosmosConfigError) -> Error { Error::cosmos_config_error(error.to_string()) diff --git a/crates/relayer/src/config/types.rs b/crates/relayer/src/config/types.rs index c79e0bd2ad..5b14b28a9d 100644 --- a/crates/relayer/src/config/types.rs +++ b/crates/relayer/src/config/types.rs @@ -125,6 +125,10 @@ pub mod max_tx_size { Ok(Self(value)) } + pub fn unsafe_new(value: usize) -> Self { + Self(value) + } + pub fn max() -> Self { Self(Self::MAX_BOUND) } From b7b90966f88876b714065e93c3c61e00e83ce183 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Mon, 8 Jan 2024 14:25:51 +0100 Subject: [PATCH 02/25] Do not relay packets which have memo or receiver field too big --- crates/relayer-cli/src/commands/clear.rs | 2 ++ crates/relayer-cli/src/commands/tx/packet.rs | 4 +++ .../src/core/ics04_channel/error.rs | 5 +++ .../src/core/ics04_channel/packet.rs | 35 +++++++++++++++++++ crates/relayer/src/link.rs | 20 +++++++++-- crates/relayer/src/link/relay_path.rs | 23 +++++++++++- crates/relayer/src/worker.rs | 2 ++ .../src/tests/execute_schedule.rs | 5 ++- .../src/tests/ordered_channel_clear.rs | 12 +++++-- .../src/tests/query_packet.rs | 5 ++- 10 files changed, 106 insertions(+), 7 deletions(-) diff --git a/crates/relayer-cli/src/commands/clear.rs b/crates/relayer-cli/src/commands/clear.rs index 9e3b9c06a4..6fba08ee6b 100644 --- a/crates/relayer-cli/src/commands/clear.rs +++ b/crates/relayer-cli/src/commands/clear.rs @@ -135,6 +135,8 @@ impl Runnable for ClearPacketsCmd { let opts = LinkParameters { src_port_id: self.port_id.clone(), src_channel_id: self.channel_id.clone(), + max_memo_size: config.mode.packets.max_memo_size.to_usize(), + max_receiver_size: config.mode.packets.max_receiver_size.to_usize(), }; let fwd_link = match Link::new_from_opts(chains.src.clone(), chains.dst, opts, false, false) diff --git a/crates/relayer-cli/src/commands/tx/packet.rs b/crates/relayer-cli/src/commands/tx/packet.rs index 7313a46a3e..bf0ed2aa7b 100644 --- a/crates/relayer-cli/src/commands/tx/packet.rs +++ b/crates/relayer-cli/src/commands/tx/packet.rs @@ -70,6 +70,8 @@ impl Runnable for TxPacketRecvCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), + max_memo_size: config.mode.packets.max_memo_size.to_usize(), + max_receiver_size: config.mode.packets.max_receiver_size.to_usize(), }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) { Ok(link) => link, @@ -151,6 +153,8 @@ impl Runnable for TxPacketAckCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), + max_memo_size: config.mode.packets.max_memo_size.to_usize(), + max_receiver_size: config.mode.packets.max_receiver_size.to_usize(), }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) { Ok(link) => link, diff --git a/crates/relayer-types/src/core/ics04_channel/error.rs b/crates/relayer-types/src/core/ics04_channel/error.rs index 817f0337f8..4a62d96755 100644 --- a/crates/relayer-types/src/core/ics04_channel/error.rs +++ b/crates/relayer-types/src/core/ics04_channel/error.rs @@ -12,11 +12,16 @@ use crate::timestamp::Timestamp; use crate::Height; use flex_error::{define_error, TraceError}; +use prost::DecodeError; use tendermint_proto::Error as TendermintError; define_error! { #[derive(Debug, PartialEq, Eq)] Error { + Decode + [ TraceError ] + | _ | { "error decoding byte array" }, + Ics03Connection [ connection_error::Error ] | _ | { "ics03 connection error" }, diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index bd1630c1a3..034aee75b6 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -2,9 +2,11 @@ use std::str::FromStr; use serde_derive::{Deserialize, Serialize}; +use ibc_proto::ibc::applications::transfer::v2::FungibleTokenPacketData as RawPacketData; use ibc_proto::ibc::core::channel::v1::Packet as RawPacket; use super::timeout::TimeoutHeight; +use crate::applications::transfer::packet::PacketData as APacketData; use crate::core::ics04_channel::error::Error; use crate::core::ics24_host::identifier::{ChannelId, PortId}; use crate::timestamp::{Expiry::Expired, Timestamp}; @@ -178,6 +180,39 @@ impl Packet { height_timed_out || timestamp_timed_out } + + pub fn validate_fields( + &self, + max_memo_size: usize, + max_receiver_size: usize, + ) -> Result<(usize, usize), Error> { + let any_packet: RawPacketData = match serde_json::from_slice(&self.data) { + Ok(v) => v, + Err(e) => { + return Err(Error::app_module(e.to_string())); + } + }; + let packet: APacketData = match any_packet.try_into() { + Ok(p) => p, + Err(e) => { + return Err(Error::app_module(e.to_string())); + } + }; + let memo_size = if let Some(memo) = &packet.memo { + if memo.len() > max_memo_size { + memo.len() + } else { + 0 + } + } else { + 0 + }; + if packet.receiver.to_string().len() > max_receiver_size { + Ok((memo_size, packet.receiver.to_string().len())) + } else { + Ok((memo_size, 0)) + } + } } /// Custom debug output to omit the packet data diff --git a/crates/relayer/src/link.rs b/crates/relayer/src/link.rs index 5e9716aee4..a387679022 100644 --- a/crates/relayer/src/link.rs +++ b/crates/relayer/src/link.rs @@ -33,6 +33,8 @@ pub use relay_path::{RelayPath, Resubmit}; pub struct LinkParameters { pub src_port_id: PortId, pub src_channel_id: ChannelId, + pub max_memo_size: usize, + pub max_receiver_size: usize, } pub struct Link { @@ -43,9 +45,16 @@ impl Link { pub fn new( channel: Channel, with_tx_confirmation: bool, + max_memo_size: usize, + max_receiver_size: usize, ) -> Result { Ok(Self { - a_to_b: RelayPath::new(channel, with_tx_confirmation)?, + a_to_b: RelayPath::new( + channel, + with_tx_confirmation, + max_memo_size, + max_receiver_size, + )?, }) } @@ -169,7 +178,12 @@ impl Link { .map_err(LinkError::relayer)?; } - Link::new(channel, with_tx_confirmation) + Link::new( + channel, + with_tx_confirmation, + opts.max_memo_size, + opts.max_receiver_size, + ) } /// Constructs a link around the channel that is reverse to the channel @@ -182,6 +196,8 @@ impl Link { let opts = LinkParameters { src_port_id: self.a_to_b.dst_port_id().clone(), src_channel_id: self.a_to_b.dst_channel_id().clone(), + max_memo_size: self.a_to_b.max_memo_size, + max_receiver_size: self.a_to_b.max_receiver_size, }; let chain_b = self.a_to_b.dst_chain().clone(); let chain_a = self.a_to_b.src_chain().clone(); diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 35abb01258..0436890ade 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -108,12 +108,17 @@ pub struct RelayPath { // transactions if [`confirm_txes`] is true. pending_txs_src: PendingTxs, pending_txs_dst: PendingTxs, + + pub max_memo_size: usize, + pub max_receiver_size: usize, } impl RelayPath { pub fn new( channel: Channel, with_tx_confirmation: bool, + max_memo_size: usize, + max_receiver_size: usize, ) -> Result { let src_chain = channel.src_chain().clone(); let dst_chain = channel.dst_chain().clone(); @@ -152,6 +157,9 @@ impl RelayPath { confirm_txes: with_tx_confirmation, pending_txs_src: PendingTxs::new(src_chain, src_channel_id, src_port_id, dst_chain_id), pending_txs_dst: PendingTxs::new(dst_chain, dst_channel_id, dst_port_id, src_chain_id), + + max_memo_size, + max_receiver_size, }) } @@ -1385,7 +1393,20 @@ impl RelayPath { if timeout.is_some() { Ok((None, timeout)) } else { - Ok((self.build_recv_packet(&event.packet, height)?, None)) + match event + .packet + .validate_fields(self.max_memo_size, self.max_receiver_size) + { + Ok((0, 0)) => Ok((self.build_recv_packet(&event.packet, height)?, None)), + Ok((memo_size, receiver_size)) => { + warn!("memo and/or receiver fields were bigger than maximum allowed. Memo field {memo_size}, maximum allowed {}. Receiver field {receiver_size}, maximum allowed {}", self.max_memo_size, self.max_receiver_size); + Ok((None, None)) + } + Err(e) => { + warn!("error validating fields, will simply relay message: {e}"); + Ok((self.build_recv_packet(&event.packet, height)?, None)) + } + } } } diff --git a/crates/relayer/src/worker.rs b/crates/relayer/src/worker.rs index 915c49ce66..fb02a38413 100644 --- a/crates/relayer/src/worker.rs +++ b/crates/relayer/src/worker.rs @@ -117,6 +117,8 @@ pub fn spawn_worker_tasks( LinkParameters { src_port_id: path.src_port_id.clone(), src_channel_id: path.src_channel_id.clone(), + max_memo_size: packets_config.max_memo_size.to_usize(), + max_receiver_size: packets_config.max_receiver_size.to_usize(), }, packets_config.tx_confirmation, packets_config.auto_register_counterparty_payee, diff --git a/tools/integration-test/src/tests/execute_schedule.rs b/tools/integration-test/src/tests/execute_schedule.rs index 1dd6b5262a..d013669b5d 100644 --- a/tools/integration-test/src/tests/execute_schedule.rs +++ b/tools/integration-test/src/tests/execute_schedule.rs @@ -37,15 +37,18 @@ impl BinaryChannelTest for ExecuteScheduleTest { fn run( &self, _config: &TestConfig, - _relayer: RelayerDriver, + relayer: RelayerDriver, chains: ConnectedChains, channel: ConnectedChannel, ) -> Result<(), Error> { let amount1 = random_u128_range(1000, 5000); + let packet_config = relayer.config.mode.packets; let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), + max_memo_size: packet_config.max_memo_size.to_usize(), + max_receiver_size: packet_config.max_receiver_size.to_usize(), }; let chain_a_link = Link::new_from_opts( diff --git a/tools/integration-test/src/tests/ordered_channel_clear.rs b/tools/integration-test/src/tests/ordered_channel_clear.rs index 696a536a27..1555dfe5cd 100644 --- a/tools/integration-test/src/tests/ordered_channel_clear.rs +++ b/tools/integration-test/src/tests/ordered_channel_clear.rs @@ -78,11 +78,12 @@ impl BinaryChannelTest for OrderedChannelClearTest { fn run( &self, _config: &TestConfig, - _relayer: RelayerDriver, + relayer: RelayerDriver, chains: ConnectedChains, channel: ConnectedChannel, ) -> Result<(), Error> { let denom_a = chains.node_a.denom(); + let packet_config = relayer.config.mode.packets; let wallet_a = chains.node_a.wallets().user1().cloned(); let wallet_b = chains.node_b.wallets().user1().cloned(); @@ -118,6 +119,8 @@ impl BinaryChannelTest for OrderedChannelClearTest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), + max_memo_size: packet_config.max_memo_size.to_usize(), + max_receiver_size: packet_config.max_receiver_size.to_usize(), }; let chain_a_link = Link::new_from_opts( @@ -131,6 +134,8 @@ impl BinaryChannelTest for OrderedChannelClearTest { let chain_b_link_opts = LinkParameters { src_port_id: channel.port_b.clone().into_value(), src_channel_id: channel.channel_id_b.clone().into_value(), + max_memo_size: packet_config.max_memo_size.to_usize(), + max_receiver_size: packet_config.max_receiver_size.to_usize(), }; let chain_b_link = Link::new_from_opts( @@ -224,11 +229,12 @@ impl BinaryChannelTest for OrderedChannelClearEqualCLITest { fn run( &self, _config: &TestConfig, - _relayer: RelayerDriver, + relayer: RelayerDriver, chains: ConnectedChains, channel: ConnectedChannel, ) -> Result<(), Error> { let num_msgs = 5_usize; + let packet_config = relayer.config.mode.packets; info!( "Performing {} IBC transfers on an ordered channel", @@ -264,6 +270,8 @@ impl BinaryChannelTest for OrderedChannelClearEqualCLITest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.into_value(), + max_memo_size: packet_config.max_memo_size.to_usize(), + max_receiver_size: packet_config.max_receiver_size.to_usize(), }; let chain_a_link = Link::new_from_opts( diff --git a/tools/integration-test/src/tests/query_packet.rs b/tools/integration-test/src/tests/query_packet.rs index 6c97c871d0..efd84e8a68 100644 --- a/tools/integration-test/src/tests/query_packet.rs +++ b/tools/integration-test/src/tests/query_packet.rs @@ -30,11 +30,12 @@ impl BinaryChannelTest for QueryPacketPendingTest { fn run( &self, _config: &TestConfig, - _relayer: RelayerDriver, + relayer: RelayerDriver, chains: ConnectedChains, channel: ConnectedChannel, ) -> Result<(), Error> { let denom_a = chains.node_a.denom(); + let packet_config = relayer.config.mode.packets; let wallet_a = chains.node_a.wallets().user1().cloned(); let wallet_b = chains.node_b.wallets().user1().cloned(); @@ -59,6 +60,8 @@ impl BinaryChannelTest for QueryPacketPendingTest { let opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), + max_memo_size: packet_config.max_memo_size.to_usize(), + max_receiver_size: packet_config.max_receiver_size.to_usize(), }; let link = Link::new_from_opts( chains.handle_a().clone(), From 3ea0c6fdb75b88fd35df36c7f50f230f8426c04c Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 08:46:54 +0100 Subject: [PATCH 03/25] Update defaults --- crates/relayer/src/config.rs | 8 ++++++-- crates/relayer/src/link/relay_path.rs | 7 +++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index b75585d0e5..3a50fd76a7 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -231,12 +231,16 @@ pub mod default { } } + /// Use limit of 32768 bytes used by ibc-go v8 as default + /// See: https://github.com/cosmos/ibc-go/blob/04e14a7079e18323c64d4e00e12cf77708d24701/modules/apps/transfer/types/msgs.go#L17 pub fn max_memo_size() -> MaxTxSize { - MaxTxSize::unsafe_new(180000) + MaxTxSize::unsafe_new(32768) } + /// Use limit of 2048 bytes used by ibc-go v8 as default + /// See: https://github.com/cosmos/ibc-go/blob/04e14a7079e18323c64d4e00e12cf77708d24701/modules/apps/transfer/types/msgs.go#L16 pub fn max_receiver_size() -> MaxTxSize { - MaxTxSize::unsafe_new(180000) + MaxTxSize::unsafe_new(2048) } } diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 0436890ade..0639b8c649 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -1389,6 +1389,13 @@ impl RelayPath { dst_info: &ChainStatus, height: Height, ) -> Result<(Option, Option), LinkError> { + crate::time!( + "build_recv_or_timeout_from_send_packet_event", + { + "src_channel": event.packet.source_channel, + "dst_channel": event.packet.destination_channel, + } + ); let timeout = self.build_timeout_from_send_packet_event(event, dst_info)?; if timeout.is_some() { Ok((None, timeout)) From 72c32bd74450e63a238b4b015f57acac845eabdf Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 09:55:55 +0100 Subject: [PATCH 04/25] Rename configurations max_memo_size and max_receiver_size to ics20_max_memo_size and ics20_max_receiver_size --- crates/relayer-cli/src/commands/clear.rs | 4 ++-- crates/relayer-cli/src/commands/tx/packet.rs | 8 ++++---- .../src/core/ics04_channel/error.rs | 12 ++++++++++- .../src/core/ics04_channel/packet.rs | 20 +++++++------------ crates/relayer/src/config.rs | 16 +++++++-------- crates/relayer/src/worker.rs | 4 ++-- .../src/tests/execute_schedule.rs | 4 ++-- .../src/tests/ordered_channel_clear.rs | 12 +++++------ .../src/tests/query_packet.rs | 4 ++-- 9 files changed, 44 insertions(+), 40 deletions(-) diff --git a/crates/relayer-cli/src/commands/clear.rs b/crates/relayer-cli/src/commands/clear.rs index 6fba08ee6b..9084aa0173 100644 --- a/crates/relayer-cli/src/commands/clear.rs +++ b/crates/relayer-cli/src/commands/clear.rs @@ -135,8 +135,8 @@ impl Runnable for ClearPacketsCmd { let opts = LinkParameters { src_port_id: self.port_id.clone(), src_channel_id: self.channel_id.clone(), - max_memo_size: config.mode.packets.max_memo_size.to_usize(), - max_receiver_size: config.mode.packets.max_receiver_size.to_usize(), + max_memo_size: config.mode.packets.ics20_max_memo_size.to_usize(), + max_receiver_size: config.mode.packets.ics20_max_receiver_size.to_usize(), }; let fwd_link = match Link::new_from_opts(chains.src.clone(), chains.dst, opts, false, false) diff --git a/crates/relayer-cli/src/commands/tx/packet.rs b/crates/relayer-cli/src/commands/tx/packet.rs index bf0ed2aa7b..d596fa3584 100644 --- a/crates/relayer-cli/src/commands/tx/packet.rs +++ b/crates/relayer-cli/src/commands/tx/packet.rs @@ -70,8 +70,8 @@ impl Runnable for TxPacketRecvCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), - max_memo_size: config.mode.packets.max_memo_size.to_usize(), - max_receiver_size: config.mode.packets.max_receiver_size.to_usize(), + max_memo_size: config.mode.packets.ics20_max_memo_size.to_usize(), + max_receiver_size: config.mode.packets.ics20_max_receiver_size.to_usize(), }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) { Ok(link) => link, @@ -153,8 +153,8 @@ impl Runnable for TxPacketAckCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), - max_memo_size: config.mode.packets.max_memo_size.to_usize(), - max_receiver_size: config.mode.packets.max_receiver_size.to_usize(), + max_memo_size: config.mode.packets.ics20_max_memo_size.to_usize(), + max_receiver_size: config.mode.packets.ics20_max_receiver_size.to_usize(), }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) { Ok(link) => link, diff --git a/crates/relayer-types/src/core/ics04_channel/error.rs b/crates/relayer-types/src/core/ics04_channel/error.rs index 4a62d96755..ac46a1176d 100644 --- a/crates/relayer-types/src/core/ics04_channel/error.rs +++ b/crates/relayer-types/src/core/ics04_channel/error.rs @@ -348,7 +348,17 @@ define_error! { AbciConversionFailed { abci_event: String } - | e | { format_args!("Failed to convert abci event to IbcEvent: {}", e.abci_event)} + | e | { format_args!("Failed to convert abci event to IbcEvent: {}", e.abci_event)}, + + SerdeJsonError + [ TraceError ] + | _ | { "error deserializing JSON" }, + + DecodeIcs20Packet + { description: String } + | e | { + format_args!("error decoding ICS20 packet from proto to domain type: {}", e.description) + }, } } diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index 034aee75b6..b0f0e54ac8 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -6,7 +6,8 @@ use ibc_proto::ibc::applications::transfer::v2::FungibleTokenPacketData as RawPa use ibc_proto::ibc::core::channel::v1::Packet as RawPacket; use super::timeout::TimeoutHeight; -use crate::applications::transfer::packet::PacketData as APacketData; +use crate::applications::transfer::error::Error as Ics20Error; +use crate::applications::transfer::packet::PacketData as Ics20PacketData; use crate::core::ics04_channel::error::Error; use crate::core::ics24_host::identifier::{ChannelId, PortId}; use crate::timestamp::{Expiry::Expired, Timestamp}; @@ -186,18 +187,11 @@ impl Packet { max_memo_size: usize, max_receiver_size: usize, ) -> Result<(usize, usize), Error> { - let any_packet: RawPacketData = match serde_json::from_slice(&self.data) { - Ok(v) => v, - Err(e) => { - return Err(Error::app_module(e.to_string())); - } - }; - let packet: APacketData = match any_packet.try_into() { - Ok(p) => p, - Err(e) => { - return Err(Error::app_module(e.to_string())); - } - }; + let any_packet: RawPacketData = + serde_json::from_slice(&self.data).map_err(Error::serde_json_error)?; + let packet: Ics20PacketData = any_packet + .try_into() + .map_err(|e: Ics20Error| Error::decode_ics20_packet(e.to_string()))?; let memo_size = if let Some(memo) = &packet.memo { if memo.len() > max_memo_size { memo.len() diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index 3a50fd76a7..7f36c54850 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -233,13 +233,13 @@ pub mod default { /// Use limit of 32768 bytes used by ibc-go v8 as default /// See: https://github.com/cosmos/ibc-go/blob/04e14a7079e18323c64d4e00e12cf77708d24701/modules/apps/transfer/types/msgs.go#L17 - pub fn max_memo_size() -> MaxTxSize { + pub fn ics20_max_memo_size() -> MaxTxSize { MaxTxSize::unsafe_new(32768) } /// Use limit of 2048 bytes used by ibc-go v8 as default /// See: https://github.com/cosmos/ibc-go/blob/04e14a7079e18323c64d4e00e12cf77708d24701/modules/apps/transfer/types/msgs.go#L16 - pub fn max_receiver_size() -> MaxTxSize { + pub fn ics20_max_receiver_size() -> MaxTxSize { MaxTxSize::unsafe_new(2048) } } @@ -412,10 +412,10 @@ pub struct Packets { pub tx_confirmation: bool, #[serde(default = "default::auto_register_counterparty_payee")] pub auto_register_counterparty_payee: bool, - #[serde(default = "default::max_memo_size")] - pub max_memo_size: MaxTxSize, - #[serde(default = "default::max_receiver_size")] - pub max_receiver_size: MaxTxSize, + #[serde(default = "default::ics20_max_memo_size")] + pub ics20_max_memo_size: MaxTxSize, + #[serde(default = "default::ics20_max_receiver_size")] + pub ics20_max_receiver_size: MaxTxSize, } impl Default for Packets { @@ -426,8 +426,8 @@ impl Default for Packets { clear_on_start: default::clear_on_start(), tx_confirmation: default::tx_confirmation(), auto_register_counterparty_payee: default::auto_register_counterparty_payee(), - max_memo_size: default::max_memo_size(), - max_receiver_size: default::max_receiver_size(), + ics20_max_memo_size: default::ics20_max_memo_size(), + ics20_max_receiver_size: default::ics20_max_receiver_size(), } } } diff --git a/crates/relayer/src/worker.rs b/crates/relayer/src/worker.rs index fb02a38413..ee88d254b4 100644 --- a/crates/relayer/src/worker.rs +++ b/crates/relayer/src/worker.rs @@ -117,8 +117,8 @@ pub fn spawn_worker_tasks( LinkParameters { src_port_id: path.src_port_id.clone(), src_channel_id: path.src_channel_id.clone(), - max_memo_size: packets_config.max_memo_size.to_usize(), - max_receiver_size: packets_config.max_receiver_size.to_usize(), + max_memo_size: packets_config.ics20_max_memo_size.to_usize(), + max_receiver_size: packets_config.ics20_max_receiver_size.to_usize(), }, packets_config.tx_confirmation, packets_config.auto_register_counterparty_payee, diff --git a/tools/integration-test/src/tests/execute_schedule.rs b/tools/integration-test/src/tests/execute_schedule.rs index d013669b5d..f0e2d8c27c 100644 --- a/tools/integration-test/src/tests/execute_schedule.rs +++ b/tools/integration-test/src/tests/execute_schedule.rs @@ -47,8 +47,8 @@ impl BinaryChannelTest for ExecuteScheduleTest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.max_memo_size.to_usize(), - max_receiver_size: packet_config.max_receiver_size.to_usize(), + max_memo_size: packet_config.ics20_max_memo_size.to_usize(), + max_receiver_size: packet_config.ics20_max_receiver_size.to_usize(), }; let chain_a_link = Link::new_from_opts( diff --git a/tools/integration-test/src/tests/ordered_channel_clear.rs b/tools/integration-test/src/tests/ordered_channel_clear.rs index 1555dfe5cd..2ad0ac2b7e 100644 --- a/tools/integration-test/src/tests/ordered_channel_clear.rs +++ b/tools/integration-test/src/tests/ordered_channel_clear.rs @@ -119,8 +119,8 @@ impl BinaryChannelTest for OrderedChannelClearTest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.max_memo_size.to_usize(), - max_receiver_size: packet_config.max_receiver_size.to_usize(), + max_memo_size: packet_config.ics20_max_memo_size.to_usize(), + max_receiver_size: packet_config.ics20_max_receiver_size.to_usize(), }; let chain_a_link = Link::new_from_opts( @@ -134,8 +134,8 @@ impl BinaryChannelTest for OrderedChannelClearTest { let chain_b_link_opts = LinkParameters { src_port_id: channel.port_b.clone().into_value(), src_channel_id: channel.channel_id_b.clone().into_value(), - max_memo_size: packet_config.max_memo_size.to_usize(), - max_receiver_size: packet_config.max_receiver_size.to_usize(), + max_memo_size: packet_config.ics20_max_memo_size.to_usize(), + max_receiver_size: packet_config.ics20_max_receiver_size.to_usize(), }; let chain_b_link = Link::new_from_opts( @@ -270,8 +270,8 @@ impl BinaryChannelTest for OrderedChannelClearEqualCLITest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.into_value(), - max_memo_size: packet_config.max_memo_size.to_usize(), - max_receiver_size: packet_config.max_receiver_size.to_usize(), + max_memo_size: packet_config.ics20_max_memo_size.to_usize(), + max_receiver_size: packet_config.ics20_max_receiver_size.to_usize(), }; let chain_a_link = Link::new_from_opts( diff --git a/tools/integration-test/src/tests/query_packet.rs b/tools/integration-test/src/tests/query_packet.rs index efd84e8a68..a9dbd83b33 100644 --- a/tools/integration-test/src/tests/query_packet.rs +++ b/tools/integration-test/src/tests/query_packet.rs @@ -60,8 +60,8 @@ impl BinaryChannelTest for QueryPacketPendingTest { let opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.max_memo_size.to_usize(), - max_receiver_size: packet_config.max_receiver_size.to_usize(), + max_memo_size: packet_config.ics20_max_memo_size.to_usize(), + max_receiver_size: packet_config.ics20_max_receiver_size.to_usize(), }; let link = Link::new_from_opts( chains.handle_a().clone(), From 5e4aa5e0d2ec5a3c03b557125d4e5ba2f5f02356 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 10:40:17 +0100 Subject: [PATCH 05/25] Improve max memo and receiver configuration --- config.toml | 12 +++ crates/relayer/src/config.rs | 27 ++--- crates/relayer/src/config/types.rs | 158 +++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 19 deletions(-) diff --git a/config.toml b/config.toml index 2ceb166ccb..b5e32bde02 100644 --- a/config.toml +++ b/config.toml @@ -79,6 +79,18 @@ tx_confirmation = false # [Default: false] auto_register_counterparty_payee = false +# Set the maximum size for the memo field in ICS20 packets. +# If the size of the memo field is bigger than the configured +# one, the packet will not be relayed. +# [Default: 32768 (32Kb)] +#ics20_max_memo_size = 32768 + +# Set the maximum size for the receiver field in ICS20 packets. +# If the size of the receiver field is bigger than the configured +# one, the packet will not be relayed. +# [Default: 32768 (2Kb)] +#ics20_max_receiver_size = 2048 + # The REST section defines parameters for Hermes' built-in RESTful API. # https://hermes.informal.systems/rest.html [rest] diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index 7f36c54850..666b288dad 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -26,6 +26,8 @@ use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId use ibc_relayer_types::timestamp::ZERO_DURATION; use crate::chain::cosmos::config::CosmosSdkConfig; +use crate::config::types::ics20_max_memo_size::Ics20MaxMemoSize; +use crate::config::types::ics20_max_receiver_size::Ics20MaxReceiverSize; use crate::config::types::TrustThreshold; use crate::error::Error as RelayerError; use crate::extension_options::ExtensionOptionDynamicFeeTx; @@ -230,18 +232,6 @@ pub mod default { buckets: 10, } } - - /// Use limit of 32768 bytes used by ibc-go v8 as default - /// See: https://github.com/cosmos/ibc-go/blob/04e14a7079e18323c64d4e00e12cf77708d24701/modules/apps/transfer/types/msgs.go#L17 - pub fn ics20_max_memo_size() -> MaxTxSize { - MaxTxSize::unsafe_new(32768) - } - - /// Use limit of 2048 bytes used by ibc-go v8 as default - /// See: https://github.com/cosmos/ibc-go/blob/04e14a7079e18323c64d4e00e12cf77708d24701/modules/apps/transfer/types/msgs.go#L16 - pub fn ics20_max_receiver_size() -> MaxTxSize { - MaxTxSize::unsafe_new(2048) - } } #[derive(Clone, Debug, Default, Deserialize, Serialize)] @@ -412,10 +402,10 @@ pub struct Packets { pub tx_confirmation: bool, #[serde(default = "default::auto_register_counterparty_payee")] pub auto_register_counterparty_payee: bool, - #[serde(default = "default::ics20_max_memo_size")] - pub ics20_max_memo_size: MaxTxSize, - #[serde(default = "default::ics20_max_receiver_size")] - pub ics20_max_receiver_size: MaxTxSize, + #[serde(default)] + pub ics20_max_memo_size: Ics20MaxMemoSize, + #[serde(default)] + pub ics20_max_receiver_size: Ics20MaxReceiverSize, } impl Default for Packets { @@ -426,8 +416,8 @@ impl Default for Packets { clear_on_start: default::clear_on_start(), tx_confirmation: default::tx_confirmation(), auto_register_counterparty_payee: default::auto_register_counterparty_payee(), - ics20_max_memo_size: default::ics20_max_memo_size(), - ics20_max_receiver_size: default::ics20_max_receiver_size(), + ics20_max_memo_size: Ics20MaxMemoSize::default(), + ics20_max_receiver_size: Ics20MaxReceiverSize::default(), } } } @@ -761,7 +751,6 @@ impl> From> for Diagnostic { use crate::chain::cosmos::config::error::Error as CosmosConfigError; -use self::types::MaxTxSize; impl From for Error { fn from(error: CosmosConfigError) -> Error { Error::cosmos_config_error(error.to_string()) diff --git a/crates/relayer/src/config/types.rs b/crates/relayer/src/config/types.rs index 5b14b28a9d..91131c9013 100644 --- a/crates/relayer/src/config/types.rs +++ b/crates/relayer/src/config/types.rs @@ -179,6 +179,164 @@ pub mod max_tx_size { } } +pub mod ics20_max_receiver_size { + flex_error::define_error! { + Error { + TooBig + { value: usize } + |e| { + format_args!("`ics20_max_receiver_size` must be less than or equal to {}, found {}", + Ics20MaxReceiverSize::MAX_BOUND, e.value) + }, + } + } + + #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] + pub struct Ics20MaxReceiverSize(usize); + + impl Ics20MaxReceiverSize { + /// Use limit of 2048 bytes used by ibc-go v8 as default + /// See: https://github.com/cosmos/ibc-go/blob/04e14a7079e18323c64d4e00e12cf77708d24701/modules/apps/transfer/types/msgs.go#L16 + const DEFAULT: usize = 2048; + const MAX_BOUND: usize = 1048576; // 1 MBytes + + pub fn new(value: usize) -> Result { + if value > Self::MAX_BOUND { + return Err(Error::too_big(value)); + } + + Ok(Self(value)) + } + + pub fn max() -> Self { + Self(Self::MAX_BOUND) + } + + pub fn to_usize(self) -> usize { + self.0 + } + } + + impl Default for Ics20MaxReceiverSize { + fn default() -> Self { + Self(Self::DEFAULT) + } + } + + use serde::de::Unexpected; + use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; + + impl<'de> Deserialize<'de> for Ics20MaxReceiverSize { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let value = usize::deserialize(deserializer)?; + + Ics20MaxReceiverSize::new(value).map_err(|e| match e.detail() { + ErrorDetail::TooBig(_) => D::Error::invalid_value( + Unexpected::Unsigned(value as u64), + &format!("a usize less than or equal to {}", Self::MAX_BOUND).as_str(), + ), + }) + } + } + + impl Serialize for Ics20MaxReceiverSize { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.0.serialize(serializer) + } + } + + impl From for usize { + fn from(m: Ics20MaxReceiverSize) -> Self { + m.0 + } + } +} + +pub mod ics20_max_memo_size { + flex_error::define_error! { + Error { + TooBig + { value: usize } + |e| { + format_args!("`ics20_max_memo_size` must be less than or equal to {}, found {}", + Ics20MaxMemoSize::MAX_BOUND, e.value) + }, + } + } + + #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] + pub struct Ics20MaxMemoSize(usize); + + impl Ics20MaxMemoSize { + /// Use limit of 32768 bytes used by ibc-go v8 as default + /// See: https://github.com/cosmos/ibc-go/blob/04e14a7079e18323c64d4e00e12cf77708d24701/modules/apps/transfer/types/msgs.go#L17 + const DEFAULT: usize = 32768; + const MAX_BOUND: usize = 4 * 1048576; // 4 MBytes + + pub fn new(value: usize) -> Result { + if value > Self::MAX_BOUND { + return Err(Error::too_big(value)); + } + + Ok(Self(value)) + } + + pub fn max() -> Self { + Self(Self::MAX_BOUND) + } + + pub fn to_usize(self) -> usize { + self.0 + } + } + + impl Default for Ics20MaxMemoSize { + fn default() -> Self { + Self(Self::DEFAULT) + } + } + + use serde::de::Unexpected; + use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; + + impl<'de> Deserialize<'de> for Ics20MaxMemoSize { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let value = usize::deserialize(deserializer)?; + + Ics20MaxMemoSize::new(value).map_err(|e| match e.detail() { + ErrorDetail::TooBig(_) => D::Error::invalid_value( + Unexpected::Unsigned(value as u64), + &format!("a usize less than or equal to {}", Self::MAX_BOUND).as_str(), + ), + }) + } + } + + impl Serialize for Ics20MaxMemoSize { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.0.serialize(serializer) + } + } + + impl From for usize { + fn from(m: Ics20MaxMemoSize) -> Self { + m.0 + } + } +} + pub use memo::Memo; pub mod memo { From 8e504b5f0c6dd82c810fc95dec618ed221dba42c Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 10:52:21 +0100 Subject: [PATCH 06/25] Add changelog entry --- .../features/ibc-relayer/3766-max-memo-receiver-config.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changelog/unreleased/features/ibc-relayer/3766-max-memo-receiver-config.md diff --git a/.changelog/unreleased/features/ibc-relayer/3766-max-memo-receiver-config.md b/.changelog/unreleased/features/ibc-relayer/3766-max-memo-receiver-config.md new file mode 100644 index 0000000000..aca27309b9 --- /dev/null +++ b/.changelog/unreleased/features/ibc-relayer/3766-max-memo-receiver-config.md @@ -0,0 +1,6 @@ +- Add two new packet configurations: + * `ics20_max_memo_size` which filters ICS20 packets with memo + field bigger than the configured value + * `ics20_max_receiver_size` which filters ICS20 packets with receiver + field bigger than the configured value + ([\#3766](https://github.com/informalsystems/hermes/issues/3766)) \ No newline at end of file From 43a79a35c048b1eeef4d50114b2f2e060154fb4e Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 15:43:04 +0100 Subject: [PATCH 07/25] Change type of ics20_max_memo_size and ics20_max_receiver_size to byte_unit Byte type --- crates/relayer-cli/src/commands/clear.rs | 4 +- crates/relayer-cli/src/commands/tx/packet.rs | 8 +- .../src/core/ics04_channel/packet.rs | 8 +- crates/relayer/src/config.rs | 22 ++- crates/relayer/src/config/types.rs | 158 ------------------ crates/relayer/src/link.rs | 8 +- crates/relayer/src/link/relay_path.rs | 8 +- crates/relayer/src/worker.rs | 4 +- .../src/tests/execute_schedule.rs | 4 +- .../src/tests/ordered_channel_clear.rs | 12 +- .../src/tests/query_packet.rs | 4 +- 11 files changed, 44 insertions(+), 196 deletions(-) diff --git a/crates/relayer-cli/src/commands/clear.rs b/crates/relayer-cli/src/commands/clear.rs index 9084aa0173..82a4f6f50b 100644 --- a/crates/relayer-cli/src/commands/clear.rs +++ b/crates/relayer-cli/src/commands/clear.rs @@ -135,8 +135,8 @@ impl Runnable for ClearPacketsCmd { let opts = LinkParameters { src_port_id: self.port_id.clone(), src_channel_id: self.channel_id.clone(), - max_memo_size: config.mode.packets.ics20_max_memo_size.to_usize(), - max_receiver_size: config.mode.packets.ics20_max_receiver_size.to_usize(), + max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes(), + max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes(), }; let fwd_link = match Link::new_from_opts(chains.src.clone(), chains.dst, opts, false, false) diff --git a/crates/relayer-cli/src/commands/tx/packet.rs b/crates/relayer-cli/src/commands/tx/packet.rs index d596fa3584..d444f2ca30 100644 --- a/crates/relayer-cli/src/commands/tx/packet.rs +++ b/crates/relayer-cli/src/commands/tx/packet.rs @@ -70,8 +70,8 @@ impl Runnable for TxPacketRecvCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), - max_memo_size: config.mode.packets.ics20_max_memo_size.to_usize(), - max_receiver_size: config.mode.packets.ics20_max_receiver_size.to_usize(), + max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes(), + max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes(), }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) { Ok(link) => link, @@ -153,8 +153,8 @@ impl Runnable for TxPacketAckCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), - max_memo_size: config.mode.packets.ics20_max_memo_size.to_usize(), - max_receiver_size: config.mode.packets.ics20_max_receiver_size.to_usize(), + max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes(), + max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes(), }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) { Ok(link) => link, diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index b0f0e54ac8..dc5fa07c8a 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -184,8 +184,8 @@ impl Packet { pub fn validate_fields( &self, - max_memo_size: usize, - max_receiver_size: usize, + max_memo_size: u64, + max_receiver_size: u64, ) -> Result<(usize, usize), Error> { let any_packet: RawPacketData = serde_json::from_slice(&self.data).map_err(Error::serde_json_error)?; @@ -193,7 +193,7 @@ impl Packet { .try_into() .map_err(|e: Ics20Error| Error::decode_ics20_packet(e.to_string()))?; let memo_size = if let Some(memo) = &packet.memo { - if memo.len() > max_memo_size { + if memo.len() > max_memo_size as usize { memo.len() } else { 0 @@ -201,7 +201,7 @@ impl Packet { } else { 0 }; - if packet.receiver.to_string().len() > max_receiver_size { + if packet.receiver.to_string().len() > max_receiver_size as usize { Ok((memo_size, packet.receiver.to_string().len())) } else { Ok((memo_size, 0)) diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index 666b288dad..fc16b82f09 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -26,8 +26,6 @@ use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId use ibc_relayer_types::timestamp::ZERO_DURATION; use crate::chain::cosmos::config::CosmosSdkConfig; -use crate::config::types::ics20_max_memo_size::Ics20MaxMemoSize; -use crate::config::types::ics20_max_receiver_size::Ics20MaxReceiverSize; use crate::config::types::TrustThreshold; use crate::error::Error as RelayerError; use crate::extension_options::ExtensionOptionDynamicFeeTx; @@ -232,6 +230,14 @@ pub mod default { buckets: 10, } } + + pub fn ics20_max_memo_size() -> Byte { + Byte::from_bytes(32768) + } + + pub fn ics20_max_receiver_size() -> Byte { + Byte::from_bytes(2048) + } } #[derive(Clone, Debug, Default, Deserialize, Serialize)] @@ -402,10 +408,10 @@ pub struct Packets { pub tx_confirmation: bool, #[serde(default = "default::auto_register_counterparty_payee")] pub auto_register_counterparty_payee: bool, - #[serde(default)] - pub ics20_max_memo_size: Ics20MaxMemoSize, - #[serde(default)] - pub ics20_max_receiver_size: Ics20MaxReceiverSize, + #[serde(default = "default::ics20_max_memo_size")] + pub ics20_max_memo_size: Byte, + #[serde(default = "default::ics20_max_receiver_size")] + pub ics20_max_receiver_size: Byte, } impl Default for Packets { @@ -416,8 +422,8 @@ impl Default for Packets { clear_on_start: default::clear_on_start(), tx_confirmation: default::tx_confirmation(), auto_register_counterparty_payee: default::auto_register_counterparty_payee(), - ics20_max_memo_size: Ics20MaxMemoSize::default(), - ics20_max_receiver_size: Ics20MaxReceiverSize::default(), + ics20_max_memo_size: default::ics20_max_memo_size(), + ics20_max_receiver_size: default::ics20_max_receiver_size(), } } } diff --git a/crates/relayer/src/config/types.rs b/crates/relayer/src/config/types.rs index 91131c9013..5b14b28a9d 100644 --- a/crates/relayer/src/config/types.rs +++ b/crates/relayer/src/config/types.rs @@ -179,164 +179,6 @@ pub mod max_tx_size { } } -pub mod ics20_max_receiver_size { - flex_error::define_error! { - Error { - TooBig - { value: usize } - |e| { - format_args!("`ics20_max_receiver_size` must be less than or equal to {}, found {}", - Ics20MaxReceiverSize::MAX_BOUND, e.value) - }, - } - } - - #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] - pub struct Ics20MaxReceiverSize(usize); - - impl Ics20MaxReceiverSize { - /// Use limit of 2048 bytes used by ibc-go v8 as default - /// See: https://github.com/cosmos/ibc-go/blob/04e14a7079e18323c64d4e00e12cf77708d24701/modules/apps/transfer/types/msgs.go#L16 - const DEFAULT: usize = 2048; - const MAX_BOUND: usize = 1048576; // 1 MBytes - - pub fn new(value: usize) -> Result { - if value > Self::MAX_BOUND { - return Err(Error::too_big(value)); - } - - Ok(Self(value)) - } - - pub fn max() -> Self { - Self(Self::MAX_BOUND) - } - - pub fn to_usize(self) -> usize { - self.0 - } - } - - impl Default for Ics20MaxReceiverSize { - fn default() -> Self { - Self(Self::DEFAULT) - } - } - - use serde::de::Unexpected; - use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; - - impl<'de> Deserialize<'de> for Ics20MaxReceiverSize { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let value = usize::deserialize(deserializer)?; - - Ics20MaxReceiverSize::new(value).map_err(|e| match e.detail() { - ErrorDetail::TooBig(_) => D::Error::invalid_value( - Unexpected::Unsigned(value as u64), - &format!("a usize less than or equal to {}", Self::MAX_BOUND).as_str(), - ), - }) - } - } - - impl Serialize for Ics20MaxReceiverSize { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - self.0.serialize(serializer) - } - } - - impl From for usize { - fn from(m: Ics20MaxReceiverSize) -> Self { - m.0 - } - } -} - -pub mod ics20_max_memo_size { - flex_error::define_error! { - Error { - TooBig - { value: usize } - |e| { - format_args!("`ics20_max_memo_size` must be less than or equal to {}, found {}", - Ics20MaxMemoSize::MAX_BOUND, e.value) - }, - } - } - - #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] - pub struct Ics20MaxMemoSize(usize); - - impl Ics20MaxMemoSize { - /// Use limit of 32768 bytes used by ibc-go v8 as default - /// See: https://github.com/cosmos/ibc-go/blob/04e14a7079e18323c64d4e00e12cf77708d24701/modules/apps/transfer/types/msgs.go#L17 - const DEFAULT: usize = 32768; - const MAX_BOUND: usize = 4 * 1048576; // 4 MBytes - - pub fn new(value: usize) -> Result { - if value > Self::MAX_BOUND { - return Err(Error::too_big(value)); - } - - Ok(Self(value)) - } - - pub fn max() -> Self { - Self(Self::MAX_BOUND) - } - - pub fn to_usize(self) -> usize { - self.0 - } - } - - impl Default for Ics20MaxMemoSize { - fn default() -> Self { - Self(Self::DEFAULT) - } - } - - use serde::de::Unexpected; - use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer}; - - impl<'de> Deserialize<'de> for Ics20MaxMemoSize { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - let value = usize::deserialize(deserializer)?; - - Ics20MaxMemoSize::new(value).map_err(|e| match e.detail() { - ErrorDetail::TooBig(_) => D::Error::invalid_value( - Unexpected::Unsigned(value as u64), - &format!("a usize less than or equal to {}", Self::MAX_BOUND).as_str(), - ), - }) - } - } - - impl Serialize for Ics20MaxMemoSize { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - self.0.serialize(serializer) - } - } - - impl From for usize { - fn from(m: Ics20MaxMemoSize) -> Self { - m.0 - } - } -} - pub use memo::Memo; pub mod memo { diff --git a/crates/relayer/src/link.rs b/crates/relayer/src/link.rs index a387679022..8077d29ab1 100644 --- a/crates/relayer/src/link.rs +++ b/crates/relayer/src/link.rs @@ -33,8 +33,8 @@ pub use relay_path::{RelayPath, Resubmit}; pub struct LinkParameters { pub src_port_id: PortId, pub src_channel_id: ChannelId, - pub max_memo_size: usize, - pub max_receiver_size: usize, + pub max_memo_size: u64, + pub max_receiver_size: u64, } pub struct Link { @@ -45,8 +45,8 @@ impl Link { pub fn new( channel: Channel, with_tx_confirmation: bool, - max_memo_size: usize, - max_receiver_size: usize, + max_memo_size: u64, + max_receiver_size: u64, ) -> Result { Ok(Self { a_to_b: RelayPath::new( diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 0639b8c649..eb772632e2 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -109,16 +109,16 @@ pub struct RelayPath { pending_txs_src: PendingTxs, pending_txs_dst: PendingTxs, - pub max_memo_size: usize, - pub max_receiver_size: usize, + pub max_memo_size: u64, + pub max_receiver_size: u64, } impl RelayPath { pub fn new( channel: Channel, with_tx_confirmation: bool, - max_memo_size: usize, - max_receiver_size: usize, + max_memo_size: u64, + max_receiver_size: u64, ) -> Result { let src_chain = channel.src_chain().clone(); let dst_chain = channel.dst_chain().clone(); diff --git a/crates/relayer/src/worker.rs b/crates/relayer/src/worker.rs index ee88d254b4..0347795609 100644 --- a/crates/relayer/src/worker.rs +++ b/crates/relayer/src/worker.rs @@ -117,8 +117,8 @@ pub fn spawn_worker_tasks( LinkParameters { src_port_id: path.src_port_id.clone(), src_channel_id: path.src_channel_id.clone(), - max_memo_size: packets_config.ics20_max_memo_size.to_usize(), - max_receiver_size: packets_config.ics20_max_receiver_size.to_usize(), + max_memo_size: packets_config.ics20_max_memo_size.get_bytes(), + max_receiver_size: packets_config.ics20_max_receiver_size.get_bytes(), }, packets_config.tx_confirmation, packets_config.auto_register_counterparty_payee, diff --git a/tools/integration-test/src/tests/execute_schedule.rs b/tools/integration-test/src/tests/execute_schedule.rs index f0e2d8c27c..f9180d47f1 100644 --- a/tools/integration-test/src/tests/execute_schedule.rs +++ b/tools/integration-test/src/tests/execute_schedule.rs @@ -47,8 +47,8 @@ impl BinaryChannelTest for ExecuteScheduleTest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.to_usize(), - max_receiver_size: packet_config.ics20_max_receiver_size.to_usize(), + max_memo_size: packet_config.ics20_max_memo_size.get_bytes(), + max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes(), }; let chain_a_link = Link::new_from_opts( diff --git a/tools/integration-test/src/tests/ordered_channel_clear.rs b/tools/integration-test/src/tests/ordered_channel_clear.rs index 2ad0ac2b7e..06b7c61ea4 100644 --- a/tools/integration-test/src/tests/ordered_channel_clear.rs +++ b/tools/integration-test/src/tests/ordered_channel_clear.rs @@ -119,8 +119,8 @@ impl BinaryChannelTest for OrderedChannelClearTest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.to_usize(), - max_receiver_size: packet_config.ics20_max_receiver_size.to_usize(), + max_memo_size: packet_config.ics20_max_memo_size.get_bytes(), + max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes(), }; let chain_a_link = Link::new_from_opts( @@ -134,8 +134,8 @@ impl BinaryChannelTest for OrderedChannelClearTest { let chain_b_link_opts = LinkParameters { src_port_id: channel.port_b.clone().into_value(), src_channel_id: channel.channel_id_b.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.to_usize(), - max_receiver_size: packet_config.ics20_max_receiver_size.to_usize(), + max_memo_size: packet_config.ics20_max_memo_size.get_bytes(), + max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes(), }; let chain_b_link = Link::new_from_opts( @@ -270,8 +270,8 @@ impl BinaryChannelTest for OrderedChannelClearEqualCLITest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.into_value(), - max_memo_size: packet_config.ics20_max_memo_size.to_usize(), - max_receiver_size: packet_config.ics20_max_receiver_size.to_usize(), + max_memo_size: packet_config.ics20_max_memo_size.get_bytes(), + max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes(), }; let chain_a_link = Link::new_from_opts( diff --git a/tools/integration-test/src/tests/query_packet.rs b/tools/integration-test/src/tests/query_packet.rs index a9dbd83b33..3da2ca6f5d 100644 --- a/tools/integration-test/src/tests/query_packet.rs +++ b/tools/integration-test/src/tests/query_packet.rs @@ -60,8 +60,8 @@ impl BinaryChannelTest for QueryPacketPendingTest { let opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.to_usize(), - max_receiver_size: packet_config.ics20_max_receiver_size.to_usize(), + max_memo_size: packet_config.ics20_max_memo_size.get_bytes(), + max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes(), }; let link = Link::new_from_opts( chains.handle_a().clone(), From f2b3a4fc46891f7f047dfb3a6336bf2a31409b59 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 15:50:52 +0100 Subject: [PATCH 08/25] Pass LinkParameters to new method for Link and RelayPath --- crates/relayer/src/link.rs | 19 ++++--------------- crates/relayer/src/link/relay_path.rs | 8 ++++---- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/crates/relayer/src/link.rs b/crates/relayer/src/link.rs index 8077d29ab1..e4aaaf7d79 100644 --- a/crates/relayer/src/link.rs +++ b/crates/relayer/src/link.rs @@ -45,16 +45,10 @@ impl Link { pub fn new( channel: Channel, with_tx_confirmation: bool, - max_memo_size: u64, - max_receiver_size: u64, + link_parameters: LinkParameters, ) -> Result { Ok(Self { - a_to_b: RelayPath::new( - channel, - with_tx_confirmation, - max_memo_size, - max_receiver_size, - )?, + a_to_b: RelayPath::new(channel, with_tx_confirmation, link_parameters)?, }) } @@ -149,7 +143,7 @@ impl Link { a_connection.client_id().clone(), a_connection_id, opts.src_port_id.clone(), - Some(opts.src_channel_id), + Some(opts.src_channel_id.clone()), None, ), b_side: ChannelSide::new( @@ -178,12 +172,7 @@ impl Link { .map_err(LinkError::relayer)?; } - Link::new( - channel, - with_tx_confirmation, - opts.max_memo_size, - opts.max_receiver_size, - ) + Link::new(channel, with_tx_confirmation, opts) } /// Constructs a link around the channel that is reverse to the channel diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index eb772632e2..081cf6ff25 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -55,6 +55,7 @@ use crate::link::packet_events::query_write_ack_events; use crate::link::pending::PendingTxs; use crate::link::relay_sender::{AsyncReply, SubmitReply}; use crate::link::relay_summary::RelaySummary; +use crate::link::LinkParameters; use crate::link::{pending, relay_sender}; use crate::path::PathIdentifiers; use crate::telemetry; @@ -117,8 +118,7 @@ impl RelayPath { pub fn new( channel: Channel, with_tx_confirmation: bool, - max_memo_size: u64, - max_receiver_size: u64, + link_parameters: LinkParameters, ) -> Result { let src_chain = channel.src_chain().clone(); let dst_chain = channel.dst_chain().clone(); @@ -158,8 +158,8 @@ impl RelayPath { pending_txs_src: PendingTxs::new(src_chain, src_channel_id, src_port_id, dst_chain_id), pending_txs_dst: PendingTxs::new(dst_chain, dst_channel_id, dst_port_id, src_chain_id), - max_memo_size, - max_receiver_size, + max_memo_size: link_parameters.max_memo_size, + max_receiver_size: link_parameters.max_receiver_size, }) } From b535b44861ae5eb9482bdb0485a23b2f58090349 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 16:10:56 +0100 Subject: [PATCH 09/25] Refactor memo and receiver field validation for ICS20 packets --- .../src/core/ics04_channel/packet.rs | 36 ++++++------------- crates/relayer/src/link/relay_path.rs | 24 ++++++------- 2 files changed, 20 insertions(+), 40 deletions(-) diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index dc5fa07c8a..9d9ef2e0b1 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -1,13 +1,10 @@ -use std::str::FromStr; - use serde_derive::{Deserialize, Serialize}; +use std::str::FromStr; use ibc_proto::ibc::applications::transfer::v2::FungibleTokenPacketData as RawPacketData; use ibc_proto::ibc::core::channel::v1::Packet as RawPacket; use super::timeout::TimeoutHeight; -use crate::applications::transfer::error::Error as Ics20Error; -use crate::applications::transfer::packet::PacketData as Ics20PacketData; use crate::core::ics04_channel::error::Error; use crate::core::ics24_host::identifier::{ChannelId, PortId}; use crate::timestamp::{Expiry::Expired, Timestamp}; @@ -182,29 +179,16 @@ impl Packet { height_timed_out || timestamp_timed_out } - pub fn validate_fields( - &self, - max_memo_size: u64, - max_receiver_size: u64, - ) -> Result<(usize, usize), Error> { - let any_packet: RawPacketData = - serde_json::from_slice(&self.data).map_err(Error::serde_json_error)?; - let packet: Ics20PacketData = any_packet - .try_into() - .map_err(|e: Ics20Error| Error::decode_ics20_packet(e.to_string()))?; - let memo_size = if let Some(memo) = &packet.memo { - if memo.len() > max_memo_size as usize { - memo.len() - } else { - 0 + /// This method will return if the memo and receiver fields are valid, e.g. not too big. + /// If it fails to decode the ICS20 PacketData then there is no need to validate fields + /// so the method will return true. + pub fn are_fields_valid(&self, max_memo_size: u64, max_receiver_size: u64) -> bool { + match serde_json::from_slice::(&self.data).map_err(Error::serde_json_error) { + Ok(packet) => { + packet.memo.len() <= max_memo_size as usize + || packet.receiver.to_string().len() <= max_receiver_size as usize } - } else { - 0 - }; - if packet.receiver.to_string().len() > max_receiver_size as usize { - Ok((memo_size, packet.receiver.to_string().len())) - } else { - Ok((memo_size, 0)) + Err(_) => true, } } } diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 081cf6ff25..4cef6dcb4c 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -1399,21 +1399,17 @@ impl RelayPath { let timeout = self.build_timeout_from_send_packet_event(event, dst_info)?; if timeout.is_some() { Ok((None, timeout)) + } else if event + .packet + .are_fields_valid(self.max_memo_size, self.max_receiver_size) + { + Ok((self.build_recv_packet(&event.packet, height)?, None)) } else { - match event - .packet - .validate_fields(self.max_memo_size, self.max_receiver_size) - { - Ok((0, 0)) => Ok((self.build_recv_packet(&event.packet, height)?, None)), - Ok((memo_size, receiver_size)) => { - warn!("memo and/or receiver fields were bigger than maximum allowed. Memo field {memo_size}, maximum allowed {}. Receiver field {receiver_size}, maximum allowed {}", self.max_memo_size, self.max_receiver_size); - Ok((None, None)) - } - Err(e) => { - warn!("error validating fields, will simply relay message: {e}"); - Ok((self.build_recv_packet(&event.packet, height)?, None)) - } - } + debug!( + "memo and/or receiver field are too big for packet {:#?}", + event.packet + ); + Ok((None, None)) } } From fecc4506e733f25940ecc82f1368ec9b029fc097 Mon Sep 17 00:00:00 2001 From: Luca Joss <43531661+ljoss17@users.noreply.github.com> Date: Wed, 10 Jan 2024 16:35:34 +0100 Subject: [PATCH 10/25] Update crates/relayer-types/src/core/ics04_channel/packet.rs Co-authored-by: Romain Ruetschi Signed-off-by: Luca Joss <43531661+ljoss17@users.noreply.github.com> --- crates/relayer-types/src/core/ics04_channel/packet.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index 9d9ef2e0b1..e6ca9cab85 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -186,7 +186,7 @@ impl Packet { match serde_json::from_slice::(&self.data).map_err(Error::serde_json_error) { Ok(packet) => { packet.memo.len() <= max_memo_size as usize - || packet.receiver.to_string().len() <= max_receiver_size as usize + && packet.receiver.as_ref().len() <= max_receiver_size as usize } Err(_) => true, } From 508cb939f18fd960561477e5fb240070d398fb88 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 16:41:54 +0100 Subject: [PATCH 11/25] Remove unnecessary errors and error maping --- .../src/core/ics04_channel/error.rs | 17 +---------------- .../src/core/ics04_channel/packet.rs | 4 ++-- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/crates/relayer-types/src/core/ics04_channel/error.rs b/crates/relayer-types/src/core/ics04_channel/error.rs index ac46a1176d..817f0337f8 100644 --- a/crates/relayer-types/src/core/ics04_channel/error.rs +++ b/crates/relayer-types/src/core/ics04_channel/error.rs @@ -12,16 +12,11 @@ use crate::timestamp::Timestamp; use crate::Height; use flex_error::{define_error, TraceError}; -use prost::DecodeError; use tendermint_proto::Error as TendermintError; define_error! { #[derive(Debug, PartialEq, Eq)] Error { - Decode - [ TraceError ] - | _ | { "error decoding byte array" }, - Ics03Connection [ connection_error::Error ] | _ | { "ics03 connection error" }, @@ -348,17 +343,7 @@ define_error! { AbciConversionFailed { abci_event: String } - | e | { format_args!("Failed to convert abci event to IbcEvent: {}", e.abci_event)}, - - SerdeJsonError - [ TraceError ] - | _ | { "error deserializing JSON" }, - - DecodeIcs20Packet - { description: String } - | e | { - format_args!("error decoding ICS20 packet from proto to domain type: {}", e.description) - }, + | e | { format_args!("Failed to convert abci event to IbcEvent: {}", e.abci_event)} } } diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index e6ca9cab85..5f5e0e8dbf 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -183,10 +183,10 @@ impl Packet { /// If it fails to decode the ICS20 PacketData then there is no need to validate fields /// so the method will return true. pub fn are_fields_valid(&self, max_memo_size: u64, max_receiver_size: u64) -> bool { - match serde_json::from_slice::(&self.data).map_err(Error::serde_json_error) { + match serde_json::from_slice::(&self.data) { Ok(packet) => { packet.memo.len() <= max_memo_size as usize - && packet.receiver.as_ref().len() <= max_receiver_size as usize + && packet.receiver.len() <= max_receiver_size as usize } Err(_) => true, } From f950ef95440e04517b054072e1983acbfbe8e921 Mon Sep 17 00:00:00 2001 From: Luca Joss <43531661+ljoss17@users.noreply.github.com> Date: Wed, 10 Jan 2024 16:44:05 +0100 Subject: [PATCH 12/25] Apply suggestions from code review Co-authored-by: Romain Ruetschi Signed-off-by: Luca Joss <43531661+ljoss17@users.noreply.github.com> --- config.toml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/config.toml b/config.toml index b5e32bde02..59c36d038f 100644 --- a/config.toml +++ b/config.toml @@ -82,14 +82,14 @@ auto_register_counterparty_payee = false # Set the maximum size for the memo field in ICS20 packets. # If the size of the memo field is bigger than the configured # one, the packet will not be relayed. -# [Default: 32768 (32Kb)] -#ics20_max_memo_size = 32768 +# [Default: "32KiB"] +#ics20_max_memo_size = "32KiB" # Set the maximum size for the receiver field in ICS20 packets. # If the size of the receiver field is bigger than the configured # one, the packet will not be relayed. -# [Default: 32768 (2Kb)] -#ics20_max_receiver_size = 2048 +# [Default: "2KiB"] +#ics20_max_receiver_size = "2KiB" # The REST section defines parameters for Hermes' built-in RESTful API. # https://hermes.informal.systems/rest.html From 0fd448cd8e91d9a825357093c9694cc2e817e6af Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 16:47:35 +0100 Subject: [PATCH 13/25] Change max_memo_size and max_receiver_size fields from u64 to usize --- crates/relayer-cli/src/commands/clear.rs | 4 ++-- crates/relayer-cli/src/commands/tx/packet.rs | 8 ++++---- .../relayer-types/src/core/ics04_channel/packet.rs | 5 ++--- crates/relayer/src/link.rs | 4 ++-- crates/relayer/src/link/relay_path.rs | 4 ++-- crates/relayer/src/worker.rs | 4 ++-- tools/integration-test/src/tests/execute_schedule.rs | 4 ++-- .../src/tests/ordered_channel_clear.rs | 12 ++++++------ tools/integration-test/src/tests/query_packet.rs | 4 ++-- 9 files changed, 24 insertions(+), 25 deletions(-) diff --git a/crates/relayer-cli/src/commands/clear.rs b/crates/relayer-cli/src/commands/clear.rs index 82a4f6f50b..c60c06e46e 100644 --- a/crates/relayer-cli/src/commands/clear.rs +++ b/crates/relayer-cli/src/commands/clear.rs @@ -135,8 +135,8 @@ impl Runnable for ClearPacketsCmd { let opts = LinkParameters { src_port_id: self.port_id.clone(), src_channel_id: self.channel_id.clone(), - max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes(), - max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes(), + max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes() as usize, + max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes() as usize, }; let fwd_link = match Link::new_from_opts(chains.src.clone(), chains.dst, opts, false, false) diff --git a/crates/relayer-cli/src/commands/tx/packet.rs b/crates/relayer-cli/src/commands/tx/packet.rs index d444f2ca30..fec2f2f62c 100644 --- a/crates/relayer-cli/src/commands/tx/packet.rs +++ b/crates/relayer-cli/src/commands/tx/packet.rs @@ -70,8 +70,8 @@ impl Runnable for TxPacketRecvCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), - max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes(), - max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes(), + max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes() as usize, + max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes() as usize, }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) { Ok(link) => link, @@ -153,8 +153,8 @@ impl Runnable for TxPacketAckCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), - max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes(), - max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes(), + max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes() as usize, + max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes() as usize, }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) { Ok(link) => link, diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index 5f5e0e8dbf..765e56a790 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -182,11 +182,10 @@ impl Packet { /// This method will return if the memo and receiver fields are valid, e.g. not too big. /// If it fails to decode the ICS20 PacketData then there is no need to validate fields /// so the method will return true. - pub fn are_fields_valid(&self, max_memo_size: u64, max_receiver_size: u64) -> bool { + pub fn are_fields_valid(&self, max_memo_size: usize, max_receiver_size: usize) -> bool { match serde_json::from_slice::(&self.data) { Ok(packet) => { - packet.memo.len() <= max_memo_size as usize - && packet.receiver.len() <= max_receiver_size as usize + packet.memo.len() <= max_memo_size && packet.receiver.len() <= max_receiver_size } Err(_) => true, } diff --git a/crates/relayer/src/link.rs b/crates/relayer/src/link.rs index e4aaaf7d79..3124740cc9 100644 --- a/crates/relayer/src/link.rs +++ b/crates/relayer/src/link.rs @@ -33,8 +33,8 @@ pub use relay_path::{RelayPath, Resubmit}; pub struct LinkParameters { pub src_port_id: PortId, pub src_channel_id: ChannelId, - pub max_memo_size: u64, - pub max_receiver_size: u64, + pub max_memo_size: usize, + pub max_receiver_size: usize, } pub struct Link { diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 4cef6dcb4c..fb25935c17 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -110,8 +110,8 @@ pub struct RelayPath { pending_txs_src: PendingTxs, pending_txs_dst: PendingTxs, - pub max_memo_size: u64, - pub max_receiver_size: u64, + pub max_memo_size: usize, + pub max_receiver_size: usize, } impl RelayPath { diff --git a/crates/relayer/src/worker.rs b/crates/relayer/src/worker.rs index 0347795609..e8054ba033 100644 --- a/crates/relayer/src/worker.rs +++ b/crates/relayer/src/worker.rs @@ -117,8 +117,8 @@ pub fn spawn_worker_tasks( LinkParameters { src_port_id: path.src_port_id.clone(), src_channel_id: path.src_channel_id.clone(), - max_memo_size: packets_config.ics20_max_memo_size.get_bytes(), - max_receiver_size: packets_config.ics20_max_receiver_size.get_bytes(), + max_memo_size: packets_config.ics20_max_memo_size.get_bytes() as usize, + max_receiver_size: packets_config.ics20_max_receiver_size.get_bytes() as usize, }, packets_config.tx_confirmation, packets_config.auto_register_counterparty_payee, diff --git a/tools/integration-test/src/tests/execute_schedule.rs b/tools/integration-test/src/tests/execute_schedule.rs index f9180d47f1..8a21d418ce 100644 --- a/tools/integration-test/src/tests/execute_schedule.rs +++ b/tools/integration-test/src/tests/execute_schedule.rs @@ -47,8 +47,8 @@ impl BinaryChannelTest for ExecuteScheduleTest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.get_bytes(), - max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes(), + max_memo_size: packet_config.ics20_max_memo_size.get_bytes() as usize, + max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes() as usize, }; let chain_a_link = Link::new_from_opts( diff --git a/tools/integration-test/src/tests/ordered_channel_clear.rs b/tools/integration-test/src/tests/ordered_channel_clear.rs index 06b7c61ea4..27b5328b34 100644 --- a/tools/integration-test/src/tests/ordered_channel_clear.rs +++ b/tools/integration-test/src/tests/ordered_channel_clear.rs @@ -119,8 +119,8 @@ impl BinaryChannelTest for OrderedChannelClearTest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.get_bytes(), - max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes(), + max_memo_size: packet_config.ics20_max_memo_size.get_bytes() as usize, + max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes() as usize, }; let chain_a_link = Link::new_from_opts( @@ -134,8 +134,8 @@ impl BinaryChannelTest for OrderedChannelClearTest { let chain_b_link_opts = LinkParameters { src_port_id: channel.port_b.clone().into_value(), src_channel_id: channel.channel_id_b.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.get_bytes(), - max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes(), + max_memo_size: packet_config.ics20_max_memo_size.get_bytes() as usize, + max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes() as usize, }; let chain_b_link = Link::new_from_opts( @@ -270,8 +270,8 @@ impl BinaryChannelTest for OrderedChannelClearEqualCLITest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.into_value(), - max_memo_size: packet_config.ics20_max_memo_size.get_bytes(), - max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes(), + max_memo_size: packet_config.ics20_max_memo_size.get_bytes() as usize, + max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes() as usize, }; let chain_a_link = Link::new_from_opts( diff --git a/tools/integration-test/src/tests/query_packet.rs b/tools/integration-test/src/tests/query_packet.rs index 3da2ca6f5d..ef59cc3840 100644 --- a/tools/integration-test/src/tests/query_packet.rs +++ b/tools/integration-test/src/tests/query_packet.rs @@ -60,8 +60,8 @@ impl BinaryChannelTest for QueryPacketPendingTest { let opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.get_bytes(), - max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes(), + max_memo_size: packet_config.ics20_max_memo_size.get_bytes() as usize, + max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes() as usize, }; let link = Link::new_from_opts( chains.handle_a().clone(), From 1f5534d6b7224f65ec19e01e3238c306bb182e1d Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 16:50:22 +0100 Subject: [PATCH 14/25] Improve doc for 'are_fields_valid' method --- crates/relayer-types/src/core/ics04_channel/packet.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index 765e56a790..7388b620eb 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -179,9 +179,9 @@ impl Packet { height_timed_out || timestamp_timed_out } - /// This method will return if the memo and receiver fields are valid, e.g. not too big. - /// If it fails to decode the ICS20 PacketData then there is no need to validate fields - /// so the method will return true. + /// If this packet data is an ICS-20 packet, check whether or not the memo and receiver + /// fields are under the given max size. + /// Returns `true` if this packet data is not an ICS-20 or fails to decode as an ICS-20 packet. pub fn are_fields_valid(&self, max_memo_size: usize, max_receiver_size: usize) -> bool { match serde_json::from_slice::(&self.data) { Ok(packet) => { From 9af4e6d0688748f09be91d09b22c7f272e82fb8c Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Wed, 10 Jan 2024 18:43:16 +0100 Subject: [PATCH 15/25] Improve logging when validating memo and receiver fields --- .../src/core/ics04_channel/packet.rs | 57 ++++++++++++++++++- crates/relayer/src/link/relay_path.rs | 50 ++++++++++++---- 2 files changed, 93 insertions(+), 14 deletions(-) diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index 7388b620eb..d63697c718 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -103,6 +103,27 @@ impl core::ops::Add for Sequence { } } +pub enum ValidationResult { + Valid, + MemoTooBig { + size: usize, + max: usize, + }, + ReceiverTooBig { + size: usize, + max: usize, + }, + BothTooBig { + memo_size: usize, + memo_max: usize, + receiver_size: usize, + receiver_max: usize, + }, + DecodeFailure { + details: String, + }, +} + #[derive(Clone, Default, Hash, PartialEq, Eq, Deserialize, Serialize)] pub struct Packet { pub sequence: Sequence, @@ -182,12 +203,42 @@ impl Packet { /// If this packet data is an ICS-20 packet, check whether or not the memo and receiver /// fields are under the given max size. /// Returns `true` if this packet data is not an ICS-20 or fails to decode as an ICS-20 packet. - pub fn are_fields_valid(&self, max_memo_size: usize, max_receiver_size: usize) -> bool { + /// Return a ValidationResult::DecodeFailure this packet data is not an ICS-20 or fails to decode + /// as an ICS-20 packet. + /// Returns ValidationResult::Valid if both the memo and receiver field are valid. + /// Returns ValidationResult containing information on the failed fields if one or both fields fail + /// the validation. + pub fn are_fields_valid( + &self, + max_memo_size: usize, + max_receiver_size: usize, + ) -> ValidationResult { match serde_json::from_slice::(&self.data) { Ok(packet) => { - packet.memo.len() <= max_memo_size && packet.receiver.len() <= max_receiver_size + match ( + packet.memo.len() <= max_memo_size, + packet.receiver.len() <= max_receiver_size, + ) { + (true, true) => ValidationResult::Valid, + (false, true) => ValidationResult::MemoTooBig { + size: max_memo_size, + max: packet.memo.len(), + }, + (true, false) => ValidationResult::ReceiverTooBig { + size: max_receiver_size, + max: packet.receiver.len(), + }, + (false, false) => ValidationResult::BothTooBig { + memo_size: max_memo_size, + memo_max: packet.memo.len(), + receiver_size: max_receiver_size, + receiver_max: packet.receiver.len(), + }, + } } - Err(_) => true, + Err(e) => ValidationResult::DecodeFailure { + details: e.to_string(), + }, } } } diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index fb25935c17..7117f52da8 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -16,7 +16,7 @@ use ibc_relayer_types::core::ics04_channel::msgs::{ acknowledgement::MsgAcknowledgement, chan_close_confirm::MsgChannelCloseConfirm, recv_packet::MsgRecvPacket, timeout::MsgTimeout, timeout_on_close::MsgTimeoutOnClose, }; -use ibc_relayer_types::core::ics04_channel::packet::{Packet, PacketMsgType}; +use ibc_relayer_types::core::ics04_channel::packet::{Packet, PacketMsgType, ValidationResult}; use ibc_relayer_types::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; use ibc_relayer_types::events::{IbcEvent, IbcEventType, WithBlockDataType}; use ibc_relayer_types::signer::Signer; @@ -1399,17 +1399,45 @@ impl RelayPath { let timeout = self.build_timeout_from_send_packet_event(event, dst_info)?; if timeout.is_some() { Ok((None, timeout)) - } else if event - .packet - .are_fields_valid(self.max_memo_size, self.max_receiver_size) - { - Ok((self.build_recv_packet(&event.packet, height)?, None)) } else { - debug!( - "memo and/or receiver field are too big for packet {:#?}", - event.packet - ); - Ok((None, None)) + match event + .packet + .are_fields_valid(self.max_memo_size, self.max_receiver_size) + { + ValidationResult::Valid => { + Ok((self.build_recv_packet(&event.packet, height)?, None)) + } + ValidationResult::MemoTooBig { size, max } => { + trace!("packet: {:#?}", event.packet); + debug!("memo field of packet with sequence {} is too big. Memo field size {size}, maximum configured size {max}", event.packet.sequence); + Ok((None, None)) + } + ValidationResult::ReceiverTooBig { size, max } => { + trace!("packet: {:#?}", event.packet); + debug!("receiver field of packet with sequence {} is too big. Receiver field size {size}, maximum configured size {max}", event.packet.sequence); + Ok((None, None)) + } + ValidationResult::BothTooBig { + memo_size, + memo_max, + receiver_size, + receiver_max, + } => { + trace!("packet: {:#?}", event.packet); + debug!( + "memo and receiver field of packet with sequence {} are too big. \ + Memo field size {memo_size}, maximum configured size {memo_max}, \ + receiver field size {receiver_size}, maximum configured size {receiver_max}", + event.packet.sequence + ); + Ok((None, None)) + } + ValidationResult::DecodeFailure { details } => { + trace!("packet: {:#?}", event.packet); + trace!("failed to decode packet or packet isn't an ICS20 packet, details: {details}"); + Ok((self.build_recv_packet(&event.packet, height)?, None)) + } + } } } From 9d0eb867f726b1476ae9aaeb66216cf7acb757b3 Mon Sep 17 00:00:00 2001 From: Luca Joss <43531661+ljoss17@users.noreply.github.com> Date: Mon, 15 Jan 2024 14:18:02 +0100 Subject: [PATCH 16/25] Apply suggestions from code review Co-authored-by: Romain Ruetschi Signed-off-by: Luca Joss <43531661+ljoss17@users.noreply.github.com> --- crates/relayer/src/link/relay_path.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 7117f52da8..bfa44a68f1 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -1408,12 +1408,12 @@ impl RelayPath { Ok((self.build_recv_packet(&event.packet, height)?, None)) } ValidationResult::MemoTooBig { size, max } => { - trace!("packet: {:#?}", event.packet); + trace!("ICS-20 packet failed fields length check: {:#?}", event.packet); debug!("memo field of packet with sequence {} is too big. Memo field size {size}, maximum configured size {max}", event.packet.sequence); Ok((None, None)) } ValidationResult::ReceiverTooBig { size, max } => { - trace!("packet: {:#?}", event.packet); + trace!("ICS-20 packet failed fields length check: {:#?}", event.packet); debug!("receiver field of packet with sequence {} is too big. Receiver field size {size}, maximum configured size {max}", event.packet.sequence); Ok((None, None)) } @@ -1423,7 +1423,7 @@ impl RelayPath { receiver_size, receiver_max, } => { - trace!("packet: {:#?}", event.packet); + trace!("ICS-20 packet failed fields length check: {:#?}", event.packet); debug!( "memo and receiver field of packet with sequence {} are too big. \ Memo field size {memo_size}, maximum configured size {memo_max}, \ From 451b5781f809fd6bf13b1c66340a67a0e4bc6b27 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Mon, 15 Jan 2024 16:01:27 +0100 Subject: [PATCH 17/25] Refactor ICS20 memo and receiver field configuration and validation --- config.toml | 4 +- crates/relayer-cli/src/commands/clear.rs | 4 +- crates/relayer-cli/src/commands/tx/packet.rs | 8 +- .../src/core/ics04_channel/packet.rs | 64 --------------- crates/relayer/src/config.rs | 13 +-- crates/relayer/src/config/types.rs | 71 ++++++++++++++++ crates/relayer/src/link.rs | 9 ++- crates/relayer/src/link/relay_path.rs | 81 +++++++++---------- crates/relayer/src/worker.rs | 4 +- .../config/fixtures/relayer_conf_example.toml | 2 + .../src/tests/execute_schedule.rs | 4 +- .../src/tests/ordered_channel_clear.rs | 12 +-- .../src/tests/query_packet.rs | 4 +- 13 files changed, 146 insertions(+), 134 deletions(-) diff --git a/config.toml b/config.toml index 59c36d038f..ef6f07320c 100644 --- a/config.toml +++ b/config.toml @@ -83,13 +83,13 @@ auto_register_counterparty_payee = false # If the size of the memo field is bigger than the configured # one, the packet will not be relayed. # [Default: "32KiB"] -#ics20_max_memo_size = "32KiB" +#ics20_max_memo_size = { enabled = true, size = "32KiB" } # Set the maximum size for the receiver field in ICS20 packets. # If the size of the receiver field is bigger than the configured # one, the packet will not be relayed. # [Default: "2KiB"] -#ics20_max_receiver_size = "2KiB" +#ics20_max_receiver_size = { enabled = true, size = "2KiB" } # The REST section defines parameters for Hermes' built-in RESTful API. # https://hermes.informal.systems/rest.html diff --git a/crates/relayer-cli/src/commands/clear.rs b/crates/relayer-cli/src/commands/clear.rs index c60c06e46e..4fde819480 100644 --- a/crates/relayer-cli/src/commands/clear.rs +++ b/crates/relayer-cli/src/commands/clear.rs @@ -135,8 +135,8 @@ impl Runnable for ClearPacketsCmd { let opts = LinkParameters { src_port_id: self.port_id.clone(), src_channel_id: self.channel_id.clone(), - max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes() as usize, - max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes() as usize, + max_memo_size: config.mode.packets.ics20_max_memo_size, + max_receiver_size: config.mode.packets.ics20_max_receiver_size, }; let fwd_link = match Link::new_from_opts(chains.src.clone(), chains.dst, opts, false, false) diff --git a/crates/relayer-cli/src/commands/tx/packet.rs b/crates/relayer-cli/src/commands/tx/packet.rs index fec2f2f62c..0eeaad8df5 100644 --- a/crates/relayer-cli/src/commands/tx/packet.rs +++ b/crates/relayer-cli/src/commands/tx/packet.rs @@ -70,8 +70,8 @@ impl Runnable for TxPacketRecvCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), - max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes() as usize, - max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes() as usize, + max_memo_size: config.mode.packets.ics20_max_memo_size, + max_receiver_size: config.mode.packets.ics20_max_receiver_size, }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) { Ok(link) => link, @@ -153,8 +153,8 @@ impl Runnable for TxPacketAckCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), src_channel_id: self.src_channel_id.clone(), - max_memo_size: config.mode.packets.ics20_max_memo_size.get_bytes() as usize, - max_receiver_size: config.mode.packets.ics20_max_receiver_size.get_bytes() as usize, + max_memo_size: config.mode.packets.ics20_max_memo_size, + max_receiver_size: config.mode.packets.ics20_max_receiver_size, }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false, false) { Ok(link) => link, diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index d63697c718..5aa248973e 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -1,7 +1,6 @@ use serde_derive::{Deserialize, Serialize}; use std::str::FromStr; -use ibc_proto::ibc::applications::transfer::v2::FungibleTokenPacketData as RawPacketData; use ibc_proto::ibc::core::channel::v1::Packet as RawPacket; use super::timeout::TimeoutHeight; @@ -103,27 +102,6 @@ impl core::ops::Add for Sequence { } } -pub enum ValidationResult { - Valid, - MemoTooBig { - size: usize, - max: usize, - }, - ReceiverTooBig { - size: usize, - max: usize, - }, - BothTooBig { - memo_size: usize, - memo_max: usize, - receiver_size: usize, - receiver_max: usize, - }, - DecodeFailure { - details: String, - }, -} - #[derive(Clone, Default, Hash, PartialEq, Eq, Deserialize, Serialize)] pub struct Packet { pub sequence: Sequence, @@ -199,48 +177,6 @@ impl Packet { height_timed_out || timestamp_timed_out } - - /// If this packet data is an ICS-20 packet, check whether or not the memo and receiver - /// fields are under the given max size. - /// Returns `true` if this packet data is not an ICS-20 or fails to decode as an ICS-20 packet. - /// Return a ValidationResult::DecodeFailure this packet data is not an ICS-20 or fails to decode - /// as an ICS-20 packet. - /// Returns ValidationResult::Valid if both the memo and receiver field are valid. - /// Returns ValidationResult containing information on the failed fields if one or both fields fail - /// the validation. - pub fn are_fields_valid( - &self, - max_memo_size: usize, - max_receiver_size: usize, - ) -> ValidationResult { - match serde_json::from_slice::(&self.data) { - Ok(packet) => { - match ( - packet.memo.len() <= max_memo_size, - packet.receiver.len() <= max_receiver_size, - ) { - (true, true) => ValidationResult::Valid, - (false, true) => ValidationResult::MemoTooBig { - size: max_memo_size, - max: packet.memo.len(), - }, - (true, false) => ValidationResult::ReceiverTooBig { - size: max_receiver_size, - max: packet.receiver.len(), - }, - (false, false) => ValidationResult::BothTooBig { - memo_size: max_memo_size, - memo_max: packet.memo.len(), - receiver_size: max_receiver_size, - receiver_max: packet.receiver.len(), - }, - } - } - Err(e) => ValidationResult::DecodeFailure { - details: e.to_string(), - }, - } - } } /// Custom debug output to omit the packet data diff --git a/crates/relayer/src/config.rs b/crates/relayer/src/config.rs index fc16b82f09..1427f1e7e1 100644 --- a/crates/relayer/src/config.rs +++ b/crates/relayer/src/config.rs @@ -26,6 +26,7 @@ use ibc_relayer_types::core::ics24_host::identifier::{ChainId, ChannelId, PortId use ibc_relayer_types::timestamp::ZERO_DURATION; use crate::chain::cosmos::config::CosmosSdkConfig; +use crate::config::types::ics20_field_size_limit::Ics20FieldSizeLimit; use crate::config::types::TrustThreshold; use crate::error::Error as RelayerError; use crate::extension_options::ExtensionOptionDynamicFeeTx; @@ -231,12 +232,12 @@ pub mod default { } } - pub fn ics20_max_memo_size() -> Byte { - Byte::from_bytes(32768) + pub fn ics20_max_memo_size() -> Ics20FieldSizeLimit { + Ics20FieldSizeLimit::new(true, Byte::from_bytes(32768)) } - pub fn ics20_max_receiver_size() -> Byte { - Byte::from_bytes(2048) + pub fn ics20_max_receiver_size() -> Ics20FieldSizeLimit { + Ics20FieldSizeLimit::new(true, Byte::from_bytes(2048)) } } @@ -409,9 +410,9 @@ pub struct Packets { #[serde(default = "default::auto_register_counterparty_payee")] pub auto_register_counterparty_payee: bool, #[serde(default = "default::ics20_max_memo_size")] - pub ics20_max_memo_size: Byte, + pub ics20_max_memo_size: Ics20FieldSizeLimit, #[serde(default = "default::ics20_max_receiver_size")] - pub ics20_max_receiver_size: Byte, + pub ics20_max_receiver_size: Ics20FieldSizeLimit, } impl Default for Packets { diff --git a/crates/relayer/src/config/types.rs b/crates/relayer/src/config/types.rs index 5b14b28a9d..f2ccbd0ca9 100644 --- a/crates/relayer/src/config/types.rs +++ b/crates/relayer/src/config/types.rs @@ -265,6 +265,77 @@ pub mod memo { } } +pub mod ics20_field_size_limit { + use byte_unit::Byte; + use serde_derive::{Deserialize, Serialize}; + use std::fmt::Display; + + use ibc_proto::ibc::applications::transfer::v2::FungibleTokenPacketData as RawPacketData; + + pub enum ValidationResult { + Valid, + Invalid { size: usize, max: usize }, + } + + impl Display for ValidationResult { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Valid => write!(f, "field is valid"), + Self::Invalid { size, max } => { + write!(f, "field is not valid, size `{size}`, max limit `{max}`") + } + } + } + } + + #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] + pub struct Ics20FieldSizeLimit { + enabled: bool, + size: Byte, + } + + impl Ics20FieldSizeLimit { + pub fn new(enabled: bool, size: Byte) -> Self { + Self { enabled, size } + } + + /// If the limit is disabled consider the field as valid. + /// If the limit is enabled assert the field is smaller or equal + /// to the configured value. + pub fn is_memo_field_valid(&self, packet_data: &RawPacketData) -> ValidationResult { + if self.enabled { + let size_limit = self.size.get_bytes() as usize; + if packet_data.memo.len() <= size_limit { + ValidationResult::Valid + } else { + ValidationResult::Invalid { + size: packet_data.memo.len(), + max: size_limit, + } + } + } else { + ValidationResult::Valid + } + } + + pub fn is_receiver_field_valid(&self, packet_data: &RawPacketData) -> ValidationResult { + if self.enabled { + let size_limit = self.size.get_bytes() as usize; + if packet_data.receiver.len() <= size_limit { + ValidationResult::Valid + } else { + ValidationResult::Invalid { + size: packet_data.receiver.len(), + max: size_limit, + } + } + } else { + ValidationResult::Valid + } + } + } +} + #[cfg(test)] #[allow(dead_code)] // the fields of the structs defined below are never accessed mod tests { diff --git a/crates/relayer/src/link.rs b/crates/relayer/src/link.rs index 3124740cc9..4c0fc849b2 100644 --- a/crates/relayer/src/link.rs +++ b/crates/relayer/src/link.rs @@ -5,11 +5,14 @@ use ibc_relayer_types::core::{ }; use tracing::info; -use crate::chain::requests::{QueryChannelRequest, QueryHeight}; use crate::chain::{counterparty::check_channel_counterparty, requests::QueryConnectionRequest}; use crate::chain::{handle::ChainHandle, requests::IncludeProof}; use crate::channel::{Channel, ChannelSide}; use crate::link::error::LinkError; +use crate::{ + chain::requests::{QueryChannelRequest, QueryHeight}, + config::types::ics20_field_size_limit::Ics20FieldSizeLimit, +}; pub mod cli; pub mod error; @@ -33,8 +36,8 @@ pub use relay_path::{RelayPath, Resubmit}; pub struct LinkParameters { pub src_port_id: PortId, pub src_channel_id: ChannelId, - pub max_memo_size: usize, - pub max_receiver_size: usize, + pub max_memo_size: Ics20FieldSizeLimit, + pub max_receiver_size: Ics20FieldSizeLimit, } pub struct Link { diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index bfa44a68f1..4a8c6d3def 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -4,6 +4,7 @@ use std::ops::Sub; use std::time::{Duration, Instant}; use ibc_proto::google::protobuf::Any; +use ibc_proto::ibc::applications::transfer::v2::FungibleTokenPacketData as RawPacketData; use itertools::Itertools; use tracing::{debug, error, info, span, trace, warn, Level}; @@ -16,7 +17,7 @@ use ibc_relayer_types::core::ics04_channel::msgs::{ acknowledgement::MsgAcknowledgement, chan_close_confirm::MsgChannelCloseConfirm, recv_packet::MsgRecvPacket, timeout::MsgTimeout, timeout_on_close::MsgTimeoutOnClose, }; -use ibc_relayer_types::core::ics04_channel::packet::{Packet, PacketMsgType, ValidationResult}; +use ibc_relayer_types::core::ics04_channel::packet::{Packet, PacketMsgType}; use ibc_relayer_types::core::ics24_host::identifier::{ChannelId, ClientId, ConnectionId, PortId}; use ibc_relayer_types::events::{IbcEvent, IbcEventType, WithBlockDataType}; use ibc_relayer_types::signer::Signer; @@ -42,6 +43,8 @@ use crate::chain::tracking::TrackedMsgs; use crate::chain::tracking::TrackingId; use crate::channel::error::ChannelError; use crate::channel::Channel; +use crate::config::types::ics20_field_size_limit::Ics20FieldSizeLimit; +use crate::config::types::ics20_field_size_limit::ValidationResult; use crate::event::source::EventBatch; use crate::event::IbcEventWithHeight; use crate::foreign_client::{ForeignClient, ForeignClientError}; @@ -110,8 +113,8 @@ pub struct RelayPath { pending_txs_src: PendingTxs, pending_txs_dst: PendingTxs, - pub max_memo_size: usize, - pub max_receiver_size: usize, + pub max_memo_size: Ics20FieldSizeLimit, + pub max_receiver_size: Ics20FieldSizeLimit, } impl RelayPath { @@ -1399,45 +1402,15 @@ impl RelayPath { let timeout = self.build_timeout_from_send_packet_event(event, dst_info)?; if timeout.is_some() { Ok((None, timeout)) + } else if are_ics20_fields_valid( + &event.packet.data, + self.max_memo_size, + self.max_receiver_size, + ) { + Ok((self.build_recv_packet(&event.packet, height)?, None)) } else { - match event - .packet - .are_fields_valid(self.max_memo_size, self.max_receiver_size) - { - ValidationResult::Valid => { - Ok((self.build_recv_packet(&event.packet, height)?, None)) - } - ValidationResult::MemoTooBig { size, max } => { - trace!("ICS-20 packet failed fields length check: {:#?}", event.packet); - debug!("memo field of packet with sequence {} is too big. Memo field size {size}, maximum configured size {max}", event.packet.sequence); - Ok((None, None)) - } - ValidationResult::ReceiverTooBig { size, max } => { - trace!("ICS-20 packet failed fields length check: {:#?}", event.packet); - debug!("receiver field of packet with sequence {} is too big. Receiver field size {size}, maximum configured size {max}", event.packet.sequence); - Ok((None, None)) - } - ValidationResult::BothTooBig { - memo_size, - memo_max, - receiver_size, - receiver_max, - } => { - trace!("ICS-20 packet failed fields length check: {:#?}", event.packet); - debug!( - "memo and receiver field of packet with sequence {} are too big. \ - Memo field size {memo_size}, maximum configured size {memo_max}, \ - receiver field size {receiver_size}, maximum configured size {receiver_max}", - event.packet.sequence - ); - Ok((None, None)) - } - ValidationResult::DecodeFailure { details } => { - trace!("packet: {:#?}", event.packet); - trace!("failed to decode packet or packet isn't an ICS20 packet, details: {details}"); - Ok((self.build_recv_packet(&event.packet, height)?, None)) - } - } + trace!("packet: {:#?}", event.packet); + Ok((None, None)) } } @@ -1938,3 +1911,29 @@ impl RelayPath { } } } + +fn are_ics20_fields_valid( + data: &[u8], + ics20_memo_limit: Ics20FieldSizeLimit, + ics20_receiver_limit: Ics20FieldSizeLimit, +) -> bool { + match serde_json::from_slice::(data) { + Ok(packet_data) => { + match ( + ics20_memo_limit.is_memo_field_valid(&packet_data), + ics20_receiver_limit.is_memo_field_valid(&packet_data), + ) { + (ValidationResult::Valid, ValidationResult::Valid) => true, + (memo_validity, receiver_validity) => { + debug!("memo validity: {memo_validity}"); + debug!("receiver validity: {receiver_validity}"); + false + } + } + } + Err(e) => { + debug!("failed to decode ICS20 packet data with error `{e}`"); + false + } + } +} diff --git a/crates/relayer/src/worker.rs b/crates/relayer/src/worker.rs index e8054ba033..400ff811af 100644 --- a/crates/relayer/src/worker.rs +++ b/crates/relayer/src/worker.rs @@ -117,8 +117,8 @@ pub fn spawn_worker_tasks( LinkParameters { src_port_id: path.src_port_id.clone(), src_channel_id: path.src_channel_id.clone(), - max_memo_size: packets_config.ics20_max_memo_size.get_bytes() as usize, - max_receiver_size: packets_config.ics20_max_receiver_size.get_bytes() as usize, + max_memo_size: packets_config.ics20_max_memo_size, + max_receiver_size: packets_config.ics20_max_receiver_size, }, packets_config.tx_confirmation, packets_config.auto_register_counterparty_payee, diff --git a/crates/relayer/tests/config/fixtures/relayer_conf_example.toml b/crates/relayer/tests/config/fixtures/relayer_conf_example.toml index 3665bb595f..f864efecda 100644 --- a/crates/relayer/tests/config/fixtures/relayer_conf_example.toml +++ b/crates/relayer/tests/config/fixtures/relayer_conf_example.toml @@ -19,6 +19,8 @@ enabled = true clear_interval = 100 clear_on_start = true tx_confirmation = true +ics20_max_memo_size = { enabled = true, size = "32KiB" } +ics20_max_receiver_size = { enabled = true, size = "2KiB" } [[chains]] type = "CosmosSdk" diff --git a/tools/integration-test/src/tests/execute_schedule.rs b/tools/integration-test/src/tests/execute_schedule.rs index 8a21d418ce..cd14968f33 100644 --- a/tools/integration-test/src/tests/execute_schedule.rs +++ b/tools/integration-test/src/tests/execute_schedule.rs @@ -47,8 +47,8 @@ impl BinaryChannelTest for ExecuteScheduleTest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.get_bytes() as usize, - max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes() as usize, + max_memo_size: packet_config.ics20_max_memo_size, + max_receiver_size: packet_config.ics20_max_receiver_size, }; let chain_a_link = Link::new_from_opts( diff --git a/tools/integration-test/src/tests/ordered_channel_clear.rs b/tools/integration-test/src/tests/ordered_channel_clear.rs index 27b5328b34..63c5373ccb 100644 --- a/tools/integration-test/src/tests/ordered_channel_clear.rs +++ b/tools/integration-test/src/tests/ordered_channel_clear.rs @@ -119,8 +119,8 @@ impl BinaryChannelTest for OrderedChannelClearTest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.get_bytes() as usize, - max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes() as usize, + max_memo_size: packet_config.ics20_max_memo_size, + max_receiver_size: packet_config.ics20_max_receiver_size, }; let chain_a_link = Link::new_from_opts( @@ -134,8 +134,8 @@ impl BinaryChannelTest for OrderedChannelClearTest { let chain_b_link_opts = LinkParameters { src_port_id: channel.port_b.clone().into_value(), src_channel_id: channel.channel_id_b.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.get_bytes() as usize, - max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes() as usize, + max_memo_size: packet_config.ics20_max_memo_size, + max_receiver_size: packet_config.ics20_max_receiver_size, }; let chain_b_link = Link::new_from_opts( @@ -270,8 +270,8 @@ impl BinaryChannelTest for OrderedChannelClearEqualCLITest { let chain_a_link_opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.into_value(), - max_memo_size: packet_config.ics20_max_memo_size.get_bytes() as usize, - max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes() as usize, + max_memo_size: packet_config.ics20_max_memo_size, + max_receiver_size: packet_config.ics20_max_receiver_size, }; let chain_a_link = Link::new_from_opts( diff --git a/tools/integration-test/src/tests/query_packet.rs b/tools/integration-test/src/tests/query_packet.rs index ef59cc3840..b4f76d9b5c 100644 --- a/tools/integration-test/src/tests/query_packet.rs +++ b/tools/integration-test/src/tests/query_packet.rs @@ -60,8 +60,8 @@ impl BinaryChannelTest for QueryPacketPendingTest { let opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), - max_memo_size: packet_config.ics20_max_memo_size.get_bytes() as usize, - max_receiver_size: packet_config.ics20_max_receiver_size.get_bytes() as usize, + max_memo_size: packet_config.ics20_max_memo_size, + max_receiver_size: packet_config.ics20_max_receiver_size, }; let link = Link::new_from_opts( chains.handle_a().clone(), From dbbc53db16e829e866a7d57ed11a4e5d078e2598 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Mon, 15 Jan 2024 16:49:52 +0100 Subject: [PATCH 18/25] Improve documentation of memo and receiver filter configuration --- config.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config.toml b/config.toml index ef6f07320c..2e361b17d0 100644 --- a/config.toml +++ b/config.toml @@ -82,12 +82,14 @@ auto_register_counterparty_payee = false # Set the maximum size for the memo field in ICS20 packets. # If the size of the memo field is bigger than the configured # one, the packet will not be relayed. +# The filter can be disabled by setting `enabled = false`. # [Default: "32KiB"] #ics20_max_memo_size = { enabled = true, size = "32KiB" } # Set the maximum size for the receiver field in ICS20 packets. # If the size of the receiver field is bigger than the configured # one, the packet will not be relayed. +# The filter can be disabled by setting `enabled = false`. # [Default: "2KiB"] #ics20_max_receiver_size = { enabled = true, size = "2KiB" } From 3401a19ca1b670bff4513f7898a3141eeebdaa30 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Mon, 15 Jan 2024 16:58:16 +0100 Subject: [PATCH 19/25] Fix clippy error --- tools/integration-test/src/tests/clear_packet.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/integration-test/src/tests/clear_packet.rs b/tools/integration-test/src/tests/clear_packet.rs index 4a02ba0d14..6da6147806 100644 --- a/tools/integration-test/src/tests/clear_packet.rs +++ b/tools/integration-test/src/tests/clear_packet.rs @@ -441,11 +441,12 @@ impl BinaryChannelTest for ClearPacketSequencesTest { fn run( &self, _config: &TestConfig, - _relayer: RelayerDriver, + relayer: RelayerDriver, chains: ConnectedChains, channel: ConnectedChannel, ) -> Result<(), Error> { const NUM_TRANSFERS: usize = 20; + let packet_config = relayer.config.mode.packets; let denom_a = chains.node_a.denom(); @@ -484,6 +485,8 @@ impl BinaryChannelTest for ClearPacketSequencesTest { let opts = LinkParameters { src_port_id: channel.port_a.clone().into_value(), src_channel_id: channel.channel_id_a.clone().into_value(), + max_memo_size: packet_config.ics20_max_memo_size, + max_receiver_size: packet_config.ics20_max_receiver_size, }; // Clear all even packets From 657dde1d90d9b9095d4b73bded39e71960e75459 Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Tue, 16 Jan 2024 08:22:51 +0100 Subject: [PATCH 20/25] Add test for memo filter --- Cargo.lock | 1 + crates/relayer/src/link/relay_path.rs | 4 +- tools/integration-test/Cargo.toml | 5 + .../src/tests/ics20_filter/memo.rs | 130 ++++++++++++++++++ .../src/tests/ics20_filter/mod.rs | 1 + tools/integration-test/src/tests/mod.rs | 1 + 6 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 tools/integration-test/src/tests/ics20_filter/memo.rs create mode 100644 tools/integration-test/src/tests/ics20_filter/mod.rs diff --git a/Cargo.lock b/Cargo.lock index b3dda08491..227eee23f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1361,6 +1361,7 @@ dependencies = [ name = "ibc-integration-test" version = "0.26.4" dependencies = [ + "byte-unit", "http", "ibc-relayer", "ibc-relayer-types", diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 4a8c6d3def..ca76b0a012 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -1921,7 +1921,7 @@ fn are_ics20_fields_valid( Ok(packet_data) => { match ( ics20_memo_limit.is_memo_field_valid(&packet_data), - ics20_receiver_limit.is_memo_field_valid(&packet_data), + ics20_receiver_limit.is_receiver_field_valid(&packet_data), ) { (ValidationResult::Valid, ValidationResult::Valid) => true, (memo_validity, receiver_validity) => { @@ -1933,7 +1933,7 @@ fn are_ics20_fields_valid( } Err(e) => { debug!("failed to decode ICS20 packet data with error `{e}`"); - false + true } } } diff --git a/tools/integration-test/Cargo.toml b/tools/integration-test/Cargo.toml index e93ec8ece7..da2cb19f1e 100644 --- a/tools/integration-test/Cargo.toml +++ b/tools/integration-test/Cargo.toml @@ -56,3 +56,8 @@ version = "0.34.0" [dependencies.tendermint-rpc] version = "0.34.0" features = ["http-client"] + +[dependencies.byte-unit] +version = "4.0.19" +default-features = false +features = ["serde"] \ No newline at end of file diff --git a/tools/integration-test/src/tests/ics20_filter/memo.rs b/tools/integration-test/src/tests/ics20_filter/memo.rs new file mode 100644 index 0000000000..4d9f43a37d --- /dev/null +++ b/tools/integration-test/src/tests/ics20_filter/memo.rs @@ -0,0 +1,130 @@ +use byte_unit::Byte; + +use ibc_relayer::config::types::ics20_field_size_limit::Ics20FieldSizeLimit; +use ibc_test_framework::prelude::*; + +#[test] +fn test_memo_filter() -> Result<(), Error> { + run_binary_channel_test(&IbcMemoFilterTest) +} + +const MEMO_SIZE_LIMIT: usize = 2000; + +pub struct IbcMemoFilterTest; + +impl TestOverrides for IbcMemoFilterTest { + fn modify_relayer_config(&self, config: &mut Config) { + config.mode.packets.ics20_max_memo_size = + Ics20FieldSizeLimit::new(true, Byte::from_bytes(MEMO_SIZE_LIMIT as u64)); + + config.mode.clients.misbehaviour = false; + } +} + +impl BinaryChannelTest for IbcMemoFilterTest { + fn run( + &self, + _config: &TestConfig, + _relayer: RelayerDriver, + chains: ConnectedChains, + channel: ConnectedChannel, + ) -> Result<(), Error> { + let denom_a = chains.node_a.denom(); + + let wallet_a = chains.node_a.wallets().user1().cloned(); + let wallet_b = chains.node_b.wallets().user1().cloned(); + + let balance_a = chains + .node_a + .chain_driver() + .query_balance(&wallet_a.address(), &denom_a)?; + + let a_to_b_amount = 23456u128; + + info!( + "Sending invalid IBC transfer from chain {} to chain {} with amount of {} {}", + chains.chain_id_a(), + chains.chain_id_b(), + a_to_b_amount, + denom_a + ); + + // Create a memo bigger than the allowed limit + let memo = "a".repeat(MEMO_SIZE_LIMIT + 1); + + chains + .node_a + .chain_driver() + .ibc_transfer_token_with_memo_and_timeout( + &channel.port_a.as_ref(), + &channel.channel_id_a.as_ref(), + &wallet_a.as_ref(), + &wallet_b.address(), + &denom_a.with_amount(a_to_b_amount).as_ref(), + Some(memo), + None, + )?; + + // Wait a bit before asserting that the transaction has not been relayed + sleep(Duration::from_secs(10)); + + info!("Assert that the IBC transfer was filtered"); + + let denom_b = derive_ibc_denom( + &channel.port_b.as_ref(), + &channel.channel_id_b.as_ref(), + &denom_a, + )?; + + // The sender tokens will be escrowed since the packet will not have timed out + chains + .node_a + .chain_driver() + .assert_eventual_wallet_amount(&wallet_a.address(), &(balance_a.clone() - a_to_b_amount).as_ref())?; + + // + chains.node_b.chain_driver().assert_eventual_wallet_amount( + &wallet_b.address(), + &denom_b.with_amount(0u64).as_ref(), + )?; + + // Retry the IBC transfer without the memo field + chains + .node_a + .chain_driver() + .ibc_transfer_token_with_memo_and_timeout( + &channel.port_a.as_ref(), + &channel.channel_id_a.as_ref(), + &wallet_a.as_ref(), + &wallet_b.address(), + &denom_a.with_amount(a_to_b_amount).as_ref(), + None, + None, + )?; + + info!( + "Waiting for user on chain B to receive IBC transferred amount of {}", + a_to_b_amount + ); + + // The sender tokens from the first transaction will still be + // escrowed since the packet will not have timed out + chains.node_a.chain_driver().assert_eventual_wallet_amount( + &wallet_a.address(), + &(balance_a - a_to_b_amount - a_to_b_amount).as_ref(), + )?; + + chains.node_b.chain_driver().assert_eventual_wallet_amount( + &wallet_b.address(), + &denom_b.with_amount(a_to_b_amount).as_ref(), + )?; + + info!( + "successfully performed IBC transfer from chain {} to chain {}", + chains.chain_id_a(), + chains.chain_id_b(), + ); + + Ok(()) + } +} diff --git a/tools/integration-test/src/tests/ics20_filter/mod.rs b/tools/integration-test/src/tests/ics20_filter/mod.rs new file mode 100644 index 0000000000..80d0261cba --- /dev/null +++ b/tools/integration-test/src/tests/ics20_filter/mod.rs @@ -0,0 +1 @@ +pub mod memo; diff --git a/tools/integration-test/src/tests/mod.rs b/tools/integration-test/src/tests/mod.rs index 225c2bab19..b56c22b49d 100644 --- a/tools/integration-test/src/tests/mod.rs +++ b/tools/integration-test/src/tests/mod.rs @@ -18,6 +18,7 @@ pub mod denom_trace; pub mod error_events; pub mod execute_schedule; pub mod handshake_on_start; +pub mod ics20_filter; pub mod memo; pub mod python; pub mod query_packet; From 9db94e04a857f6adab92de4de97c915f6b093495 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi <106849+romac@users.noreply.github.com> Date: Tue, 16 Jan 2024 15:48:49 +0100 Subject: [PATCH 21/25] Formatting --- tools/integration-test/src/tests/ics20_filter/memo.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/integration-test/src/tests/ics20_filter/memo.rs b/tools/integration-test/src/tests/ics20_filter/memo.rs index 4d9f43a37d..8cf6c983d7 100644 --- a/tools/integration-test/src/tests/ics20_filter/memo.rs +++ b/tools/integration-test/src/tests/ics20_filter/memo.rs @@ -77,12 +77,12 @@ impl BinaryChannelTest for IbcMemoFilterTest { )?; // The sender tokens will be escrowed since the packet will not have timed out - chains - .node_a - .chain_driver() - .assert_eventual_wallet_amount(&wallet_a.address(), &(balance_a.clone() - a_to_b_amount).as_ref())?; + chains.node_a.chain_driver().assert_eventual_wallet_amount( + &wallet_a.address(), + &(balance_a.clone() - a_to_b_amount).as_ref(), + )?; - // + // chains.node_b.chain_driver().assert_eventual_wallet_amount( &wallet_b.address(), &denom_b.with_amount(0u64).as_ref(), From bfe93f83a7973ad2c6adc6507d037a3e3e20f4cd Mon Sep 17 00:00:00 2001 From: Romain Ruetschi <106849+romac@users.noreply.github.com> Date: Tue, 16 Jan 2024 15:49:48 +0100 Subject: [PATCH 22/25] Revert formatting change to otherwise untouched file --- crates/relayer-types/src/core/ics04_channel/packet.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/relayer-types/src/core/ics04_channel/packet.rs b/crates/relayer-types/src/core/ics04_channel/packet.rs index b248d53e37..316227c08f 100644 --- a/crates/relayer-types/src/core/ics04_channel/packet.rs +++ b/crates/relayer-types/src/core/ics04_channel/packet.rs @@ -1,6 +1,7 @@ -use serde_derive::{Deserialize, Serialize}; use std::str::FromStr; +use serde_derive::{Deserialize, Serialize}; + use ibc_proto::ibc::core::channel::v1::Packet as RawPacket; use super::timeout::TimeoutHeight; From 06fe76ac0b3948ab8cdd316986370038c20b9450 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi <106849+romac@users.noreply.github.com> Date: Tue, 16 Jan 2024 15:59:32 +0100 Subject: [PATCH 23/25] Simplify ICS-20 checks a little bit --- crates/relayer/src/config/types.rs | 28 ++++++--------------------- crates/relayer/src/link/relay_path.rs | 25 +++++++++++++++--------- 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/crates/relayer/src/config/types.rs b/crates/relayer/src/config/types.rs index f2ccbd0ca9..eec382a01d 100644 --- a/crates/relayer/src/config/types.rs +++ b/crates/relayer/src/config/types.rs @@ -270,8 +270,6 @@ pub mod ics20_field_size_limit { use serde_derive::{Deserialize, Serialize}; use std::fmt::Display; - use ibc_proto::ibc::applications::transfer::v2::FungibleTokenPacketData as RawPacketData; - pub enum ValidationResult { Valid, Invalid { size: usize, max: usize }, @@ -280,9 +278,10 @@ pub mod ics20_field_size_limit { impl Display for ValidationResult { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Valid => write!(f, "field is valid"), + Self::Valid => write!(f, "valid"), + Self::Invalid { size, max } => { - write!(f, "field is not valid, size `{size}`, max limit `{max}`") + write!(f, "invalid, size `{size}` is greater than max `{max}`") } } } @@ -302,30 +301,15 @@ pub mod ics20_field_size_limit { /// If the limit is disabled consider the field as valid. /// If the limit is enabled assert the field is smaller or equal /// to the configured value. - pub fn is_memo_field_valid(&self, packet_data: &RawPacketData) -> ValidationResult { + pub fn check_field_size(&self, field: &str) -> ValidationResult { if self.enabled { let size_limit = self.size.get_bytes() as usize; - if packet_data.memo.len() <= size_limit { - ValidationResult::Valid - } else { - ValidationResult::Invalid { - size: packet_data.memo.len(), - max: size_limit, - } - } - } else { - ValidationResult::Valid - } - } - pub fn is_receiver_field_valid(&self, packet_data: &RawPacketData) -> ValidationResult { - if self.enabled { - let size_limit = self.size.get_bytes() as usize; - if packet_data.receiver.len() <= size_limit { + if field.len() <= size_limit { ValidationResult::Valid } else { ValidationResult::Invalid { - size: packet_data.receiver.len(), + size: field.len(), max: size_limit, } } diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index ca76b0a012..4b5b10bc84 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -1399,10 +1399,12 @@ impl RelayPath { "dst_channel": event.packet.destination_channel, } ); + let timeout = self.build_timeout_from_send_packet_event(event, dst_info)?; + if timeout.is_some() { Ok((None, timeout)) - } else if are_ics20_fields_valid( + } else if check_ics20_fields_size( &event.packet.data, self.max_memo_size, self.max_receiver_size, @@ -1912,27 +1914,32 @@ impl RelayPath { } } -fn are_ics20_fields_valid( +#[tracing::instrument(skip(data))] +fn check_ics20_fields_size( data: &[u8], - ics20_memo_limit: Ics20FieldSizeLimit, - ics20_receiver_limit: Ics20FieldSizeLimit, + memo_limit: Ics20FieldSizeLimit, + receiver_limit: Ics20FieldSizeLimit, ) -> bool { match serde_json::from_slice::(data) { Ok(packet_data) => { match ( - ics20_memo_limit.is_memo_field_valid(&packet_data), - ics20_receiver_limit.is_receiver_field_valid(&packet_data), + memo_limit.check_field_size(&packet_data.memo), + receiver_limit.check_field_size(&packet_data.receiver), ) { (ValidationResult::Valid, ValidationResult::Valid) => true, + (memo_validity, receiver_validity) => { - debug!("memo validity: {memo_validity}"); - debug!("receiver validity: {receiver_validity}"); + debug!("found invalid ICS-20 packet data, not relaying packet!"); + debug!(" ICS-20 memo: {memo_validity}"); + debug!(" ICS-20 receiver: {receiver_validity}"); + false } } } Err(e) => { - debug!("failed to decode ICS20 packet data with error `{e}`"); + trace!("failed to decode ICS20 packet data with error `{e}`"); + true } } From 3d5687eb534dc72a94c08e95e7afdfb6492d3d22 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi <106849+romac@users.noreply.github.com> Date: Tue, 16 Jan 2024 16:06:48 +0100 Subject: [PATCH 24/25] Perform ICS-20 checks on all packet events --- crates/relayer/src/link/relay_path.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/relayer/src/link/relay_path.rs b/crates/relayer/src/link/relay_path.rs index 4b5b10bc84..2a1d4307be 100644 --- a/crates/relayer/src/link/relay_path.rs +++ b/crates/relayer/src/link/relay_path.rs @@ -560,6 +560,18 @@ impl RelayPath { for event_with_height in input { trace!(event = %event_with_height, "processing event"); + if let Some(packet) = event_with_height.event.packet() { + // If the event is a ICS-04 packet event, and the packet contains ICS-20 + // packet data, check that the ICS-20 fields are within the configured limits. + if !check_ics20_fields_size( + &packet.data, + self.max_memo_size, + self.max_receiver_size, + ) { + continue; + } + } + let (dst_msg, src_msg) = match &event_with_height.event { IbcEvent::CloseInitChannel(_) => ( self.build_chan_close_confirm_from_event(event_with_height)?, @@ -1374,6 +1386,7 @@ impl RelayPath { dst_info: &ChainStatus, ) -> Result, LinkError> { let packet = event.packet.clone(); + if self .dst_channel(QueryHeight::Specific(dst_info.height))? .state_matches(&ChannelState::Closed) @@ -1404,15 +1417,8 @@ impl RelayPath { if timeout.is_some() { Ok((None, timeout)) - } else if check_ics20_fields_size( - &event.packet.data, - self.max_memo_size, - self.max_receiver_size, - ) { - Ok((self.build_recv_packet(&event.packet, height)?, None)) } else { - trace!("packet: {:#?}", event.packet); - Ok((None, None)) + Ok((self.build_recv_packet(&event.packet, height)?, None)) } } From 41fe478acf88938de4b828487f9c13fb6929d4bc Mon Sep 17 00:00:00 2001 From: Luca Joss Date: Tue, 16 Jan 2024 16:30:44 +0100 Subject: [PATCH 25/25] Improve comment in memo filter test --- tools/integration-test/src/tests/ics20_filter/memo.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/integration-test/src/tests/ics20_filter/memo.rs b/tools/integration-test/src/tests/ics20_filter/memo.rs index 8cf6c983d7..0ec207af37 100644 --- a/tools/integration-test/src/tests/ics20_filter/memo.rs +++ b/tools/integration-test/src/tests/ics20_filter/memo.rs @@ -82,7 +82,8 @@ impl BinaryChannelTest for IbcMemoFilterTest { &(balance_a.clone() - a_to_b_amount).as_ref(), )?; - // + // The receiver will not have received the tokens since the packet should be + // filtered chains.node_b.chain_driver().assert_eventual_wallet_amount( &wallet_b.address(), &denom_b.with_amount(0u64).as_ref(),