Skip to content

Commit cde4d6e

Browse files
tuantran1702rnbguy
tuantran1702
and
rnbguy
authored
imp: remove order_matches from ChannelEnd impl (#1095)
* replace if-else branch to match statement, remove order_matches helper from ChannelEnd impl * add unclog * deprecate instead of delete the method * update changelog * Update .changelog/unreleased/improvements/394-remove-order-match.md Co-authored-by: Rano | Ranadeep <[email protected]> Signed-off-by: Tuan Tran <[email protected]> * fix: return err on Order::None * Update ibc-core/ics04-channel/src/handler/timeout.rs additional blank line for consistency Co-authored-by: Rano | Ranadeep <[email protected]> Signed-off-by: Tuan Tran <[email protected]> --------- Signed-off-by: Tuan Tran <[email protected]> Co-authored-by: Rano | Ranadeep <[email protected]>
1 parent f799be8 commit cde4d6e

File tree

5 files changed

+120
-84
lines changed

5 files changed

+120
-84
lines changed

‎.changelog/unreleased/improvements/394-remove-order-match.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- [ibc-core] Deprecate `ChannelEnd::order_matches` method
2+
([\#394](https://github.com/cosmos/ibc-rs/issues/394))

‎ibc-core/ics04-channel/src/handler/recv_packet.rs

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -224,40 +224,49 @@ where
224224
.map_err(PacketError::Channel)?;
225225
}
226226

227-
if chan_end_on_b.order_matches(&Order::Ordered) {
228-
let seq_recv_path_on_b =
229-
SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b);
230-
let next_seq_recv = ctx_b.get_next_sequence_recv(&seq_recv_path_on_b)?;
231-
if msg.packet.seq_on_a > next_seq_recv {
232-
return Err(PacketError::InvalidPacketSequence {
233-
given_sequence: msg.packet.seq_on_a,
234-
next_sequence: next_seq_recv,
227+
match chan_end_on_b.ordering {
228+
Order::Ordered => {
229+
let seq_recv_path_on_b =
230+
SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b);
231+
let next_seq_recv = ctx_b.get_next_sequence_recv(&seq_recv_path_on_b)?;
232+
if msg.packet.seq_on_a > next_seq_recv {
233+
return Err(PacketError::InvalidPacketSequence {
234+
given_sequence: msg.packet.seq_on_a,
235+
next_sequence: next_seq_recv,
236+
}
237+
.into());
235238
}
236-
.into());
237-
}
238239

239-
if msg.packet.seq_on_a == next_seq_recv {
240+
if msg.packet.seq_on_a == next_seq_recv {
241+
// Case where the recvPacket is successful and an
242+
// acknowledgement will be written (not a no-op)
243+
validate_write_acknowledgement(ctx_b, msg)?;
244+
}
245+
}
246+
Order::Unordered => {
247+
let receipt_path_on_b = ReceiptPath::new(
248+
&msg.packet.port_id_on_a,
249+
&msg.packet.chan_id_on_a,
250+
msg.packet.seq_on_a,
251+
);
252+
let packet_rec = ctx_b.get_packet_receipt(&receipt_path_on_b);
253+
match packet_rec {
254+
Ok(_receipt) => {}
255+
Err(ContextError::PacketError(PacketError::PacketReceiptNotFound { sequence }))
256+
if sequence == msg.packet.seq_on_a => {}
257+
Err(e) => return Err(e),
258+
}
240259
// Case where the recvPacket is successful and an
241260
// acknowledgement will be written (not a no-op)
242261
validate_write_acknowledgement(ctx_b, msg)?;
243262
}
244-
} else {
245-
let receipt_path_on_b = ReceiptPath::new(
246-
&msg.packet.port_id_on_a,
247-
&msg.packet.chan_id_on_a,
248-
msg.packet.seq_on_a,
249-
);
250-
let packet_rec = ctx_b.get_packet_receipt(&receipt_path_on_b);
251-
match packet_rec {
252-
Ok(_receipt) => {}
253-
Err(ContextError::PacketError(PacketError::PacketReceiptNotFound { sequence }))
254-
if sequence == msg.packet.seq_on_a => {}
255-
Err(e) => return Err(e),
263+
Order::None => {
264+
return Err(ContextError::ChannelError(ChannelError::InvalidOrderType {
265+
expected: "Channel ordering cannot be None".to_string(),
266+
actual: chan_end_on_b.ordering.to_string(),
267+
}))
256268
}
257-
// Case where the recvPacket is successful and an
258-
// acknowledgement will be written (not a no-op)
259-
validate_write_acknowledgement(ctx_b, msg)?;
260-
};
269+
}
261270

262271
Ok(())
263272
}

‎ibc-core/ics04-channel/src/handler/timeout.rs

Lines changed: 39 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -215,38 +215,48 @@ where
215215

216216
verify_conn_delay_passed(ctx_a, msg.proof_height_on_b, &conn_end_on_a)?;
217217

218-
let next_seq_recv_verification_result = if chan_end_on_a.order_matches(&Order::Ordered) {
219-
if msg.packet.seq_on_a < msg.next_seq_recv_on_b {
220-
return Err(PacketError::InvalidPacketSequence {
221-
given_sequence: msg.packet.seq_on_a,
222-
next_sequence: msg.next_seq_recv_on_b,
218+
let next_seq_recv_verification_result = match chan_end_on_a.ordering {
219+
Order::Ordered => {
220+
if msg.packet.seq_on_a < msg.next_seq_recv_on_b {
221+
return Err(PacketError::InvalidPacketSequence {
222+
given_sequence: msg.packet.seq_on_a,
223+
next_sequence: msg.next_seq_recv_on_b,
224+
}
225+
.into());
223226
}
224-
.into());
227+
let seq_recv_path_on_b =
228+
SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b);
229+
230+
client_state_of_b_on_a.verify_membership(
231+
conn_end_on_a.counterparty().prefix(),
232+
&msg.proof_unreceived_on_b,
233+
consensus_state_of_b_on_a.root(),
234+
Path::SeqRecv(seq_recv_path_on_b),
235+
msg.packet.seq_on_a.to_vec(),
236+
)
237+
}
238+
Order::Unordered => {
239+
let receipt_path_on_b = ReceiptPath::new(
240+
&msg.packet.port_id_on_b,
241+
&msg.packet.chan_id_on_b,
242+
msg.packet.seq_on_a,
243+
);
244+
245+
client_state_of_b_on_a.verify_non_membership(
246+
conn_end_on_a.counterparty().prefix(),
247+
&msg.proof_unreceived_on_b,
248+
consensus_state_of_b_on_a.root(),
249+
Path::Receipt(receipt_path_on_b),
250+
)
251+
}
252+
Order::None => {
253+
return Err(ContextError::ChannelError(ChannelError::InvalidOrderType {
254+
expected: "Channel ordering cannot be None".to_string(),
255+
actual: chan_end_on_a.ordering.to_string(),
256+
}))
225257
}
226-
let seq_recv_path_on_b =
227-
SeqRecvPath::new(&msg.packet.port_id_on_b, &msg.packet.chan_id_on_b);
228-
229-
client_state_of_b_on_a.verify_membership(
230-
conn_end_on_a.counterparty().prefix(),
231-
&msg.proof_unreceived_on_b,
232-
consensus_state_of_b_on_a.root(),
233-
Path::SeqRecv(seq_recv_path_on_b),
234-
msg.packet.seq_on_a.to_vec(),
235-
)
236-
} else {
237-
let receipt_path_on_b = ReceiptPath::new(
238-
&msg.packet.port_id_on_b,
239-
&msg.packet.chan_id_on_b,
240-
msg.packet.seq_on_a,
241-
);
242-
243-
client_state_of_b_on_a.verify_non_membership(
244-
conn_end_on_a.counterparty().prefix(),
245-
&msg.proof_unreceived_on_b,
246-
consensus_state_of_b_on_a.root(),
247-
Path::Receipt(receipt_path_on_b),
248-
)
249258
};
259+
250260
next_seq_recv_verification_result
251261
.map_err(|e| ChannelError::PacketVerificationFailed {
252262
sequence: msg.next_seq_recv_on_b,

‎ibc-core/ics04-channel/src/handler/timeout_on_close.rs

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -120,37 +120,48 @@ where
120120

121121
verify_conn_delay_passed(ctx_a, msg.proof_height_on_b, &conn_end_on_a)?;
122122

123-
let next_seq_recv_verification_result = if chan_end_on_a.order_matches(&Order::Ordered) {
124-
if packet.seq_on_a < msg.next_seq_recv_on_b {
125-
return Err(PacketError::InvalidPacketSequence {
126-
given_sequence: packet.seq_on_a,
127-
next_sequence: msg.next_seq_recv_on_b,
123+
let next_seq_recv_verification_result = match chan_end_on_a.ordering {
124+
Order::Ordered => {
125+
if packet.seq_on_a < msg.next_seq_recv_on_b {
126+
return Err(PacketError::InvalidPacketSequence {
127+
given_sequence: packet.seq_on_a,
128+
next_sequence: msg.next_seq_recv_on_b,
129+
}
130+
.into());
128131
}
129-
.into());
132+
let seq_recv_path_on_b =
133+
SeqRecvPath::new(&packet.port_id_on_b, &packet.chan_id_on_b);
134+
135+
client_state_of_b_on_a.verify_membership(
136+
conn_end_on_a.counterparty().prefix(),
137+
&msg.proof_unreceived_on_b,
138+
consensus_state_of_b_on_a.root(),
139+
Path::SeqRecv(seq_recv_path_on_b),
140+
packet.seq_on_a.to_vec(),
141+
)
142+
}
143+
Order::Unordered => {
144+
let receipt_path_on_b = ReceiptPath::new(
145+
&msg.packet.port_id_on_b,
146+
&msg.packet.chan_id_on_b,
147+
msg.packet.seq_on_a,
148+
);
149+
150+
client_state_of_b_on_a.verify_non_membership(
151+
conn_end_on_a.counterparty().prefix(),
152+
&msg.proof_unreceived_on_b,
153+
consensus_state_of_b_on_a.root(),
154+
Path::Receipt(receipt_path_on_b),
155+
)
156+
}
157+
Order::None => {
158+
return Err(ContextError::ChannelError(ChannelError::InvalidOrderType {
159+
expected: "Channel ordering cannot be None".to_string(),
160+
actual: chan_end_on_a.ordering.to_string(),
161+
}))
130162
}
131-
let seq_recv_path_on_b = SeqRecvPath::new(&packet.port_id_on_b, &packet.chan_id_on_b);
132-
133-
client_state_of_b_on_a.verify_membership(
134-
conn_end_on_a.counterparty().prefix(),
135-
&msg.proof_unreceived_on_b,
136-
consensus_state_of_b_on_a.root(),
137-
Path::SeqRecv(seq_recv_path_on_b),
138-
packet.seq_on_a.to_vec(),
139-
)
140-
} else {
141-
let receipt_path_on_b = ReceiptPath::new(
142-
&msg.packet.port_id_on_b,
143-
&msg.packet.chan_id_on_b,
144-
msg.packet.seq_on_a,
145-
);
146-
147-
client_state_of_b_on_a.verify_non_membership(
148-
conn_end_on_a.counterparty().prefix(),
149-
&msg.proof_unreceived_on_b,
150-
consensus_state_of_b_on_a.root(),
151-
Path::Receipt(receipt_path_on_b),
152-
)
153163
};
164+
154165
next_seq_recv_verification_result
155166
.map_err(|e| ChannelError::PacketVerificationFailed {
156167
sequence: msg.next_seq_recv_on_b,

‎ibc-core/ics04-channel/types/src/channel.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,10 @@ impl ChannelEnd {
280280
}
281281

282282
/// Helper function to compare the order of this end with another order.
283+
#[deprecated(
284+
since = "0.50.1",
285+
note = "Use `Eq` or `match` directly on the `Order` enum instead"
286+
)]
283287
pub fn order_matches(&self, other: &Order) -> bool {
284288
self.ordering.eq(other)
285289
}

0 commit comments

Comments
 (0)