Make the sender set MASP transaction to IBC memo

.changelog/unreleased/bug-fixes/3438-fix-ibc-shielding-transfer.md

@@ -0,0 +1,2 @@
+- Fix IBC shielding transfer for the receiver not to be replaced by a malicious
+  relayer ([\#3438](https://github.com/anoma/namada/issues/3438))

.github/workflows/scripts/hermes.txt

@@ -1 +1 @@
-1.8.2-namada-beta11-rc2
+1.9.0-namada-beta13-rc

crates/apps_lib/src/cli.rs

@@ -292,6 +292,7 @@ pub mod cmds {
                 // Actions
                 .subcommand(SignTx::def().display_order(6))
                 .subcommand(ShieldedSync::def().display_order(6))
+                .subcommand(GenIbcShieldingTransfer::def().display_order(6))
                 // Utils
                 .subcommand(ClientUtils::def().display_order(7))
         }
@@ -380,6 +381,8 @@ pub mod cmds {
                 Self::parse_with_ctx(matches, AddToEthBridgePool);
             let sign_tx = Self::parse_with_ctx(matches, SignTx);
             let shielded_sync = Self::parse_with_ctx(matches, ShieldedSync);
+            let gen_ibc_shielding =
+                Self::parse_with_ctx(matches, GenIbcShieldingTransfer);
             let utils = SubCmd::parse(matches).map(Self::WithoutContext);
             tx_custom
                 .or(tx_transparent_transfer)
@@ -434,6 +437,7 @@ pub mod cmds {
                 .or(query_account)
                 .or(sign_tx)
                 .or(shielded_sync)
+                .or(gen_ibc_shielding)
                 .or(utils)
         }
     }
@@ -524,6 +528,7 @@ pub mod cmds {
         QueryRewards(QueryRewards),
         SignTx(SignTx),
         ShieldedSync(ShieldedSync),
+        GenIbcShieldingTransfer(GenIbcShieldingTransfer),
     }
 
     #[allow(clippy::large_enum_variant)]
@@ -2259,6 +2264,29 @@ pub mod cmds {
         }
     }
 
+    #[derive(Clone, Debug)]
+    pub struct GenIbcShieldingTransfer(
+        pub args::GenIbcShieldingTransfer<args::CliTypes>,
+    );
+
+    impl SubCmd for GenIbcShieldingTransfer {
+        const CMD: &'static str = "ibc-gen-shielding";
+
+        fn parse(matches: &ArgMatches) -> Option<Self> {
+            matches.subcommand_matches(Self::CMD).map(|matches| {
+                GenIbcShieldingTransfer(args::GenIbcShieldingTransfer::parse(
+                    matches,
+                ))
+            })
+        }
+
+        fn def() -> App {
+            App::new(Self::CMD)
+                .about("Generate shielding transfer for IBC.")
+                .add_args::<args::GenIbcShieldingTransfer<args::CliTypes>>()
+        }
+    }
+
     #[derive(Clone, Debug)]
     pub struct EpochSleep(pub args::Query<args::CliTypes>);
 
@@ -6432,19 +6460,19 @@ pub mod args {
         }
     }
 
