From 24c2d7b64d29dbee4c2d6329dd4e5dcc382d2ebf Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 6 Apr 2022 18:39:14 +0530 Subject: [PATCH 01/10] Remodel ChannelId as newtype u64 --- modules/src/core/ics24_host/error.rs | 14 ++++++- modules/src/core/ics24_host/identifier.rs | 46 +++++++++++------------ 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/modules/src/core/ics24_host/error.rs b/modules/src/core/ics24_host/error.rs index 8de566e646..de71ad7974 100644 --- a/modules/src/core/ics24_host/error.rs +++ b/modules/src/core/ics24_host/error.rs @@ -1,7 +1,10 @@ -use crate::prelude::*; -use flex_error::define_error; +use core::num::ParseIntError; + +use flex_error::{define_error, TraceError}; use serde::Serialize; +use crate::prelude::*; + define_error! { #[derive(Debug, PartialEq, Eq, Serialize)] ValidationError { @@ -29,6 +32,13 @@ define_error! { { id: String } | e | { format_args!("chain identifiers are expected to be in epoch format {0}", e.id) }, + ChannelIdInvalidFormat + | _ | { "channel identifiers are expected to be in `channel-{N}` format" }, + + ChannelIdParseFailure + [ TraceError ] + | _ | { "failed to parse channel identifier" }, + InvalidCounterpartyChannelId |_| { "Invalid channel id in counterparty" } } diff --git a/modules/src/core/ics24_host/identifier.rs b/modules/src/core/ics24_host/identifier.rs index 8746d26d80..21985c28ae 100644 --- a/modules/src/core/ics24_host/identifier.rs +++ b/modules/src/core/ics24_host/identifier.rs @@ -337,7 +337,7 @@ impl Default for PortId { } #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] -pub struct ChannelId(String); +pub struct ChannelId(u64); impl ChannelId { /// Builds a new channel identifier. Like client and connection identifiers, channel ids are @@ -352,29 +352,27 @@ impl ChannelId { /// assert_eq!(&chan_id, "channel-27"); /// ``` pub fn new(counter: u64) -> Self { - let id = format!("{}-{}", Self::prefix(), counter); - Self::from_str(id.as_str()).unwrap() + Self(counter) } - pub fn prefix() -> &'static str { - "channel" + pub fn counter(&self) -> u64 { + self.0 } - /// Get this identifier as a borrowed `&str` - pub fn as_str(&self) -> &str { - &self.0 + const fn prefix() -> &'static str { + "channel-" } - /// Get this identifier as a borrowed byte slice - pub fn as_bytes(&self) -> &[u8] { - self.0.as_bytes() + fn is_valid_fmt(chan_id_str: &str) -> bool { + let re = safe_regex::regex!(br"^channel-[0-9]{1,20}$"); + re.is_match(chan_id_str.as_bytes()) } } /// This implementation provides a `to_string` method. impl Display for ChannelId { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), fmt::Error> { - write!(f, "{}", self.0) + write!(f, "{}{}", Self::prefix(), self.0) } } @@ -382,13 +380,18 @@ impl FromStr for ChannelId { type Err = ValidationError; fn from_str(s: &str) -> Result { - validate_channel_identifier(s).map(|_| Self(s.to_string())) - } -} + if !Self::is_valid_fmt(s) { + return Err(ValidationError::channel_id_invalid_format()); + } -impl AsRef for ChannelId { - fn as_ref(&self) -> &str { - self.0.as_str() + let counter = { + let s = s + .strip_prefix(Self::prefix()) + .expect("missing prefix although `is_valid_fmt()`"); + u64::from_str(s).map_err(ValidationError::channel_id_parse_failure)? + }; + + Ok(Self(counter)) } } @@ -398,13 +401,6 @@ impl Default for ChannelId { } } -/// Equality check against string literal (satisfies &ChannelId == &str). -impl PartialEq for ChannelId { - fn eq(&self, other: &str) -> bool { - self.as_str().eq(other) - } -} - /// A pair of [`PortId`] and [`ChannelId`] are used together for sending IBC packets. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] pub struct PortChannelId { From 72b7b769180c1f33cea5446f98be69da5dec2c22 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 6 Apr 2022 18:39:42 +0530 Subject: [PATCH 02/10] Fix AsRef usages --- modules/src/core/ics04_channel/channel.rs | 2 +- modules/src/core/ics04_channel/events.rs | 4 ++-- modules/src/core/ics04_channel/msgs/chan_open_try.rs | 2 +- relayer/src/config/filter.rs | 8 ++++---- tools/test-framework/src/chain/driver/transfer.rs | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/modules/src/core/ics04_channel/channel.rs b/modules/src/core/ics04_channel/channel.rs index b33208a9e8..0e8087e291 100644 --- a/modules/src/core/ics04_channel/channel.rs +++ b/modules/src/core/ics04_channel/channel.rs @@ -292,7 +292,7 @@ impl From for RawCounterparty { port_id: value.port_id.as_str().to_string(), channel_id: value .channel_id - .map_or_else(|| "".to_string(), |v| v.as_str().to_string()), + .map_or_else(|| "".to_string(), |v| v.to_string()), } } } diff --git a/modules/src/core/ics04_channel/events.rs b/modules/src/core/ics04_channel/events.rs index ebb5117e2f..f7204b1ca5 100644 --- a/modules/src/core/ics04_channel/events.rs +++ b/modules/src/core/ics04_channel/events.rs @@ -256,7 +256,7 @@ impl From for Vec { if let Some(channel_id) = a.channel_id { let channel_id = Tag { key: CHANNEL_ID_ATTRIBUTE_KEY.parse().unwrap(), - value: channel_id.as_str().parse().unwrap(), + value: channel_id.to_string().parse().unwrap(), }; attributes.push(channel_id); } @@ -273,7 +273,7 @@ impl From for Vec { if let Some(channel_id) = a.counterparty_channel_id { let channel_id = Tag { key: COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY.parse().unwrap(), - value: channel_id.as_str().parse().unwrap(), + value: channel_id.to_string().parse().unwrap(), }; attributes.push(channel_id); } diff --git a/modules/src/core/ics04_channel/msgs/chan_open_try.rs b/modules/src/core/ics04_channel/msgs/chan_open_try.rs index 8464ec3b5e..e9c031ac94 100644 --- a/modules/src/core/ics04_channel/msgs/chan_open_try.rs +++ b/modules/src/core/ics04_channel/msgs/chan_open_try.rs @@ -120,7 +120,7 @@ impl From for RawMsgChannelOpenTry { port_id: domain_msg.port_id.to_string(), previous_channel_id: domain_msg .previous_channel_id - .map_or_else(|| "".to_string(), |v| v.as_str().to_string()), + .map_or_else(|| "".to_string(), |v| v.to_string()), channel: Some(domain_msg.channel.into()), counterparty_version: domain_msg.counterparty_version.to_string(), proof_init: domain_msg.proofs.object_proof().clone().into(), diff --git a/relayer/src/config/filter.rs b/relayer/src/config/filter.rs index 398dd8bb5a..297ae7bb30 100644 --- a/relayer/src/config/filter.rs +++ b/relayer/src/config/filter.rs @@ -201,11 +201,11 @@ impl FilterPattern { /// wildcard matching if the filter is a `Pattern`. pub fn matches(&self, value: &T) -> bool where - T: PartialEq + AsRef, + T: PartialEq + ToString, { match self { FilterPattern::Exact(v) => value == v, - FilterPattern::Wildcard(regex) => regex.is_match(value.as_ref()), + FilterPattern::Wildcard(regex) => regex.is_match(&value.to_string()), } } @@ -230,14 +230,14 @@ impl fmt::Display for FilterPattern { impl Serialize for FilterPattern where - T: AsRef, + T: ToString, { fn serialize(&self, serializer: S) -> Result where S: Serializer, { match self { - FilterPattern::Exact(e) => serializer.serialize_str(e.as_ref()), + FilterPattern::Exact(e) => serializer.serialize_str(&e.to_string()), FilterPattern::Wildcard(t) => serializer.serialize_str(&t.to_string()), } } diff --git a/tools/test-framework/src/chain/driver/transfer.rs b/tools/test-framework/src/chain/driver/transfer.rs index 4f46f1da89..063213daa0 100644 --- a/tools/test-framework/src/chain/driver/transfer.rs +++ b/tools/test-framework/src/chain/driver/transfer.rs @@ -33,7 +33,7 @@ pub fn transfer_token( "ibc-transfer", "transfer", port_id.as_str(), - channel_id.as_str(), + &channel_id.to_string(), &recipient.0, &format!("{}{}", amount, denom), "--from", From b301f0580b9e06553b295052d05378673d6c80b7 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 6 Apr 2022 20:51:09 +0530 Subject: [PATCH 03/10] Modify channel-id validation to not use regex --- modules/src/core/ics24_host/identifier.rs | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/modules/src/core/ics24_host/identifier.rs b/modules/src/core/ics24_host/identifier.rs index 21985c28ae..4335a623e6 100644 --- a/modules/src/core/ics24_host/identifier.rs +++ b/modules/src/core/ics24_host/identifier.rs @@ -362,11 +362,6 @@ impl ChannelId { const fn prefix() -> &'static str { "channel-" } - - fn is_valid_fmt(chan_id_str: &str) -> bool { - let re = safe_regex::regex!(br"^channel-[0-9]{1,20}$"); - re.is_match(chan_id_str.as_bytes()) - } } /// This implementation provides a `to_string` method. @@ -380,17 +375,10 @@ impl FromStr for ChannelId { type Err = ValidationError; fn from_str(s: &str) -> Result { - if !Self::is_valid_fmt(s) { - return Err(ValidationError::channel_id_invalid_format()); - } - - let counter = { - let s = s - .strip_prefix(Self::prefix()) - .expect("missing prefix although `is_valid_fmt()`"); - u64::from_str(s).map_err(ValidationError::channel_id_parse_failure)? - }; - + let s = s + .strip_prefix(Self::prefix()) + .ok_or_else(|| ValidationError::channel_id_invalid_format())?; + let counter = u64::from_str(s).map_err(ValidationError::channel_id_parse_failure)?; Ok(Self(counter)) } } From bbebc344fa7816c273be348fcc058850f2812547 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 6 Apr 2022 20:52:05 +0530 Subject: [PATCH 04/10] Update tests to use valid channel ids --- modules/src/core/ics04_channel/events.rs | 8 ++++---- modules/src/core/ics04_channel/msgs/chan_close_confirm.rs | 4 ++-- modules/src/core/ics04_channel/msgs/chan_close_init.rs | 4 ++-- modules/src/core/ics04_channel/msgs/chan_open_ack.rs | 8 ++++---- modules/src/core/ics04_channel/msgs/chan_open_confirm.rs | 4 ++-- modules/src/core/ics04_channel/msgs/chan_open_try.rs | 4 ++-- modules/src/core/ics04_channel/packet.rs | 8 ++++---- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/modules/src/core/ics04_channel/events.rs b/modules/src/core/ics04_channel/events.rs index f7204b1ca5..7cd7685ee5 100644 --- a/modules/src/core/ics04_channel/events.rs +++ b/modules/src/core/ics04_channel/events.rs @@ -1112,10 +1112,10 @@ mod tests { let attributes = Attributes { height: Height::default(), port_id: "test_port".parse().unwrap(), - channel_id: Some("test_channel".parse().unwrap()), + channel_id: Some("channel-0".parse().unwrap()), connection_id: "test_connection".parse().unwrap(), counterparty_port_id: "counterparty_test_port".parse().unwrap(), - counterparty_channel_id: Some("counterparty_test_channel".parse().unwrap()), + counterparty_channel_id: Some("channel-1".parse().unwrap()), }; let mut abci_events = vec![]; let open_init = OpenInit::try_from(attributes.clone()).unwrap(); @@ -1164,9 +1164,9 @@ mod tests { let packet = Packet { sequence: Sequence::from(10), source_port: "a_test_port".parse().unwrap(), - source_channel: "a_test_channel".parse().unwrap(), + source_channel: "channel-0".parse().unwrap(), destination_port: "b_test_port".parse().unwrap(), - destination_channel: "b_test_channel".parse().unwrap(), + destination_channel: "channel-1".parse().unwrap(), data: "test_data".as_bytes().to_vec(), timeout_height: Height::new(1, 10), timeout_timestamp: Timestamp::now(), diff --git a/modules/src/core/ics04_channel/msgs/chan_close_confirm.rs b/modules/src/core/ics04_channel/msgs/chan_close_confirm.rs index b1e15c149b..f5ca89607e 100644 --- a/modules/src/core/ics04_channel/msgs/chan_close_confirm.rs +++ b/modules/src/core/ics04_channel/msgs/chan_close_confirm.rs @@ -170,7 +170,7 @@ mod tests { Test { name: "Correct channel identifier".to_string(), raw: RawMsgChannelCloseConfirm { - channel_id: "channelid34".to_string(), + channel_id: "channel-34".to_string(), ..default_raw_msg.clone() }, want_pass: true, @@ -187,7 +187,7 @@ mod tests { name: "Bad channel, name too long".to_string(), raw: RawMsgChannelCloseConfirm { channel_id: - "abcdefghiasdfadsfasdfgdfsadfasdasdfasdasdfasddsfasdfasdjklmnopqrstu" + "channel-12839128379182739812739879" .to_string(), ..default_raw_msg.clone() }, diff --git a/modules/src/core/ics04_channel/msgs/chan_close_init.rs b/modules/src/core/ics04_channel/msgs/chan_close_init.rs index 1077239248..3dfac2565f 100644 --- a/modules/src/core/ics04_channel/msgs/chan_close_init.rs +++ b/modules/src/core/ics04_channel/msgs/chan_close_init.rs @@ -140,7 +140,7 @@ mod tests { Test { name: "Correct channel identifier".to_string(), raw: RawMsgChannelCloseInit { - channel_id: "channelid34".to_string(), + channel_id: "channel-34".to_string(), ..default_raw_msg.clone() }, want_pass: true, @@ -156,7 +156,7 @@ mod tests { Test { name: "Bad channel, name too long".to_string(), raw: RawMsgChannelCloseInit { - channel_id: "abcdeasdfasdfasdfasdfasdfasdfasdfasdfdgasdfasdfasdfghijklmnopqrstu".to_string(), + channel_id: "channel-12839128379182739812739879".to_string(), ..default_raw_msg }, want_pass: false, diff --git a/modules/src/core/ics04_channel/msgs/chan_open_ack.rs b/modules/src/core/ics04_channel/msgs/chan_open_ack.rs index 987571c9e4..46161037af 100644 --- a/modules/src/core/ics04_channel/msgs/chan_open_ack.rs +++ b/modules/src/core/ics04_channel/msgs/chan_open_ack.rs @@ -184,7 +184,7 @@ mod tests { Test { name: "Correct channel identifier".to_string(), raw: RawMsgChannelOpenAck { - channel_id: "channelid34".to_string(), + channel_id: "channel-34".to_string(), ..default_raw_msg.clone() }, want_pass: true, @@ -200,7 +200,7 @@ mod tests { Test { name: "Bad channel, name too long".to_string(), raw: RawMsgChannelOpenAck { - channel_id: "abcdefghsdfasdfasfdasfdwewefsdfasdfasdfasdfasdfasdfsfdijklmnopqrstu".to_string(), + channel_id: "channel-12839128379182739812739879".to_string(), ..default_raw_msg.clone() }, want_pass: false, @@ -208,7 +208,7 @@ mod tests { Test { name: "[Counterparty] Correct channel identifier".to_string(), raw: RawMsgChannelOpenAck { - counterparty_channel_id: "channelid34".to_string(), + counterparty_channel_id: "channel-34".to_string(), ..default_raw_msg.clone() }, want_pass: true, @@ -224,7 +224,7 @@ mod tests { Test { name: "[Counterparty] Bad channel, name too long".to_string(), raw: RawMsgChannelOpenAck { - counterparty_channel_id: "abcdefghsdfasdfasfdasfdwewefsdfasdfasdfasdfasdfasdfsfdijklmnopqrstu".to_string(), + counterparty_channel_id: "channel-12839128379182739812739879".to_string(), ..default_raw_msg.clone() }, want_pass: false, diff --git a/modules/src/core/ics04_channel/msgs/chan_open_confirm.rs b/modules/src/core/ics04_channel/msgs/chan_open_confirm.rs index 57342c818b..a283e516d7 100644 --- a/modules/src/core/ics04_channel/msgs/chan_open_confirm.rs +++ b/modules/src/core/ics04_channel/msgs/chan_open_confirm.rs @@ -164,7 +164,7 @@ mod tests { Test { name: "Correct channel identifier".to_string(), raw: RawMsgChannelOpenConfirm { - channel_id: "channelid34".to_string(), + channel_id: "channel-34".to_string(), ..default_raw_msg.clone() }, want_pass: true, @@ -180,7 +180,7 @@ mod tests { Test { name: "Bad channel, name too long".to_string(), raw: RawMsgChannelOpenConfirm { - channel_id: "abcdefghijklmnoasdfasdfasdfasdfasdgsdghasdfasdfasdfasdfadsfasgdasdfasdfasfdpqrstu".to_string(), + channel_id: "channel-12839128379182739812739879".to_string(), ..default_raw_msg.clone() }, want_pass: false, diff --git a/modules/src/core/ics04_channel/msgs/chan_open_try.rs b/modules/src/core/ics04_channel/msgs/chan_open_try.rs index e9c031ac94..68122e2d44 100644 --- a/modules/src/core/ics04_channel/msgs/chan_open_try.rs +++ b/modules/src/core/ics04_channel/msgs/chan_open_try.rs @@ -211,7 +211,7 @@ mod tests { Test { name: "Correct channel identifier".to_string(), raw: RawMsgChannelOpenTry { - previous_channel_id: "channelid34".to_string(), + previous_channel_id: "channel-34".to_string(), ..default_raw_msg.clone() }, want_pass: true, @@ -227,7 +227,7 @@ mod tests { Test { name: "Bad channel, name too long".to_string(), raw: RawMsgChannelOpenTry { - previous_channel_id: "abcdefghijkasdfasdfasdfasgdasdgasdfasdfadflmnoasdasdasdfasdfasdfasdfadadgadgadsfpqrstu".to_string(), + previous_channel_id: "channel-12839128379182739812739879".to_string(), ..default_raw_msg.clone() }, want_pass: false, diff --git a/modules/src/core/ics04_channel/packet.rs b/modules/src/core/ics04_channel/packet.rs index 29e0cd54d7..3d6a49886b 100644 --- a/modules/src/core/ics04_channel/packet.rs +++ b/modules/src/core/ics04_channel/packet.rs @@ -388,7 +388,7 @@ mod tests { Test { name: "Src channel validation: correct".to_string(), raw: RawPacket { - source_channel: "srcchannelp34".to_string(), + source_channel: "channel-1".to_string(), ..default_raw_msg.clone() }, want_pass: true, @@ -404,7 +404,7 @@ mod tests { Test { name: "Bad src channel, name too long".to_string(), raw: RawPacket { - source_channel: "abcdefghijasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfadgasgasdfasdfasdfasdfaklmnopqrstu".to_string(), + source_channel: "channel-12839128379182739812739879".to_string(), ..default_raw_msg.clone() }, want_pass: false, @@ -412,7 +412,7 @@ mod tests { Test { name: "Dst channel validation: correct".to_string(), raw: RawPacket { - destination_channel: "dstchannelp34".to_string(), + destination_channel: "channel-34".to_string(), ..default_raw_msg.clone() }, want_pass: true, @@ -428,7 +428,7 @@ mod tests { Test { name: "Bad dst channel, name too long".to_string(), raw: RawPacket { - destination_channel: "abcdefghijasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfadgasgasdfasdfasdfasdfaklmnopqrstu".to_string(), + destination_channel: "channel-12839128379182739812739879".to_string(), ..default_raw_msg.clone() }, want_pass: false, From 99a16d9ed0a725a90fc3154bff5936c25d3fd8cb Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 6 Apr 2022 21:05:33 +0530 Subject: [PATCH 05/10] Fix clippy errors --- modules/src/core/ics24_host/identifier.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/src/core/ics24_host/identifier.rs b/modules/src/core/ics24_host/identifier.rs index 4335a623e6..bdf2d11baa 100644 --- a/modules/src/core/ics24_host/identifier.rs +++ b/modules/src/core/ics24_host/identifier.rs @@ -377,7 +377,7 @@ impl FromStr for ChannelId { fn from_str(s: &str) -> Result { let s = s .strip_prefix(Self::prefix()) - .ok_or_else(|| ValidationError::channel_id_invalid_format())?; + .ok_or_else(ValidationError::channel_id_invalid_format)?; let counter = u64::from_str(s).map_err(ValidationError::channel_id_parse_failure)?; Ok(Self(counter)) } From b83fa5bcefa533a13c6f2c577fd4a6a3376a2759 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Wed, 6 Apr 2022 21:51:04 +0530 Subject: [PATCH 06/10] Impl Clone for ChannelId --- .../relay_application_logic/send_transfer.rs | 11 ++--- .../clients/ics07_tendermint/client_def.rs | 10 ++--- modules/src/core/ics04_channel/context.rs | 34 ++++++---------- modules/src/core/ics04_channel/events.rs | 4 +- .../ics04_channel/handler/acknowledgement.rs | 22 +++++----- .../handler/chan_close_confirm.rs | 15 ++++--- .../ics04_channel/handler/chan_close_init.rs | 12 +++--- .../ics04_channel/handler/chan_open_ack.rs | 33 ++++++--------- .../handler/chan_open_confirm.rs | 15 ++++--- .../ics04_channel/handler/chan_open_init.rs | 2 +- .../ics04_channel/handler/chan_open_try.rs | 30 +++++--------- .../core/ics04_channel/handler/recv_packet.rs | 34 ++++++---------- .../core/ics04_channel/handler/send_packet.rs | 8 ++-- .../src/core/ics04_channel/handler/timeout.rs | 22 +++++----- .../ics04_channel/handler/timeout_on_close.rs | 22 +++++----- .../handler/write_acknowledgement.rs | 17 +++----- modules/src/core/ics24_host/identifier.rs | 2 +- modules/src/mock/context.rs | 5 +-- relayer-cli/src/commands/clear.rs | 2 +- .../src/commands/query/channel_ends.rs | 2 +- relayer-cli/src/commands/tx/channel.rs | 20 +++++----- relayer-cli/src/commands/tx/packet.rs | 4 +- relayer-cli/src/commands/tx/transfer.rs | 2 +- relayer/src/chain/cosmos.rs | 12 +----- relayer/src/chain/counterparty.rs | 10 ++--- relayer/src/chain/handle/base.rs | 6 +-- relayer/src/chain/handle/cache.rs | 2 +- relayer/src/channel.rs | 40 +++++++++---------- relayer/src/link.rs | 34 +++++----------- relayer/src/link/relay_path.rs | 24 +++++------ relayer/src/object.rs | 10 ++--- relayer/src/supervisor/client_state_filter.rs | 4 +- relayer/src/supervisor/scan.rs | 14 +++---- relayer/src/supervisor/spawn.rs | 4 +- relayer/src/transfer.rs | 2 +- relayer/src/worker.rs | 2 +- .../src/bootstrap/binary/channel.rs | 10 ++--- tools/test-framework/src/relayer/channel.rs | 5 +-- tools/test-framework/src/relayer/transfer.rs | 2 +- 39 files changed, 216 insertions(+), 293 deletions(-) diff --git a/modules/src/applications/ics20_fungible_token_transfer/relay_application_logic/send_transfer.rs b/modules/src/applications/ics20_fungible_token_transfer/relay_application_logic/send_transfer.rs index e4cd744f4a..5b8be1c3c7 100644 --- a/modules/src/applications/ics20_fungible_token_transfer/relay_application_logic/send_transfer.rs +++ b/modules/src/applications/ics20_fungible_token_transfer/relay_application_logic/send_transfer.rs @@ -15,7 +15,7 @@ where Ctx: Ics20Context, { let source_channel_end = ctx - .channel_end(&(msg.source_port.clone(), msg.source_channel.clone())) + .channel_end(&(msg.source_port.clone(), msg.source_channel)) .map_err(Error::ics04_channel)?; let destination_port = source_channel_end.counterparty().port_id().clone(); @@ -23,15 +23,12 @@ where .counterparty() .channel_id() .ok_or_else(|| { - Error::destination_channel_not_found( - msg.source_port.clone(), - msg.source_channel.clone(), - ) + Error::destination_channel_not_found(msg.source_port.clone(), msg.source_channel) })?; // get the next sequence let sequence = ctx - .get_next_sequence_send(&(msg.source_port.clone(), msg.source_channel.clone())) + .get_next_sequence_send(&(msg.source_port.clone(), msg.source_channel)) .map_err(Error::ics04_channel)?; //TODO: Application LOGIC. @@ -41,7 +38,7 @@ where source_port: msg.source_port, source_channel: msg.source_channel, destination_port, - destination_channel: destination_channel.clone(), + destination_channel: *destination_channel, data: vec![0], timeout_height: msg.timeout_height, timeout_timestamp: msg.timeout_timestamp, diff --git a/modules/src/clients/ics07_tendermint/client_def.rs b/modules/src/clients/ics07_tendermint/client_def.rs index 7845dc9dc7..80eed2701f 100644 --- a/modules/src/clients/ics07_tendermint/client_def.rs +++ b/modules/src/clients/ics07_tendermint/client_def.rs @@ -242,7 +242,7 @@ impl ClientDef for TendermintClient { ) -> Result<(), Ics02Error> { client_state.verify_height(height)?; - let path = ChannelEndsPath(port_id.clone(), channel_id.clone()); + let path = ChannelEndsPath(port_id.clone(), *channel_id); let value = expected_channel_end.encode_vec().unwrap(); verify_membership(client_state, prefix, proof, root, path, value) } @@ -282,7 +282,7 @@ impl ClientDef for TendermintClient { let commitment_path = CommitmentsPath { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id: *channel_id, sequence, }; @@ -319,7 +319,7 @@ impl ClientDef for TendermintClient { let ack_path = AcksPath { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id: *channel_id, sequence, }; verify_membership( @@ -352,7 +352,7 @@ impl ClientDef for TendermintClient { .encode(&mut seq_bytes) .expect("buffer size too small"); - let seq_path = SeqRecvsPath(port_id.clone(), channel_id.clone()); + let seq_path = SeqRecvsPath(port_id.clone(), *channel_id); verify_membership( client_state, connection_end.counterparty().prefix(), @@ -380,7 +380,7 @@ impl ClientDef for TendermintClient { let receipt_path = ReceiptsPath { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id: *channel_id, sequence, }; verify_non_membership( diff --git a/modules/src/core/ics04_channel/context.rs b/modules/src/core/ics04_channel/context.rs index 71de87b887..8443ef9bfa 100644 --- a/modules/src/core/ics04_channel/context.rs +++ b/modules/src/core/ics04_channel/context.rs @@ -121,7 +121,7 @@ pub trait ChannelKeeper { fn store_channel_result(&mut self, result: ChannelResult) -> Result<(), Error> { // The handler processed this channel & some modifications occurred, store the new end. self.store_channel( - (result.port_id.clone(), result.channel_id.clone()), + (result.port_id.clone(), result.channel_id), &result.channel_end, )?; @@ -133,18 +133,12 @@ pub trait ChannelKeeper { // Associate also the channel end to its connection. self.store_connection_channels( result.channel_end.connection_hops()[0].clone(), - &(result.port_id.clone(), result.channel_id.clone()), + &(result.port_id.clone(), result.channel_id), )?; // Initialize send, recv, and ack sequence numbers. - self.store_next_sequence_send( - (result.port_id.clone(), result.channel_id.clone()), - 1.into(), - )?; - self.store_next_sequence_recv( - (result.port_id.clone(), result.channel_id.clone()), - 1.into(), - )?; + self.store_next_sequence_send((result.port_id.clone(), result.channel_id), 1.into())?; + self.store_next_sequence_recv((result.port_id.clone(), result.channel_id), 1.into())?; self.store_next_sequence_ack((result.port_id, result.channel_id), 1.into())?; } @@ -155,12 +149,12 @@ pub trait ChannelKeeper { match general_result { PacketResult::Send(res) => { self.store_next_sequence_send( - (res.port_id.clone(), res.channel_id.clone()), + (res.port_id.clone(), res.channel_id), res.seq_number, )?; self.store_packet_commitment( - (res.port_id.clone(), res.channel_id.clone(), res.seq), + (res.port_id.clone(), res.channel_id, res.seq), res.timeout_timestamp, res.timeout_height, res.data, @@ -175,14 +169,14 @@ pub trait ChannelKeeper { None => { // Ordered channel self.store_next_sequence_recv( - (res.port_id.clone(), res.channel_id.clone()), + (res.port_id.clone(), res.channel_id), res.seq_number, )? } Some(r) => { // Unordered channel self.store_packet_receipt( - (res.port_id.clone(), res.channel_id.clone(), res.seq), + (res.port_id.clone(), res.channel_id, res.seq), r, )? } @@ -190,7 +184,7 @@ pub trait ChannelKeeper { } PacketResult::WriteAck(res) => { self.store_packet_acknowledgement( - (res.port_id.clone(), res.channel_id.clone(), res.seq), + (res.port_id.clone(), res.channel_id, res.seq), res.ack, )?; } @@ -204,7 +198,7 @@ pub trait ChannelKeeper { //Unordered Channel self.delete_packet_commitment(( res.port_id.clone(), - res.channel_id.clone(), + res.channel_id, res.seq, ))?; } @@ -213,13 +207,9 @@ pub trait ChannelKeeper { PacketResult::Timeout(res) => { if let Some(c) = res.channel { //Ordered Channel - self.store_channel((res.port_id.clone(), res.channel_id.clone()), &c)?; + self.store_channel((res.port_id.clone(), res.channel_id), &c)?; } - self.delete_packet_commitment(( - res.port_id.clone(), - res.channel_id.clone(), - res.seq, - ))?; + self.delete_packet_commitment((res.port_id.clone(), res.channel_id, res.seq))?; } } Ok(()) diff --git a/modules/src/core/ics04_channel/events.rs b/modules/src/core/ics04_channel/events.rs index 7cd7685ee5..b705892327 100644 --- a/modules/src/core/ics04_channel/events.rs +++ b/modules/src/core/ics04_channel/events.rs @@ -612,10 +612,10 @@ impl TryFrom for CloseInit { Ok(CloseInit { height: attrs.height, port_id: attrs.port_id.clone(), - channel_id: channel_id.clone(), + channel_id: *channel_id, connection_id: attrs.connection_id.clone(), counterparty_port_id: attrs.counterparty_port_id.clone(), - counterparty_channel_id: attrs.counterparty_channel_id.clone(), + counterparty_channel_id: attrs.counterparty_channel_id, }) } else { Err(EventError::channel(Error::missing_channel_id())) diff --git a/modules/src/core/ics04_channel/handler/acknowledgement.rs b/modules/src/core/ics04_channel/handler/acknowledgement.rs index 7ebf3b1070..62de3ed913 100644 --- a/modules/src/core/ics04_channel/handler/acknowledgement.rs +++ b/modules/src/core/ics04_channel/handler/acknowledgement.rs @@ -28,23 +28,23 @@ pub fn process( let packet = &msg.packet; let source_channel_end = - ctx.channel_end(&(packet.source_port.clone(), packet.source_channel.clone()))?; + ctx.channel_end(&(packet.source_port.clone(), packet.source_channel))?; if !source_channel_end.state_matches(&State::Open) { - return Err(Error::channel_closed(packet.source_channel.clone())); + return Err(Error::channel_closed(packet.source_channel)); } let _channel_cap = ctx.authenticated_capability(&packet.source_port)?; let counterparty = Counterparty::new( packet.destination_port.clone(), - Some(packet.destination_channel.clone()), + Some(packet.destination_channel), ); if !source_channel_end.counterparty_matches(&counterparty) { return Err(Error::invalid_packet_counterparty( packet.destination_port.clone(), - packet.destination_channel.clone(), + packet.destination_channel, )); } @@ -59,7 +59,7 @@ pub fn process( // Verify packet commitment let packet_commitment = ctx.get_packet_commitment(&( packet.source_port.clone(), - packet.source_channel.clone(), + packet.source_channel, packet.sequence, ))?; @@ -83,8 +83,8 @@ pub fn process( )?; let result = if source_channel_end.order_matches(&Order::Ordered) { - let next_seq_ack = ctx - .get_next_sequence_ack(&(packet.source_port.clone(), packet.source_channel.clone()))?; + let next_seq_ack = + ctx.get_next_sequence_ack(&(packet.source_port.clone(), packet.source_channel))?; if packet.sequence != next_seq_ack { return Err(Error::invalid_packet_sequence( @@ -95,14 +95,14 @@ pub fn process( PacketResult::Ack(AckPacketResult { port_id: packet.source_port.clone(), - channel_id: packet.source_channel.clone(), + channel_id: packet.source_channel, seq: packet.sequence, seq_number: Some(next_seq_ack.increment()), }) } else { PacketResult::Ack(AckPacketResult { port_id: packet.source_port.clone(), - channel_id: packet.source_channel.clone(), + channel_id: packet.source_channel, seq: packet.sequence, seq_number: None, }) @@ -171,7 +171,7 @@ mod tests { Order::default(), Counterparty::new( packet.destination_port.clone(), - Some(packet.destination_channel.clone()), + Some(packet.destination_channel), ), vec![ConnectionId::default()], Version::ics20(), @@ -215,7 +215,7 @@ mod tests { .with_port_capability(packet.destination_port.clone()) .with_channel( packet.source_port.clone(), - packet.source_channel.clone(), + packet.source_channel, source_channel_end, ) .with_packet_commitment( diff --git a/modules/src/core/ics04_channel/handler/chan_close_confirm.rs b/modules/src/core/ics04_channel/handler/chan_close_confirm.rs index 8786561989..997755cbde 100644 --- a/modules/src/core/ics04_channel/handler/chan_close_confirm.rs +++ b/modules/src/core/ics04_channel/handler/chan_close_confirm.rs @@ -18,11 +18,11 @@ pub(crate) fn process( let mut output = HandlerOutput::builder(); // Retrieve the old channel end and validate it against the message. - let mut channel_end = ctx.channel_end(&(msg.port_id.clone(), msg.channel_id.clone()))?; + let mut channel_end = ctx.channel_end(&(msg.port_id.clone(), msg.channel_id))?; // Validate that the channel end is in a state where it can be closed. if channel_end.state_matches(&State::Closed) { - return Err(Error::channel_closed(msg.channel_id.clone())); + return Err(Error::channel_closed(msg.channel_id)); } // Channel capabilities @@ -47,8 +47,7 @@ pub(crate) fn process( // Proof verification in two steps: // 1. Setup: build the Channel as we expect to find it on the other party. - let expected_counterparty = - Counterparty::new(msg.port_id.clone(), Some(msg.channel_id.clone())); + let expected_counterparty = Counterparty::new(msg.port_id.clone(), Some(msg.channel_id)); let counterparty = conn.counterparty(); let ccid = counterparty.connection_id().ok_or_else(|| { @@ -81,14 +80,14 @@ pub(crate) fn process( let result = ChannelResult { port_id: msg.port_id.clone(), - channel_id: msg.channel_id.clone(), + channel_id: msg.channel_id, channel_id_state: ChannelIdState::Reused, channel_cap, channel_end, }; let event_attributes = Attributes { - channel_id: Some(msg.channel_id.clone()), + channel_id: Some(msg.channel_id), height: ctx.host_height(), ..Default::default() }; @@ -151,7 +150,7 @@ mod tests { Order::default(), Counterparty::new( msg_chan_close_confirm.port_id.clone(), - Some(msg_chan_close_confirm.channel_id.clone()), + Some(msg_chan_close_confirm.channel_id), ), vec![conn_id.clone()], Version::default(), @@ -163,7 +162,7 @@ mod tests { .with_port_capability(msg_chan_close_confirm.port_id.clone()) .with_channel( msg_chan_close_confirm.port_id.clone(), - msg_chan_close_confirm.channel_id.clone(), + msg_chan_close_confirm.channel_id, chan_end, ); diff --git a/modules/src/core/ics04_channel/handler/chan_close_init.rs b/modules/src/core/ics04_channel/handler/chan_close_init.rs index 7248219aa1..4bd981fadd 100644 --- a/modules/src/core/ics04_channel/handler/chan_close_init.rs +++ b/modules/src/core/ics04_channel/handler/chan_close_init.rs @@ -16,12 +16,12 @@ pub(crate) fn process( let mut output = HandlerOutput::builder(); // Unwrap the old channel end and validate it against the message. - let mut channel_end = ctx.channel_end(&(msg.port_id.clone(), msg.channel_id.clone()))?; + let mut channel_end = ctx.channel_end(&(msg.port_id.clone(), msg.channel_id))?; // Validate that the channel end is in a state where it can be closed. if channel_end.state_matches(&State::Closed) { return Err(Error::invalid_channel_state( - msg.channel_id.clone(), + msg.channel_id, channel_end.state, )); } @@ -52,14 +52,14 @@ pub(crate) fn process( let result = ChannelResult { port_id: msg.port_id.clone(), - channel_id: msg.channel_id.clone(), + channel_id: msg.channel_id, channel_id_state: ChannelIdState::Reused, channel_cap, channel_end, }; let event_attributes = Attributes { - channel_id: Some(msg.channel_id.clone()), + channel_id: Some(msg.channel_id), height: ctx.host_height(), ..Default::default() }; @@ -118,7 +118,7 @@ mod tests { Order::default(), Counterparty::new( msg_chan_close_init.port_id.clone(), - Some(msg_chan_close_init.channel_id.clone()), + Some(msg_chan_close_init.channel_id), ), vec![conn_id.clone()], Version::default(), @@ -134,7 +134,7 @@ mod tests { .with_port_capability(msg_chan_close_init.port_id.clone()) .with_channel( msg_chan_close_init.port_id.clone(), - msg_chan_close_init.channel_id.clone(), + msg_chan_close_init.channel_id, chan_end, ) }; diff --git a/modules/src/core/ics04_channel/handler/chan_open_ack.rs b/modules/src/core/ics04_channel/handler/chan_open_ack.rs index a1d84b2166..f4772dbb65 100644 --- a/modules/src/core/ics04_channel/handler/chan_open_ack.rs +++ b/modules/src/core/ics04_channel/handler/chan_open_ack.rs @@ -18,12 +18,12 @@ pub(crate) fn process( let mut output = HandlerOutput::builder(); // Unwrap the old channel end and validate it against the message. - let mut channel_end = ctx.channel_end(&(msg.port_id.clone(), msg.channel_id.clone()))?; + let mut channel_end = ctx.channel_end(&(msg.port_id.clone(), msg.channel_id))?; // Validate that the channel end is in a state where it can be ack. if !channel_end.state_matches(&State::Init) && !channel_end.state_matches(&State::TryOpen) { return Err(Error::invalid_channel_state( - msg.channel_id.clone(), + msg.channel_id, channel_end.state, )); } @@ -51,8 +51,7 @@ pub(crate) fn process( // Proof verification in two steps: // 1. Setup: build the Channel as we expect to find it on the other party. - let expected_counterparty = - Counterparty::new(msg.port_id.clone(), Some(msg.channel_id.clone())); + let expected_counterparty = Counterparty::new(msg.port_id.clone(), Some(msg.channel_id)); let counterparty = conn.counterparty(); let ccid = counterparty.connection_id().ok_or_else(|| { @@ -70,7 +69,7 @@ pub(crate) fn process( ); // set the counterparty channel id to verify against it - channel_end.set_counterparty_channel_id(msg.counterparty_channel_id.clone()); + channel_end.set_counterparty_channel_id(msg.counterparty_channel_id); //2. Verify proofs verify_channel_proofs( @@ -90,14 +89,14 @@ pub(crate) fn process( let result = ChannelResult { port_id: msg.port_id.clone(), - channel_id: msg.channel_id.clone(), + channel_id: msg.channel_id, channel_id_state: ChannelIdState::Reused, channel_cap, channel_end, }; let event_attributes = Attributes { - channel_id: Some(msg.channel_id.clone()), + channel_id: Some(msg.channel_id), height: ctx.host_height(), ..Default::default() }; @@ -199,10 +198,7 @@ mod tests { let chan_end = ChannelEnd::new( State::Init, *msg_chan_try.channel.ordering(), - Counterparty::new( - msg_chan_ack.port_id.clone(), - Some(msg_chan_ack.channel_id.clone()), - ), + Counterparty::new(msg_chan_ack.port_id.clone(), Some(msg_chan_ack.channel_id)), connection_vec0.clone(), msg_chan_try.channel.version().clone(), ); @@ -210,10 +206,7 @@ mod tests { let failed_chan_end = ChannelEnd::new( State::Open, *msg_chan_try.channel.ordering(), - Counterparty::new( - msg_chan_ack.port_id.clone(), - Some(msg_chan_ack.channel_id.clone()), - ), + Counterparty::new(msg_chan_ack.port_id.clone(), Some(msg_chan_ack.channel_id)), connection_vec0, msg_chan_try.channel.version().clone(), ); @@ -236,7 +229,7 @@ mod tests { .with_port_capability(msg_chan_ack.port_id.clone()) .with_channel( msg_chan_ack.port_id.clone(), - msg_chan_ack.channel_id.clone(), + msg_chan_ack.channel_id, failed_chan_end, ), msg: ChannelMsg::ChannelOpenAck(msg_chan_ack.clone()), @@ -254,7 +247,7 @@ mod tests { .with_connection(cid.clone(), conn_end.clone()) .with_channel( msg_chan_ack.port_id.clone(), - msg_chan_ack.channel_id.clone(), + msg_chan_ack.channel_id, chan_end.clone(), ), msg: ChannelMsg::ChannelOpenAck(msg_chan_ack.clone()), @@ -271,7 +264,7 @@ mod tests { .with_port_capability(msg_chan_ack.port_id.clone()) .with_channel( msg_chan_ack.port_id.clone(), - msg_chan_ack.channel_id.clone(), + msg_chan_ack.channel_id, chan_end.clone(), ), msg: ChannelMsg::ChannelOpenAck(msg_chan_ack.clone()), @@ -285,7 +278,7 @@ mod tests { .with_port_capability(msg_chan_ack.port_id.clone()) .with_channel( msg_chan_ack.port_id.clone(), - msg_chan_ack.channel_id.clone(), + msg_chan_ack.channel_id, chan_end.clone(), ), msg: ChannelMsg::ChannelOpenAck(msg_chan_ack.clone()), @@ -302,7 +295,7 @@ mod tests { .with_port_capability(msg_chan_ack.port_id.clone()) .with_channel( msg_chan_ack.port_id.clone(), - msg_chan_ack.channel_id.clone(), + msg_chan_ack.channel_id, chan_end, ), msg: ChannelMsg::ChannelOpenAck(msg_chan_ack), diff --git a/modules/src/core/ics04_channel/handler/chan_open_confirm.rs b/modules/src/core/ics04_channel/handler/chan_open_confirm.rs index b855fdbd67..1c40eb4c72 100644 --- a/modules/src/core/ics04_channel/handler/chan_open_confirm.rs +++ b/modules/src/core/ics04_channel/handler/chan_open_confirm.rs @@ -18,12 +18,12 @@ pub(crate) fn process( let mut output = HandlerOutput::builder(); // Unwrap the old channel end and validate it against the message. - let mut channel_end = ctx.channel_end(&(msg.port_id.clone(), msg.channel_id.clone()))?; + let mut channel_end = ctx.channel_end(&(msg.port_id.clone(), msg.channel_id))?; // Validate that the channel end is in a state where it can be confirmed. if !channel_end.state_matches(&State::TryOpen) { return Err(Error::invalid_channel_state( - msg.channel_id.clone(), + msg.channel_id, channel_end.state, )); } @@ -50,8 +50,7 @@ pub(crate) fn process( // Proof verification in two steps: // 1. Setup: build the Channel as we expect to find it on the other party. - let expected_counterparty = - Counterparty::new(msg.port_id.clone(), Some(msg.channel_id.clone())); + let expected_counterparty = Counterparty::new(msg.port_id.clone(), Some(msg.channel_id)); let connection_counterparty = conn.counterparty(); let ccid = connection_counterparty.connection_id().ok_or_else(|| { @@ -85,14 +84,14 @@ pub(crate) fn process( let result = ChannelResult { port_id: msg.port_id.clone(), - channel_id: msg.channel_id.clone(), + channel_id: msg.channel_id, channel_id_state: ChannelIdState::Reused, channel_cap, channel_end, }; let event_attributes = Attributes { - channel_id: Some(msg.channel_id.clone()), + channel_id: Some(msg.channel_id), height: ctx.host_height(), ..Default::default() }; @@ -164,7 +163,7 @@ mod tests { Order::default(), Counterparty::new( msg_chan_confirm.port_id.clone(), - Some(msg_chan_confirm.channel_id.clone()), + Some(msg_chan_confirm.channel_id), ), vec![conn_id.clone()], Version::default(), @@ -178,7 +177,7 @@ mod tests { .with_port_capability(msg_chan_confirm.port_id.clone()) .with_channel( msg_chan_confirm.port_id.clone(), - msg_chan_confirm.channel_id.clone(), + msg_chan_confirm.channel_id, chan_end, ), msg: ChannelMsg::ChannelOpenConfirm(msg_chan_confirm), diff --git a/modules/src/core/ics04_channel/handler/chan_open_init.rs b/modules/src/core/ics04_channel/handler/chan_open_init.rs index f1f9859cc5..fab19c81e2 100644 --- a/modules/src/core/ics04_channel/handler/chan_open_init.rs +++ b/modules/src/core/ics04_channel/handler/chan_open_init.rs @@ -61,7 +61,7 @@ pub(crate) fn process( let result = ChannelResult { port_id: msg.port_id.clone(), - channel_id: chan_id.clone(), + channel_id: chan_id, channel_end: new_channel_end, channel_id_state: ChannelIdState::Generated, channel_cap, diff --git a/modules/src/core/ics04_channel/handler/chan_open_try.rs b/modules/src/core/ics04_channel/handler/chan_open_try.rs index bac06233ad..1611def909 100644 --- a/modules/src/core/ics04_channel/handler/chan_open_try.rs +++ b/modules/src/core/ics04_channel/handler/chan_open_try.rs @@ -22,7 +22,7 @@ pub(crate) fn process( // Unwrap the old channel end (if any) and validate it against the message. let (mut new_channel_end, channel_id) = match &msg.previous_channel_id { Some(prev_id) => { - let old_channel_end = ctx.channel_end(&(msg.port_id.clone(), prev_id.clone()))?; + let old_channel_end = ctx.channel_end(&(msg.port_id.clone(), *prev_id))?; // Validate that existing channel end matches with the one we're trying to establish. if old_channel_end.state_matches(&State::Init) @@ -32,10 +32,10 @@ pub(crate) fn process( && old_channel_end.version_matches(msg.channel.version()) { // A ChannelEnd already exists and all validation passed. - Ok((old_channel_end, prev_id.clone())) + Ok((old_channel_end, *prev_id)) } else { // A ConnectionEnd already exists and validation failed. - Err(Error::channel_mismatch(prev_id.clone())) + Err(Error::channel_mismatch(*prev_id)) } } // No previous channel id was supplied. Create a new channel end & an identifier. @@ -133,7 +133,7 @@ pub(crate) fn process( } else { ChannelIdState::Reused }, - channel_id: channel_id.clone(), + channel_id, channel_end: new_channel_end, }; @@ -213,7 +213,7 @@ mod tests { // this channel should depend on connection `conn_id`. let chan_id = ChannelId::new(24); let hops = vec![conn_id.clone()]; - msg.previous_channel_id = Some(chan_id.clone()); + msg.previous_channel_id = Some(chan_id); msg.channel.connection_hops = hops; // This message does not assume a channel should already be initialized. @@ -260,7 +260,7 @@ mod tests { want_pass: false, match_error: { let port_id = msg.port_id.clone(); - let channel_id = chan_id.clone(); + let channel_id = chan_id; Box::new(move |e| match e { error::ErrorDetail::ChannelNotFound(e) => { assert_eq!(e.port_id, port_id); @@ -321,11 +321,11 @@ mod tests { .clone() .with_connection(conn_id.clone(), conn_end.clone()) .with_port_capability(msg.port_id.clone()) - .with_channel(msg.port_id.clone(), chan_id.clone(), incorrect_chan_end_ver), + .with_channel(msg.port_id.clone(), chan_id, incorrect_chan_end_ver), msg: ChannelMsg::ChannelOpenTry(msg.clone()), want_pass: false, match_error: { - let channel_id = chan_id.clone(); + let channel_id = chan_id; Box::new(move |e| match e { error::ErrorDetail::ChannelMismatch(e) => { assert_eq!(e.channel_id, channel_id); @@ -342,15 +342,11 @@ mod tests { .clone() .with_connection(conn_id.clone(), conn_end.clone()) .with_port_capability(msg.port_id.clone()) - .with_channel( - msg.port_id.clone(), - chan_id.clone(), - incorrect_chan_end_hops, - ), + .with_channel(msg.port_id.clone(), chan_id, incorrect_chan_end_hops), msg: ChannelMsg::ChannelOpenTry(msg.clone()), want_pass: false, match_error: { - let channel_id = chan_id.clone(); + let channel_id = chan_id; Box::new(move |e| match e { error::ErrorDetail::ChannelMismatch(e) => { assert_eq!(e.channel_id, channel_id); @@ -367,11 +363,7 @@ mod tests { .clone() .with_connection(conn_id.clone(), conn_end.clone()) .with_port_capability(msg.port_id.clone()) - .with_channel( - msg.port_id.clone(), - chan_id.clone(), - correct_chan_end.clone(), - ), + .with_channel(msg.port_id.clone(), chan_id, correct_chan_end.clone()), msg: ChannelMsg::ChannelOpenTry(msg.clone()), want_pass: false, match_error: Box::new(|e| match e { diff --git a/modules/src/core/ics04_channel/handler/recv_packet.rs b/modules/src/core/ics04_channel/handler/recv_packet.rs index d0244a368b..ab4c2132ca 100644 --- a/modules/src/core/ics04_channel/handler/recv_packet.rs +++ b/modules/src/core/ics04_channel/handler/recv_packet.rs @@ -32,29 +32,24 @@ pub fn process(ctx: &dyn ChannelReader, msg: &MsgRecvPacket) -> HandlerResult HandlerResult HandlerResult HandlerResult HandlerResult HandlerResult HandlerResult HandlerResult HandlerResult HandlerResult HandlerResult HandlerResult HandlerResult HandlerResult { let mut output = HandlerOutput::builder(); - let dest_channel_end = ctx.channel_end(&( - packet.destination_port.clone(), - packet.destination_channel.clone(), - ))?; + let dest_channel_end = + ctx.channel_end(&(packet.destination_port.clone(), packet.destination_channel))?; if !dest_channel_end.state_matches(&State::Open) { return Err(Error::invalid_channel_state( @@ -43,7 +41,7 @@ pub fn process( // set on the store and return an error if so. match ctx.get_packet_acknowledgement(&( packet.destination_port.clone(), - packet.destination_channel.clone(), + packet.destination_channel, packet.sequence, )) { Ok(_) => return Err(Error::acknowledgement_exists(packet.sequence)), @@ -58,7 +56,7 @@ pub fn process( let result = PacketResult::WriteAck(WriteAckPacketResult { port_id: packet.source_port.clone(), - channel_id: packet.source_channel.clone(), + channel_id: packet.source_channel, seq: packet.sequence, ack: ack.clone(), }); @@ -119,10 +117,7 @@ mod tests { let dest_channel_end = ChannelEnd::new( State::Open, Order::default(), - Counterparty::new( - packet.source_port.clone(), - Some(packet.source_channel.clone()), - ), + Counterparty::new(packet.source_port.clone(), Some(packet.source_channel)), vec![ConnectionId::default()], Version::ics20(), ); @@ -168,7 +163,7 @@ mod tests { .with_port_capability(packet.destination_port.clone()) .with_channel( packet.destination_port.clone(), - packet.destination_channel.clone(), + packet.destination_channel, dest_channel_end.clone(), ), packet: packet.clone(), diff --git a/modules/src/core/ics24_host/identifier.rs b/modules/src/core/ics24_host/identifier.rs index bdf2d11baa..507ed716cb 100644 --- a/modules/src/core/ics24_host/identifier.rs +++ b/modules/src/core/ics24_host/identifier.rs @@ -336,7 +336,7 @@ impl Default for PortId { } } -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] pub struct ChannelId(u64); impl ChannelId { diff --git a/modules/src/mock/context.rs b/modules/src/mock/context.rs index de061d51f2..5c7bb65495 100644 --- a/modules/src/mock/context.rs +++ b/modules/src/mock/context.rs @@ -642,10 +642,7 @@ impl ChannelReader for MockContext { fn channel_end(&self, pcid: &(PortId, ChannelId)) -> Result { match self.channels.get(pcid) { Some(channel_end) => Ok(channel_end.clone()), - None => Err(Ics04Error::channel_not_found( - pcid.0.clone(), - pcid.1.clone(), - )), + None => Err(Ics04Error::channel_not_found(pcid.0.clone(), pcid.1)), } } diff --git a/relayer-cli/src/commands/clear.rs b/relayer-cli/src/commands/clear.rs index 618774a094..933c09a12c 100644 --- a/relayer-cli/src/commands/clear.rs +++ b/relayer-cli/src/commands/clear.rs @@ -52,7 +52,7 @@ impl Runnable for ClearPacketsCmd { // Construct links in both directions. let opts = LinkParameters { src_port_id: self.port_id.clone(), - src_channel_id: self.channel_id.clone(), + src_channel_id: self.channel_id, }; let fwd_link = match Link::new_from_opts(chains.src.clone(), chains.dst, opts, false) { Ok(link) => link, diff --git a/relayer-cli/src/commands/query/channel_ends.rs b/relayer-cli/src/commands/query/channel_ends.rs index 77cdaf955c..669cd02c6c 100644 --- a/relayer-cli/src/commands/query/channel_ends.rs +++ b/relayer-cli/src/commands/query/channel_ends.rs @@ -68,7 +68,7 @@ fn do_run(cmd: &QueryChannelEndsCmd) -> Result<(), Box>::new((*config).clone()); let chain = registry.get_or_spawn(&chain_id)?; diff --git a/relayer-cli/src/commands/tx/channel.rs b/relayer-cli/src/commands/tx/channel.rs index 641a34137c..a206221b59 100644 --- a/relayer-cli/src/commands/tx/channel.rs +++ b/relayer-cli/src/commands/tx/channel.rs @@ -174,7 +174,7 @@ impl Runnable for TxRawChanOpenTryCmd { ClientId::default(), ConnectionId::default(), self.src_port_id.clone(), - Some(self.src_chan_id.clone()), + Some(self.src_chan_id), None, ), b_side: ChannelSide::new( @@ -182,7 +182,7 @@ impl Runnable for TxRawChanOpenTryCmd { dst_connection.client_id().clone(), self.dst_conn_id.clone(), self.dst_port_id.clone(), - self.dst_chan_id.clone(), + self.dst_chan_id, None, ), } @@ -242,7 +242,7 @@ impl Runnable for TxRawChanOpenAckCmd { ClientId::default(), ConnectionId::default(), self.src_port_id.clone(), - Some(self.src_chan_id.clone()), + Some(self.src_chan_id), None, ), b_side: ChannelSide::new( @@ -250,7 +250,7 @@ impl Runnable for TxRawChanOpenAckCmd { dst_connection.client_id().clone(), self.dst_conn_id.clone(), self.dst_port_id.clone(), - Some(self.dst_chan_id.clone()), + Some(self.dst_chan_id), None, ), } @@ -310,7 +310,7 @@ impl Runnable for TxRawChanOpenConfirmCmd { ClientId::default(), ConnectionId::default(), self.src_port_id.clone(), - Some(self.src_chan_id.clone()), + Some(self.src_chan_id), None, ), b_side: ChannelSide::new( @@ -318,7 +318,7 @@ impl Runnable for TxRawChanOpenConfirmCmd { dst_connection.client_id().clone(), self.dst_conn_id.clone(), self.dst_port_id.clone(), - Some(self.dst_chan_id.clone()), + Some(self.dst_chan_id), None, ), } @@ -378,7 +378,7 @@ impl Runnable for TxRawChanCloseInitCmd { ClientId::default(), ConnectionId::default(), self.src_port_id.clone(), - Some(self.src_chan_id.clone()), + Some(self.src_chan_id), None, ), b_side: ChannelSide::new( @@ -386,7 +386,7 @@ impl Runnable for TxRawChanCloseInitCmd { dst_connection.client_id().clone(), self.dst_conn_id.clone(), self.dst_port_id.clone(), - Some(self.dst_chan_id.clone()), + Some(self.dst_chan_id), None, ), } @@ -446,7 +446,7 @@ impl Runnable for TxRawChanCloseConfirmCmd { ClientId::default(), ConnectionId::default(), self.src_port_id.clone(), - Some(self.src_chan_id.clone()), + Some(self.src_chan_id), None, ), b_side: ChannelSide::new( @@ -454,7 +454,7 @@ impl Runnable for TxRawChanCloseConfirmCmd { dst_connection.client_id().clone(), self.dst_conn_id.clone(), self.dst_port_id.clone(), - Some(self.dst_chan_id.clone()), + Some(self.dst_chan_id), None, ), } diff --git a/relayer-cli/src/commands/tx/packet.rs b/relayer-cli/src/commands/tx/packet.rs index 8bab19761b..2df1302d66 100644 --- a/relayer-cli/src/commands/tx/packet.rs +++ b/relayer-cli/src/commands/tx/packet.rs @@ -36,7 +36,7 @@ impl Runnable for TxRawPacketRecvCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), - src_channel_id: self.src_channel_id.clone(), + src_channel_id: self.src_channel_id, }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false) { Ok(link) => link, @@ -80,7 +80,7 @@ impl Runnable for TxRawPacketAckCmd { let opts = LinkParameters { src_port_id: self.src_port_id.clone(), - src_channel_id: self.src_channel_id.clone(), + src_channel_id: self.src_channel_id, }; let link = match Link::new_from_opts(chains.src, chains.dst, opts, false) { Ok(link) => link, diff --git a/relayer-cli/src/commands/tx/transfer.rs b/relayer-cli/src/commands/tx/transfer.rs index 5dca709156..40a44006a8 100644 --- a/relayer-cli/src/commands/tx/transfer.rs +++ b/relayer-cli/src/commands/tx/transfer.rs @@ -136,7 +136,7 @@ impl TxIcs20MsgTransferCmd { let opts = TransferOptions { packet_src_port_id: self.src_port_id.clone(), - packet_src_channel_id: self.src_channel_id.clone(), + packet_src_channel_id: self.src_channel_id, amount: self.amount, denom, receiver: self.receiver.clone(), diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index 79639bd817..ba9ff59da7 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -1574,11 +1574,7 @@ impl ChainEndpoint for CosmosSdkChain { crate::time!("query_channel"); crate::telemetry!(query, self.id(), "query_channel"); - let res = self.query( - ChannelEndsPath(port_id.clone(), channel_id.clone()), - height, - false, - )?; + let res = self.query(ChannelEndsPath(port_id.clone(), *channel_id), height, false)?; let channel_end = ChannelEnd::decode_vec(&res.value).map_err(Error::decode)?; Ok(channel_end) @@ -2017,11 +2013,7 @@ impl ChainEndpoint for CosmosSdkChain { channel_id: &ChannelId, height: ICSHeight, ) -> Result<(ChannelEnd, MerkleProof), Error> { - let res = self.query( - ChannelEndsPath(port_id.clone(), channel_id.clone()), - height, - true, - )?; + let res = self.query(ChannelEndsPath(port_id.clone(), *channel_id), height, true)?; let channel_end = ChannelEnd::decode_vec(&res.value).map_err(Error::decode)?; diff --git a/relayer/src/chain/counterparty.rs b/relayer/src/chain/counterparty.rs index 3a3de610aa..5aee9a3547 100644 --- a/relayer/src/chain/counterparty.rs +++ b/relayer/src/chain/counterparty.rs @@ -135,7 +135,7 @@ pub fn channel_connection_client( if channel_end.state_matches(&State::Uninitialized) { return Err(Error::channel_uninitialized( port_id.clone(), - channel_id.clone(), + *channel_id, chain.id(), )); } @@ -143,7 +143,7 @@ pub fn channel_connection_client( let connection_id = channel_end .connection_hops() .first() - .ok_or_else(|| Error::missing_connection_hops(channel_id.clone(), chain.id()))?; + .ok_or_else(|| Error::missing_connection_hops(*channel_id, chain.id()))?; let connection_end = chain .query_connection(connection_id, Height::zero()) @@ -152,7 +152,7 @@ pub fn channel_connection_client( if !connection_end.is_open() { return Err(Error::connection_not_open( connection_id.clone(), - channel_id.clone(), + *channel_id, chain.id(), )); } @@ -164,7 +164,7 @@ pub fn channel_connection_client( let client = IdentifiedAnyClientState::new(client_id.clone(), client_state); let connection = IdentifiedConnectionEnd::new(connection_id.clone(), connection_end); - let channel = IdentifiedChannelEnd::new(port_id.clone(), channel_id.clone(), channel_end); + let channel = IdentifiedChannelEnd::new(port_id.clone(), *channel_id, channel_end); Ok(ChannelConnectionClient::new(channel, connection, client)) } @@ -230,7 +230,7 @@ pub fn channel_on_destination( ) .map(|c| IdentifiedChannelEnd { port_id: channel.channel_end.counterparty().port_id().clone(), - channel_id: remote_channel_id.clone(), + channel_id: *remote_channel_id, channel_end: c, }) .map_err(Error::relayer)?; diff --git a/relayer/src/chain/handle/base.rs b/relayer/src/chain/handle/base.rs index 39cdcda106..9dbf33c6dc 100644 --- a/relayer/src/chain/handle/base.rs +++ b/relayer/src/chain/handle/base.rs @@ -263,7 +263,7 @@ impl ChainHandle for BaseChainHandle { ) -> Result { self.send(|reply_to| ChainRequest::QueryChannel { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id: *channel_id, height, reply_to, }) @@ -392,7 +392,7 @@ impl ChainHandle for BaseChainHandle { ) -> Result { self.send(|reply_to| ChainRequest::BuildChannelProofs { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id: *channel_id, height, reply_to, }) @@ -409,7 +409,7 @@ impl ChainHandle for BaseChainHandle { self.send(|reply_to| ChainRequest::BuildPacketProofs { packet_type, port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id: *channel_id, sequence, height, reply_to, diff --git a/relayer/src/chain/handle/cache.rs b/relayer/src/chain/handle/cache.rs index 1eec45c81f..9a3870d7b7 100644 --- a/relayer/src/chain/handle/cache.rs +++ b/relayer/src/chain/handle/cache.rs @@ -281,7 +281,7 @@ impl ChainHandle for CachingChainHandle { let handle = self.inner(); if height.is_zero() { let (result, in_cache) = self.cache.get_or_try_insert_channel_with( - &PortChannelId::new(channel_id.clone(), port_id.clone()), + &PortChannelId::new(*channel_id, port_id.clone()), || handle.query_channel(port_id, channel_id, height), )?; diff --git a/relayer/src/channel.rs b/relayer/src/channel.rs index 1224e9e059..dcd23238aa 100644 --- a/relayer/src/channel.rs +++ b/relayer/src/channel.rs @@ -201,7 +201,7 @@ impl Channel { .ok_or_else(|| ChannelError::invalid_event(channel_open_event))?; let port_id = channel_event_attributes.port_id.clone(); - let channel_id = channel_event_attributes.channel_id.clone(); + let channel_id = channel_event_attributes.channel_id; let connection_id = channel_event_attributes.connection_id.clone(); let connection = chain @@ -255,7 +255,7 @@ impl Channel { let a_connection_id = a_channel.connection_hops().first().ok_or_else(|| { ChannelError::supervisor(SupervisorError::missing_connection_hops( - channel.src_channel_id.clone(), + channel.src_channel_id, chain.id(), )) })?; @@ -270,7 +270,7 @@ impl Channel { .cloned() .ok_or_else(|| { ChannelError::supervisor(SupervisorError::channel_connection_uninitialized( - channel.src_channel_id.clone(), + channel.src_channel_id, chain.id(), a_connection.counterparty().clone(), )) @@ -283,7 +283,7 @@ impl Channel { a_connection.client_id().clone(), a_connection_id.clone(), channel.src_port_id.clone(), - Some(channel.src_channel_id.clone()), + Some(channel.src_channel_id), None, ), b_side: ChannelSide::new( @@ -291,7 +291,7 @@ impl Channel { a_connection.counterparty().client_id().clone(), b_connection_id.clone(), a_channel.remote.port_id.clone(), - a_channel.remote.channel_id.clone(), + a_channel.remote.channel_id, None, ), connection_delay: a_connection.delay_period(), @@ -383,7 +383,7 @@ impl Channel { info!("done {} => {:#?}\n", self.src_chain().id(), event); let channel_id = extract_channel_id(&event)?; - self.a_side.channel_id = Some(channel_id.clone()); + self.a_side.channel_id = Some(*channel_id); info!("successfully opened init channel"); Ok(()) @@ -413,7 +413,7 @@ impl Channel { })?; let channel_id = extract_channel_id(&event)?; - self.b_side.channel_id = Some(channel_id.clone()); + self.b_side.channel_id = Some(*channel_id); println!("done {} => {:#?}\n", self.dst_chain().id(), event); Ok(()) @@ -481,7 +481,7 @@ impl Channel { .map_err(|e| { ChannelError::handshake_finalize( channel.src_port_id().clone(), - src_channel_id.clone(), + *src_channel_id, channel.src_chain().id(), e, ) @@ -493,7 +493,7 @@ impl Channel { .map_err(|e| { ChannelError::handshake_finalize( channel.dst_port_id().clone(), - dst_channel_id.clone(), + *dst_channel_id, channel.dst_chain().id(), e, ) @@ -632,14 +632,14 @@ impl Channel { let channel_deps = channel_connection_client(self.src_chain(), self.src_port_id(), channel_id) - .map_err(|e| ChannelError::query_channel(channel_id.clone(), e))?; + .map_err(|e| ChannelError::query_channel(*channel_id, e))?; channel_state_on_destination( &channel_deps.channel, &channel_deps.connection, self.dst_chain(), ) - .map_err(|e| ChannelError::query_channel(channel_id.clone(), e)) + .map_err(|e| ChannelError::query_channel(*channel_id, e)) } pub fn handshake_step( @@ -833,7 +833,7 @@ impl Channel { } check_destination_channel_state( - dst_channel_id.clone(), + *dst_channel_id, dst_channel, dst_expected_channel.clone(), )?; @@ -859,7 +859,7 @@ impl Channel { self.dst_port_id().clone(), self.src_chain().id(), src_channel.counterparty().port_id.clone(), - src_channel_id.clone(), + *src_channel_id, )); } @@ -902,9 +902,9 @@ impl Channel { .map_err(|e| ChannelError::fetch_signer(self.dst_chain().id(), e))?; let previous_channel_id = if src_channel.counterparty().channel_id.is_none() { - self.b_side.channel_id.clone() + self.b_side.channel_id } else { - src_channel.counterparty().channel_id.clone() + src_channel.counterparty().channel_id }; // Build the domain type message @@ -994,8 +994,8 @@ impl Channel { // Build the domain type message let new_msg = MsgChannelOpenAck { port_id: self.dst_port_id().clone(), - channel_id: dst_channel_id.clone(), - counterparty_channel_id: src_channel_id.clone(), + channel_id: *dst_channel_id, + counterparty_channel_id: *src_channel_id, counterparty_version: src_channel.version().clone(), proofs, signer, @@ -1094,7 +1094,7 @@ impl Channel { // Build the domain type message let new_msg = MsgChannelOpenConfirm { port_id: self.dst_port_id().clone(), - channel_id: dst_channel_id.clone(), + channel_id: *dst_channel_id, proofs, signer, }; @@ -1163,7 +1163,7 @@ impl Channel { // Build the domain type message let new_msg = MsgChannelCloseInit { port_id: self.dst_port_id().clone(), - channel_id: dst_channel_id.clone(), + channel_id: *dst_channel_id, signer, }; @@ -1242,7 +1242,7 @@ impl Channel { // Build the domain type message let new_msg = MsgChannelCloseConfirm { port_id: self.dst_port_id().clone(), - channel_id: dst_channel_id.clone(), + channel_id: *dst_channel_id, proofs, signer, }; diff --git a/relayer/src/link.rs b/relayer/src/link.rs index de11e12e74..dd0530fbe0 100644 --- a/relayer/src/link.rs +++ b/relayer/src/link.rs @@ -56,7 +56,7 @@ impl Link { .map_err(|e| { LinkError::channel_not_found( self.a_to_b.src_port_id().clone(), - a_channel_id.clone(), + *a_channel_id, self.a_to_b.src_chain().id(), e, ) @@ -71,7 +71,7 @@ impl Link { .map_err(|e| { LinkError::channel_not_found( self.a_to_b.dst_port_id().clone(), - b_channel_id.clone(), + *b_channel_id, self.a_to_b.dst_chain().id(), e, ) @@ -98,19 +98,14 @@ impl Link { let a_channel = a_chain .query_channel(a_port_id, a_channel_id, Height::default()) .map_err(|e| { - LinkError::channel_not_found( - a_port_id.clone(), - a_channel_id.clone(), - a_chain.id(), - e, - ) + LinkError::channel_not_found(a_port_id.clone(), *a_channel_id, a_chain.id(), e) })?; if !a_channel.state_matches(&ChannelState::Open) && !a_channel.state_matches(&ChannelState::Closed) { return Err(LinkError::invalid_channel_state( - a_channel_id.clone(), + *a_channel_id, a_chain.id(), )); } @@ -118,25 +113,21 @@ impl Link { let b_channel_id = a_channel .counterparty() .channel_id - .clone() - .ok_or_else(|| LinkError::counterparty_channel_not_found(a_channel_id.clone()))?; + .ok_or_else(|| LinkError::counterparty_channel_not_found(*a_channel_id))?; if a_channel.connection_hops().is_empty() { - return Err(LinkError::no_connection_hop( - a_channel_id.clone(), - a_chain.id(), - )); + return Err(LinkError::no_connection_hop(*a_channel_id, a_chain.id())); } // Check that the counterparty details on the destination chain matches the source chain check_channel_counterparty( b_chain.clone(), &PortChannelId { - channel_id: b_channel_id.clone(), + channel_id: b_channel_id, port_id: a_channel.counterparty().port_id.clone(), }, &PortChannelId { - channel_id: a_channel_id.clone(), + channel_id: *a_channel_id, port_id: opts.src_port_id.clone(), }, ) @@ -149,10 +140,7 @@ impl Link { .map_err(LinkError::relayer)?; if !a_connection.state_matches(&ConnectionState::Open) { - return Err(LinkError::channel_not_opened( - a_channel_id.clone(), - a_chain.id(), - )); + return Err(LinkError::channel_not_opened(*a_channel_id, a_chain.id())); } let channel = Channel { @@ -162,7 +150,7 @@ impl Link { a_connection.client_id().clone(), a_connection_id, opts.src_port_id.clone(), - Some(opts.src_channel_id.clone()), + Some(opts.src_channel_id), None, ), b_side: ChannelSide::new( @@ -184,7 +172,7 @@ impl Link { pub fn reverse(&self, with_tx_confirmation: bool) -> Result, LinkError> { 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(), + src_channel_id: *self.a_to_b.dst_channel_id(), }; let chain_b = self.a_to_b.dst_chain().clone(); let chain_a = self.a_to_b.src_chain().clone(); diff --git a/relayer/src/link/relay_path.rs b/relayer/src/link/relay_path.rs index 7e286a9219..0561d3ceae 100644 --- a/relayer/src/link/relay_path.rs +++ b/relayer/src/link/relay_path.rs @@ -99,15 +99,13 @@ impl RelayPath { let src_chain_id = src_chain.id(); let dst_chain_id = dst_chain.id(); - let src_channel_id = channel + let src_channel_id = *channel .src_channel_id() - .ok_or_else(|| LinkError::missing_channel_id(src_chain.id()))? - .clone(); + .ok_or_else(|| LinkError::missing_channel_id(src_chain.id()))?; - let dst_channel_id = channel + let dst_channel_id = *channel .dst_channel_id() - .ok_or_else(|| LinkError::missing_channel_id(dst_chain.id()))? - .clone(); + .ok_or_else(|| LinkError::missing_channel_id(dst_chain.id()))?; let src_port_id = channel.src_port_id().clone(); let dst_port_id = channel.dst_port_id().clone(); @@ -115,9 +113,9 @@ impl RelayPath { Ok(Self { channel, - src_channel_id: src_channel_id.clone(), + src_channel_id, src_port_id: src_port_id.clone(), - dst_channel_id: dst_channel_id.clone(), + dst_channel_id, dst_port_id: dst_port_id.clone(), src_operational_data: Queue::new(), @@ -275,7 +273,7 @@ impl RelayPath { // Build the domain type message let new_msg = MsgChannelCloseConfirm { port_id: self.dst_port_id().clone(), - channel_id: self.dst_channel_id().clone(), + channel_id: *self.dst_channel_id(), proofs, signer: self.dst_signer()?, }; @@ -983,9 +981,9 @@ impl RelayPath { let mut query = QueryPacketEventDataRequest { event_id: WithBlockDataType::SendPacket, source_port_id: self.src_port_id().clone(), - source_channel_id: src_channel_id.clone(), + source_channel_id: *src_channel_id, destination_port_id: self.dst_port_id().clone(), - destination_channel_id: dst_channel_id.clone(), + destination_channel_id: *dst_channel_id, sequences, height: query_height, }; @@ -1098,9 +1096,9 @@ impl RelayPath { .query_txs(QueryTxRequest::Packet(QueryPacketEventDataRequest { event_id: WithBlockDataType::WriteAck, source_port_id: self.dst_port_id().clone(), - source_channel_id: dst_channel_id.clone(), + source_channel_id: *dst_channel_id, destination_port_id: self.src_port_id().clone(), - destination_channel_id: src_channel_id.clone(), + destination_channel_id: *src_channel_id, sequences, height: query_height, })) diff --git a/relayer/src/object.rs b/relayer/src/object.rs index d62f923a0a..34c44f7993 100644 --- a/relayer/src/object.rs +++ b/relayer/src/object.rs @@ -375,7 +375,7 @@ impl Object { Ok(Channel { dst_chain_id, src_chain_id: src_chain.id(), - src_channel_id: channel_id.clone(), + src_channel_id: *channel_id, src_port_id: attributes.port_id().clone(), } .into()) @@ -396,7 +396,7 @@ impl Object { Ok(Packet { dst_chain_id, src_chain_id: src_chain.id(), - src_channel_id: e.packet.source_channel.clone(), + src_channel_id: e.packet.source_channel, src_port_id: e.packet.source_port.clone(), } .into()) @@ -417,7 +417,7 @@ impl Object { Ok(Packet { dst_chain_id, src_chain_id: src_chain.id(), - src_channel_id: e.packet.destination_channel.clone(), + src_channel_id: e.packet.destination_channel, src_port_id: e.packet.destination_port.clone(), } .into()) @@ -438,7 +438,7 @@ impl Object { Ok(Packet { dst_chain_id, src_chain_id: src_chain.id(), - src_channel_id: e.src_channel_id().clone(), + src_channel_id: *e.src_channel_id(), src_port_id: e.src_port_id().clone(), } .into()) @@ -455,7 +455,7 @@ impl Object { Ok(Packet { dst_chain_id, src_chain_id: src_chain.id(), - src_channel_id: e.channel_id().clone(), + src_channel_id: *e.channel_id(), src_port_id: e.port_id().clone(), } .into()) diff --git a/relayer/src/supervisor/client_state_filter.rs b/relayer/src/supervisor/client_state_filter.rs index 55e100a6d8..e34776dfed 100644 --- a/relayer/src/supervisor/client_state_filter.rs +++ b/relayer/src/supervisor/client_state_filter.rs @@ -271,7 +271,7 @@ impl FilterPolicy { port_id: &PortId, channel_id: &ChannelId, ) -> Result { - let identifier = CacheKey::Channel(chain_id.clone(), port_id.clone(), channel_id.clone()); + let identifier = CacheKey::Channel(chain_id.clone(), port_id.clone(), *channel_id); trace!( "[client filter] controlling permissions for {:?}", @@ -315,7 +315,7 @@ impl FilterPolicy { conn_id, )?; - let key = CacheKey::Channel(chain_id.clone(), port_id.clone(), channel_id.clone()); + let key = CacheKey::Channel(chain_id.clone(), port_id.clone(), *channel_id); debug!( "[client filter] {:?}: relay for channel {:?}: ", diff --git a/relayer/src/supervisor/scan.rs b/relayer/src/supervisor/scan.rs index fd248a8692..8983cb242e 100644 --- a/relayer/src/supervisor/scan.rs +++ b/relayer/src/supervisor/scan.rs @@ -349,7 +349,7 @@ impl<'a, Chain: ChainHandle> ChainScanner<'a, Chain> { connection_scan .channels - .entry(channel.channel_id.clone()) + .entry(channel.channel_id) .or_insert_with(|| ChannelScan::new(channel, counterparty_channel)); } Err(e) => error!(channel = %channel_id, "failed to scan channel, reason: {}", e), @@ -485,7 +485,7 @@ impl<'a, Chain: ChainHandle> ChainScanner<'a, Chain> { counterparty, }; - (scan.id().clone(), scan) + (*scan.id(), scan) }) .collect(); @@ -623,7 +623,7 @@ fn scan_allowed_channel( { return Err(Error::uninitialized_channel( port_id.clone(), - channel_id.clone(), + *channel_id, chain.id(), )); } @@ -708,7 +708,7 @@ fn query_channel( Ok(IdentifiedChannelEnd::new( port_id.clone(), - channel_id.clone(), + *channel_id, channel_end, )) } @@ -723,11 +723,7 @@ fn query_connection_for_channel( .first() .cloned() .ok_or_else(|| { - Error::missing_connection_hop( - channel.port_id.clone(), - channel.channel_id.clone(), - chain.id(), - ) + Error::missing_connection_hop(channel.port_id.clone(), channel.channel_id, chain.id()) })?; query_connection(chain, &connection_id) diff --git a/relayer/src/supervisor/spawn.rs b/relayer/src/supervisor/spawn.rs index 8562b823ee..b0da31bc1a 100644 --- a/relayer/src/supervisor/spawn.rs +++ b/relayer/src/supervisor/spawn.rs @@ -259,7 +259,7 @@ impl<'a, Chain: ChainHandle> SpawnContext<'a, Chain> { let path_object = Object::Packet(Packet { dst_chain_id: counterparty_chain.id(), src_chain_id: chain.id(), - src_channel_id: channel_scan.id().clone(), + src_channel_id: *channel_scan.id(), src_port_id: channel_scan.channel.port_id.clone(), }); @@ -283,7 +283,7 @@ impl<'a, Chain: ChainHandle> SpawnContext<'a, Chain> { let channel_object = Object::Channel(Channel { dst_chain_id: counterparty_chain.id(), src_chain_id: chain.id(), - src_channel_id: channel_scan.id().clone(), + src_channel_id: *channel_scan.id(), src_port_id: channel_scan.channel.port_id, }); diff --git a/relayer/src/transfer.rs b/relayer/src/transfer.rs index e924f9742c..f6e928431a 100644 --- a/relayer/src/transfer.rs +++ b/relayer/src/transfer.rs @@ -112,7 +112,7 @@ pub fn build_and_send_transfer_messages( chains.b, LinkParameters { src_port_id: path.src_port_id.clone(), - src_channel_id: path.src_channel_id.clone(), + src_channel_id: path.src_channel_id, }, packets_config.tx_confirmation, ); diff --git a/tools/test-framework/src/bootstrap/binary/channel.rs b/tools/test-framework/src/bootstrap/binary/channel.rs index bcbef60f5a..6399c5852f 100644 --- a/tools/test-framework/src/bootstrap/binary/channel.rs +++ b/tools/test-framework/src/bootstrap/binary/channel.rs @@ -102,17 +102,15 @@ pub fn bootstrap_channel_with_connection TaggedChannelEndExt for DualTagged { fn tagged_counterparty_channel_id(&self) -> Option> { - self.contra_map(|c| c.counterparty().channel_id.clone()) - .transpose() + self.contra_map(|c| c.counterparty().channel_id).transpose() } fn tagged_counterparty_port_id(&self) -> TaggedPortId { @@ -65,7 +64,7 @@ pub fn init_channel( let event = channel.build_chan_open_init_and_send()?; - let channel_id = extract_channel_id(&event)?.clone(); + let channel_id = *extract_channel_id(&event)?; let channel2 = Channel::restore_from_event(handle_b.clone(), handle_a.clone(), event)?; diff --git a/tools/test-framework/src/relayer/transfer.rs b/tools/test-framework/src/relayer/transfer.rs index 1ed1741a91..cd0575a76e 100644 --- a/tools/test-framework/src/relayer/transfer.rs +++ b/tools/test-framework/src/relayer/transfer.rs @@ -51,7 +51,7 @@ pub fn tx_raw_ft_transfer( ) -> Result, Error> { let transfer_options = TransferOptions { packet_src_port_id: channel.port_a.value().clone(), - packet_src_channel_id: channel.channel_id_a.value().clone(), + packet_src_channel_id: *channel.channel_id_a.value(), amount: Amount(amount.into()), denom: denom.value().to_string(), receiver: Some(recipient.value().0.clone()), From 8f4770b9e37e764f95604901c144c09a01c1260f Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Thu, 7 Apr 2022 01:39:16 +0530 Subject: [PATCH 07/10] Delete unused validate_channel_identifier() function --- modules/src/core/ics24_host/validate.rs | 28 ++----------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/modules/src/core/ics24_host/validate.rs b/modules/src/core/ics24_host/validate.rs index 5a19a13703..722d35b124 100644 --- a/modules/src/core/ics24_host/validate.rs +++ b/modules/src/core/ics24_host/validate.rs @@ -67,19 +67,11 @@ pub fn validate_port_identifier(id: &str) -> Result<(), Error> { validate_identifier(id, 2, 128) } -/// Default validator function for Channel identifiers. -/// -/// A valid Identifier must be between 10-64 characters and only contain lowercase -/// alphabetic characters, -pub fn validate_channel_identifier(id: &str) -> Result<(), Error> { - validate_identifier(id, 8, 64) -} - #[cfg(test)] mod tests { use crate::core::ics24_host::validate::{ - validate_channel_identifier, validate_client_identifier, validate_connection_identifier, - validate_identifier, validate_port_identifier, + validate_client_identifier, validate_connection_identifier, validate_identifier, + validate_port_identifier, }; use test_log::test; @@ -131,22 +123,6 @@ mod tests { assert!(id.is_err()) } - #[test] - fn parse_invalid_channel_id_min() { - // invalid min channel id - let id = validate_channel_identifier("channel"); - assert!(id.is_err()) - } - - #[test] - fn parse_channel_id_max() { - // invalid max channel id (test string length is 65) - let id = validate_channel_identifier( - "hlkbzrbmrh0rjrh8f8a8d9lmtjuhww7a9ev3blskc58amhwfq07zwp1xevxz1p098", - ); - assert!(id.is_err()) - } - #[test] fn parse_invalid_id_chars() { // invalid id chars From 1c81c0a2738ef62302914d99ffcbd6156c55d6bb Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Thu, 7 Apr 2022 02:10:36 +0530 Subject: [PATCH 08/10] Custom serde for ChannelId --- modules/src/core/ics24_host/identifier.rs | 31 ++++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/modules/src/core/ics24_host/identifier.rs b/modules/src/core/ics24_host/identifier.rs index 507ed716cb..2c5e706ca0 100644 --- a/modules/src/core/ics24_host/identifier.rs +++ b/modules/src/core/ics24_host/identifier.rs @@ -1,14 +1,13 @@ -use crate::prelude::*; - use core::convert::{From, Infallible}; use core::fmt::{self, Display, Formatter}; use core::str::FromStr; -use serde::{Deserialize, Serialize}; -use crate::core::ics02_client::client_type::ClientType; -use crate::core::ics24_host::error::ValidationError; +use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use super::validate::*; +use crate::core::ics02_client::client_type::ClientType; +use crate::core::ics24_host::error::ValidationError; +use crate::prelude::*; /// This type is subject to future changes. /// @@ -336,7 +335,7 @@ impl Default for PortId { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct ChannelId(u64); impl ChannelId { @@ -389,6 +388,26 @@ impl Default for ChannelId { } } +impl Serialize for ChannelId { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serializer.collect_str(self) + } +} + +impl<'de> Deserialize<'de> for ChannelId { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + String::deserialize(deserializer)? + .parse() + .map_err(de::Error::custom) + } +} + /// A pair of [`PortId`] and [`ChannelId`] are used together for sending IBC packets. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] pub struct PortChannelId { From a632ed223c577840d835048852c11e8ce64b79ac Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Thu, 7 Apr 2022 02:23:36 +0530 Subject: [PATCH 09/10] Add .changelog entry --- .changelog/unreleased/improvements/ibc/2068-chan-id-u64.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 .changelog/unreleased/improvements/ibc/2068-chan-id-u64.md diff --git a/.changelog/unreleased/improvements/ibc/2068-chan-id-u64.md b/.changelog/unreleased/improvements/ibc/2068-chan-id-u64.md new file mode 100644 index 0000000000..f9ffd9d8cb --- /dev/null +++ b/.changelog/unreleased/improvements/ibc/2068-chan-id-u64.md @@ -0,0 +1 @@ +- Improve `ChannelId` validation. ([#2068](https://github.com/informalsystems/ibc-rs/issues/2068)) From 1a6b41ad4ec5e2f2183bcdc74e4be99a4713f414 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Thu, 7 Apr 2022 02:49:08 +0530 Subject: [PATCH 10/10] Fix failing tests --- modules/src/core/ics24_host/identifier.rs | 2 +- relayer/src/config/filter.rs | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/modules/src/core/ics24_host/identifier.rs b/modules/src/core/ics24_host/identifier.rs index 2c5e706ca0..be1b4634a3 100644 --- a/modules/src/core/ics24_host/identifier.rs +++ b/modules/src/core/ics24_host/identifier.rs @@ -348,7 +348,7 @@ impl ChannelId { /// ``` /// # use ibc::core::ics24_host::identifier::ChannelId; /// let chan_id = ChannelId::new(27); - /// assert_eq!(&chan_id, "channel-27"); + /// assert_eq!(chan_id.to_string(), "channel-27"); /// ``` pub fn new(counter: u64) -> Self { Self(counter) diff --git a/relayer/src/config/filter.rs b/relayer/src/config/filter.rs index 297ae7bb30..36cb759e91 100644 --- a/relayer/src/config/filter.rs +++ b/relayer/src/config/filter.rs @@ -369,7 +369,7 @@ mod tests { ['ica*', '*'], ['transfer', 'channel-0'], ['transfer*', 'channel-1'], - ['ft-transfer', 'network-0'], + ['ft-transfer', 'channel-2'], ] "#; @@ -386,7 +386,7 @@ mod tests { ), ( &PortId::from_str("ft-transfer").unwrap(), - &ChannelId::from_str("network-0").unwrap() + &ChannelId::from_str("channel-2").unwrap() ) ] ); @@ -404,7 +404,7 @@ mod tests { ['ica*', '*'], ['transfer', 'channel-0'], ['transfer*', 'channel-1'], - ['ft-transfer', 'network-0'], + ['ft-transfer', 'channel-2'], ] "#; @@ -412,11 +412,11 @@ mod tests { assert!(!pf.is_allowed( &PortId::from_str("ft-transfer").unwrap(), - &ChannelId::from_str("network-0").unwrap() + &ChannelId::from_str("channel-2").unwrap() )); assert!(pf.is_allowed( &PortId::from_str("ft-transfer").unwrap(), - &ChannelId::from_str("network-1").unwrap() + &ChannelId::from_str("channel-1").unwrap() )); assert!(pf.is_allowed( &PortId::from_str("transfer").unwrap(), @@ -437,7 +437,7 @@ mod tests { ['ica*', '*'], ['transfer', 'channel-0'], ['transfer*', 'channel-1'], - ['ft-transfer', 'network-0'], + ['ft-transfer', 'channel-2'], ] "#; @@ -445,11 +445,11 @@ mod tests { assert!(pf.is_allowed( &PortId::from_str("ft-transfer").unwrap(), - &ChannelId::from_str("network-0").unwrap() + &ChannelId::from_str("channel-2").unwrap() )); assert!(!pf.is_allowed( &PortId::from_str("ft-transfer").unwrap(), - &ChannelId::from_str("network-1").unwrap() + &ChannelId::from_str("channel-1").unwrap() )); assert!(!pf.is_allowed( &PortId::from_str("transfer-1").unwrap(), @@ -461,7 +461,7 @@ mod tests { )); assert!(pf.is_allowed( &PortId::from_str("ica").unwrap(), - &ChannelId::from_str("channel1").unwrap() + &ChannelId::from_str("channel-1").unwrap() )); } }