From 33fb8560e9a669a5966191547c47df5ba5aee959 Mon Sep 17 00:00:00 2001 From: Ranadeep Biswas Date: Thu, 24 Oct 2024 12:08:57 +0200 Subject: [PATCH] refactor packet handlers after channel types removal --- .../src/handler/acknowledgement.rs | 28 +----- .../ics04-channel/src/handler/recv_packet.rs | 94 ++++--------------- .../ics04-channel/src/handler/send_packet.rs | 7 +- .../ics04-channel/src/handler/timeout.rs | 75 +++------------ .../src/handler/timeout_on_close.rs | 48 ++-------- 5 files changed, 41 insertions(+), 211 deletions(-) diff --git a/ibc-eureka-core/ics04-channel/src/handler/acknowledgement.rs b/ibc-eureka-core/ics04-channel/src/handler/acknowledgement.rs index 234c046d2..04f8fc6d1 100644 --- a/ibc-eureka-core/ics04-channel/src/handler/acknowledgement.rs +++ b/ibc-eureka-core/ics04-channel/src/handler/acknowledgement.rs @@ -6,9 +6,7 @@ use ibc_eureka_core_channel_types::events::AcknowledgePacket; use ibc_eureka_core_channel_types::msgs::MsgAcknowledgement; use ibc_eureka_core_client::context::prelude::*; use ibc_eureka_core_handler_types::events::{IbcEvent, MessageEvent}; -use ibc_eureka_core_host::types::path::{ - AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, SeqAckPath, -}; +use ibc_eureka_core_host::types::path::{AckPath, ClientConsensusStatePath, CommitmentPath, Path}; use ibc_eureka_core_host::{ExecutionContext, ValidationContext}; use ibc_eureka_core_router::module::Module; use ibc_primitives::prelude::*; @@ -41,8 +39,6 @@ where let channel_id_on_a = &packet.header.source_client; let seq_on_a = &packet.header.seq_on_a; - let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, channel_id_on_a); - // In all cases, this event is emitted let event = IbcEvent::AcknowledgePacket(AcknowledgePacket::new(packet.clone())); ctx_a.emit_ibc_event(IbcEvent::Message(MessageEvent::Channel))?; @@ -67,13 +63,6 @@ where // apply state changes { ctx_a.delete_packet_commitment(&commitment_path_on_a)?; - - if let Order::Ordered = chan_end_on_a.ordering { - // Note: in validation, we verified that `msg.packet.sequence == nextSeqRecv` - // (where `nextSeqRecv` is the value in the store) - let seq_ack_path_on_a = SeqAckPath::new(port_id_on_a, channel_id_on_a); - ctx_a.store_next_sequence_ack(&seq_ack_path_on_a, (*seq_on_a).increment())?; - } } // emit events and logs @@ -110,10 +99,6 @@ where let seq_on_a = &packet.header.seq_on_a; let data = &payload.data; - let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, channel_id_on_a); - - let counterparty = Counterparty::new(port_id_on_b.clone(), Some(channel_id_on_b.clone())); - let commitment_path_on_a = CommitmentPath::new(port_id_on_a, channel_id_on_a, *seq_on_a); // Verify packet commitment @@ -138,17 +123,6 @@ where }); } - if let Order::Ordered = chan_end_on_a.ordering { - let seq_ack_path_on_a = SeqAckPath::new(port_id_on_a, channel_id_on_a); - let next_seq_ack = ctx_a.get_next_sequence_ack(&seq_ack_path_on_a)?; - if seq_on_a != &next_seq_ack { - return Err(ChannelError::MismatchedPacketSequence { - actual: *seq_on_a, - expected: next_seq_ack, - }); - } - } - // Verify proofs { // TODO(rano): avoid a vs b confusion diff --git a/ibc-eureka-core/ics04-channel/src/handler/recv_packet.rs b/ibc-eureka-core/ics04-channel/src/handler/recv_packet.rs index 54b778e6e..9689e4ff8 100644 --- a/ibc-eureka-core/ics04-channel/src/handler/recv_packet.rs +++ b/ibc-eureka-core/ics04-channel/src/handler/recv_packet.rs @@ -4,12 +4,10 @@ use ibc_eureka_core_channel_types::commitment::{ use ibc_eureka_core_channel_types::error::ChannelError; use ibc_eureka_core_channel_types::events::{ReceivePacket, WriteAcknowledgement}; use ibc_eureka_core_channel_types::msgs::MsgRecvPacket; -use ibc_eureka_core_channel_types::packet::Receipt; use ibc_eureka_core_client::context::prelude::*; use ibc_eureka_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_eureka_core_host::types::path::{ - AckPath, ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, - SeqRecvPath, + AckPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, SeqRecvPath, }; use ibc_eureka_core_host::{ExecutionContext, ValidationContext}; use ibc_eureka_core_router::module::Module; @@ -41,26 +39,12 @@ where let channel_id_on_b = &packet.header.target_client; let seq_on_a = &packet.header.seq_on_a; - let chan_end_path_on_b = ChannelEndPath::new(port_id_on_b, channel_id_on_b); - // Check if another relayer already relayed the packet. // We don't want to fail the transaction in this case. { - let packet_already_received = match chan_end_on_b.ordering { - // Note: ibc-go doesn't make the check for `Order::None` channels - Order::None => false, - Order::Unordered => { - let receipt_path_on_b = ReceiptPath::new(port_id_on_b, channel_id_on_b, *seq_on_a); - ctx_b.get_packet_receipt(&receipt_path_on_b)?.is_ok() - } - Order::Ordered => { - let seq_recv_path_on_b = SeqRecvPath::new(port_id_on_b, channel_id_on_b); - let next_seq_recv = ctx_b.get_next_sequence_recv(&seq_recv_path_on_b)?; - - // the sequence number has already been incremented, so - // another relayer already relayed the packet - seq_on_a < &next_seq_recv - } + let packet_already_received = { + let receipt_path_on_b = ReceiptPath::new(port_id_on_b, channel_id_on_b, *seq_on_a); + ctx_b.get_packet_receipt(&receipt_path_on_b)?.is_ok() }; if packet_already_received { @@ -73,22 +57,10 @@ where // state changes { // `recvPacket` core handler state changes - match chan_end_on_b.ordering { - Order::Unordered => { - let receipt_path_on_b = ReceiptPath { - port_id: port_id_on_b.clone(), - channel_id: channel_id_on_b.clone(), - sequence: *seq_on_a, - }; - - ctx_b.store_packet_receipt(&receipt_path_on_b, Receipt::Ok)?; - } - Order::Ordered => { - let seq_recv_path_on_b = SeqRecvPath::new(port_id_on_b, channel_id_on_b); - let next_seq_recv = ctx_b.get_next_sequence_recv(&seq_recv_path_on_b)?; - ctx_b.store_next_sequence_recv(&seq_recv_path_on_b, next_seq_recv.increment())?; - } - _ => {} + { + let seq_recv_path_on_b = SeqRecvPath::new(port_id_on_b, channel_id_on_b); + let next_seq_recv = ctx_b.get_next_sequence_recv(&seq_recv_path_on_b)?; + ctx_b.store_next_sequence_recv(&seq_recv_path_on_b, next_seq_recv.increment())?; } let ack_path_on_b = AckPath::new(port_id_on_b, channel_id_on_b, *seq_on_a); // `writeAcknowledgement` handler state changes @@ -134,15 +106,11 @@ where let (_, port_id_on_a) = &payload.header.source_port; let channel_id_on_a = &packet.header.source_client; - let (prefix_on_b, port_id_on_b) = &payload.header.target_port; - let channel_id_on_b = &packet.header.target_client; + let (prefix_on_b, _) = &payload.header.target_port; + let _ = &packet.header.target_client; let seq_on_a = &packet.header.seq_on_a; let data = &payload.data; - let chan_end_path_on_b = ChannelEndPath::new(port_id_on_b, channel_id_on_b); - - let counterparty = Counterparty::new(port_id_on_a.clone(), Some(channel_id_on_a.clone())); - let latest_height = ctx_b.host_height()?; if packet.header.timeout_height_on_b.has_expired(latest_height) { return Err(ChannelError::InsufficientPacketHeight { @@ -198,39 +166,15 @@ where )?; } - match chan_end_on_b.ordering { - Order::Ordered => { - let seq_recv_path_on_b = SeqRecvPath::new(port_id_on_b, channel_id_on_b); - let next_seq_recv = ctx_b.get_next_sequence_recv(&seq_recv_path_on_b)?; - if seq_on_a > &next_seq_recv { - return Err(ChannelError::MismatchedPacketSequence { - actual: *seq_on_a, - expected: next_seq_recv, - }); - } - - if 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 => { - // Note: We don't check for the packet receipt here because another - // relayer may have already relayed the packet. If that's the case, - // we want to avoid failing the transaction and consuming - // unnecessary fees. - - // Case where the recvPacket is successful and an - // acknowledgement will be written (not a no-op) - validate_write_acknowledgement(ctx_b, msg)?; - } - Order::None => { - return Err(ChannelError::InvalidState { - expected: "Channel ordering to not be None".to_string(), - actual: chan_end_on_b.ordering.to_string(), - }) - } + { + // Note: We don't check for the packet receipt here because another + // relayer may have already relayed the packet. If that's the case, + // we want to avoid failing the transaction and consuming + // unnecessary fees. + + // 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-eureka-core/ics04-channel/src/handler/send_packet.rs b/ibc-eureka-core/ics04-channel/src/handler/send_packet.rs index d5e03449c..433e0ba6e 100644 --- a/ibc-eureka-core/ics04-channel/src/handler/send_packet.rs +++ b/ibc-eureka-core/ics04-channel/src/handler/send_packet.rs @@ -4,9 +4,7 @@ use ibc_eureka_core_channel_types::events::SendPacket; use ibc_eureka_core_channel_types::packet::Packet; use ibc_eureka_core_client::context::prelude::*; use ibc_eureka_core_handler_types::events::{IbcEvent, MessageEvent}; -use ibc_eureka_core_host::types::path::{ - ChannelEndPath, ClientConsensusStatePath, CommitmentPath, SeqSendPath, -}; +use ibc_eureka_core_host::types::path::{ClientConsensusStatePath, CommitmentPath, SeqSendPath}; use ibc_primitives::prelude::*; use crate::context::{SendPacketExecutionContext, SendPacketValidationContext}; @@ -120,9 +118,6 @@ pub fn send_packet_execute( // emit events and logs { - let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, channel_id_on_a); - let chan_end_on_a = ctx_a.channel_end(&chan_end_path_on_a)?; - ctx_a.log_message("success: packet send".to_string())?; let event = IbcEvent::SendPacket(SendPacket::new(packet)); ctx_a.emit_ibc_event(IbcEvent::Message(MessageEvent::Channel))?; diff --git a/ibc-eureka-core/ics04-channel/src/handler/timeout.rs b/ibc-eureka-core/ics04-channel/src/handler/timeout.rs index 7346b06c6..6a9e6f006 100644 --- a/ibc-eureka-core/ics04-channel/src/handler/timeout.rs +++ b/ibc-eureka-core/ics04-channel/src/handler/timeout.rs @@ -1,11 +1,11 @@ use ibc_eureka_core_channel_types::commitment::compute_packet_commitment; use ibc_eureka_core_channel_types::error::ChannelError; -use ibc_eureka_core_channel_types::events::{ChannelClosed, TimeoutPacket}; +use ibc_eureka_core_channel_types::events::TimeoutPacket; use ibc_eureka_core_channel_types::msgs::{MsgTimeout, MsgTimeoutOnClose}; use ibc_eureka_core_client::context::prelude::*; use ibc_eureka_core_handler_types::events::{IbcEvent, MessageEvent}; use ibc_eureka_core_host::types::path::{ - ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, SeqRecvPath, + ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, }; use ibc_eureka_core_host::{ExecutionContext, ValidationContext}; use ibc_eureka_core_router::module::Module; @@ -58,8 +58,6 @@ where let channel_id_on_a = &packet.header.source_client; let seq_on_a = &packet.header.seq_on_a; - let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, channel_id_on_a); - // In all cases, this event is emitted let event = IbcEvent::TimeoutPacket(TimeoutPacket::new(packet.clone())); ctx_a.emit_ibc_event(IbcEvent::Message(MessageEvent::Channel))?; @@ -80,36 +78,10 @@ where cb_result?; - // apply state changes - let chan_end_on_a = { - ctx_a.delete_packet_commitment(&commitment_path_on_a)?; - - if let Order::Ordered = chan_end_on_a.ordering { - let mut chan_end_on_a = chan_end_on_a; - chan_end_on_a.state = State::Closed; - ctx_a.store_channel(&chan_end_path_on_a, chan_end_on_a.clone())?; - - chan_end_on_a - } else { - chan_end_on_a - } - }; - // emit events and logs { ctx_a.log_message("success: packet timeout".to_string())?; - if let Order::Ordered = chan_end_on_a.ordering { - let event = IbcEvent::ChannelClosed(ChannelClosed::new( - port_id_on_a.clone(), - channel_id_on_a.clone(), - chan_end_on_a.counterparty().port_id.clone(), - chan_end_on_a.counterparty().channel_id.clone(), - )); - ctx_a.emit_ibc_event(IbcEvent::Message(MessageEvent::Channel))?; - ctx_a.emit_ibc_event(event)?; - } - for module_event in extras.events { ctx_a.emit_ibc_event(IbcEvent::Module(module_event))?; } @@ -192,40 +164,15 @@ where }); } - let next_seq_recv_verification_result = match chan_end_on_a.ordering { - Order::Ordered => { - if seq_on_a < &msg.next_seq_recv_on_b { - return Err(ChannelError::MismatchedPacketSequence { - actual: *seq_on_a, - expected: msg.next_seq_recv_on_b, - }); - } - let seq_recv_path_on_b = SeqRecvPath::new(port_id_on_b, channel_id_on_b); - - client_state_of_b_on_a.verify_membership( - prefix_on_a, - &msg.proof_unreceived_on_b, - consensus_state_of_b_on_a.root(), - Path::SeqRecv(seq_recv_path_on_b), - seq_on_a.to_vec(), - ) - } - Order::Unordered => { - let receipt_path_on_b = ReceiptPath::new(port_id_on_b, channel_id_on_b, *seq_on_a); - - client_state_of_b_on_a.verify_non_membership( - prefix_on_a, - &msg.proof_unreceived_on_b, - consensus_state_of_b_on_a.root(), - Path::Receipt(receipt_path_on_b), - ) - } - Order::None => { - return Err(ChannelError::InvalidState { - expected: "Channel ordering to not be None".to_string(), - actual: chan_end_on_a.ordering.to_string(), - }) - } + let next_seq_recv_verification_result = { + let receipt_path_on_b = ReceiptPath::new(port_id_on_b, channel_id_on_b, *seq_on_a); + + client_state_of_b_on_a.verify_non_membership( + prefix_on_a, + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + Path::Receipt(receipt_path_on_b), + ) }; next_seq_recv_verification_result?; diff --git a/ibc-eureka-core/ics04-channel/src/handler/timeout_on_close.rs b/ibc-eureka-core/ics04-channel/src/handler/timeout_on_close.rs index e87c21a87..9d4845220 100644 --- a/ibc-eureka-core/ics04-channel/src/handler/timeout_on_close.rs +++ b/ibc-eureka-core/ics04-channel/src/handler/timeout_on_close.rs @@ -3,11 +3,10 @@ use ibc_eureka_core_channel_types::error::ChannelError; use ibc_eureka_core_channel_types::msgs::MsgTimeoutOnClose; use ibc_eureka_core_client::context::prelude::*; use ibc_eureka_core_host::types::path::{ - ChannelEndPath, ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, SeqRecvPath, + ClientConsensusStatePath, CommitmentPath, Path, ReceiptPath, }; use ibc_eureka_core_host::ValidationContext; use ibc_primitives::prelude::*; -use ibc_primitives::proto::Protobuf; pub fn validate(ctx_a: &Ctx, msg: &MsgTimeoutOnClose) -> Result<(), ChannelError> where @@ -25,8 +24,6 @@ where let seq_on_a = &packet.header.seq_on_a; let data = &payload.data; - let chan_end_path_on_a = ChannelEndPath::new(port_id_on_a, channel_id_on_a); - let commitment_path_on_a = CommitmentPath::new(port_id_on_a, channel_id_on_a, *seq_on_a); //verify the packet was sent, check the store @@ -70,42 +67,15 @@ where let consensus_state_of_b_on_a = client_val_ctx_a.consensus_state(&client_cons_state_path_on_a)?; - let chan_end_path_on_b = ChannelEndPath(port_id_on_b.clone(), channel_id_on_b.clone()); - - let next_seq_recv_verification_result = match chan_end_on_a.ordering { - Order::Ordered => { - if seq_on_a < &msg.next_seq_recv_on_b { - return Err(ChannelError::MismatchedPacketSequence { - actual: *seq_on_a, - expected: msg.next_seq_recv_on_b, - }); - } - let seq_recv_path_on_b = SeqRecvPath::new(&port_id_on_b, channel_id_on_b); - - client_state_of_b_on_a.verify_membership( - prefix_on_a, - &msg.proof_unreceived_on_b, - consensus_state_of_b_on_a.root(), - Path::SeqRecv(seq_recv_path_on_b), - seq_on_a.to_vec(), - ) - } - Order::Unordered => { - let receipt_path_on_b = ReceiptPath::new(&port_id_on_b, channel_id_on_b, *seq_on_a); + let next_seq_recv_verification_result = { + let receipt_path_on_b = ReceiptPath::new(port_id_on_b, channel_id_on_b, *seq_on_a); - client_state_of_b_on_a.verify_non_membership( - prefix_on_a, - &msg.proof_unreceived_on_b, - consensus_state_of_b_on_a.root(), - Path::Receipt(receipt_path_on_b), - ) - } - Order::None => { - return Err(ChannelError::InvalidState { - expected: "Channel ordering to not be None".to_string(), - actual: chan_end_on_a.ordering.to_string(), - }) - } + client_state_of_b_on_a.verify_non_membership( + prefix_on_a, + &msg.proof_unreceived_on_b, + consensus_state_of_b_on_a.root(), + Path::Receipt(receipt_path_on_b), + ) }; next_seq_recv_verification_result?;