Now check that the IBC events are valid with respect to storage changes.

crates/events/src/extend.rs

@@ -483,21 +483,6 @@ impl EventAttributeEntry<'static> for Info {
     }
 }
 
-/// Extend an [`Event`] with `packet_ack` data, indicating the success or
-/// failure of processing a received packet.
-pub struct PacketAck(pub Vec<u8>);
-
-impl EventAttributeEntry<'static> for PacketAck {
-    type Value = Vec<u8>;
-    type ValueOwned = Self::Value;
-
-    const KEY: &'static str = "packet_ack";
-
-    fn into_value(self) -> Self::Value {
-        self.0
-    }
-}
-
 /// Extend an [`Event`] with `masp_tx_block_index` data, indicating that the tx
 /// at the specified index in the block contains a valid masp transaction.
 pub struct MaspTxBlockIndex(pub TxIndex);

crates/ibc/src/context/common.rs

@@ -510,7 +510,6 @@ pub trait IbcCommonContext: IbcStorageContext {
 
     /// Calculate the packet commitment
     fn compute_packet_commitment(
-        &self,
         packet_data: &[u8],
         timeout_height: &TimeoutHeight,
         timeout_timestamp: &Timestamp,

crates/namada/src/ledger/native_vp/masp.rs

@@ -55,13 +55,21 @@ use crate::vm::WasmCacheAccess;
 use crate::ledger::ibc::storage::ibc_token;
 use crate::sdk::ibc::is_ibc_denom;
 use crate::sdk::ibc::IbcTokenHash;
-use crate::ledger::events::extend::PacketAck;
 use crate::sdk::ibc::core::channel::types::acknowledgement::AcknowledgementStatus;
+use crate::sdk::ibc::core::channel::types::commitment::PacketCommitment;
+use namada_ibc::event::PacketAck;
+use crate::ledger::native_vp::masp::ibc_events::PacketSrcPort;
+use crate::ledger::native_vp::masp::ibc_events::PacketSrcChannel;
+use crate::ledger::native_vp::masp::ibc_events::PacketSequence;
+use namada_events::extend::ReadFromEventAttributes;
+use namada_ibc::IbcCommonContext;
+use crate::ledger::native_vp::ibc::context::VpValidationContext;
 
 /// Packet event types
 const SEND_PACKET_EVENT: &str = "send_packet";
 const RECEIVE_PACKET_EVENT: &str = "recv_packet";
 const WRITE_ACK_EVENT: &str = "write_acknowledgement";
+const ACK_PACKET_EVENT: &str = "acknowledge_packet";
 
 #[allow(missing_docs)]
 #[derive(Error, Debug)]
@@ -428,6 +436,8 @@ where
             if let Ok(packet_data) =
                 serde_json::from_slice::<PacketData>(&msg.data)
             {
+                // Get the packet commitment from post-storage that corresponds
+                // to this event
                 let address =
                     MaybeIbcAddress::Ibc(packet_data.receiver.to_string());
                 // Public keys must be the hash of the sources/targets
@@ -445,11 +455,12 @@ where
                     .post
                     .entry(ibc_address_hash)
                     .or_insert(ValueSum::zero());
-                // Also enable the tracking of received IBC
+                // Required for the IBC internal Address to release funds
                 let pre_entry = changed_balances
                     .pre
                     .entry(ibc_address_hash)
                     .or_insert(ValueSum::zero());
+                // Required for the packet's receiver to get funds
                 let post_entry = changed_balances
                     .post
                     .entry(address_hash)
@@ -458,6 +469,35 @@ where
                 match ibc_event.kind().sub_domain() {
                     // This event is emitted on the sender
                     SEND_PACKET_EVENT => {
+                        // Ensure that the event corresponds to the current changes to
+                        // storage
+                        let commitment_key = storage::commitment_key(
+                            &msg.port_id_on_a,
+                            &msg.chan_id_on_a,
+                            msg.seq_on_a,
+                        );
+                        if !keys_changed.contains(&commitment_key) {
+                            // Otherwise ignore this event
+                            continue;
+                        }
+                        let Some(storage_commitment): Option<PacketCommitment> = self
+                            .ctx
+                            .read_bytes_post(&commitment_key)?
+                            .map(Into::into) else {
+                                // Ignore this event if it does not exist
+                                continue;
+                            };
+                        // Check that the packet commitment in storage is the same as
+                        // that derived from this event
+                        let packet_commitment = VpValidationContext::<'a, 'a, S, CA>::compute_packet_commitment(
+                            &msg.data,
+                            &msg.timeout_height_on_b,
+                            &msg.timeout_timestamp_on_b,
+                        );
+                        if packet_commitment != storage_commitment {
+                            // Ignore this event if the commitments are not equal
+                            continue;
+                        }
                         // Since IBC denominations are derived from Addresses
                         // when sending, we have to do a reverse look-up of the
                         // relevant token Address
@@ -484,33 +524,65 @@ where
                             .map_err(native_vp::Error::new)?
                     },
                     // This event is emitted on the receiver
-                    RECEIVE_PACKET_EVENT => {
-                        // Mirror how the IBC token is derived in
-                        // gen_ibc_shielded_transfer in the non-refund case
-                        let ibc_denom = self.query_ibc_denom(
-                            &packet_data.token.denom.to_string(),
-                            Some(&Address::Internal(InternalAddress::Ibc)),
-                        )?;
-                        let token = namada_ibc::received_ibc_token(
-                            ibc_denom,
+                    WRITE_ACK_EVENT => {
+                        // Ensure that the event corresponds to the current changes to
+                        // storage
+                        let ack_key = storage::ack_key(
                             &msg.port_id_on_a,
                             &msg.chan_id_on_a,
-                            &msg.port_id_on_b,
-                            &msg.chan_id_on_b,
-                        ).into_storage_result()
-                            .map_err(Error::NativeVpError)?;
-                        let delta = ValueSum::from_pair(
-                            token.clone(),
-                            Amount::from_uint(Uint(*packet_data.token.amount), 0)
-                                .unwrap(),
+                            msg.seq_on_a,
                         );
-                        // Enable funds to be taken from the IBC internal
-                        // address and be deposited elsewhere
-                        *pre_entry = checked!(pre_entry + &delta)
-                            .map_err(native_vp::Error::new)?
+                        if !keys_changed.contains(&ack_key) {
+                            // Otherwise ignore this event
+                            continue;
+                        }
+                        let acknowledgement = ibc_event
+                            .raw_read_attribute::<PacketAck>()
+                            .ok_or_err_msg("packet_ack attribute missing from write_acknowledgement")?;
+                        let acknowledgement: AcknowledgementStatus =
+                            serde_json::from_slice(acknowledgement.as_ref())
+                            .wrap_err("Decoding acknowledment failed")?;
+                        // If the transfer was a failure, then enable funds to
+                        // be withdrawn from the IBC internal address
+                        if acknowledgement.is_successful() {
+                            // Mirror how the IBC token is derived in
+                            // gen_ibc_shielded_transfer in the non-refund case
+                            let ibc_denom = self.query_ibc_denom(
+                                &packet_data.token.denom.to_string(),
+                                Some(&Address::Internal(InternalAddress::Ibc)),
+                            )?;
+                            let token = namada_ibc::received_ibc_token(
+                                ibc_denom,
+                                &msg.port_id_on_a,
+                                &msg.chan_id_on_a,
+                                &msg.port_id_on_b,
+                                &msg.chan_id_on_b,
+                            ).into_storage_result()
+                                .map_err(Error::NativeVpError)?;
+                            let delta = ValueSum::from_pair(
+                                token.clone(),
+                                Amount::from_uint(Uint(*packet_data.token.amount), 0)
+                                    .unwrap(),
+                            );
+                            // Enable funds to be taken from the IBC internal
+                            // address and be deposited elsewhere
+                            *pre_entry = checked!(pre_entry + &delta)
+                                .map_err(native_vp::Error::new)?;
+                        }
                     },
                     // This event is emitted on the sender
-                    WRITE_ACK_EVENT => {
+                    ACK_PACKET_EVENT => {
+                        // Ensure that the event corresponds to the current changes to
+                        // storage
+                        let ack_key = storage::ack_key(
+                            &msg.port_id_on_a,
+                            &msg.chan_id_on_a,
+                            msg.seq_on_a,
+                        );
+                        if !keys_changed.contains(&ack_key) {
+                            // Otherwise ignore this event
+                            continue;
+                        }
                         let acknowledgement = ibc_event
                             .raw_read_attribute::<PacketAck>()
                             .ok_or_err_msg("packet_ack attribute missing from write_acknowledgement")?;