Skip to content

imp: remove order_matches from ChannelEnd impl #1095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 21, 2024
2 changes: 2 additions & 0 deletions .changelog/unreleased/improvements/394-remove-order-match.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- [ibc-core] Deprecate `ChannelEnd::order_matches` method
([\#394](https://github.com/cosmos/ibc-rs/issues/394))
63 changes: 36 additions & 27 deletions ibc-core/ics04-channel/src/handler/recv_packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
68 changes: 39 additions & 29 deletions ibc-core/ics04-channel/src/handler/timeout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
67 changes: 39 additions & 28 deletions ibc-core/ics04-channel/src/handler/timeout_on_close.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions ibc-core/ics04-channel/types/src/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down