-    impl CliToSdk<GenIbcShieldedTransfer<SdkTypes>>
-        for GenIbcShieldedTransfer<CliTypes>
+    impl CliToSdk<GenIbcShieldingTransfer<SdkTypes>>
+        for GenIbcShieldingTransfer<CliTypes>
     {
         type Error = std::convert::Infallible;
 
         fn to_sdk(
             self,
             ctx: &mut Context,
-        ) -> Result<GenIbcShieldedTransfer<SdkTypes>, Self::Error> {
+        ) -> Result<GenIbcShieldingTransfer<SdkTypes>, Self::Error> {
             let query = self.query.to_sdk(ctx)?;
             let chain_ctx = ctx.borrow_chain_or_exit();
 
-            Ok(GenIbcShieldedTransfer::<SdkTypes> {
+            Ok(GenIbcShieldingTransfer::<SdkTypes> {
                 query,
                 output_folder: self.output_folder,
                 target: chain_ctx.get(&self.target),
@@ -6457,7 +6485,7 @@ pub mod args {
         }
     }
 
-    impl Args for GenIbcShieldedTransfer<CliTypes> {
+    impl Args for GenIbcShieldingTransfer<CliTypes> {
         fn parse(matches: &ArgMatches) -> Self {
             let query = Query::parse(matches);
             let output_folder = OUTPUT_FOLDER_PATH.parse(matches);
@@ -6498,7 +6526,7 @@ pub mod args {
                     "The channel ID via which the token is received."
                 )))
                 .arg(REFUND.def().help(wrap!(
-                    "Generate the shielded transfer for refunding."
+                    "Generate the shielding transfer for refunding."
                 )))
         }
     }

crates/apps_lib/src/cli/client.rs

@@ -370,6 +370,20 @@ impl CliApi {
                         )
                         .await?;
                     }
+                    Sub::GenIbcShieldingTransfer(GenIbcShieldingTransfer(
+                        args,
+                    )) => {
+                        let chain_ctx = ctx.borrow_mut_chain_or_exit();
+                        let ledger_address =
+                            chain_ctx.get(&args.query.ledger_address);
+                        let client = client.unwrap_or_else(|| {
+                            C::from_tendermint_address(&ledger_address)
+                        });
+                        client.wait_until_node_is_synced(&io).await?;
+                        let args = args.to_sdk(&mut ctx)?;
+                        let namada = ctx.to_sdk(client, io);
+                        tx::gen_ibc_shielding_transfer(&namada, args).await?;
+                    }
                     #[cfg(feature = "namada-eth-bridge")]
                     Sub::AddToEthBridgePool(args) => {
                         let args = args.0;

crates/apps_lib/src/client/tx.rs

@@ -1,4 +1,5 @@
 use std::fs::File;
+use std::io::Write;
 
 use borsh::BorshDeserialize;
 use borsh_ext::BorshSerializeExt;
@@ -11,6 +12,7 @@ use namada::core::key::*;
 use namada::governance::cli::onchain::{
     DefaultProposal, PgfFundingProposal, PgfStewardProposal,
 };
+use namada::ibc::convert_masp_tx_to_ibc_memo;
 use namada::io::Io;
 use namada::state::EPOCH_SWITCH_BLOCKS_DELAY;
 use namada::tx::{CompressedAuthorization, Section, Signer, Tx};
@@ -1352,3 +1354,32 @@ pub async fn submit_tx(
 ) -> Result<TxResponse, error::Error> {
     tx::submit_tx(namada, to_broadcast).await
 }
+
+/// Generate MASP transaction and output it
+pub async fn gen_ibc_shielding_transfer(
+    context: &impl Namada,
+    args: args::GenIbcShieldingTransfer,
+) -> Result<(), error::Error> {
+    if let Some(masp_tx) =
+        tx::gen_ibc_shielding_transfer(context, args.clone()).await?
+    {
+        let tx_id = masp_tx.txid().to_string();
+        let filename = format!("ibc_masp_tx_{}.memo", tx_id);
+        let output_path = match &args.output_folder {
+            Some(path) => path.join(filename),
+            None => filename.into(),
+        };
+        let mut out = File::create(&output_path)
+            .expect("Creating a new file for IBC MASP transaction failed.");
+        let bytes = convert_masp_tx_to_ibc_memo(&masp_tx);
+        out.write_all(bytes.as_bytes())
+            .expect("Writing IBC MASP transaction file failed.");
+        println!(
+            "Output IBC shielding transfer for {tx_id} to {}",
+            output_path.to_string_lossy()
+        );
+    } else {
+        eprintln!("No shielded transfer for this IBC transfer.")
+    }
+    Ok(())
+}

crates/ibc/Cargo.toml

@@ -32,6 +32,7 @@ namada_storage = { path = "../storage" }
 namada_token = { path = "../token" }
 
 borsh.workspace = true
+data-encoding.workspace = true
 konst.workspace = true
 linkme = {workspace = true, optional = true}
 ibc.workspace = true

crates/ibc/src/lib.rs

@@ -71,6 +71,7 @@ use ibc::core::host::types::identifiers::{ChannelId, PortId, Sequence};
 use ibc::core::router::types::error::RouterError;
 use ibc::primitives::proto::Any;
 pub use ibc::*;
+use masp_primitives::transaction::Transaction as MaspTransaction;
 pub use msg::*;
 use namada_core::address::{self, Address};
 use namada_core::arith::checked;
@@ -151,7 +152,8 @@ where
     pub fn execute(
         &mut self,
         tx_data: &[u8],
-    ) -> Result<Option<ShieldingTransfer>, Error> {
+    ) -> Result<(Option<ShieldingTransfer>, Option<MaspTransaction>), Error>
+    {
         let message = decode_message(tx_data)?;
         match &message {
             IbcMessage::Transfer(msg) => {
@@ -166,7 +168,7 @@ where
                     msg.message.clone(),
                 )
                 .map_err(Error::TokenTransfer)?;
-                Ok(msg.transfer.clone())
+                Ok((msg.transfer.clone(), None))
             }
             IbcMessage::NftTransfer(msg) => {
                 let mut nft_transfer_ctx =
@@ -177,47 +179,45 @@ where
                     msg.message.clone(),
                 )
                 .map_err(Error::NftTransfer)?;
-                Ok(msg.transfer.clone())
-            }
-            IbcMessage::RecvPacket(msg) => {
-                let envelope =
-                    MsgEnvelope::Packet(PacketMsg::Recv(msg.message.clone()));
-                execute(&mut self.ctx, &mut self.router, envelope)
-                    .map_err(|e| Error::Context(Box::new(e)))?;
-                let transfer = if self.is_receiving_success(&msg.message)? {
-                    // For receiving the token to a shielded address
-                    msg.transfer.clone()
-                } else {
-                    None
-                };
-                Ok(transfer)
-            }
-            IbcMessage::AckPacket(msg) => {
-                let envelope =
-                    MsgEnvelope::Packet(PacketMsg::Ack(msg.message.clone()));
-                execute(&mut self.ctx, &mut self.router, envelope)
-                    .map_err(|e| Error::Context(Box::new(e)))?;
-                let transfer =
-                    if !is_ack_successful(&msg.message.acknowledgement)? {
-                        // For refunding the token to a shielded address
-                        msg.transfer.clone()
-                    } else {
-                        None
-                    };
-                Ok(transfer)
-            }
-            IbcMessage::Timeout(msg) => {
-                let envelope = MsgEnvelope::Packet(PacketMsg::Timeout(
-                    msg.message.clone(),
-                ));
-                execute(&mut self.ctx, &mut self.router, envelope)
-                    .map_err(|e| Error::Context(Box::new(e)))?;
-                Ok(msg.transfer.clone())
+                Ok((msg.transfer.clone(), None))
             }
             IbcMessage::Envelope(envelope) => {
-                execute(&mut self.ctx, &mut self.router, *envelope.clone())
+                execute(&mut self.ctx, &mut self.router, envelope.clone())
                     .map_err(|e| Error::Context(Box::new(e)))?;
-                Ok(None)
+                // 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,
+                        }
+                    }
+                    _ => None,
+                };
+                Ok((None, masp_tx))
             }
         }
     }
