From 8d34ed207fbcb7ceb41e51d7157003209b039b78 Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 20 Dec 2023 12:09:20 +0100 Subject: [PATCH 1/6] add MsgShieldedTransfer --- apps/src/lib/cli.rs | 4 +- apps/src/lib/client/tx.rs | 7 +- core/src/types/ibc.rs | 38 +++++- sdk/src/args.rs | 4 +- sdk/src/lib.rs | 2 +- sdk/src/tx.rs | 240 ++++++++++++++++++++++++++------------ 6 files changed, 211 insertions(+), 84 deletions(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index c04f5a531b..0d8909d70d 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -4025,7 +4025,7 @@ pub mod args { let chain_ctx = ctx.borrow_mut_chain_or_exit(); TxIbcTransfer:: { tx, - source: chain_ctx.get(&self.source), + source: chain_ctx.get_cached(&self.source), receiver: self.receiver, token: chain_ctx.get(&self.token), amount: self.amount, @@ -4042,7 +4042,7 @@ pub mod args { impl Args for TxIbcTransfer { fn parse(matches: &ArgMatches) -> Self { let tx = Tx::parse(matches); - let source = SOURCE.parse(matches); + let source = TRANSFER_SOURCE.parse(matches); let receiver = RECEIVER.parse(matches); let token = TOKEN.parse(matches); let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index 74ef2bab4c..ee098859f7 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -989,7 +989,12 @@ pub async fn submit_ibc_transfer( where ::Error: std::fmt::Display, { - submit_reveal_aux(namada, args.tx.clone(), &args.source).await?; + submit_reveal_aux( + namada, + args.tx.clone(), + &args.source.effective_address(), + ) + .await?; let (mut tx, signing_data, _epoch) = args.build(namada).await?; signing::generate_test_vector(namada, &tx).await?; diff --git a/core/src/types/ibc.rs b/core/src/types/ibc.rs index 2f04e11709..70fd372993 100644 --- a/core/src/types/ibc.rs +++ b/core/src/types/ibc.rs @@ -11,12 +11,15 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use super::address::HASH_LEN; +use crate::ibc::apps::transfer::types::msgs::transfer::MsgTransfer; use crate::ibc::apps::transfer::types::{Memo, PrefixedDenom, TracePath}; use crate::ibc::core::handler::types::events::{ Error as IbcEventError, IbcEvent as RawIbcEvent, }; +use crate::ibc::primitives::proto::Protobuf; use crate::tendermint::abci::Event as AbciEvent; use crate::types::masp::PaymentAddress; +use crate::types::token::Transfer; /// The event type defined in ibc-rs for receiving a token pub const EVENT_TYPE_PACKET: &str = "fungible_token_packet"; @@ -98,11 +101,44 @@ impl std::fmt::Display for IbcEvent { } } +/// IBC transfer message to send from a shielded address +#[derive(Debug, Clone)] +pub struct MsgShieldedTransfer { + /// IBC transfer message + pub message: MsgTransfer, + /// Token transfer with the masp section hash + pub transfer: Transfer, +} + +impl BorshSerialize for MsgShieldedTransfer { + fn serialize( + &self, + writer: &mut W, + ) -> std::io::Result<()> { + let encoded_msg = self.message.clone().encode_vec(); + let members = (encoded_msg, self.transfer.clone()); + BorshSerialize::serialize(&members, writer) + } +} + +impl BorshDeserialize for MsgShieldedTransfer { + fn deserialize_reader( + reader: &mut R, + ) -> std::io::Result { + use std::io::{Error, ErrorKind}; + let (msg, transfer): (Vec, Transfer) = + BorshDeserialize::deserialize_reader(reader)?; + let message = MsgTransfer::decode_vec(&msg) + .map_err(|err| Error::new(ErrorKind::InvalidData, err))?; + Ok(Self { message, transfer }) + } +} + /// IBC shielded transfer #[derive(Debug, Clone, BorshSerialize, BorshDeserialize)] pub struct IbcShieldedTransfer { /// The IBC event type - pub transfer: crate::types::token::Transfer, + pub transfer: Transfer, /// The attributes of the IBC event pub masp_tx: masp_primitives::transaction::Transaction, } diff --git a/sdk/src/args.rs b/sdk/src/args.rs index 6599b0e9fb..0947788f34 100644 --- a/sdk/src/args.rs +++ b/sdk/src/args.rs @@ -296,7 +296,7 @@ pub struct TxIbcTransfer { /// Common tx arguments pub tx: Tx, /// Transfer source address - pub source: C::Address, + pub source: C::TransferSource, /// Transfer target address pub receiver: String, /// Transferred token address @@ -331,7 +331,7 @@ impl TxBuilder for TxIbcTransfer { impl TxIbcTransfer { /// Transfer source address - pub fn source(self, source: C::Address) -> Self { + pub fn source(self, source: C::TransferSource) -> Self { Self { source, ..self } } diff --git a/sdk/src/lib.rs b/sdk/src/lib.rs index 29aab3041a..8ad9f5752f 100644 --- a/sdk/src/lib.rs +++ b/sdk/src/lib.rs @@ -245,7 +245,7 @@ pub trait Namada: Sized + MaybeSync + MaybeSend { /// Make a TxIbcTransfer builder from the given minimum set of arguments fn new_ibc_transfer( &self, - source: Address, + source: TransferSource, receiver: String, token: Address, amount: InputAmount, diff --git a/sdk/src/tx.rs b/sdk/src/tx.rs index d99a9c6f44..0035c55eea 100644 --- a/sdk/src/tx.rs +++ b/sdk/src/tx.rs @@ -6,6 +6,7 @@ use std::path::{Path, PathBuf}; use std::time::Duration; use borsh::BorshSerialize; +use borsh_ext::BorshSerializeExt; use masp_primitives::asset_type::AssetType; use masp_primitives::transaction::builder; use masp_primitives::transaction::builder::Builder; @@ -34,7 +35,7 @@ use namada_core::ledger::pgf::cli::steward::Commission; use namada_core::types::address::{Address, InternalAddress, MASP}; use namada_core::types::dec::Dec; use namada_core::types::hash::Hash; -use namada_core::types::ibc::IbcShieldedTransfer; +use namada_core::types::ibc::{IbcShieldedTransfer, MsgShieldedTransfer}; use namada_core::types::key::*; use namada_core::types::masp::{TransferSource, TransferTarget}; use namada_core::types::storage::Epoch; @@ -1924,18 +1925,17 @@ pub async fn build_ibc_transfer( context: &impl Namada, args: &args::TxIbcTransfer, ) -> Result<(Tx, SigningTxData, Option)> { - let default_signer = Some(args.source.clone()); + let source = args.source.effective_address(); let signing_data = signing::aux_signing_data( context, &args.tx, - Some(args.source.clone()), - default_signer, + Some(source.clone()), + Some(source.clone()), ) .await?; // Check that the source address exists on chain let source = - source_exists_or_err(args.source.clone(), args.tx.force, context) - .await?; + source_exists_or_err(source.clone(), args.tx.force, context).await?; // We cannot check the receiver // validate the amount given @@ -1973,6 +1973,19 @@ pub async fn build_ibc_transfer( .await .map_err(|e| Error::from(QueryError::Wasm(e.to_string())))?; + // For transfer from a spending key + let shielded_parts = construct_shielded_part( + context, + &args.source, + // Replace the target address as a MASP address because the target + // address could be a payment address, but the address isn't that + // of this chain. + &TransferTarget::Address(MASP), + &args.token, + validated_amount, + ) + .await?; + let ibc_denom = rpc::query_ibc_denom(context, &args.token, Some(&source)).await; let token = PrefixedCoin { @@ -2014,7 +2027,7 @@ pub async fn build_ibc_transfer( IbcTimestamp::none() }; - let msg = MsgTransfer { + let message = MsgTransfer { port_id_on_a: args.port_id.clone(), chan_id_on_a: args.channel_id.clone(), packet_data, @@ -2022,13 +2035,41 @@ pub async fn build_ibc_transfer( timeout_timestamp_on_b: timeout_timestamp, }; - let any_msg = msg.to_any(); - let mut data = vec![]; - prost::Message::encode(&any_msg, &mut data) - .map_err(TxError::EncodeFailure)?; - let chain_id = args.tx.chain_id.clone().unwrap(); let mut tx = Tx::new(chain_id, args.tx.expiration); + + let data = match shielded_parts { + Some((shielded_transfer, asset_types)) => { + let masp_tx_hash = + tx.add_masp_tx_section(shielded_transfer.masp_tx).1; + let transfer = token::Transfer { + source: source.clone(), + target: MASP, + token: args.token.clone(), + amount: validated_amount, + // The address could be a payment address, but the address isn't + // that of this chain. + key: None, + // Link the Transfer to the MASP Transaction by hash code + shielded: Some(masp_tx_hash), + }; + tx.add_masp_builder(MaspBuilder { + asset_types, + metadata: shielded_transfer.metadata, + builder: shielded_transfer.builder, + target: masp_tx_hash, + }); + MsgShieldedTransfer { message, transfer }.serialize_to_vec() + } + None => { + let any_msg = message.to_any(); + let mut data = vec![]; + prost::Message::encode(&any_msg, &mut data) + .map_err(TxError::EncodeFailure)?; + data + } + }; + tx.add_code_from_hash( tx_code_hash, Some(args.tx_code_path.to_string_lossy().into_owned()), @@ -2234,42 +2275,15 @@ pub async fn build_transfer( _ => None, }; - // Construct the shielded part of the transaction, if any - let stx_result = - ShieldedContext::::gen_shielded_transfer( - context, - &args.source, - &args.target, - &args.token, - validated_amount, - ) - .await; - - let shielded_parts = match stx_result { - Ok(stx) => Ok(stx), - Err(Build(builder::Error::InsufficientFunds(_))) => { - Err(TxError::NegativeBalanceAfterTransfer( - Box::new(source.clone()), - validated_amount.amount.to_string_native(), - Box::new(token.clone()), - )) - } - Err(err) => Err(TxError::MaspError(err.to_string())), - }?; - - let shielded_tx_epoch = shielded_parts.clone().map(|trans| trans.epoch); - - let asset_types = match shielded_parts.clone() { - None => None, - Some(transfer) => { - // Get the decoded asset types used in the transaction to give - // offline wallet users more information - let asset_types = used_asset_types(context, &transfer.builder) - .await - .unwrap_or_default(); - Some(asset_types) - } - }; + // if this transfer is shielded, `on_tx` adds the shielded part to the tx + let (on_tx, shielded_tx_epoch) = get_shielded_transfer_appender( + context, + &args.source, + &args.target, + &args.token, + validated_amount, + ) + .await?; // Construct the corresponding transparent Transfer object let transfer = token::Transfer { @@ -2282,40 +2296,12 @@ pub async fn build_transfer( shielded: None, }; - let add_shielded = |tx: &mut Tx, transfer: &mut token::Transfer| { - // Add the MASP Transaction and its Builder to facilitate validation - if let Some(ShieldedTransfer { - builder, - masp_tx, - metadata, - epoch: _, - }) = shielded_parts - { - // Add a MASP Transaction section to the Tx and get the tx hash - let masp_tx_hash = tx.add_masp_tx_section(masp_tx).1; - transfer.shielded = Some(masp_tx_hash); - - tracing::debug!("Transfer data {:?}", transfer); - - tx.add_masp_builder(MaspBuilder { - // Is safe - asset_types: asset_types.unwrap(), - // Store how the Info objects map to Descriptors/Outputs - metadata, - // Store the data that was used to construct the Transaction - builder, - // Link the Builder to the Transaction by hash code - target: masp_tx_hash, - }); - }; - Ok(()) - }; let (tx, unshielding_epoch) = build_pow_flag( context, &args.tx, args.tx_code_path.clone(), transfer, - add_shielded, + on_tx, &signing_data.fee_payer, tx_source_balance, ) @@ -2342,6 +2328,106 @@ pub async fn build_transfer( Ok((tx, signing_data, masp_epoch)) } +// Returns a function to add the shielded parts and the shielded epoch if this +// transfer is shielded. Otherwise, it returns the function is `do_nothing` and +// None as the shielded epoch +async fn get_shielded_transfer_appender( + context: &N, + source: &TransferSource, + target: &TransferTarget, + token: &Address, + amount: token::DenominatedAmount, +) -> Result<( + Box Result<()>>, + Option, +)> { + match construct_shielded_part(context, &source, &target, &token, amount) + .await? + { + Some((shielded_parts, asset_types)) => { + let epoch = shielded_parts.epoch; + Ok(( + add_shielded_transfer(shielded_parts, asset_types), + Some(epoch), + )) + } + None => Ok(( + Box::new(do_nothing as fn(&mut Tx, &mut _) -> Result<()>) + as Box Result<()>>, + None, + )), + } +} + +// Construct the shielded part of the transaction, if any +async fn construct_shielded_part( + context: &N, + source: &TransferSource, + target: &TransferTarget, + token: &Address, + amount: token::DenominatedAmount, +) -> Result)>> { + let stx_result = + ShieldedContext::::gen_shielded_transfer( + context, source, target, token, amount, + ) + .await; + + let shielded_parts = match stx_result { + Ok(Some(stx)) => stx, + Ok(None) => return Ok(None), + Err(Build(builder::Error::InsufficientFunds(_))) => { + return Err(TxError::NegativeBalanceAfterTransfer( + Box::new(source.effective_address()), + amount.amount.to_string_native(), + Box::new(token.clone()), + ) + .into()); + } + Err(err) => return Err(TxError::MaspError(err.to_string()).into()), + }; + + // Get the decoded asset types used in the transaction to give offline + // wallet users more information + let asset_types = used_asset_types(context, &shielded_parts.builder) + .await + .unwrap_or_default(); + + Ok(Some((shielded_parts, asset_types))) +} + +/// Add the MASP Transaction and its Builder to facilitate validation +fn add_shielded_transfer( + shielded_parts: ShieldedTransfer, + asset_types: HashSet<(Address, MaspDenom, Epoch)>, +) -> Box Result<()>> { + Box::new(|tx: &mut Tx, transfer: &mut token::Transfer| { + let ShieldedTransfer { + builder, + masp_tx, + metadata, + epoch: _, + } = shielded_parts; + // Add a MASP Transaction section to the Tx and get the tx hash + let masp_tx_hash = tx.add_masp_tx_section(masp_tx).1; + transfer.shielded = Some(masp_tx_hash); + + tracing::debug!("Transfer data {:?}", transfer); + + tx.add_masp_builder(MaspBuilder { + // Is safe + asset_types, + // Store how the Info objects map to Descriptors/Outputs + metadata, + // Store the data that was used to construct the Transaction + builder, + // Link the Builder to the Transaction by hash code + target: masp_tx_hash, + }); + Ok(()) + }) +} + /// Submit a transaction to initialize an account pub async fn build_init_account( context: &impl Namada, From 26c49e16999b9b92c53668dd48f799dd05f51226 Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 20 Dec 2023 15:00:37 +0100 Subject: [PATCH 2/6] decode MsgShieldedTransfer --- core/src/ledger/ibc/mod.rs | 90 ++++++++++++++++++++++++++++---------- core/src/ledger/vp_env.rs | 16 +++++-- core/src/types/ibc.rs | 13 +++--- sdk/src/tx.rs | 12 ++++- 4 files changed, 97 insertions(+), 34 deletions(-) diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 1b01017d3f..ed972cb688 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -8,6 +8,7 @@ use std::fmt::Debug; use std::rc::Rc; use std::str::FromStr; +use borsh::BorshDeserialize; pub use context::common::IbcCommonContext; use context::router::IbcRouter; pub use context::storage::{IbcStorageContext, ProofSpec}; @@ -37,16 +38,16 @@ use crate::ibc::core::router::types::module::ModuleId; use crate::ibc::primitives::proto::Any; use crate::types::address::{Address, MASP}; use crate::types::ibc::{ - get_shielded_transfer, is_ibc_denom, EVENT_TYPE_DENOM_TRACE, - EVENT_TYPE_PACKET, + get_shielded_transfer, is_ibc_denom, MsgShieldedTransfer, + EVENT_TYPE_DENOM_TRACE, EVENT_TYPE_PACKET, }; use crate::types::masp::PaymentAddress; #[allow(missing_docs)] #[derive(Error, Debug)] pub enum Error { - #[error("Decoding IBC data error: {0}")] - DecodingData(prost::DecodeError), + #[error("Decoding IBC data error")] + DecodingData, #[error("Decoding message error: {0}")] DecodingMessage(RouterError), #[error("IBC context error: {0}")] @@ -99,28 +100,37 @@ where /// Execute according to the message in an IBC transaction or VP pub fn execute(&mut self, tx_data: &[u8]) -> Result<(), Error> { - let any_msg = Any::decode(tx_data).map_err(Error::DecodingData)?; - match MsgTransfer::try_from(any_msg.clone()) { - Ok(msg) => { + let message = decode_message(tx_data)?; + match &message { + IbcMessage::Transfer(msg) => { let mut token_transfer_ctx = TokenTransferContext::new(self.ctx.inner.clone()); send_transfer_execute( &mut self.ctx, &mut token_transfer_ctx, - msg, + msg.clone(), ) .map_err(Error::TokenTransfer) } - Err(_) => { - let envelope = MsgEnvelope::try_from(any_msg) - .map_err(Error::DecodingMessage)?; + IbcMessage::ShieldedTransfer(msg) => { + let mut token_transfer_ctx = + TokenTransferContext::new(self.ctx.inner.clone()); + send_transfer_execute( + &mut self.ctx, + &mut token_transfer_ctx, + msg.message.clone(), + ) + .map_err(Error::TokenTransfer)?; + self.handle_masp_tx(message) + } + IbcMessage::Envelope(envelope) => { execute(&mut self.ctx, &mut self.router, envelope.clone()) .map_err(|e| Error::Context(Box::new(e)))?; - // For receiving the token to a shielded address - self.handle_masp_tx(&envelope)?; // the current ibc-rs execution doesn't store the denom for the // token hash when transfer with MsgRecvPacket - self.store_denom(&envelope) + self.store_denom(&envelope)?; + // For receiving the token to a shielded address + self.handle_masp_tx(message) } } } @@ -218,17 +228,25 @@ where /// Validate according to the message in IBC VP pub fn validate(&self, tx_data: &[u8]) -> Result<(), Error> { - let any_msg = Any::decode(tx_data).map_err(Error::DecodingData)?; - match MsgTransfer::try_from(any_msg.clone()) { - Ok(msg) => { + let message = decode_message(tx_data)?; + match message { + IbcMessage::Transfer(msg) => { let token_transfer_ctx = TokenTransferContext::new(self.ctx.inner.clone()); send_transfer_validate(&self.ctx, &token_transfer_ctx, msg) .map_err(Error::TokenTransfer) } - Err(_) => { - let envelope = MsgEnvelope::try_from(any_msg) - .map_err(Error::DecodingMessage)?; + IbcMessage::ShieldedTransfer(msg) => { + let token_transfer_ctx = + TokenTransferContext::new(self.ctx.inner.clone()); + send_transfer_validate( + &self.ctx, + &token_transfer_ctx, + msg.message, + ) + .map_err(Error::TokenTransfer) + } + IbcMessage::Envelope(envelope) => { validate(&self.ctx, &self.router, envelope) .map_err(|e| Error::Context(Box::new(e))) } @@ -236,9 +254,9 @@ where } /// Handle the MASP transaction if needed - fn handle_masp_tx(&mut self, envelope: &MsgEnvelope) -> Result<(), Error> { - let shielded_transfer = match envelope { - MsgEnvelope::Packet(PacketMsg::Recv(_)) => { + fn handle_masp_tx(&mut self, message: IbcMessage) -> Result<(), Error> { + let shielded_transfer = match message { + IbcMessage::Envelope(MsgEnvelope::Packet(PacketMsg::Recv(_))) => { let event = self .ctx .inner @@ -257,6 +275,7 @@ where None => return Ok(()), } } + IbcMessage::ShieldedTransfer(msg) => Some(msg.shielded_transfer), _ => return Ok(()), }; if let Some(shielded_transfer) = shielded_transfer { @@ -272,6 +291,31 @@ where } } +enum IbcMessage { + Envelope(MsgEnvelope), + Transfer(MsgTransfer), + ShieldedTransfer(MsgShieldedTransfer), +} + +fn decode_message(tx_data: &[u8]) -> Result { + // ibc-rs message + if let Ok(any_msg) = Any::decode(tx_data) { + if let Ok(transfer_msg) = MsgTransfer::try_from(any_msg.clone()) { + return Ok(IbcMessage::Transfer(transfer_msg)); + } + if let Ok(envelope) = MsgEnvelope::try_from(any_msg) { + return Ok(IbcMessage::Envelope(envelope)); + } + } + + // Message with Transfer for the shielded transfer + if let Ok(msg) = MsgShieldedTransfer::try_from_slice(tx_data) { + return Ok(IbcMessage::ShieldedTransfer(msg)); + } + + Err(Error::DecodingData) +} + /// Get the IbcToken from the source/destination ports and channels pub fn received_ibc_token( ibc_denom: &PrefixedDenom, diff --git a/core/src/ledger/vp_env.rs b/core/src/ledger/vp_env.rs index 664e030786..728e66d9a6 100644 --- a/core/src/ledger/vp_env.rs +++ b/core/src/ledger/vp_env.rs @@ -8,7 +8,9 @@ use super::storage_api::{self, OptionExt, ResultExt, StorageRead}; use crate::proto::Tx; use crate::types::address::Address; use crate::types::hash::Hash; -use crate::types::ibc::{get_shielded_transfer, IbcEvent, EVENT_TYPE_PACKET}; +use crate::types::ibc::{ + get_shielded_transfer, IbcEvent, MsgShieldedTransfer, EVENT_TYPE_PACKET, +}; use crate::types::storage::{ BlockHash, BlockHeight, Epoch, Header, Key, TxIndex, }; @@ -112,9 +114,8 @@ where tx_data: &Tx, ) -> Result<(Transfer, Transaction), storage_api::Error> { let signed = tx_data; - if let Ok(transfer) = - Transfer::try_from_slice(&signed.data().unwrap()[..]) - { + let data = signed.data().ok_or_err_msg("No transaction data")?; + if let Ok(transfer) = Transfer::try_from_slice(&data) { let shielded_hash = transfer .shielded .ok_or_err_msg("unable to find shielded hash")?; @@ -125,6 +126,13 @@ where return Ok((transfer, masp_tx)); } + if let Ok(message) = MsgShieldedTransfer::try_from_slice(&data) { + return Ok(( + message.shielded_transfer.transfer, + message.shielded_transfer.masp_tx, + )); + } + // Shielded transfer over IBC let events = self.get_ibc_events(EVENT_TYPE_PACKET.to_string())?; // The receiving event should be only one in the single IBC transaction diff --git a/core/src/types/ibc.rs b/core/src/types/ibc.rs index 70fd372993..a66a84fae0 100644 --- a/core/src/types/ibc.rs +++ b/core/src/types/ibc.rs @@ -106,8 +106,8 @@ impl std::fmt::Display for IbcEvent { pub struct MsgShieldedTransfer { /// IBC transfer message pub message: MsgTransfer, - /// Token transfer with the masp section hash - pub transfer: Transfer, + /// MASP tx with token transfer + pub shielded_transfer: IbcShieldedTransfer, } impl BorshSerialize for MsgShieldedTransfer { @@ -116,7 +116,7 @@ impl BorshSerialize for MsgShieldedTransfer { writer: &mut W, ) -> std::io::Result<()> { let encoded_msg = self.message.clone().encode_vec(); - let members = (encoded_msg, self.transfer.clone()); + let members = (encoded_msg, self.shielded_transfer.clone()); BorshSerialize::serialize(&members, writer) } } @@ -126,11 +126,14 @@ impl BorshDeserialize for MsgShieldedTransfer { reader: &mut R, ) -> std::io::Result { use std::io::{Error, ErrorKind}; - let (msg, transfer): (Vec, Transfer) = + let (msg, shielded_transfer): (Vec, IbcShieldedTransfer) = BorshDeserialize::deserialize_reader(reader)?; let message = MsgTransfer::decode_vec(&msg) .map_err(|err| Error::new(ErrorKind::InvalidData, err))?; - Ok(Self { message, transfer }) + Ok(Self { + message, + shielded_transfer, + }) } } diff --git a/sdk/src/tx.rs b/sdk/src/tx.rs index 0035c55eea..758b6648b9 100644 --- a/sdk/src/tx.rs +++ b/sdk/src/tx.rs @@ -2041,7 +2041,7 @@ pub async fn build_ibc_transfer( let data = match shielded_parts { Some((shielded_transfer, asset_types)) => { let masp_tx_hash = - tx.add_masp_tx_section(shielded_transfer.masp_tx).1; + tx.add_masp_tx_section(shielded_transfer.masp_tx.clone()).1; let transfer = token::Transfer { source: source.clone(), target: MASP, @@ -2059,7 +2059,15 @@ pub async fn build_ibc_transfer( builder: shielded_transfer.builder, target: masp_tx_hash, }); - MsgShieldedTransfer { message, transfer }.serialize_to_vec() + let shielded_transfer = IbcShieldedTransfer { + transfer, + masp_tx: shielded_transfer.masp_tx, + }; + MsgShieldedTransfer { + message, + shielded_transfer, + } + .serialize_to_vec() } None => { let any_msg = message.to_any(); From ced7545790ac497c9d384cb6d49a1e830d8987fa Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 20 Dec 2023 21:20:11 +0100 Subject: [PATCH 3/6] fix target addr for shielded_parts --- core/src/ledger/ibc/mod.rs | 2 +- sdk/src/tx.rs | 109 +++++++++++++------------------------ tests/src/e2e/ibc_tests.rs | 33 ++++++----- 3 files changed, 58 insertions(+), 86 deletions(-) diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index ed972cb688..936d8641e0 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -128,7 +128,7 @@ where .map_err(|e| Error::Context(Box::new(e)))?; // the current ibc-rs execution doesn't store the denom for the // token hash when transfer with MsgRecvPacket - self.store_denom(&envelope)?; + self.store_denom(envelope)?; // For receiving the token to a shielded address self.handle_masp_tx(message) } diff --git a/sdk/src/tx.rs b/sdk/src/tx.rs index 758b6648b9..d664bb302e 100644 --- a/sdk/src/tx.rs +++ b/sdk/src/tx.rs @@ -1977,10 +1977,8 @@ pub async fn build_ibc_transfer( let shielded_parts = construct_shielded_part( context, &args.source, - // Replace the target address as a MASP address because the target - // address could be a payment address, but the address isn't that - // of this chain. - &TransferTarget::Address(MASP), + // The token will be escrowed to IBC address + &TransferTarget::Address(Address::Internal(InternalAddress::Ibc)), &args.token, validated_amount, ) @@ -2044,7 +2042,8 @@ pub async fn build_ibc_transfer( tx.add_masp_tx_section(shielded_transfer.masp_tx.clone()).1; let transfer = token::Transfer { source: source.clone(), - target: MASP, + // The token will be escrowed to IBC address + target: Address::Internal(InternalAddress::Ibc), token: args.token.clone(), amount: validated_amount, // The address could be a payment address, but the address isn't @@ -2283,8 +2282,7 @@ pub async fn build_transfer( _ => None, }; - // if this transfer is shielded, `on_tx` adds the shielded part to the tx - let (on_tx, shielded_tx_epoch) = get_shielded_transfer_appender( + let shielded_parts = construct_shielded_part( context, &args.source, &args.target, @@ -2292,6 +2290,7 @@ pub async fn build_transfer( validated_amount, ) .await?; + let shielded_tx_epoch = shielded_parts.as_ref().map(|trans| trans.0.epoch); // Construct the corresponding transparent Transfer object let transfer = token::Transfer { @@ -2304,12 +2303,43 @@ pub async fn build_transfer( shielded: None, }; + let add_shielded = |tx: &mut Tx, transfer: &mut token::Transfer| { + // Add the MASP Transaction and its Builder to facilitate validation + if let Some(( + ShieldedTransfer { + builder, + masp_tx, + metadata, + epoch: _, + }, + asset_types, + )) = shielded_parts + { + // Add a MASP Transaction section to the Tx and get the tx hash + let masp_tx_hash = tx.add_masp_tx_section(masp_tx).1; + transfer.shielded = Some(masp_tx_hash); + + tracing::debug!("Transfer data {:?}", transfer); + + tx.add_masp_builder(MaspBuilder { + asset_types, + // Store how the Info objects map to Descriptors/Outputs + metadata, + // Store the data that was used to construct the Transaction + builder, + // Link the Builder to the Transaction by hash code + target: masp_tx_hash, + }); + }; + Ok(()) + }; + let (tx, unshielding_epoch) = build_pow_flag( context, &args.tx, args.tx_code_path.clone(), transfer, - on_tx, + add_shielded, &signing_data.fee_payer, tx_source_balance, ) @@ -2336,37 +2366,6 @@ pub async fn build_transfer( Ok((tx, signing_data, masp_epoch)) } -// Returns a function to add the shielded parts and the shielded epoch if this -// transfer is shielded. Otherwise, it returns the function is `do_nothing` and -// None as the shielded epoch -async fn get_shielded_transfer_appender( - context: &N, - source: &TransferSource, - target: &TransferTarget, - token: &Address, - amount: token::DenominatedAmount, -) -> Result<( - Box Result<()>>, - Option, -)> { - match construct_shielded_part(context, &source, &target, &token, amount) - .await? - { - Some((shielded_parts, asset_types)) => { - let epoch = shielded_parts.epoch; - Ok(( - add_shielded_transfer(shielded_parts, asset_types), - Some(epoch), - )) - } - None => Ok(( - Box::new(do_nothing as fn(&mut Tx, &mut _) -> Result<()>) - as Box Result<()>>, - None, - )), - } -} - // Construct the shielded part of the transaction, if any async fn construct_shielded_part( context: &N, @@ -2404,38 +2403,6 @@ async fn construct_shielded_part( Ok(Some((shielded_parts, asset_types))) } -/// Add the MASP Transaction and its Builder to facilitate validation -fn add_shielded_transfer( - shielded_parts: ShieldedTransfer, - asset_types: HashSet<(Address, MaspDenom, Epoch)>, -) -> Box Result<()>> { - Box::new(|tx: &mut Tx, transfer: &mut token::Transfer| { - let ShieldedTransfer { - builder, - masp_tx, - metadata, - epoch: _, - } = shielded_parts; - // Add a MASP Transaction section to the Tx and get the tx hash - let masp_tx_hash = tx.add_masp_tx_section(masp_tx).1; - transfer.shielded = Some(masp_tx_hash); - - tracing::debug!("Transfer data {:?}", transfer); - - tx.add_masp_builder(MaspBuilder { - // Is safe - asset_types, - // Store how the Info objects map to Descriptors/Outputs - metadata, - // Store the data that was used to construct the Transaction - builder, - // Link the Builder to the Transaction by hash code - target: masp_tx_hash, - }); - Ok(()) - }) -} - /// Submit a transaction to initialize an account pub async fn build_init_account( context: &impl Namada, diff --git a/tests/src/e2e/ibc_tests.rs b/tests/src/e2e/ibc_tests.rs index d69d3f1844..3806c8d362 100644 --- a/tests/src/e2e/ibc_tests.rs +++ b/tests/src/e2e/ibc_tests.rs @@ -167,7 +167,8 @@ fn run_ledger_ibc() -> Result<()> { try_invalid_transfers(&test_a, &test_b, &port_id_a, &channel_id_a)?; // Transfer 50000 received over IBC on Chain B - transfer_received_token(&port_id_b, &channel_id_b, &test_b)?; + let token = format!("{port_id_b}/{channel_id_b}/nam"); + transfer_on_chain(&test_b, BERTHA, ALBERT, token, 50000, BERTHA_KEY)?; check_balances_after_non_ibc(&port_id_b, &channel_id_b, &test_b)?; // Transfer 50000 back from the origin-specific account on Chain B to Chain @@ -862,26 +863,27 @@ fn try_invalid_transfers( Ok(()) } -fn transfer_received_token( - port_id: &PortId, - channel_id: &ChannelId, +fn transfer_on_chain( test: &Test, + sender: impl AsRef, + receiver: impl AsRef, + token: impl AsRef, + amount: u64, + signer: impl AsRef, ) -> Result<()> { let rpc = get_actor_rpc(test, &Who::Validator(0)); - let ibc_denom = format!("{port_id}/{channel_id}/nam"); - let amount = Amount::native_whole(50000).to_string_native(); let tx_args = [ "transfer", "--source", - BERTHA, + sender.as_ref(), "--target", - ALBERT, + receiver.as_ref(), "--token", - &ibc_denom, + token.as_ref(), "--amount", - &amount, - "--gas-token", - NAM, + &amount.to_string(), + "--signing-keys", + signer.as_ref(), "--node", &rpc, ]; @@ -1046,11 +1048,14 @@ fn shielded_transfer( let file_path = get_shielded_transfer_path(&mut client)?; client.assert_success(); - // Send a token from Chain A to PA(B) on Chain B + // Send a token to the shielded address on Chain A + transfer_on_chain(test_a, ALBERT, AA_PAYMENT_ADDRESS, BTC, 10, ALBERT_KEY)?; + + // Send a token from SP(A) on Chain A to PA(B) on Chain B let amount = Amount::native_whole(10).to_string_native(); let height = transfer( test_a, - ALBERT, + A_SPENDING_KEY, AB_PAYMENT_ADDRESS, BTC, amount, From 9e092d27751cff63a4b06cbcaf2e70f080c117c1 Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 20 Dec 2023 21:33:59 +0100 Subject: [PATCH 4/6] add changelog --- .../unreleased/features/2321-ibc_shielded_transfer.md | 2 ++ sdk/src/tx.rs | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) create mode 100644 .changelog/unreleased/features/2321-ibc_shielded_transfer.md diff --git a/.changelog/unreleased/features/2321-ibc_shielded_transfer.md b/.changelog/unreleased/features/2321-ibc_shielded_transfer.md new file mode 100644 index 0000000000..1d6de75a7a --- /dev/null +++ b/.changelog/unreleased/features/2321-ibc_shielded_transfer.md @@ -0,0 +1,2 @@ +- IBC transfer from a spending key + ([\#2321](https://github.com/anoma/namada/issues/2321)) \ No newline at end of file diff --git a/sdk/src/tx.rs b/sdk/src/tx.rs index d664bb302e..70ca41a144 100644 --- a/sdk/src/tx.rs +++ b/sdk/src/tx.rs @@ -1974,7 +1974,7 @@ pub async fn build_ibc_transfer( .map_err(|e| Error::from(QueryError::Wasm(e.to_string())))?; // For transfer from a spending key - let shielded_parts = construct_shielded_part( + let shielded_parts = construct_shielded_parts( context, &args.source, // The token will be escrowed to IBC address @@ -2282,7 +2282,7 @@ pub async fn build_transfer( _ => None, }; - let shielded_parts = construct_shielded_part( + let shielded_parts = construct_shielded_parts( context, &args.source, &args.target, @@ -2367,7 +2367,7 @@ pub async fn build_transfer( } // Construct the shielded part of the transaction, if any -async fn construct_shielded_part( +async fn construct_shielded_parts( context: &N, source: &TransferSource, target: &TransferTarget, From a3927cd08c0748d3eed65d0640265c5049627d28 Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 20 Dec 2023 23:07:43 +0100 Subject: [PATCH 5/6] check epoch --- sdk/src/tx.rs | 54 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/sdk/src/tx.rs b/sdk/src/tx.rs index 70ca41a144..084a7b16b5 100644 --- a/sdk/src/tx.rs +++ b/sdk/src/tx.rs @@ -1983,6 +1983,7 @@ pub async fn build_ibc_transfer( validated_amount, ) .await?; + let shielded_tx_epoch = shielded_parts.as_ref().map(|trans| trans.0.epoch); let ibc_denom = rpc::query_ibc_denom(context, &args.token, Some(&source)).await; @@ -2083,7 +2084,7 @@ pub async fn build_ibc_transfer( ) .add_serialized_data(data); - let epoch = prepare_tx( + let unshielding_epoch = prepare_tx( context, &args.tx, &mut tx, @@ -2092,6 +2093,9 @@ pub async fn build_ibc_transfer( ) .await?; + let epoch = + check_epochs(unshielding_epoch, shielded_tx_epoch, args.tx.force)?; + Ok((tx, signing_data, epoch)) } @@ -2344,25 +2348,10 @@ pub async fn build_transfer( tx_source_balance, ) .await?; - // Manage the two masp epochs - let masp_epoch = match (unshielding_epoch, shielded_tx_epoch) { - (Some(fee_unshield_epoch), Some(transfer_unshield_epoch)) => { - // If the two masp epochs are different, either the wrapper or the - // inner tx will fail, so abort tx creation - if fee_unshield_epoch != transfer_unshield_epoch && !args.tx.force { - return Err(Error::Other( - "Fee unshielding masp tx and inner tx masp transaction \ - were crafted on an epoch boundary" - .to_string(), - )); - } - // Take the smaller of the two epochs - Some(fee_unshield_epoch.min(transfer_unshield_epoch)) - } - (Some(_fee_unshielding_epoch), None) => unshielding_epoch, - (None, Some(_transfer_unshield_epoch)) => shielded_tx_epoch, - (None, None) => None, - }; + + let masp_epoch = + check_epochs(unshielding_epoch, shielded_tx_epoch, args.tx.force)?; + Ok((tx, signing_data, masp_epoch)) } @@ -2403,6 +2392,31 @@ async fn construct_shielded_parts( Ok(Some((shielded_parts, asset_types))) } +fn check_epochs( + unshielding_epoch: Option, + shielded_tx_epoch: Option, + force: bool, +) -> Result> { + match (unshielding_epoch, shielded_tx_epoch) { + (Some(fee_unshield_epoch), Some(transfer_unshield_epoch)) => { + // If the two masp epochs are different, either the wrapper or the + // inner tx will fail, so abort tx creation + if fee_unshield_epoch != transfer_unshield_epoch && !force { + return Err(Error::Other( + "Fee unshielding masp tx and inner tx masp transaction \ + were crafted on an epoch boundary" + .to_string(), + )); + } + // Take the smaller of the two epochs + Ok(Some(fee_unshield_epoch.min(transfer_unshield_epoch))) + } + (Some(_fee_unshielding_epoch), None) => Ok(unshielding_epoch), + (None, Some(_transfer_unshield_epoch)) => Ok(shielded_tx_epoch), + (None, None) => Ok(None), + } +} + /// Submit a transaction to initialize an account pub async fn build_init_account( context: &impl Namada, From 57f94e94ea61beaa424920d8c70f674e710abbc3 Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 27 Dec 2023 16:22:05 +0100 Subject: [PATCH 6/6] add SDK changelog --- .changelog/unreleased/SDK/2321-ibc_shielded_transfer.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/SDK/2321-ibc_shielded_transfer.md diff --git a/.changelog/unreleased/SDK/2321-ibc_shielded_transfer.md b/.changelog/unreleased/SDK/2321-ibc_shielded_transfer.md new file mode 100644 index 0000000000..a90b2c4075 --- /dev/null +++ b/.changelog/unreleased/SDK/2321-ibc_shielded_transfer.md @@ -0,0 +1,2 @@ +- ibc-transfer can set a spending key to the source + ([\#2321](https://github.com/anoma/namada/issues/2321)) \ No newline at end of file