From 96024fe265e534fdf9d722dbe0d276de625d5441 Mon Sep 17 00:00:00 2001 From: Davirain Date: Thu, 16 Mar 2023 21:06:19 +0800 Subject: [PATCH 1/3] make Model ChannelId as newtype u64 --- .../ibc/src/applications/transfer/context.rs | 2 +- crates/ibc/src/applications/transfer/relay.rs | 4 +- .../transfer/relay/on_recv_packet.rs | 6 +- .../transfer/relay/send_transfer.rs | 32 ++++----- .../ibc/src/core/context/acknowledgement.rs | 11 ++- .../src/core/context/chan_close_confirm.rs | 7 +- .../ibc/src/core/context/chan_close_init.rs | 7 +- crates/ibc/src/core/context/chan_open_ack.rs | 12 ++-- .../ibc/src/core/context/chan_open_confirm.rs | 3 +- crates/ibc/src/core/context/chan_open_init.rs | 2 +- crates/ibc/src/core/context/chan_open_try.rs | 10 +-- crates/ibc/src/core/context/recv_packet.rs | 2 +- crates/ibc/src/core/context/timeout.rs | 15 ++-- crates/ibc/src/core/ics04_channel/events.rs | 18 ++--- .../events/channel_attributes.rs | 8 ++- .../ics04_channel/events/packet_attributes.rs | 12 +++- .../ics04_channel/handler/acknowledgement.rs | 11 ++- .../handler/chan_close_confirm.rs | 8 +-- .../ics04_channel/handler/chan_close_init.rs | 6 +- .../ics04_channel/handler/chan_open_ack.rs | 26 ++----- .../handler/chan_open_confirm.rs | 4 +- .../ics04_channel/handler/chan_open_try.rs | 2 +- .../core/ics04_channel/handler/recv_packet.rs | 16 ++--- .../core/ics04_channel/handler/send_packet.rs | 9 +-- .../src/core/ics04_channel/handler/timeout.rs | 6 +- .../ics04_channel/handler/timeout_on_close.rs | 17 ++--- crates/ibc/src/core/ics24_host/error.rs | 2 + crates/ibc/src/core/ics24_host/identifier.rs | 68 ++++++++++--------- crates/ibc/src/core/ics24_host/path.rs | 14 ++-- crates/ibc/src/mock/context.rs | 52 +++++++------- 30 files changed, 183 insertions(+), 209 deletions(-) diff --git a/crates/ibc/src/applications/transfer/context.rs b/crates/ibc/src/applications/transfer/context.rs index d58efff41..b948d2174 100644 --- a/crates/ibc/src/applications/transfer/context.rs +++ b/crates/ibc/src/applications/transfer/context.rs @@ -442,7 +442,7 @@ pub(crate) mod test { let connection_hops = vec![ConnectionId::new(1)]; let port_id = PortId::transfer(); let channel_id = ChannelId::new(1); - let counterparty = Counterparty::new(port_id.clone(), Some(channel_id.clone())); + let counterparty = Counterparty::new(port_id.clone(), Some(channel_id)); ( ctx, diff --git a/crates/ibc/src/applications/transfer/relay.rs b/crates/ibc/src/applications/transfer/relay.rs index 3d69e82c7..3ddb1467f 100644 --- a/crates/ibc/src/applications/transfer/relay.rs +++ b/crates/ibc/src/applications/transfer/relay.rs @@ -23,7 +23,7 @@ pub fn refund_packet_token_execute( if is_sender_chain_source( packet.port_id_on_a.clone(), - packet.chan_id_on_a.clone(), + packet.chan_id_on_a, &data.token.denom, ) { // unescrow tokens back to sender @@ -51,7 +51,7 @@ pub fn refund_packet_token_validate( if is_sender_chain_source( packet.port_id_on_a.clone(), - packet.chan_id_on_a.clone(), + packet.chan_id_on_a, &data.token.denom, ) { let escrow_address = diff --git a/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs b/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs index 33d086381..7bf4aad27 100644 --- a/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs +++ b/crates/ibc/src/applications/transfer/relay/on_recv_packet.rs @@ -31,11 +31,11 @@ pub fn process_recv_packet_execute( let extras = if is_receiver_chain_source( packet.port_id_on_a.clone(), - packet.chan_id_on_a.clone(), + packet.chan_id_on_a, &data.token.denom, ) { // sender chain is not the source, unescrow tokens - let prefix = TracePrefix::new(packet.port_id_on_a.clone(), packet.chan_id_on_a.clone()); + let prefix = TracePrefix::new(packet.port_id_on_a.clone(), packet.chan_id_on_a); let coin = { let mut c = data.token; c.denom.remove_trace_prefix(&prefix); @@ -67,7 +67,7 @@ pub fn process_recv_packet_execute( ModuleExtras::empty() } else { // sender chain is the source, mint vouchers - let prefix = TracePrefix::new(packet.port_id_on_b.clone(), packet.chan_id_on_b.clone()); + let prefix = TracePrefix::new(packet.port_id_on_b.clone(), packet.chan_id_on_b); let coin = { let mut c = data.token; c.denom.add_trace_prefix(prefix); diff --git a/crates/ibc/src/applications/transfer/relay/send_transfer.rs b/crates/ibc/src/applications/transfer/relay/send_transfer.rs index 2b99133a0..cb9ec5201 100644 --- a/crates/ibc/src/applications/transfer/relay/send_transfer.rs +++ b/crates/ibc/src/applications/transfer/relay/send_transfer.rs @@ -40,14 +40,12 @@ where .map_err(TokenTransferError::ContextError)?; let port_id_on_b = chan_end_on_a.counterparty().port_id().clone(); - let chan_id_on_b = chan_end_on_a - .counterparty() - .channel_id() - .ok_or_else(|| TokenTransferError::DestinationChannelNotFound { + let chan_id_on_b = chan_end_on_a.counterparty().channel_id().ok_or_else(|| { + TokenTransferError::DestinationChannelNotFound { port_id: msg.port_id_on_a.clone(), - channel_id: msg.chan_id_on_a.clone(), - })? - .clone(); + channel_id: msg.chan_id_on_a, + } + })?; let seq_send_path_on_a = SeqSendPath::new(&msg.port_id_on_a, &msg.chan_id_on_a); let sequence = ctx_a @@ -70,7 +68,7 @@ where .try_into() .map_err(|_| TokenTransferError::ParseAccountFailure)?; - if is_sender_chain_source(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone(), &denom) { + if is_sender_chain_source(msg.port_id_on_a.clone(), msg.chan_id_on_a, &denom) { let escrow_address = ctx_a.get_escrow_account(&msg.port_id_on_a, &msg.chan_id_on_a)?; ctx_a.send_coins_validate(&sender, &escrow_address, &coin)?; } else { @@ -91,7 +89,7 @@ where port_id_on_a: msg.port_id_on_a, chan_id_on_a: msg.chan_id_on_a, port_id_on_b, - chan_id_on_b, + chan_id_on_b: *chan_id_on_b, data, timeout_height_on_b: msg.timeout_height_on_b, timeout_timestamp_on_b: msg.timeout_timestamp_on_b, @@ -116,14 +114,12 @@ where .map_err(TokenTransferError::ContextError)?; let port_on_b = chan_end_on_a.counterparty().port_id().clone(); - let chan_on_b = chan_end_on_a - .counterparty() - .channel_id() - .ok_or_else(|| TokenTransferError::DestinationChannelNotFound { + let chan_on_b = chan_end_on_a.counterparty().channel_id().ok_or_else(|| { + TokenTransferError::DestinationChannelNotFound { port_id: msg.port_id_on_a.clone(), - channel_id: msg.chan_id_on_a.clone(), - })? - .clone(); + channel_id: msg.chan_id_on_a, + } + })?; // get the next sequence let seq_send_path_on_a = SeqSendPath::new(&msg.port_id_on_a, &msg.chan_id_on_a); @@ -147,7 +143,7 @@ where .try_into() .map_err(|_| TokenTransferError::ParseAccountFailure)?; - if is_sender_chain_source(msg.port_id_on_a.clone(), msg.chan_id_on_a.clone(), &denom) { + if is_sender_chain_source(msg.port_id_on_a.clone(), msg.chan_id_on_a, &denom) { let escrow_address = ctx_a.get_escrow_account(&msg.port_id_on_a, &msg.chan_id_on_a)?; ctx_a.send_coins_execute(&sender, &escrow_address, &coin)?; } else { @@ -168,7 +164,7 @@ where port_id_on_a: msg.port_id_on_a, chan_id_on_a: msg.chan_id_on_a, port_id_on_b: port_on_b, - chan_id_on_b: chan_on_b, + chan_id_on_b: *chan_on_b, data, timeout_height_on_b: msg.timeout_height_on_b, timeout_timestamp_on_b: msg.timeout_timestamp_on_b, diff --git a/crates/ibc/src/core/context/acknowledgement.rs b/crates/ibc/src/core/context/acknowledgement.rs index ab6399d58..4b4de36ce 100644 --- a/crates/ibc/src/core/context/acknowledgement.rs +++ b/crates/ibc/src/core/context/acknowledgement.rs @@ -81,7 +81,7 @@ where { let commitment_path_on_a = CommitmentPath { port_id: msg.packet.port_id_on_a.clone(), - channel_id: msg.packet.chan_id_on_a.clone(), + channel_id: msg.packet.chan_id_on_a, sequence: msg.packet.seq_on_a, }; ctx_a.delete_packet_commitment(&commitment_path_on_a)?; @@ -177,10 +177,7 @@ mod tests { let chan_end_on_a_unordered = ChannelEnd::new( State::Open, Order::Unordered, - Counterparty::new( - packet.port_id_on_b.clone(), - Some(packet.chan_id_on_b.clone()), - ), + Counterparty::new(packet.port_id_on_b.clone(), Some(packet.chan_id_on_b)), vec![ConnectionId::default()], Version::new("ics20-1".to_string()), ); @@ -236,7 +233,7 @@ mod tests { .with_connection(ConnectionId::default(), conn_end_on_a) .with_packet_commitment( msg.packet.port_id_on_a.clone(), - msg.packet.chan_id_on_a.clone(), + msg.packet.chan_id_on_a, msg.packet.seq_on_a, packet_commitment, ); @@ -272,7 +269,7 @@ mod tests { .with_connection(ConnectionId::default(), conn_end_on_a) .with_packet_commitment( msg.packet.port_id_on_a.clone(), - msg.packet.chan_id_on_a.clone(), + msg.packet.chan_id_on_a, msg.packet.seq_on_a, packet_commitment, ); diff --git a/crates/ibc/src/core/context/chan_close_confirm.rs b/crates/ibc/src/core/context/chan_close_confirm.rs index 7042443fa..7a7d85d01 100644 --- a/crates/ibc/src/core/context/chan_close_confirm.rs +++ b/crates/ibc/src/core/context/chan_close_confirm.rs @@ -63,7 +63,6 @@ where let chan_id_on_a = chan_end_on_b .counterparty() .channel_id - .clone() .ok_or(ContextError::ChannelError(ChannelError::Other { description: "internal error: ChannelEnd doesn't have a counterparty channel id in CloseInit" @@ -73,7 +72,7 @@ where IbcEvent::CloseConfirmChannel(CloseConfirm::new( msg.port_id_on_b.clone(), - msg.chan_id_on_b.clone(), + msg.chan_id_on_b, port_id_on_a, chan_id_on_a, conn_id_on_b, @@ -145,7 +144,7 @@ mod tests { Order::default(), Counterparty::new( msg_chan_close_confirm.port_id_on_b.clone(), - Some(msg_chan_close_confirm.chan_id_on_b.clone()), + Some(msg_chan_close_confirm.chan_id_on_b), ), vec![conn_id.clone()], Version::default(), @@ -156,7 +155,7 @@ mod tests { .with_connection(conn_id, conn_end) .with_channel( msg_chan_close_confirm.port_id_on_b.clone(), - msg_chan_close_confirm.chan_id_on_b.clone(), + msg_chan_close_confirm.chan_id_on_b, chan_end, ); diff --git a/crates/ibc/src/core/context/chan_close_init.rs b/crates/ibc/src/core/context/chan_close_init.rs index 5c4c0dfce..74cf5a494 100644 --- a/crates/ibc/src/core/context/chan_close_init.rs +++ b/crates/ibc/src/core/context/chan_close_init.rs @@ -63,7 +63,6 @@ where let chan_id_on_b = chan_end_on_a .counterparty() .channel_id - .clone() .ok_or(ContextError::ChannelError(ChannelError::Other { description: "internal error: ChannelEnd doesn't have a counterparty channel id in CloseInit" @@ -73,7 +72,7 @@ where IbcEvent::CloseInitChannel(CloseInit::new( msg.port_id_on_a.clone(), - msg.chan_id_on_a.clone(), + msg.chan_id_on_a, port_id_on_b, chan_id_on_b, conn_id_on_a, @@ -141,7 +140,7 @@ mod tests { Order::default(), Counterparty::new( msg_chan_close_init.port_id_on_a.clone(), - Some(msg_chan_close_init.chan_id_on_a.clone()), + Some(msg_chan_close_init.chan_id_on_a), ), vec![conn_id.clone()], Version::default(), @@ -160,7 +159,7 @@ mod tests { .with_connection(conn_id, conn_end) .with_channel( msg_chan_close_init.port_id_on_a.clone(), - msg_chan_close_init.chan_id_on_a.clone(), + msg_chan_close_init.chan_id_on_a, chan_end, ) }; diff --git a/crates/ibc/src/core/context/chan_open_ack.rs b/crates/ibc/src/core/context/chan_open_ack.rs index fd638e8b2..fdf0683d8 100644 --- a/crates/ibc/src/core/context/chan_open_ack.rs +++ b/crates/ibc/src/core/context/chan_open_ack.rs @@ -53,7 +53,7 @@ where chan_end_on_a.set_state(State::Open); chan_end_on_a.set_version(msg.version_on_b.clone()); - chan_end_on_a.set_counterparty_channel_id(msg.chan_id_on_b.clone()); + chan_end_on_a.set_counterparty_channel_id(msg.chan_id_on_b); chan_end_on_a }; @@ -70,7 +70,7 @@ where IbcEvent::OpenAckChannel(OpenAck::new( msg.port_id_on_a.clone(), - msg.chan_id_on_a.clone(), + msg.chan_id_on_a, port_id_on_b, msg.chan_id_on_b, conn_id_on_a, @@ -157,7 +157,7 @@ mod tests { let chan_end_on_a = ChannelEnd::new( State::Init, Order::Unordered, - Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_b.clone())), + Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_b)), vec![conn_id_on_a.clone()], msg.version_on_b.clone(), ); @@ -191,11 +191,7 @@ mod tests { let mut context = context .with_client(&client_id_on_a, Height::new(0, proof_height).unwrap()) .with_connection(conn_id_on_a, conn_end_on_a) - .with_channel( - msg.port_id_on_a.clone(), - msg.chan_id_on_a.clone(), - chan_end_on_a, - ); + .with_channel(msg.port_id_on_a.clone(), msg.chan_id_on_a, chan_end_on_a); let res = chan_open_ack_execute(&mut context, module_id, msg); diff --git a/crates/ibc/src/core/context/chan_open_confirm.rs b/crates/ibc/src/core/context/chan_open_confirm.rs index 58ba111f2..38c3cb19f 100644 --- a/crates/ibc/src/core/context/chan_open_confirm.rs +++ b/crates/ibc/src/core/context/chan_open_confirm.rs @@ -65,7 +65,6 @@ where let chan_id_on_a = chan_end_on_b .counterparty() .channel_id - .clone() .ok_or(ContextError::ChannelError(ChannelError::Other { description: "internal error: ChannelEnd doesn't have a counterparty channel id in OpenConfirm" @@ -74,7 +73,7 @@ where let core_event = IbcEvent::OpenConfirmChannel(OpenConfirm::new( msg.port_id_on_b.clone(), - msg.chan_id_on_b.clone(), + msg.chan_id_on_b, port_id_on_a, chan_id_on_a, conn_id_on_b, diff --git a/crates/ibc/src/core/context/chan_open_init.rs b/crates/ibc/src/core/context/chan_open_init.rs index 9246d89bb..5ab9336fb 100644 --- a/crates/ibc/src/core/context/chan_open_init.rs +++ b/crates/ibc/src/core/context/chan_open_init.rs @@ -94,7 +94,7 @@ where )); let core_event = IbcEvent::OpenInitChannel(OpenInit::new( msg.port_id_on_a.clone(), - chan_id_on_a.clone(), + chan_id_on_a, msg.port_id_on_b, conn_id_on_a, version, diff --git a/crates/ibc/src/core/context/chan_open_try.rs b/crates/ibc/src/core/context/chan_open_try.rs index 3b8a48df6..98064c460 100644 --- a/crates/ibc/src/core/context/chan_open_try.rs +++ b/crates/ibc/src/core/context/chan_open_try.rs @@ -31,7 +31,7 @@ where &msg.connection_hops_on_b, &msg.port_id_on_b, &chan_id_on_b, - &Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a.clone())), + &Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a)), &msg.version_supported_on_a, )?; @@ -56,7 +56,7 @@ where &msg.connection_hops_on_b, &msg.port_id_on_b, &chan_id_on_b, - &Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a.clone())), + &Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a)), &msg.version_supported_on_a, )?; @@ -67,7 +67,7 @@ where let chan_end_on_b = ChannelEnd::new( State::TryOpen, msg.ordering, - Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a.clone())), + Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a)), msg.connection_hops_on_b.clone(), version.clone(), ); @@ -95,9 +95,9 @@ where let core_event = IbcEvent::OpenTryChannel(OpenTry::new( msg.port_id_on_b.clone(), - chan_id_on_b.clone(), + chan_id_on_b, msg.port_id_on_a.clone(), - msg.chan_id_on_a.clone(), + msg.chan_id_on_a, conn_id_on_b, version, )); diff --git a/crates/ibc/src/core/context/recv_packet.rs b/crates/ibc/src/core/context/recv_packet.rs index 7b2c1618f..5bd340db9 100644 --- a/crates/ibc/src/core/context/recv_packet.rs +++ b/crates/ibc/src/core/context/recv_packet.rs @@ -85,7 +85,7 @@ where Order::Unordered => { let receipt_path_on_b = ReceiptPath { port_id: msg.packet.port_id_on_b.clone(), - channel_id: msg.packet.chan_id_on_b.clone(), + channel_id: msg.packet.chan_id_on_b, sequence: msg.packet.seq_on_a, }; diff --git a/crates/ibc/src/core/context/timeout.rs b/crates/ibc/src/core/context/timeout.rs index 6f021fbee..64d078530 100644 --- a/crates/ibc/src/core/context/timeout.rs +++ b/crates/ibc/src/core/context/timeout.rs @@ -96,7 +96,7 @@ where let chan_end_on_a = { let commitment_path_on_a = CommitmentPath { port_id: packet.port_id_on_a.clone(), - channel_id: packet.chan_id_on_a.clone(), + channel_id: packet.chan_id_on_a, sequence: packet.seq_on_a, }; ctx_a.delete_packet_commitment(&commitment_path_on_a)?; @@ -121,9 +121,9 @@ where ctx_a.emit_ibc_event(IbcEvent::ChannelClosed(ChannelClosed::new( packet.port_id_on_a.clone(), - packet.chan_id_on_a.clone(), + packet.chan_id_on_a, chan_end_on_a.counterparty().port_id.clone(), - chan_end_on_a.counterparty().channel_id.clone(), + chan_end_on_a.counterparty().channel_id, conn_id_on_a, chan_end_on_a.ordering, ))); @@ -210,10 +210,7 @@ mod tests { let chan_end_on_a_ordered = ChannelEnd::new( State::Open, Order::Ordered, - Counterparty::new( - packet.port_id_on_b.clone(), - Some(packet.chan_id_on_b.clone()), - ), + Counterparty::new(packet.port_id_on_b.clone(), Some(packet.chan_id_on_b)), vec![ConnectionId::default()], Version::new("ics20-1".to_string()), ); @@ -269,7 +266,7 @@ mod tests { .with_connection(ConnectionId::default(), conn_end_on_a) .with_packet_commitment( msg.packet.port_id_on_a.clone(), - msg.packet.chan_id_on_a.clone(), + msg.packet.chan_id_on_a, msg.packet.seq_on_a, packet_commitment, ); @@ -306,7 +303,7 @@ mod tests { .with_connection(ConnectionId::default(), conn_end_on_a) .with_packet_commitment( msg.packet.port_id_on_a.clone(), - msg.packet.chan_id_on_a.clone(), + msg.packet.chan_id_on_a, msg.packet.seq_on_a, packet_commitment, ); diff --git a/crates/ibc/src/core/ics04_channel/events.rs b/crates/ibc/src/core/ics04_channel/events.rs index 8c43d00fd..945264c09 100644 --- a/crates/ibc/src/core/ics04_channel/events.rs +++ b/crates/ibc/src/core/ics04_channel/events.rs @@ -1101,7 +1101,7 @@ mod tests { kind: IbcEventType::OpenInitChannel, event: OpenInit::new( port_id.clone(), - channel_id.clone(), + channel_id, counterparty_port_id.clone(), connection_id.clone(), version.clone(), @@ -1118,9 +1118,9 @@ mod tests { kind: IbcEventType::OpenTryChannel, event: OpenTry::new( port_id.clone(), - channel_id.clone(), + channel_id, counterparty_port_id.clone(), - counterparty_channel_id.clone(), + counterparty_channel_id, connection_id.clone(), version, ) @@ -1132,9 +1132,9 @@ mod tests { kind: IbcEventType::OpenAckChannel, event: OpenAck::new( port_id.clone(), - channel_id.clone(), + channel_id, counterparty_port_id.clone(), - counterparty_channel_id.clone(), + counterparty_channel_id, connection_id.clone(), ) .into(), @@ -1145,9 +1145,9 @@ mod tests { kind: IbcEventType::OpenConfirmChannel, event: OpenConfirm::new( port_id.clone(), - channel_id.clone(), + channel_id, counterparty_port_id.clone(), - counterparty_channel_id.clone(), + counterparty_channel_id, connection_id.clone(), ) .into(), @@ -1158,9 +1158,9 @@ mod tests { kind: IbcEventType::CloseInitChannel, event: CloseInit::new( port_id.clone(), - channel_id.clone(), + channel_id, counterparty_port_id.clone(), - counterparty_channel_id.clone(), + counterparty_channel_id, connection_id.clone(), ) .into(), diff --git a/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs b/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs index e858527bb..20e0bb20b 100644 --- a/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs +++ b/crates/ibc/src/core/ics04_channel/events/channel_attributes.rs @@ -61,7 +61,11 @@ pub struct ChannelIdAttribute { impl From for abci::EventAttribute { fn from(attr: ChannelIdAttribute) -> Self { - (CHANNEL_ID_ATTRIBUTE_KEY, attr.channel_id.as_str()).into() + ( + CHANNEL_ID_ATTRIBUTE_KEY, + alloc::format!("{}", attr.channel_id), + ) + .into() } } #[cfg_attr( @@ -113,7 +117,7 @@ impl From for abci::EventAttribute { fn from(attr: CounterpartyChannelIdAttribute) -> Self { ( COUNTERPARTY_CHANNEL_ID_ATTRIBUTE_KEY, - attr.counterparty_channel_id.as_str(), + alloc::format!("{}", attr.counterparty_channel_id), ) .into() } diff --git a/crates/ibc/src/core/ics04_channel/events/packet_attributes.rs b/crates/ibc/src/core/ics04_channel/events/packet_attributes.rs index 319d7aaee..3b3e947da 100644 --- a/crates/ibc/src/core/ics04_channel/events/packet_attributes.rs +++ b/crates/ibc/src/core/ics04_channel/events/packet_attributes.rs @@ -196,7 +196,11 @@ pub struct SrcChannelIdAttribute { impl From for abci::EventAttribute { fn from(attr: SrcChannelIdAttribute) -> Self { - (PKT_SRC_CHANNEL_ATTRIBUTE_KEY, attr.src_channel_id.as_str()).into() + ( + PKT_SRC_CHANNEL_ATTRIBUTE_KEY, + format!("{}", attr.src_channel_id), + ) + .into() } } @@ -244,7 +248,11 @@ pub struct DstChannelIdAttribute { impl From for abci::EventAttribute { fn from(attr: DstChannelIdAttribute) -> Self { - (PKT_DST_CHANNEL_ATTRIBUTE_KEY, attr.dst_channel_id.as_str()).into() + ( + PKT_DST_CHANNEL_ATTRIBUTE_KEY, + format!("{}", attr.dst_channel_id), + ) + .into() } } diff --git a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs index cb7a6d49f..40299058c 100644 --- a/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs +++ b/crates/ibc/src/core/ics04_channel/handler/acknowledgement.rs @@ -23,20 +23,17 @@ where if !chan_end_on_a.state_matches(&State::Open) { return Err(PacketError::ChannelClosed { - channel_id: packet.chan_id_on_a.clone(), + channel_id: packet.chan_id_on_a, } .into()); } - let counterparty = Counterparty::new( - packet.port_id_on_b.clone(), - Some(packet.chan_id_on_b.clone()), - ); + let counterparty = Counterparty::new(packet.port_id_on_b.clone(), Some(packet.chan_id_on_b)); if !chan_end_on_a.counterparty_matches(&counterparty) { return Err(PacketError::InvalidPacketCounterparty { port_id: packet.port_id_on_b.clone(), - channel_id: packet.chan_id_on_b.clone(), + channel_id: packet.chan_id_on_b, } .into()); } @@ -265,7 +262,7 @@ mod tests { .with_connection(ConnectionId::default(), conn_end_on_a) .with_packet_commitment( msg.packet.port_id_on_a.clone(), - msg.packet.chan_id_on_a.clone(), + msg.packet.chan_id_on_a, msg.packet.seq_on_a, packet_commitment, ); diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs index 0080def11..222d9b5d1 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_confirm.rs @@ -19,7 +19,7 @@ where // Validate that the channel end is in a state where it can be closed. if chan_end_on_b.state_matches(&State::Closed) { return Err(ChannelError::ChannelClosed { - channel_id: msg.chan_id_on_b.clone(), + channel_id: msg.chan_id_on_b, } .into()); } @@ -72,7 +72,7 @@ where let expected_chan_end_on_a = ChannelEnd::new( State::Closed, *chan_end_on_b.ordering(), - Counterparty::new(msg.port_id_on_b.clone(), Some(msg.chan_id_on_b.clone())), + Counterparty::new(msg.port_id_on_b.clone(), Some(msg.chan_id_on_b)), vec![conn_id_on_a.clone()], chan_end_on_b.version().clone(), ); @@ -144,7 +144,7 @@ mod tests { Order::default(), Counterparty::new( msg_chan_close_confirm.port_id_on_b.clone(), - Some(msg_chan_close_confirm.chan_id_on_b.clone()), + Some(msg_chan_close_confirm.chan_id_on_b), ), vec![conn_id.clone()], Version::default(), @@ -155,7 +155,7 @@ mod tests { .with_connection(conn_id, conn_end) .with_channel( msg_chan_close_confirm.port_id_on_b.clone(), - msg_chan_close_confirm.chan_id_on_b.clone(), + msg_chan_close_confirm.chan_id_on_b, chan_end, ); diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs index 2d69816a0..91bbe2975 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_close_init.rs @@ -17,7 +17,7 @@ where // Validate that the channel end is in a state where it can be closed. if chan_end_on_a.state_matches(&State::Closed) { return Err(ChannelError::InvalidChannelState { - channel_id: msg.chan_id_on_a.clone(), + channel_id: msg.chan_id_on_a, state: chan_end_on_a.state, } .into()); @@ -89,7 +89,7 @@ mod tests { Order::default(), Counterparty::new( msg_chan_close_init.port_id_on_a.clone(), - Some(msg_chan_close_init.chan_id_on_a.clone()), + Some(msg_chan_close_init.chan_id_on_a), ), vec![conn_id.clone()], Version::default(), @@ -104,7 +104,7 @@ mod tests { .with_connection(conn_id, conn_end) .with_channel( msg_chan_close_init.port_id_on_a.clone(), - msg_chan_close_init.chan_id_on_a.clone(), + msg_chan_close_init.chan_id_on_a, chan_end, ) }; diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs index feb3f7fe9..9f48444d1 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_ack.rs @@ -18,7 +18,7 @@ where // Validate that the channel end is in a state where it can be ack. if !chan_end_on_a.state_matches(&State::Init) { return Err(ChannelError::InvalidChannelState { - channel_id: msg.chan_id_on_a.clone(), + channel_id: msg.chan_id_on_a, state: chan_end_on_a.state, } .into()); @@ -71,7 +71,7 @@ where // Note: Both ends of a channel must have the same ordering, so it's // fine to use A's ordering here *chan_end_on_a.ordering(), - Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a.clone())), + Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_a)), vec![conn_id_on_b.clone()], msg.version_on_b.clone(), ); @@ -149,7 +149,7 @@ mod tests { let chan_end_on_a = ChannelEnd::new( State::Init, Order::Unordered, - Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_b.clone())), + Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_b)), vec![conn_id_on_a.clone()], msg.version_on_b.clone(), ); @@ -203,18 +203,14 @@ mod tests { let wrong_chan_end = ChannelEnd::new( State::Open, Order::Unordered, - Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_b.clone())), + Counterparty::new(msg.port_id_on_a.clone(), Some(msg.chan_id_on_b)), vec![conn_id_on_a.clone()], msg.version_on_b.clone(), ); let context = context .with_client(&client_id_on_a, Height::new(0, proof_height).unwrap()) .with_connection(conn_id_on_a, conn_end_on_a) - .with_channel( - msg.port_id_on_a.clone(), - msg.chan_id_on_a.clone(), - wrong_chan_end, - ); + .with_channel(msg.port_id_on_a.clone(), msg.chan_id_on_a, wrong_chan_end); let res = validate(&context, &msg); @@ -237,11 +233,7 @@ mod tests { let context = context .with_client(&client_id_on_a, Height::new(0, proof_height).unwrap()) - .with_channel( - msg.port_id_on_a.clone(), - msg.chan_id_on_a.clone(), - chan_end_on_a, - ); + .with_channel(msg.port_id_on_a.clone(), msg.chan_id_on_a, chan_end_on_a); let res = validate(&context, &msg); @@ -267,11 +259,7 @@ mod tests { let context = context .with_client(&client_id_on_a, Height::new(0, proof_height).unwrap()) .with_connection(conn_id_on_a, conn_end_on_a) - .with_channel( - msg.port_id_on_a.clone(), - msg.chan_id_on_a.clone(), - chan_end_on_a, - ); + .with_channel(msg.port_id_on_a.clone(), msg.chan_id_on_a, chan_end_on_a); let res = validate(&context, &msg); diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs index df4bdda32..9ff58c367 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_confirm.rs @@ -19,7 +19,7 @@ where // Validate that the channel end is in a state where it can be confirmed. if !chan_end_on_b.state_matches(&State::TryOpen) { return Err(ChannelError::InvalidChannelState { - channel_id: msg.chan_id_on_b.clone(), + channel_id: msg.chan_id_on_b, state: chan_end_on_b.state, } .into()); @@ -73,7 +73,7 @@ where let expected_chan_end_on_a = ChannelEnd::new( State::Open, *chan_end_on_b.ordering(), - Counterparty::new(msg.port_id_on_b.clone(), Some(msg.chan_id_on_b.clone())), + Counterparty::new(msg.port_id_on_b.clone(), Some(msg.chan_id_on_b)), vec![conn_id_on_a.clone()], chan_end_on_b.version.clone(), ); diff --git a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs index 449ffa12d..8bddee534 100644 --- a/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs +++ b/crates/ibc/src/core/ics04_channel/handler/chan_open_try.rs @@ -53,7 +53,7 @@ where let consensus_state_of_a_on_b = ctx_b.consensus_state(&client_cons_state_path_on_b)?; let prefix_on_a = conn_end_on_b.counterparty().prefix(); let port_id_on_a = msg.port_id_on_a.clone(); - let chan_id_on_a = msg.chan_id_on_a.clone(); + let chan_id_on_a = msg.chan_id_on_a; let conn_id_on_a = conn_end_on_b.counterparty().connection_id().ok_or( ChannelError::UndefinedConnectionCounterparty { connection_id: msg.connection_hops_on_b[0].clone(), diff --git a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs index 775e50474..61e464e22 100644 --- a/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/recv_packet.rs @@ -22,7 +22,7 @@ where if !chan_end_on_b.state_matches(&State::Open) { return Err(PacketError::InvalidChannelState { - channel_id: msg.packet.chan_id_on_a.clone(), + channel_id: msg.packet.chan_id_on_a, state: chan_end_on_b.state, } .into()); @@ -30,13 +30,13 @@ where let counterparty = Counterparty::new( msg.packet.port_id_on_a.clone(), - Some(msg.packet.chan_id_on_a.clone()), + Some(msg.packet.chan_id_on_a), ); if !chan_end_on_b.counterparty_matches(&counterparty) { return Err(PacketError::InvalidPacketCounterparty { port_id: msg.packet.port_id_on_a.clone(), - channel_id: msg.packet.chan_id_on_a.clone(), + channel_id: msg.packet.chan_id_on_a, } .into()); } @@ -276,19 +276,15 @@ mod tests { .with_connection(ConnectionId::default(), conn_end_on_b) .with_channel( packet.port_id_on_b.clone(), - packet.chan_id_on_b.clone(), + packet.chan_id_on_b, chan_end_on_b, ) - .with_send_sequence( - packet.port_id_on_b.clone(), - packet.chan_id_on_b.clone(), - 1.into(), - ) + .with_send_sequence(packet.port_id_on_b.clone(), packet.chan_id_on_b, 1.into()) .with_height(host_height) // This `with_recv_sequence` is required for ordered channels .with_recv_sequence( packet.port_id_on_b.clone(), - packet.chan_id_on_b.clone(), + packet.chan_id_on_b, packet.seq_on_a, ); diff --git a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs index 925d95efc..dfd7c9d3e 100644 --- a/crates/ibc/src/core/ics04_channel/handler/send_packet.rs +++ b/crates/ibc/src/core/ics04_channel/handler/send_packet.rs @@ -34,20 +34,17 @@ pub fn send_packet_validate( if chan_end_on_a.state_matches(&State::Closed) { return Err(PacketError::ChannelClosed { - channel_id: packet.chan_id_on_a.clone(), + channel_id: packet.chan_id_on_a, } .into()); } - let counterparty = Counterparty::new( - packet.port_id_on_b.clone(), - Some(packet.chan_id_on_b.clone()), - ); + let counterparty = Counterparty::new(packet.port_id_on_b.clone(), Some(packet.chan_id_on_b)); if !chan_end_on_a.counterparty_matches(&counterparty) { return Err(PacketError::InvalidPacketCounterparty { port_id: packet.port_id_on_b.clone(), - channel_id: packet.chan_id_on_b.clone(), + channel_id: packet.chan_id_on_b, } .into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout.rs b/crates/ibc/src/core/ics04_channel/handler/timeout.rs index 6f1954403..c70f07ec6 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout.rs @@ -24,20 +24,20 @@ where if !chan_end_on_a.state_matches(&State::Open) { return Err(PacketError::ChannelClosed { - channel_id: msg.packet.chan_id_on_a.clone(), + channel_id: msg.packet.chan_id_on_a, } .into()); } let counterparty = Counterparty::new( msg.packet.port_id_on_b.clone(), - Some(msg.packet.chan_id_on_b.clone()), + Some(msg.packet.chan_id_on_b), ); if !chan_end_on_a.counterparty_matches(&counterparty) { return Err(PacketError::InvalidPacketCounterparty { port_id: msg.packet.port_id_on_b.clone(), - channel_id: msg.packet.chan_id_on_b.clone(), + channel_id: msg.packet.chan_id_on_b, } .into()); } diff --git a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs index 0a6c8e541..f936bffeb 100644 --- a/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs +++ b/crates/ibc/src/core/ics04_channel/handler/timeout_on_close.rs @@ -19,15 +19,12 @@ where let chan_end_path_on_a = ChannelEndPath::new(&packet.port_id_on_a, &packet.chan_id_on_a); let chan_end_on_a = ctx_a.channel_end(&chan_end_path_on_a)?; - let counterparty = Counterparty::new( - packet.port_id_on_b.clone(), - Some(packet.chan_id_on_b.clone()), - ); + let counterparty = Counterparty::new(packet.port_id_on_b.clone(), Some(packet.chan_id_on_b)); if !chan_end_on_a.counterparty_matches(&counterparty) { return Err(PacketError::InvalidPacketCounterparty { port_id: packet.port_id_on_b.clone(), - channel_id: packet.chan_id_on_b.clone(), + channel_id: packet.chan_id_on_b, } .into()); } @@ -94,10 +91,8 @@ where }, )?; let expected_conn_hops_on_b = vec![conn_id_on_b.clone()]; - let expected_counterparty = Counterparty::new( - packet.port_id_on_a.clone(), - Some(packet.chan_id_on_a.clone()), - ); + let expected_counterparty = + Counterparty::new(packet.port_id_on_a.clone(), Some(packet.chan_id_on_a)); let expected_chan_end_on_b = ChannelEnd::new( State::Closed, *chan_end_on_a.ordering(), @@ -106,7 +101,7 @@ where chan_end_on_a.version().clone(), ); - let chan_end_path_on_b = ChannelEndPath(port_id_on_b, chan_id_on_b.clone()); + let chan_end_path_on_b = ChannelEndPath(port_id_on_b, *chan_id_on_b); // Verify the proof for the channel state against the expected channel end. // A counterparty channel id of None in not possible, and is checked by validate_basic in msg. @@ -297,7 +292,7 @@ mod tests { .with_connection(ConnectionId::default(), conn_end_on_a) .with_packet_commitment( msg.packet.port_id_on_a.clone(), - msg.packet.chan_id_on_a.clone(), + msg.packet.chan_id_on_a, msg.packet.seq_on_a, packet_commitment, ); diff --git a/crates/ibc/src/core/ics24_host/error.rs b/crates/ibc/src/core/ics24_host/error.rs index ef78eb941..a9c5212ee 100644 --- a/crates/ibc/src/core/ics24_host/error.rs +++ b/crates/ibc/src/core/ics24_host/error.rs @@ -19,6 +19,8 @@ pub enum ValidationError { Empty, /// Invalid channel id in counterparty InvalidCounterpartyChannelId, + /// channel id parse failure + ChannelIdParseFailure(String), } #[cfg(feature = "std")] diff --git a/crates/ibc/src/core/ics24_host/identifier.rs b/crates/ibc/src/core/ics24_host/identifier.rs index 7edd011ce..337fd6805 100644 --- a/crates/ibc/src/core/ics24_host/identifier.rs +++ b/crates/ibc/src/core/ics24_host/identifier.rs @@ -412,9 +412,8 @@ impl Default for PortId { feature = "borsh", derive(borsh::BorshSerialize, borsh::BorshDeserialize) )] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct ChannelId(String); +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct ChannelId(u64); impl ChannelId { const PREFIX: &'static str = "channel-"; @@ -431,25 +430,18 @@ impl ChannelId { /// assert_eq!(chan_id.to_string(), "channel-27"); /// ``` pub fn new(identifier: u64) -> Self { - let id = format!("{}{}", Self::PREFIX, identifier); - Self(id) + Self(identifier) } - /// Get this identifier as a borrowed `&str` - pub fn as_str(&self) -> &str { - &self.0 - } - - /// Get this identifier as a borrowed byte slice - pub fn as_bytes(&self) -> &[u8] { - self.0.as_bytes() + pub fn counter(&self) -> u64 { + self.0 } } /// This implementation provides a `to_string` method. impl Display for ChannelId { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { - write!(f, "{}", self.0) + write!(f, "{}{}", Self::PREFIX, self.0) } } @@ -457,13 +449,12 @@ impl FromStr for ChannelId { type Err = ValidationError; fn from_str(s: &str) -> Result { - validate_channel_identifier(s).map(|_| Self(s.to_string())) - } -} - -impl AsRef for ChannelId { - fn as_ref(&self) -> &str { - &self.0 + let s = s + .strip_prefix(Self::PREFIX) + .ok_or_else(|| ValidationError::InvalidCharacter { id: s.to_string() })?; + let counter = + u64::from_str(s).map_err(|e| ValidationError::ChannelIdParseFailure(e.to_string()))?; + Ok(Self(counter)) } } @@ -473,17 +464,30 @@ impl Default for ChannelId { } } -/// Equality check against string literal (satisfies &ChannelId == &str). -/// ``` -/// use core::str::FromStr; -/// use ibc::core::ics24_host::identifier::ChannelId; -/// let channel_id = ChannelId::from_str("channelId-0"); -/// assert!(channel_id.is_ok()); -/// channel_id.map(|id| {assert_eq!(&id, "channelId-0")}); -/// ``` -impl PartialEq for ChannelId { - fn eq(&self, other: &str) -> bool { - self.as_str().eq(other) +#[cfg(feature = "serde")] +mod seald { + use super::ChannelId; + use crate::prelude::String; + use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; + + 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) + } } } diff --git a/crates/ibc/src/core/ics24_host/path.rs b/crates/ibc/src/core/ics24_host/path.rs index 8abeae066..4cb604287 100644 --- a/crates/ibc/src/core/ics24_host/path.rs +++ b/crates/ibc/src/core/ics24_host/path.rs @@ -113,7 +113,7 @@ pub struct ChannelEndPath(pub PortId, pub ChannelId); impl ChannelEndPath { pub fn new(port_id: &PortId, channel_id: &ChannelId) -> ChannelEndPath { - ChannelEndPath(port_id.clone(), channel_id.clone()) + ChannelEndPath(port_id.clone(), *channel_id) } } @@ -123,7 +123,7 @@ pub struct SeqSendPath(pub PortId, pub ChannelId); impl SeqSendPath { pub fn new(port_id: &PortId, channel_id: &ChannelId) -> SeqSendPath { - SeqSendPath(port_id.clone(), channel_id.clone()) + SeqSendPath(port_id.clone(), *channel_id) } } @@ -133,7 +133,7 @@ pub struct SeqRecvPath(pub PortId, pub ChannelId); impl SeqRecvPath { pub fn new(port_id: &PortId, channel_id: &ChannelId) -> SeqRecvPath { - SeqRecvPath(port_id.clone(), channel_id.clone()) + SeqRecvPath(port_id.clone(), *channel_id) } } @@ -143,7 +143,7 @@ pub struct SeqAckPath(pub PortId, pub ChannelId); impl SeqAckPath { pub fn new(port_id: &PortId, channel_id: &ChannelId) -> SeqAckPath { - SeqAckPath(port_id.clone(), channel_id.clone()) + SeqAckPath(port_id.clone(), *channel_id) } } @@ -159,7 +159,7 @@ impl CommitmentPath { pub fn new(port_id: &PortId, channel_id: &ChannelId, sequence: Sequence) -> CommitmentPath { CommitmentPath { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id: *channel_id, sequence, } } @@ -177,7 +177,7 @@ impl AckPath { pub fn new(port_id: &PortId, channel_id: &ChannelId, sequence: Sequence) -> AckPath { AckPath { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id: *channel_id, sequence, } } @@ -195,7 +195,7 @@ impl ReceiptPath { pub fn new(port_id: &PortId, channel_id: &ChannelId, sequence: Sequence) -> ReceiptPath { ReceiptPath { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id: *channel_id, sequence, } } diff --git a/crates/ibc/src/mock/context.rs b/crates/ibc/src/mock/context.rs index fcdbf526d..4ee0ad120 100644 --- a/crates/ibc/src/mock/context.rs +++ b/crates/ibc/src/mock/context.rs @@ -916,19 +916,19 @@ impl ValidationContext for MockContext { fn channel_end(&self, chan_end_path: &ChannelEndPath) -> Result { let port_id = &chan_end_path.0; - let channel_id = &chan_end_path.1; + let channel_id = chan_end_path.1; match self .ibc_store .lock() .channels .get(port_id) - .and_then(|map| map.get(channel_id)) + .and_then(|map| map.get(&channel_id)) { Some(channel_end) => Ok(channel_end.clone()), None => Err(ChannelError::ChannelNotFound { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id, }), } .map_err(ContextError::ChannelError) @@ -939,19 +939,19 @@ impl ValidationContext for MockContext { seq_send_path: &SeqSendPath, ) -> Result { let port_id = &seq_send_path.0; - let channel_id = &seq_send_path.1; + let channel_id = seq_send_path.1; match self .ibc_store .lock() .next_sequence_send .get(port_id) - .and_then(|map| map.get(channel_id)) + .and_then(|map| map.get(&channel_id)) { Some(sequence) => Ok(*sequence), None => Err(PacketError::MissingNextSendSeq { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id, }), } .map_err(ContextError::PacketError) @@ -962,19 +962,19 @@ impl ValidationContext for MockContext { seq_recv_path: &SeqRecvPath, ) -> Result { let port_id = &seq_recv_path.0; - let channel_id = &seq_recv_path.1; + let channel_id = seq_recv_path.1; match self .ibc_store .lock() .next_sequence_recv .get(port_id) - .and_then(|map| map.get(channel_id)) + .and_then(|map| map.get(&channel_id)) { Some(sequence) => Ok(*sequence), None => Err(PacketError::MissingNextRecvSeq { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id, }), } .map_err(ContextError::PacketError) @@ -982,19 +982,19 @@ impl ValidationContext for MockContext { fn get_next_sequence_ack(&self, seq_ack_path: &SeqAckPath) -> Result { let port_id = &seq_ack_path.0; - let channel_id = &seq_ack_path.1; + let channel_id = seq_ack_path.1; match self .ibc_store .lock() .next_sequence_ack .get(port_id) - .and_then(|map| map.get(channel_id)) + .and_then(|map| map.get(&channel_id)) { Some(sequence) => Ok(*sequence), None => Err(PacketError::MissingNextAckSeq { port_id: port_id.clone(), - channel_id: channel_id.clone(), + channel_id, }), } .map_err(ContextError::PacketError) @@ -1005,7 +1005,7 @@ impl ValidationContext for MockContext { commitment_path: &CommitmentPath, ) -> Result { let port_id = &commitment_path.port_id; - let channel_id = &commitment_path.channel_id; + let channel_id = commitment_path.channel_id; let seq = &commitment_path.sequence; match self @@ -1013,7 +1013,7 @@ impl ValidationContext for MockContext { .lock() .packet_commitment .get(port_id) - .and_then(|map| map.get(channel_id)) + .and_then(|map| map.get(&channel_id)) .and_then(|map| map.get(seq)) { Some(commitment) => Ok(commitment.clone()), @@ -1024,7 +1024,7 @@ impl ValidationContext for MockContext { fn get_packet_receipt(&self, receipt_path: &ReceiptPath) -> Result { let port_id = &receipt_path.port_id; - let channel_id = &receipt_path.channel_id; + let channel_id = receipt_path.channel_id; let seq = &receipt_path.sequence; match self @@ -1032,7 +1032,7 @@ impl ValidationContext for MockContext { .lock() .packet_receipt .get(port_id) - .and_then(|map| map.get(channel_id)) + .and_then(|map| map.get(&channel_id)) .and_then(|map| map.get(seq)) { Some(receipt) => Ok(receipt.clone()), @@ -1046,7 +1046,7 @@ impl ValidationContext for MockContext { ack_path: &AckPath, ) -> Result { let port_id = &ack_path.port_id; - let channel_id = &ack_path.channel_id; + let channel_id = ack_path.channel_id; let seq = &ack_path.sequence; match self @@ -1054,7 +1054,7 @@ impl ValidationContext for MockContext { .lock() .packet_acknowledgement .get(port_id) - .and_then(|map| map.get(channel_id)) + .and_then(|map| map.get(&channel_id)) .and_then(|map| map.get(seq)) { Some(ack) => Ok(ack.clone()), @@ -1252,7 +1252,7 @@ impl ExecutionContext for MockContext { .packet_commitment .entry(commitment_path.port_id.clone()) .or_default() - .entry(commitment_path.channel_id.clone()) + .entry(commitment_path.channel_id) .or_default() .insert(commitment_path.sequence, commitment); Ok(()) @@ -1281,7 +1281,7 @@ impl ExecutionContext for MockContext { .packet_receipt .entry(path.port_id.clone()) .or_default() - .entry(path.channel_id.clone()) + .entry(path.channel_id) .or_default() .insert(path.sequence, receipt); Ok(()) @@ -1293,7 +1293,7 @@ impl ExecutionContext for MockContext { ack_commitment: AcknowledgementCommitment, ) -> Result<(), ContextError> { let port_id = ack_path.port_id.clone(); - let channel_id = ack_path.channel_id.clone(); + let channel_id = ack_path.channel_id; let seq = ack_path.sequence; self.ibc_store @@ -1309,7 +1309,7 @@ impl ExecutionContext for MockContext { fn delete_packet_acknowledgement(&mut self, ack_path: &AckPath) -> Result<(), ContextError> { let port_id = ack_path.port_id.clone(); - let channel_id = ack_path.channel_id.clone(); + let channel_id = ack_path.channel_id; let sequence = ack_path.sequence; self.ibc_store @@ -1327,7 +1327,7 @@ impl ExecutionContext for MockContext { channel_end: ChannelEnd, ) -> Result<(), ContextError> { let port_id = channel_end_path.0.clone(); - let channel_id = channel_end_path.1.clone(); + let channel_id = channel_end_path.1; self.ibc_store .lock() @@ -1344,7 +1344,7 @@ impl ExecutionContext for MockContext { seq: Sequence, ) -> Result<(), ContextError> { let port_id = seq_send_path.0.clone(); - let channel_id = seq_send_path.1.clone(); + let channel_id = seq_send_path.1; self.ibc_store .lock() @@ -1361,7 +1361,7 @@ impl ExecutionContext for MockContext { seq: Sequence, ) -> Result<(), ContextError> { let port_id = seq_recv_path.0.clone(); - let channel_id = seq_recv_path.1.clone(); + let channel_id = seq_recv_path.1; self.ibc_store .lock() @@ -1378,7 +1378,7 @@ impl ExecutionContext for MockContext { seq: Sequence, ) -> Result<(), ContextError> { let port_id = seq_ack_path.0.clone(); - let channel_id = seq_ack_path.1.clone(); + let channel_id = seq_ack_path.1; self.ibc_store .lock() From e57284f29f2ba1acb1e3ee3bf42958b557204868 Mon Sep 17 00:00:00 2001 From: Davirain Date: Thu, 16 Mar 2023 21:08:30 +0800 Subject: [PATCH 2/3] Create 537-channel-id-u64.md --- .changelog/unreleased/improvements/537-channel-id-u64.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/537-channel-id-u64.md diff --git a/.changelog/unreleased/improvements/537-channel-id-u64.md b/.changelog/unreleased/improvements/537-channel-id-u64.md new file mode 100644 index 000000000..e8ae2ecb6 --- /dev/null +++ b/.changelog/unreleased/improvements/537-channel-id-u64.md @@ -0,0 +1,2 @@ +- [Crate] Pass ChannelId by value instead of reference since it impls Copy + ([#537](https://github.com/cosmos/ibc-rs/issues/537)) \ No newline at end of file From d500a402b5d51d89e671fa8e682d22bc03701458 Mon Sep 17 00:00:00 2001 From: Davirain Date: Thu, 16 Mar 2023 21:57:55 +0800 Subject: [PATCH 3/3] Update identifier.rs --- crates/ibc/src/core/ics24_host/identifier.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ibc/src/core/ics24_host/identifier.rs b/crates/ibc/src/core/ics24_host/identifier.rs index 337fd6805..f23e14b46 100644 --- a/crates/ibc/src/core/ics24_host/identifier.rs +++ b/crates/ibc/src/core/ics24_host/identifier.rs @@ -449,6 +449,8 @@ impl FromStr for ChannelId { type Err = ValidationError; fn from_str(s: &str) -> Result { + validate_channel_identifier(s)?; + let s = s .strip_prefix(Self::PREFIX) .ok_or_else(|| ValidationError::InvalidCharacter { id: s.to_string() })?;