From 80fc75dcd5f333f119cfad455507903dd66e131a Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Fri, 14 Jun 2024 11:34:01 +0200 Subject: [PATCH] Subdivided some functions involved in processing IBC packets. --- crates/core/src/masp.rs | 6 +- crates/namada/src/ledger/native_vp/masp.rs | 220 +++++++++++++-------- 2 files changed, 136 insertions(+), 90 deletions(-) diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index 42559838542..0cd54ebc411 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -491,7 +491,7 @@ impl Display for TransferSource { /// Represents the pre-image to a TransparentAddress #[derive(Debug, Clone, BorshDeserialize, BorshSerialize, BorshDeserializer)] pub enum TAddrData { - /// A transparent address + /// A transparent address within Namada Addr(Address), /// An IBC address Ibc(String), @@ -509,7 +509,7 @@ impl TAddrData { } /// Get the contained IBC receiver, if any - pub fn payment_address(&self) -> Option { + pub fn ibc_receiver_address(&self) -> Option { match self { Self::Ibc(address) => Some(address.clone()), _ => None, @@ -583,7 +583,7 @@ impl TransferTarget { } } - /// Get the contained MaybeIbcAddress, if any + /// Get the contained TAddrData, if any pub fn t_addr_data(&self) -> Option { match self { Self::Address(x) => Some(TAddrData::Addr(x.clone())), diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index 0d32b9199aa..c9a75a55e88 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -17,11 +17,15 @@ use namada_core::arith::{checked, CheckedAdd, CheckedSub}; use namada_core::booleans::BoolResultUnitExt; use namada_core::collections::HashSet; use namada_core::ibc::apps::transfer::types::is_sender_chain_source; +use namada_core::ibc::apps::transfer::types::msgs::transfer::MsgTransfer as IbcMsgTransfer; use namada_core::ibc::apps::transfer::types::packet::PacketData; use namada_core::masp::{addr_taddr, encode_asset_type, ibc_taddr, MaspEpoch}; use namada_core::storage::Key; use namada_gas::GasMetering; use namada_governance::storage::is_proposal_accepted; +use namada_ibc::apps::transfer::types::PrefixedDenom; +use namada_ibc::core::channel::types::msgs::MsgRecvPacket as IbcMsgRecvPacket; +use namada_ibc::core::host::types::identifiers::Sequence; use namada_ibc::event::{IbcEvent, PacketAck}; use namada_ibc::{IbcCommonContext, IbcMessage}; use namada_sdk::masp::{verify_shielded_tx, TAddrData}; @@ -333,34 +337,30 @@ where Ok(token.as_ref().to_string()) } - // Apply the given send packet to the changed balances structure - fn apply_send_packet( + // Find the given IBC message in the changed keys and return the associated + // sequence number + fn search_ibc_transfer( &self, - acc: &mut ChangedBalances, - msg: &MsgTransfer, + message: &IbcMsgTransfer, keys_changed: &BTreeSet, - ) -> Result<()> { + ) -> Result> { // Compute the packet commitment for this message - let Ok(packet_data_bytes) = - serde_json::to_vec(&msg.message.packet_data) - else { - return Ok(()); - }; + let packet_data_bytes = serde_json::to_vec(&message.packet_data) + .map_err(native_vp::Error::new)?; let packet_commitment = VpValidationContext::<'a, 'a, S, CA>::compute_packet_commitment( &packet_data_bytes, - &msg.message.timeout_height_on_b, - &msg.message.timeout_timestamp_on_b, + &message.timeout_height_on_b, + &message.timeout_timestamp_on_b, ); // Try to find a key change with the same port, channel, and commitment // as this message and note its sequence number - let mut seq_on_a = None; for key in keys_changed { let Some(path) = is_ibc_commitment_key(key) else { continue; }; - if path.port_id == msg.message.port_id_on_a - && path.channel_id == msg.message.chan_id_on_a + if path.port_id == message.port_id_on_a + && path.channel_id == message.chan_id_on_a { let Some(storage_commitment): Option = self.ctx.read_bytes_post(key)?.map(Into::into) @@ -369,54 +369,66 @@ where continue; }; if packet_commitment == storage_commitment { - seq_on_a = Some(path.sequence); - break; + return Ok(Some(path.sequence)); } } } - // If a key change with the same port, channel, and commitment as this - // message cannot be found, then ignore this message - if seq_on_a.is_none() { - return Ok(()); - } + Ok(None) + } - // Since IBC denominations are derived from Addresses - // when sending, we have to do a reverse look-up of the - // relevant token Address - let token = acc - .ibc_denoms - .get(&msg.message.packet_data.token.denom.to_string()) + // Try to determine which address would cause query_ibc_denom to yield the + // supplied denom + fn reverse_query_ibc_denom( + denom: &PrefixedDenom, + ibc_denoms: &BTreeMap, + ) -> Option
{ + ibc_denoms + .get(&denom.to_string()) .cloned() // If the reverse lookup failed, then guess the Address // that might have yielded the IBC denom. However, // guessing an IBC token address cannot possibly be // correct due to the structure of query_ibc_denom .or_else(|| { - Address::decode(msg.message.packet_data.token.denom.to_string()) - .ok() - .filter(|x| { - !matches!( - x, - Address::Internal(InternalAddress::IbcToken(_)) - ) - }) + Address::decode(denom.to_string()).ok().filter(|x| { + !matches!( + x, + Address::Internal(InternalAddress::IbcToken(_)) + ) + }) }) - .ok_or_err_msg("Unable to decode IBC token")?; + } + + // Apply the given send packet to the changed balances structure + fn apply_send_packet( + &self, + mut acc: ChangedBalances, + msg: &MsgTransfer, + keys_changed: &BTreeSet, + ) -> Result { + // If a key change with the same port, channel, and commitment as this + // message cannot be found, then ignore this message + if self + .search_ibc_transfer(&msg.message, keys_changed)? + .is_none() + { + return Ok(acc); + }; + + // Since IBC denominations are derived from Addresses + // when sending, we have to do a reverse look-up of the + // relevant token Address + let Some(token) = Self::reverse_query_ibc_denom( + &msg.message.packet_data.token.denom, + &acc.ibc_denoms, + ) else { + return Ok(acc); + }; let delta = ValueSum::from_pair( token.clone(), Amount::from_uint(Uint(*msg.message.packet_data.token.amount), 0) .unwrap(), ); - // Required for the packet's receiver to get funds - let post_entry = acc - .post - .entry(ibc_taddr(msg.message.packet_data.receiver.to_string())) - .or_insert(ValueSum::zero()); - // Enable funds to be received by the receiver in the - // IBC packet - *post_entry = - checked!(post_entry + &delta).map_err(native_vp::Error::new)?; - // If there is a transfer to the IBC account, then deduplicate the // balance increase since we already accounted for it above if is_sender_chain_source( @@ -429,41 +441,57 @@ where *post_entry = checked!(post_entry - &delta).map_err(native_vp::Error::new)?; } - Ok(()) + // Required for the packet's receiver to get funds + let post_entry = acc + .post + .entry(ibc_taddr(msg.message.packet_data.receiver.to_string())) + .or_insert(ValueSum::zero()); + // Enable funds to be received by the receiver in the + // IBC packet + *post_entry = + checked!(post_entry + &delta).map_err(native_vp::Error::new)?; + + Ok(acc) } - // Apply the given write acknowledge to the changed balances structure - fn apply_write_ack( + // Get the acknowledgement status associated with the given message + fn get_msg_ack( &self, - acc: &mut ChangedBalances, - msg: &MsgRecvPacket, - packet_data: &PacketData, + message: &IbcMsgRecvPacket, keys_changed: &BTreeSet, ibc_acks: &BTreeMap, AcknowledgementStatus>, - ) -> Result<()> { + ) -> Result> { // Ensure that the event corresponds to the current changes // to storage let ack_key = storage::ack_key( - &msg.message.packet.port_id_on_a, - &msg.message.packet.chan_id_on_a, - msg.message.packet.seq_on_a, + &message.packet.port_id_on_a, + &message.packet.chan_id_on_a, + message.packet.seq_on_a, ); if !keys_changed.contains(&ack_key) { // Otherwise ignore this event - return Ok(()); + return Ok(None); } let acknowledgement = match self.ctx.read_bytes_post(&ack_key)? { Some(value) => AcknowledgementCommitment::from(value), - None => return Ok(()), - }; - // Get the pre-image of the acknowledgement commitment - let Some(acknowledgement) = ibc_acks.get(&acknowledgement.into_vec()) - else { - return Ok(()); + None => return Ok(None), }; + Ok(ibc_acks.get(&acknowledgement.into_vec()).cloned()) + } + + // Apply the given write acknowledge to the changed balances structure + fn apply_write_ack( + &self, + mut acc: ChangedBalances, + msg: &MsgRecvPacket, + packet_data: &PacketData, + keys_changed: &BTreeSet, + ibc_acks: &BTreeMap, AcknowledgementStatus>, + ) -> Result { // If the transfer was a failure, then enable funds to // be withdrawn from the IBC internal address - if acknowledgement.is_successful() { + if matches!(self.get_msg_ack(&msg.message, keys_changed, ibc_acks)?, Some(ack) if ack.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( @@ -492,7 +520,7 @@ where *pre_entry = checked!(pre_entry + &delta).map_err(native_vp::Error::new)?; } - Ok(()) + Ok(acc) } // Apply relevant IBC packets to the changed balances structure @@ -511,7 +539,7 @@ where let receiver = msg.message.packet_data.receiver.to_string(); let addr = TAddrData::Ibc(receiver.clone()); acc.decoder.insert(ibc_taddr(receiver), addr); - self.apply_send_packet(&mut acc, msg, keys_changed)?; + acc = self.apply_send_packet(acc, msg, keys_changed)?; } // This event is emitted on the receiver IbcMessage::RecvPacket(msg) => { @@ -524,8 +552,8 @@ where let receiver = packet_data.receiver.to_string(); let addr = TAddrData::Ibc(receiver.clone()); acc.decoder.insert(ibc_taddr(receiver), addr); - self.apply_write_ack( - &mut acc, + acc = self.apply_write_ack( + acc, msg, &packet_data, keys_changed, @@ -547,6 +575,7 @@ where let denom = read_denom(&self.ctx.pre(), token)?.ok_or_err_msg( "No denomination found in storage for the given token", )?; + // Record the token without an epoch to facilitate later decoding unepoched_tokens(token, denom, &mut result.tokens)?; let counterpart_balance_key = balance_key(token, counterpart); let pre_balance: Amount = self @@ -569,14 +598,17 @@ where result .decoder .insert(address_hash, TAddrData::Addr(counterpart.clone())); + // Finally record the actual balance change starting with the initial + // state let pre_entry = result.pre.entry(address_hash).or_insert(ValueSum::zero()); - let post_entry = - result.post.entry(address_hash).or_insert(ValueSum::zero()); *pre_entry = checked!( pre_entry + &ValueSum::from_pair((*token).clone(), pre_balance) ) .map_err(native_vp::Error::new)?; + // And then record thee final state + let post_entry = + result.post.entry(address_hash).or_insert(ValueSum::zero()); *post_entry = checked!( post_entry + &ValueSum::from_pair((*token).clone(), post_balance) ) @@ -584,41 +616,55 @@ where Result::<_>::Ok(result) } - // Check that transfer is pinned correctly and record the balance changes - fn validate_state_and_get_transfer_data( + // Collect the various packet acknowledgements and store them by their + // commitments + fn extract_ibc_acknowledgements( &self, - keys_changed: &BTreeSet, - ibc_msgs: &[IbcMessage], - ) -> Result { - // Get the changed balance keys - let mut counterparts_balances = - keys_changed.iter().filter_map(is_any_token_balance_key); - - // Apply the balance changes to the changed balances structure - let mut changed_balances = counterparts_balances - .try_fold(ChangedBalances::default(), |acc, account| { - self.apply_balance_change(acc, account) - })?; - - // Collect the various packet acknowledgements + ) -> Result, AcknowledgementStatus>> { let mut ibc_acks = BTreeMap::new(); let ibc_events = self.ctx.state.write_log().get_events_of::(); for ibc_event in ibc_events { if ibc_event.kind().sub_domain() == WRITE_ACK_EVENT { + // Packet acknowledgements are an attribute of the IBC Write + // Acknowledgement event let acknowledgement = ibc_event .raw_read_attribute::>() .ok_or_err_msg( "packet_ack attribute missing from \ write_acknowledgement", )?; + // Acknowledgements adhere to the AcknowledgementStatus format let acknowledgement: AcknowledgementStatus = serde_json::from_slice(acknowledgement.as_ref()) .wrap_err("Decoding acknowledment failed")?; let commitment = compute_ack_commitment(&acknowledgement.clone().into()); + // Store the acknowledgements by their commitment to facilitate + // reverse lookups on the commitments placed in storage ibc_acks.insert(commitment.into_vec(), acknowledgement); } } + Ok(ibc_acks) + } + + // Check that transfer is pinned correctly and record the balance changes + fn validate_state_and_get_transfer_data( + &self, + keys_changed: &BTreeSet, + ibc_msgs: &[IbcMessage], + ) -> Result { + // Get the changed balance keys + let mut counterparts_balances = + keys_changed.iter().filter_map(is_any_token_balance_key); + + // Apply the balance changes to the changed balances structure + let mut changed_balances = counterparts_balances + .try_fold(ChangedBalances::default(), |acc, account| { + self.apply_balance_change(acc, account) + })?; + + // Collect the various packet acknowledgements + let ibc_acks = self.extract_ibc_acknowledgements()?; let ibc_addr = TAddrData::Addr(IBC); // Enable decoding the IBC address hash