diff --git a/.changelog/unreleased/improvements/394-remove-order-match.md b/.changelog/unreleased/improvements/394-remove-order-match.md new file mode 100644 index 000000000..1997e6026 --- /dev/null +++ b/.changelog/unreleased/improvements/394-remove-order-match.md @@ -0,0 +1,2 @@ +- [ibc-core] Deprecate `ChannelEnd::order_matches` method + ([\#394](https://github.com/cosmos/ibc-rs/issues/394)) diff --git a/ibc-core/ics04-channel/src/handler/recv_packet.rs b/ibc-core/ics04-channel/src/handler/recv_packet.rs index bd32a3258..9e42e269d 100644 --- a/ibc-core/ics04-channel/src/handler/recv_packet.rs +++ b/ibc-core/ics04-channel/src/handler/recv_packet.rs @@ -224,40 +224,49 @@ where .map_err(PacketError::Channel)?; } - if chan_end_on_b.order_matches(&Order::Ordered) { - let seq_recv_path_on_b = - SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b); - let next_seq_recv = ctx_b.get_next_sequence_recv(&seq_recv_path_on_b)?; - if msg.packet.seq_on_a > next_seq_recv { - return Err(PacketError::InvalidPacketSequence { - given_sequence: msg.packet.seq_on_a, - next_sequence: next_seq_recv, + match chan_end_on_b.ordering { + Order::Ordered => { + let seq_recv_path_on_b = + SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b); + let next_seq_recv = ctx_b.get_next_sequence_recv(&seq_recv_path_on_b)?; + if msg.packet.seq_on_a > next_seq_recv { + return Err(PacketError::InvalidPacketSequence { + given_sequence: msg.packet.seq_on_a, + next_sequence: next_seq_recv, + } + .into()); } - .into()); - } - if msg.packet.seq_on_a == next_seq_recv { + if msg.packet.seq_on_a == next_seq_recv { + // Case where the recvPacket is successful and an + // acknowledgement will be written (not a no-op) + validate_write_acknowledgement(ctx_b, msg)?; + } + } + Order::Unordered => { + let receipt_path_on_b = ReceiptPath::new( + &msg.packet.port_id_on_a, + &msg.packet.chan_id_on_a, + msg.packet.seq_on_a, + ); + let packet_rec = ctx_b.get_packet_receipt(&receipt_path_on_b); + match packet_rec { + Ok(_receipt) => {} + Err(ContextError::PacketError(PacketError::PacketReceiptNotFound { sequence })) + if sequence == msg.packet.seq_on_a => {} + Err(e) => return Err(e), + } // Case where the recvPacket is successful and an // acknowledgement will be written (not a no-op) validate_write_acknowledgement(ctx_b, msg)?; } - } else { - let receipt_path_on_b = ReceiptPath::new( - &msg.packet.port_id_on_a, - &msg.packet.chan_id_on_a, - msg.packet.seq_on_a, - ); - let packet_rec = ctx_b.get_packet_receipt(&receipt_path_on_b); - match packet_rec { - Ok(_receipt) => {} - Err(ContextError::PacketError(PacketError::PacketReceiptNotFound { sequence })) - if sequence == msg.packet.seq_on_a => {} - Err(e) => return Err(e), + Order::None => { + return Err(ContextError::ChannelError(ChannelError::InvalidOrderType { + expected: "Channel ordering cannot be None".to_string(), + actual: chan_end_on_b.ordering.to_string(), + })) } - // Case where the recvPacket is successful and an - // acknowledgement will be written (not a no-op) - validate_write_acknowledgement(ctx_b, msg)?; - }; + } Ok(()) } diff --git a/ibc-core/ics04-channel/src/handler/timeout.rs b/ibc-core/ics04-channel/src/handler/timeout.rs index adef59bfb..bff8be649 100644 --- a/ibc-core/ics04-channel/src/handler/timeout.rs +++ b/ibc-core/ics04-channel/src/handler/timeout.rs @@ -215,38 +215,48 @@ where verify_conn_delay_passed(ctx_a, msg.proof_height_on_b, &conn_end_on_a)?; - let next_seq_recv_verification_result = if chan_end_on_a.order_matches(&Order::Ordered) { - if msg.packet.seq_on_a < msg.next_seq_recv_on_b { - return Err(PacketError::InvalidPacketSequence { - given_sequence: msg.packet.seq_on_a, - next_sequence: msg.next_seq_recv_on_b, + let next_seq_recv_verification_result = match chan_end_on_a.ordering { + Order::Ordered => { + if msg.packet.seq_on_a < msg.next_seq_recv_on_b { + return Err(PacketError::InvalidPacketSequence { + given_sequence: msg.packet.seq_on_a, + next_sequence: msg.next_seq_recv_on_b, + } + .into()); } - .into()); + let seq_recv_path_on_b = + SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b); + + client_state_of_b_on_a.verify_membership( + conn_end_on_a.counterparty().prefix(), + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + Path::SeqRecv(seq_recv_path_on_b), + msg.packet.seq_on_a.to_vec(), + ) + } + Order::Unordered => { + let receipt_path_on_b = ReceiptPath::new( + &msg.packet.port_id_on_b, + &msg.packet.chan_id_on_b, + msg.packet.seq_on_a, + ); + + client_state_of_b_on_a.verify_non_membership( + conn_end_on_a.counterparty().prefix(), + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + Path::Receipt(receipt_path_on_b), + ) + } + Order::None => { + return Err(ContextError::ChannelError(ChannelError::InvalidOrderType { + expected: "Channel ordering cannot be None".to_string(), + actual: chan_end_on_a.ordering.to_string(), + })) } - let seq_recv_path_on_b = - SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b); - - client_state_of_b_on_a.verify_membership( - conn_end_on_a.counterparty().prefix(), - &msg.proof_unreceived_on_b, - consensus_state_of_b_on_a.root(), - Path::SeqRecv(seq_recv_path_on_b), - msg.packet.seq_on_a.to_vec(), - ) - } else { - let receipt_path_on_b = ReceiptPath::new( - &msg.packet.port_id_on_b, - &msg.packet.chan_id_on_b, - msg.packet.seq_on_a, - ); - - client_state_of_b_on_a.verify_non_membership( - conn_end_on_a.counterparty().prefix(), - &msg.proof_unreceived_on_b, - consensus_state_of_b_on_a.root(), - Path::Receipt(receipt_path_on_b), - ) }; + next_seq_recv_verification_result .map_err(|e| ChannelError::PacketVerificationFailed { sequence: msg.next_seq_recv_on_b, diff --git a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs index eb6224a4b..b448bba44 100644 --- a/ibc-core/ics04-channel/src/handler/timeout_on_close.rs +++ b/ibc-core/ics04-channel/src/handler/timeout_on_close.rs @@ -120,37 +120,48 @@ where verify_conn_delay_passed(ctx_a, msg.proof_height_on_b, &conn_end_on_a)?; - let next_seq_recv_verification_result = if chan_end_on_a.order_matches(&Order::Ordered) { - if packet.seq_on_a < msg.next_seq_recv_on_b { - return Err(PacketError::InvalidPacketSequence { - given_sequence: packet.seq_on_a, - next_sequence: msg.next_seq_recv_on_b, + let next_seq_recv_verification_result = match chan_end_on_a.ordering { + Order::Ordered => { + if packet.seq_on_a < msg.next_seq_recv_on_b { + return Err(PacketError::InvalidPacketSequence { + given_sequence: packet.seq_on_a, + next_sequence: msg.next_seq_recv_on_b, + } + .into()); } - .into()); + let seq_recv_path_on_b = + SeqRecvPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); + + client_state_of_b_on_a.verify_membership( + conn_end_on_a.counterparty().prefix(), + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + Path::SeqRecv(seq_recv_path_on_b), + packet.seq_on_a.to_vec(), + ) + } + Order::Unordered => { + let receipt_path_on_b = ReceiptPath::new( + &msg.packet.port_id_on_b, + &msg.packet.chan_id_on_b, + msg.packet.seq_on_a, + ); + + client_state_of_b_on_a.verify_non_membership( + conn_end_on_a.counterparty().prefix(), + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + Path::Receipt(receipt_path_on_b), + ) + } + Order::None => { + return Err(ContextError::ChannelError(ChannelError::InvalidOrderType { + expected: "Channel ordering cannot be None".to_string(), + actual: chan_end_on_a.ordering.to_string(), + })) } - let seq_recv_path_on_b = SeqRecvPath::new(&packet.port_id_on_b, &packet.chan_id_on_b); - - client_state_of_b_on_a.verify_membership( - conn_end_on_a.counterparty().prefix(), - &msg.proof_unreceived_on_b, - consensus_state_of_b_on_a.root(), - Path::SeqRecv(seq_recv_path_on_b), - packet.seq_on_a.to_vec(), - ) - } else { - let receipt_path_on_b = ReceiptPath::new( - &msg.packet.port_id_on_b, - &msg.packet.chan_id_on_b, - msg.packet.seq_on_a, - ); - - client_state_of_b_on_a.verify_non_membership( - conn_end_on_a.counterparty().prefix(), - &msg.proof_unreceived_on_b, - consensus_state_of_b_on_a.root(), - Path::Receipt(receipt_path_on_b), - ) }; + next_seq_recv_verification_result .map_err(|e| ChannelError::PacketVerificationFailed { sequence: msg.next_seq_recv_on_b, diff --git a/ibc-core/ics04-channel/types/src/channel.rs b/ibc-core/ics04-channel/types/src/channel.rs index f42d8b86f..732e17185 100644 --- a/ibc-core/ics04-channel/types/src/channel.rs +++ b/ibc-core/ics04-channel/types/src/channel.rs @@ -280,6 +280,10 @@ impl ChannelEnd { } /// Helper function to compare the order of this end with another order. + #[deprecated( + since = "0.50.1", + note = "Use `Eq` or `match` directly on the `Order` enum instead" + )] pub fn order_matches(&self, other: &Order) -> bool { self.ordering.eq(other) }