diff --git a/crates/events/src/extend.rs b/crates/events/src/extend.rs index b4111fc4a5c..fdd098b4019 100644 --- a/crates/events/src/extend.rs +++ b/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); - -impl EventAttributeEntry<'static> for PacketAck { - type Value = Vec; - 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); diff --git a/crates/ibc/src/context/common.rs b/crates/ibc/src/context/common.rs index e536bb75eef..562ac257c1d 100644 --- a/crates/ibc/src/context/common.rs +++ b/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, diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index 0e7891adf6b..6ea9483a1a3 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/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::(&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 = 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::() + .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::() .ok_or_err_msg("packet_ack attribute missing from write_acknowledgement")?;