Merge pull request #3646 from anoma/yuji/ibc-refund-transparent

Support only a transparent address as a refund target

.changelog/unreleased/bug-fixes/3620-ibc-refund-transparent.md

@@ -0,0 +1,2 @@
+- Support only a transparent address as a refund target of IBC shielding
+  transfer ([\#3620](https://github.com/anoma/namada/issues/3620))

crates/ibc/build.rs

@@ -0,0 +1,10 @@
+/// Set `is_apple_silicon` flag to avoid a wasm compilation error
+fn main() {
+    let host_arch =
+        std::env::var("HOST").expect("HOST environment variable not found");
+
+    if host_arch == "aarch64-apple-darwin" {
+        println!("cargo:rustc-cfg=is_apple_silicon");
+    }
+    println!("cargo::rustc-check-cfg=cfg(is_apple_silicon)");
+}

crates/ibc/src/context/nft_transfer.rs

@@ -14,7 +14,7 @@ use ibc::apps::nft_transfer::types::{
 };
 use ibc::core::handler::types::error::ContextError;
 use ibc::core::host::types::identifiers::{ChannelId, PortId};
-use namada_core::address::Address;
+use namada_core::address::{Address, MASP};
 use namada_core::token::Amount;
 use namada_systems::trans_token;
 
@@ -28,6 +28,7 @@ where
     C: IbcCommonContext,
 {
     inner: Rc<RefCell<C>>,
+    is_shielded: bool,
     _marker: PhantomData<Token>,
 }
 
@@ -40,10 +41,16 @@ where
     pub fn new(inner: Rc<RefCell<C>>) -> Self {
         Self {
             inner,
+            is_shielded: false,
             _marker: PhantomData,
         }
     }
 
+    /// Set to enable a shielded transfer
+    pub fn enable_shielded_transfer(&mut self) {
+        self.is_shielded = true;
+    }
+
     /// Update the mint amount of the token
     fn update_mint_amount(
         &self,
@@ -163,6 +170,12 @@ where
         // The metadata should exist
         self.get_nft(class_id, token_id)?;
 
+        let from_account = if self.is_shielded {
+            &MASP
+        } else {
+            from_account
+        };
+
         // Check the account owns the NFT
         if self.inner.borrow().is_nft_owned::<Token>(
             class_id,
@@ -228,6 +241,8 @@ where
         // Metadata should exist
         self.get_nft(class_id, token_id)?;
 
+        let account = if self.is_shielded { &MASP } else { account };
+
         // Check the account owns the NFT
         if self
             .inner
@@ -313,6 +328,12 @@ where
 
         self.add_withdraw(&ibc_token)?;
 
+        let from_account = if self.is_shielded {
+            &MASP
+        } else {
+            from_account
+        };
+
         self.inner
             .borrow_mut()
             .transfer_token(
@@ -392,6 +413,8 @@ where
         self.update_mint_amount(&ibc_token, false)?;
         self.add_withdraw(&ibc_token)?;
 
+        let account = if self.is_shielded { &MASP } else { account };
+
         self.inner
             .borrow_mut()
             .burn_token(account, &ibc_token, Amount::from_u64(1))

crates/ibc/src/context/token_transfer.rs

@@ -12,7 +12,7 @@ use ibc::apps::transfer::types::{Memo, PrefixedCoin, PrefixedDenom};
 use ibc::core::channel::types::error::ChannelError;
 use ibc::core::handler::types::error::ContextError;
 use ibc::core::host::types::identifiers::{ChannelId, PortId};
-use namada_core::address::{Address, InternalAddress};
+use namada_core::address::{Address, InternalAddress, MASP};
 use namada_core::token::Amount;
 use namada_core::uint::Uint;
 
@@ -27,6 +27,7 @@ where
 {
     inner: Rc<RefCell<C>>,
     verifiers: Rc<RefCell<BTreeSet<Address>>>,
+    is_shielded: bool,
 }
 
 impl<C> TokenTransferContext<C>
@@ -38,14 +39,23 @@ where
         inner: Rc<RefCell<C>>,
         verifiers: Rc<RefCell<BTreeSet<Address>>>,
     ) -> Self {
-        Self { inner, verifiers }
+        Self {
+            inner,
+            verifiers,
+            is_shielded: false,
+        }
     }
 
     /// Insert a verifier address whose VP will verify the tx.
     fn insert_verifier(&mut self, addr: &Address) {
         self.verifiers.borrow_mut().insert(addr.clone());
     }
 
+    /// Set to enable a shielded transfer
+    pub fn enable_shielded_transfer(&mut self) {
+        self.is_shielded = true;
+    }
+
     /// Get the token address and the amount from PrefixedCoin. If the base
     /// denom is not an address, it returns `IbcToken`
     fn get_token_amount(
@@ -251,6 +261,12 @@ where
             self.insert_verifier(&ibc_token);
         }
 
+        let from_account = if self.is_shielded {
+            &MASP
+        } else {
+            from_account
+        };
+
         self.inner
             .borrow_mut()
             .transfer_token(
@@ -325,6 +341,8 @@ where
             self.insert_verifier(&ibc_token);
         }
 
+        let account = if self.is_shielded { &MASP } else { account };
+
         // The burn is "unminting" from the minted balance
         self.inner
             .borrow_mut()

crates/ibc/src/lib.rs

@@ -61,9 +61,7 @@ use ibc::apps::transfer::handler::{
 use ibc::apps::transfer::types::error::TokenTransferError;
 use ibc::apps::transfer::types::msgs::transfer::MsgTransfer as IbcMsgTransfer;
 use ibc::apps::transfer::types::{is_receiver_chain_source, TracePrefix};
-use ibc::core::channel::types::acknowledgement::{
-    Acknowledgement, AcknowledgementStatus,
-};
+use ibc::core::channel::types::acknowledgement::AcknowledgementStatus;
 use ibc::core::channel::types::commitment::compute_ack_commitment;
 use ibc::core::channel::types::msgs::{
     MsgRecvPacket as IbcMsgRecvPacket, PacketMsg,
@@ -614,6 +612,9 @@ where
                     self.verifiers.clone(),
                 );
                 self.insert_verifiers()?;
+                if msg.transfer.is_some() {
+                    token_transfer_ctx.enable_shielded_transfer();
+                }
                 send_transfer_execute(
                     &mut self.ctx,
                     &mut token_transfer_ctx,
@@ -625,6 +626,9 @@ where
             IbcMessage::NftTransfer(msg) => {
                 let mut nft_transfer_ctx =
                     NftTransferContext::<_, Token>::new(self.ctx.inner.clone());
+                if msg.transfer.is_some() {
+                    nft_transfer_ctx.enable_shielded_transfer();
+                }
                 send_nft_transfer_execute(
                     &mut self.ctx,
                     &mut nft_transfer_ctx,
@@ -638,34 +642,17 @@ where
                     .map_err(|e| Error::Context(Box::new(e)))?;
                 // Extract MASP tx from the memo in the packet if needed
                 let masp_tx = match &*envelope {
-                    MsgEnvelope::Packet(packet_msg) => {
-                        match packet_msg {
-                            PacketMsg::Recv(msg) => {
-                                if self.is_receiving_success(msg)? {
-                                    extract_masp_tx_from_packet(
-                                        &msg.packet,
-                                        false,
-                                    )
-                                } else {
-                                    None
-                                }
-                            }
-                            PacketMsg::Ack(msg) => {
-                                if is_ack_successful(&msg.acknowledgement)? {
-                                    // No refund
-                                    None
-                                } else {
-                                    extract_masp_tx_from_packet(
-                                        &msg.packet,
-                                        true,
-                                    )
-                                }
-                            }
-                            PacketMsg::Timeout(msg) => {
-                                extract_masp_tx_from_packet(&msg.packet, true)
-                            }
-                            _ => None,
-                        }
+                    MsgEnvelope::Packet(PacketMsg::Recv(msg))
+                        if self.is_receiving_success(msg)? =>
+                    {
+                        extract_masp_tx_from_packet(&msg.packet)
+                    }
+                    #[cfg(is_apple_silicon)]
+                    MsgEnvelope::Packet(PacketMsg::Ack(msg)) => {
+                        // NOTE: This is unneeded but wasm compilation error
+                        // happened if deleted on macOS with Apple Silicon
+                        let _ = extract_masp_tx_from_packet(&msg.packet);
+                        None
                     }
                     _ => None,
                 };
@@ -746,18 +733,6 @@ where
     }
 }
 
-fn is_ack_successful(ack: &Acknowledgement) -> Result<bool, Error> {
-    let acknowledgement = serde_json::from_slice::<AcknowledgementStatus>(
-        ack.as_ref(),
-    )
-    .map_err(|e| {
-        Error::TokenTransfer(TokenTransferError::Other(format!(
-            "Decoding the acknowledgement failed: {e}"
-        )))
-    })?;
-    Ok(acknowledgement.is_successful())
-}
-
 /// Tries to decode transaction data to an `IbcMessage`
 pub fn decode_message<Transfer: BorshDeserialize>(
     tx_data: &[u8],

crates/ibc/src/msg.rs

@@ -130,12 +130,7 @@ impl<Transfer: BorshSchema> BorshSchema for MsgNftTransfer<Transfer> {
 
 /// Shielding data in IBC packet memo
 #[derive(Debug, Clone, BorshDeserialize, BorshSerialize)]
-pub struct IbcShieldingData {
-    /// MASP transaction for receiving the token
-    pub shielding: Option<MaspTransaction>,
-    /// MASP transaction for refunding the token
-    pub refund: Option<MaspTransaction>,
-}
+pub struct IbcShieldingData(pub MaspTransaction);
 
 impl From<IbcShieldingData> for String {
     fn from(data: IbcShieldingData) -> Self {
@@ -148,18 +143,9 @@ pub fn extract_masp_tx_from_envelope(
     envelope: &MsgEnvelope,
 ) -> Option<MaspTransaction> {
     match envelope {
-        MsgEnvelope::Packet(packet_msg) => match packet_msg {
-            PacketMsg::Recv(msg) => {
-                extract_masp_tx_from_packet(&msg.packet, false)
-            }
-            PacketMsg::Ack(msg) => {
-                extract_masp_tx_from_packet(&msg.packet, true)
-            }
-            PacketMsg::Timeout(msg) => {
-                extract_masp_tx_from_packet(&msg.packet, true)
-            }
-            _ => None,
-        },
+        MsgEnvelope::Packet(PacketMsg::Recv(msg)) => {
+            extract_masp_tx_from_packet(&msg.packet)
+        }
         _ => None,
     }
 }
@@ -173,22 +159,9 @@ pub fn decode_ibc_shielding_data(
 }
 
 /// Extract MASP transaction from IBC packet memo
-pub fn extract_masp_tx_from_packet(
-    packet: &Packet,
-    is_sender: bool,
-) -> Option<MaspTransaction> {
-    let port_id = if is_sender {
-        &packet.port_id_on_a
-    } else {
-        &packet.port_id_on_b
-    };
-    let memo = extract_memo_from_packet(packet, port_id)?;
-    let shielding_data = decode_ibc_shielding_data(memo)?;
-    if is_sender {
-        shielding_data.refund
-    } else {
-        shielding_data.shielding
-    }
+pub fn extract_masp_tx_from_packet(packet: &Packet) -> Option<MaspTransaction> {
+    let memo = extract_memo_from_packet(packet, &packet.port_id_on_b)?;
+    decode_ibc_shielding_data(memo).map(|data| data.0)
 }
 
 fn extract_memo_from_packet(
@@ -222,9 +195,5 @@ fn extract_memo_from_packet(
 
 /// Get IBC memo string from MASP transaction for receiving
 pub fn convert_masp_tx_to_ibc_memo(transaction: &MaspTransaction) -> String {
-    let shielding_data = IbcShieldingData {
-        shielding: Some(transaction.clone()),
-        refund: None,
-    };
-    shielding_data.into()
+    IbcShieldingData(transaction.clone()).into()
 }