@@ -275,26 +275,8 @@ where
                 )
                 .map_err(Error::NftTransfer)
             }
-            IbcMessage::RecvPacket(msg) => validate(
-                &self.ctx,
-                &self.router,
-                MsgEnvelope::Packet(PacketMsg::Recv(msg.message)),
-            )
-            .map_err(|e| Error::Context(Box::new(e))),
-            IbcMessage::AckPacket(msg) => validate(
-                &self.ctx,
-                &self.router,
-                MsgEnvelope::Packet(PacketMsg::Ack(msg.message)),
-            )
-            .map_err(|e| Error::Context(Box::new(e))),
-            IbcMessage::Timeout(msg) => validate(
-                &self.ctx,
-                &self.router,
-                MsgEnvelope::Packet(PacketMsg::Timeout(msg.message)),
-            )
-            .map_err(|e| Error::Context(Box::new(e))),
             IbcMessage::Envelope(envelope) => {
-                validate(&self.ctx, &self.router, *envelope)
+                validate(&self.ctx, &self.router, envelope)
                     .map_err(|e| Error::Context(Box::new(e)))
             }
         }
@@ -326,7 +308,7 @@ pub fn decode_message(tx_data: &[u8]) -> Result<IbcMessage, Error> {
     // ibc-rs message
     if let Ok(any_msg) = Any::decode(tx_data) {
         if let Ok(envelope) = MsgEnvelope::try_from(any_msg) {
-            return Ok(IbcMessage::Envelope(Box::new(envelope)));
+            return Ok(IbcMessage::Envelope(envelope));
         }
     }
 
@@ -340,20 +322,6 @@ pub fn decode_message(tx_data: &[u8]) -> Result<IbcMessage, Error> {
         return Ok(IbcMessage::NftTransfer(msg));
     }
 
-    // Receiving packet message with `IbcShieldedTransfer`
-    if let Ok(msg) = MsgRecvPacket::try_from_slice(tx_data) {
-        return Ok(IbcMessage::RecvPacket(msg));
-    }
-
-    // Acknowledge packet message with `IbcShieldedTransfer`
-    if let Ok(msg) = MsgAcknowledgement::try_from_slice(tx_data) {
-        return Ok(IbcMessage::AckPacket(msg));
-    }
-    // Timeout packet message with `IbcShieldedTransfer`
-    if let Ok(msg) = MsgTimeout::try_from_slice(tx_data) {
-        return Ok(IbcMessage::Timeout(msg));
-    }
-
     Err(Error::DecodingData)
 }