From 4cf16cb4b6d5a5b6d27573333f4f8f45eb60e71d Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 20 Sep 2023 00:23:53 +0200 Subject: [PATCH 01/10] WIP: receive to a shielded address --- core/src/ledger/ibc/context/storage.rs | 8 +- core/src/ledger/ibc/mod.rs | 43 +++- core/src/ledger/vp_env.rs | 50 +++- core/src/types/address.rs | 10 +- core/src/types/ibc.rs | 69 ++++- shared/src/ledger/native_vp/ibc/context.rs | 69 ++++- shared/src/ledger/native_vp/mod.rs | 13 + shared/src/ledger/vp_host_fns.rs | 15 ++ shared/src/vm/host_env.rs | 116 ++++++++- shared/src/vm/wasm/host_env.rs | 1 + tx_prelude/src/ibc.rs | 13 +- tx_prelude/src/token.rs | 78 +++--- vm_env/src/lib.rs | 6 + vp_prelude/src/lib.rs | 14 + wasm/wasm_source/src/tx_bridge_pool.rs | 9 - wasm/wasm_source/src/tx_transfer.rs | 34 ++- wasm/wasm_source/src/vp_implicit.rs | 12 - wasm/wasm_source/src/vp_masp.rs | 284 ++++++++++----------- wasm/wasm_source/src/vp_testnet_faucet.rs | 7 +- wasm/wasm_source/src/vp_user.rs | 12 - wasm/wasm_source/src/vp_validator.rs | 12 - 21 files changed, 591 insertions(+), 284 deletions(-) diff --git a/core/src/ledger/ibc/context/storage.rs b/core/src/ledger/ibc/context/storage.rs index 2d1c8afdb3..ef8b61116d 100644 --- a/core/src/ledger/ibc/context/storage.rs +++ b/core/src/ledger/ibc/context/storage.rs @@ -7,7 +7,7 @@ pub use ics23::ProofSpec; use super::super::Error; use crate::ledger::storage_api; use crate::types::address::Address; -use crate::types::ibc::IbcEvent; +use crate::types::ibc::{IbcEvent, IbcShieldedTransfer}; use crate::types::storage::{BlockHeight, Header, Key}; use crate::types::token::DenominatedAmount; @@ -70,6 +70,12 @@ pub trait IbcStorageContext { amount: DenominatedAmount, ) -> Result<(), Self::Error>; + /// Handle masp tx + fn handle_masp_tx( + &mut self, + shielded: &IbcShieldedTransfer, + ) -> Result<(), Self::Error>; + /// Mint token fn mint_token( &mut self, diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 40382282cd..05dec84b99 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -29,7 +29,8 @@ use crate::ibc_proto::google::protobuf::Any; use crate::types::address::Address; use crate::types::chain::ChainId; use crate::types::ibc::{ - is_ibc_denom, EVENT_TYPE_DENOM_TRACE, EVENT_TYPE_PACKET, + get_shielded_transfer, is_ibc_denom, EVENT_TYPE_DENOM_TRACE, + EVENT_TYPE_PACKET, }; #[allow(missing_docs)] @@ -53,6 +54,8 @@ pub enum Error { Denom(String), #[error("Invalid chain ID: {0}")] ChainId(ChainId), + #[error("Handling MASP transaction error: {0}")] + MaspTx(String), } /// IBC actions to handle IBC operations @@ -128,15 +131,17 @@ where let envelope = MsgEnvelope::try_from(any_msg).map_err(Error::Execution)?; execute(self, envelope.clone()).map_err(Error::Execution)?; + // 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) } } } /// Store the denom when transfer with MsgRecvPacket - fn store_denom(&mut self, envelope: MsgEnvelope) -> Result<(), Error> { + fn store_denom(&mut self, envelope: &MsgEnvelope) -> Result<(), Error> { match envelope { MsgEnvelope::Packet(PacketMsg::Recv(_)) => { if let Some((trace_hash, ibc_denom, receiver)) = @@ -248,6 +253,38 @@ 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(_)) => { + let event = self + .ctx + .borrow() + .get_ibc_event("fungible_token_packet") + .map_err(|_| { + Error::MaspTx( + "Reading the IBC event failed".to_string(), + ) + })?; + match event { + Some(event) => get_shielded_transfer(&event) + .map_err(|e| Error::MaspTx(e.to_string()))?, + None => return Ok(()), + } + } + _ => return Ok(()), + }; + if let Some(shielded_transfer) = shielded_transfer { + self.ctx + .borrow_mut() + .handle_masp_tx(&shielded_transfer) + .map_err(|_| { + Error::MaspTx("Writing MASP components failed".to_string()) + })?; + } + Ok(()) + } } #[derive(Debug, Default)] diff --git a/core/src/ledger/vp_env.rs b/core/src/ledger/vp_env.rs index 969452b241..c62cf2982d 100644 --- a/core/src/ledger/vp_env.rs +++ b/core/src/ledger/vp_env.rs @@ -2,14 +2,17 @@ //! inside validity predicates. use borsh::BorshDeserialize; +use masp_primitives::transaction::Transaction; -use super::storage_api::{self, StorageRead}; +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}; use crate::types::storage::{ BlockHash, BlockHeight, Epoch, Header, Key, TxIndex, }; +use crate::types::token::Transfer; /// Validity predicate's environment is available for native VPs and WASM VPs pub trait VpEnv<'view> @@ -75,6 +78,12 @@ where /// Get the address of the native token. fn get_native_token(&self) -> Result; + /// Get the IBC event. + fn get_ibc_event( + &self, + event_type: String, + ) -> Result, storage_api::Error>; + /// Storage prefix iterator, ordered by storage keys. It will try to get an /// iterator from the storage. fn iter_prefix<'iter>( @@ -97,6 +106,45 @@ where /// Get a tx hash fn get_tx_code_hash(&self) -> Result, storage_api::Error>; + /// Get the shielded action including the transfer and the masp tx + fn get_shielded_action( + &self, + tx_data: Tx, + ) -> Result<(Transfer, Transaction), storage_api::Error> { + let signed = tx_data; + match Transfer::try_from_slice(&signed.data().unwrap()[..]) { + Ok(transfer) => { + let shielded_hash = transfer + .shielded + .ok_or_err_msg("unable to find shielded hash")?; + let masp_tx = signed + .get_section(&shielded_hash) + .and_then(|x| x.as_ref().masp_tx()) + .ok_or_err_msg("unable to find shielded section")?; + Ok((transfer, masp_tx)) + } + Err(_) => { + if let Some(event) = + self.get_ibc_event("fungible_token_packet".to_string())? + { + if let Some(shielded) = + get_shielded_transfer(&event).into_storage_result()? + { + Ok((shielded.transfer, shielded.masp_tx)) + } else { + Err(storage_api::Error::new_const( + "No shielded transfer in the IBC event", + )) + } + } else { + Err(storage_api::Error::new_const( + "No IBC event for the shielded action", + )) + } + } + } + } + /// Verify a MASP transaction fn verify_masp(&self, tx: Vec) -> Result; diff --git a/core/src/types/address.rs b/core/src/types/address.rs index 416b3f059e..1a18118e83 100644 --- a/core/src/types/address.rs +++ b/core/src/types/address.rs @@ -445,12 +445,18 @@ impl FromStr for Address { } } -/// for IBC signer +/// Convert the IBC signer to an address for the IBC transfer impl TryFrom for Address { type Error = DecodeError; fn try_from(signer: Signer) -> Result { - Address::decode(signer.as_ref()) + // When IBC transfer, the address in this signer should be an address or + // a payment address. If it's a payment address, this returns + // the masp address. + Address::decode(signer.as_ref()).or( + crate::types::masp::PaymentAddress::from_str(signer.as_ref()) + .and(Ok(masp())), + ) } } diff --git a/core/src/types/ibc.rs b/core/src/types/ibc.rs index e7cb5f745f..3e7ae3c4fc 100644 --- a/core/src/types/ibc.rs +++ b/core/src/types/ibc.rs @@ -1,4 +1,4 @@ -//! IBC event without IBC-related data types +//! IBC-related data types use std::cmp::Ordering; use std::collections::HashMap; @@ -50,25 +50,41 @@ impl std::fmt::Display for IbcEvent { } } +/// IBC shielded transfer +#[derive(Debug, Clone, BorshSerialize, BorshDeserialize)] +pub struct IbcShieldedTransfer { + /// The IBC event type + pub transfer: crate::types::token::Transfer, + /// The attributes of the IBC event + pub masp_tx: masp_primitives::transaction::Transaction, +} + #[cfg(any(feature = "abciplus", feature = "abcipp"))] mod ibc_rs_conversion { use std::collections::HashMap; use std::str::FromStr; + use borsh::{BorshDeserialize, BorshSerialize}; + use data_encoding::HEXLOWER; use thiserror::Error; - use super::IbcEvent; - use crate::ibc::applications::transfer::{PrefixedDenom, TracePath}; + use super::{IbcEvent, IbcShieldedTransfer}; + use crate::ibc::applications::transfer::{Memo, PrefixedDenom, TracePath}; use crate::ibc::core::events::{ Error as IbcEventError, IbcEvent as RawIbcEvent, }; use crate::tendermint_proto::abci::Event as AbciEvent; + use crate::types::masp::PaymentAddress; #[allow(missing_docs)] #[derive(Error, Debug)] pub enum Error { #[error("IBC event error: {0}")] IbcEvent(IbcEventError), + #[error("IBC transfer memo HEX decoding error: {0}")] + DecodingHex(data_encoding::DecodeError), + #[error("IBC transfer memo decoding error: {0}")] + DecodingShieldedTransfer(std::io::Error), } /// Conversion functions result @@ -105,6 +121,53 @@ mod ibc_rs_conversion { prefixed_denom.base_denom.to_string(), )) } + + impl From for Memo { + fn from(shielded: IbcShieldedTransfer) -> Self { + let bytes = + shielded.try_to_vec().expect("Encoding shouldn't failed"); + HEXLOWER.encode(&bytes).into() + } + } + + impl TryFrom for IbcShieldedTransfer { + type Error = Error; + + fn try_from(memo: Memo) -> Result { + let bytes = HEXLOWER + .decode(memo.as_ref().as_bytes()) + .map_err(Error::DecodingHex)?; + Self::try_from_slice(&bytes) + .map_err(Error::DecodingShieldedTransfer) + } + } + + /// Get the shielded transfer from the memo + pub fn get_shielded_transfer( + event: &IbcEvent, + ) -> Result> { + if event.event_type != "fungible_token_packet" { + // This event is not for receiving a token + return Ok(None); + } + let is_success = + event.attributes.get("success") == Some(&"true".to_string()); + let receiver = event.attributes.get("receiver"); + let is_shielded = if let Some(receiver) = receiver { + PaymentAddress::from_str(&receiver).is_ok() + } else { + false + }; + if !is_success || !is_shielded { + return Ok(None); + } + + event + .attributes + .get("memo") + .map(|memo| IbcShieldedTransfer::try_from(Memo::from(memo.clone()))) + .transpose() + } } #[cfg(any(feature = "abciplus", feature = "abcipp"))] diff --git a/shared/src/ledger/native_vp/ibc/context.rs b/shared/src/ledger/native_vp/ibc/context.rs index 5af2afebf7..93784b8054 100644 --- a/shared/src/ledger/native_vp/ibc/context.rs +++ b/shared/src/ledger/native_vp/ibc/context.rs @@ -3,15 +3,21 @@ use std::collections::{BTreeSet, HashMap, HashSet}; use borsh::BorshSerialize; +use masp_primitives::transaction::Transaction; use namada_core::ledger::ibc::storage::is_ibc_key; use namada_core::ledger::ibc::{IbcCommonContext, IbcStorageContext}; use namada_core::ledger::storage::write_log::StorageModification; use namada_core::ledger::storage::{self as ledger_storage, StorageHasher}; use namada_core::ledger::storage_api::StorageRead; -use namada_core::types::address::{Address, InternalAddress}; -use namada_core::types::ibc::IbcEvent; -use namada_core::types::storage::{BlockHeight, Header, Key}; -use namada_core::types::token::{self, Amount, DenominatedAmount}; +use namada_core::types::address::{self, Address, InternalAddress}; +use namada_core::types::ibc::{IbcEvent, IbcShieldedTransfer}; +use namada_core::types::storage::{ + BlockHeight, Epoch, Header, Key, KeySeg, TxIndex, +}; +use namada_core::types::token::{ + self, Amount, DenominatedAmount, Transfer, HEAD_TX_KEY, PIN_KEY_PREFIX, + TX_KEY_PREFIX, +}; use super::Error; use crate::ledger::native_vp::CtxPreStorageRead; @@ -160,6 +166,54 @@ where ) } + fn handle_masp_tx( + &mut self, + shielded: &IbcShieldedTransfer, + ) -> Result<(), Self::Error> { + let masp_addr = address::masp(); + let head_tx_key = Key::from(masp_addr.to_db_key()) + .push(&HEAD_TX_KEY.to_owned()) + .expect("Cannot obtain a storage key"); + let current_tx_idx: u64 = + self.ctx.read(&head_tx_key).unwrap_or(None).unwrap_or(0); + let current_tx_key = Key::from(masp_addr.to_db_key()) + .push(&(TX_KEY_PREFIX.to_owned() + ¤t_tx_idx.to_string())) + .expect("Cannot obtain a storage key"); + // Save the Transfer object and its location within the blockchain + // so that clients do not have to separately look these + // up + let record: (Epoch, BlockHeight, TxIndex, Transfer, Transaction) = ( + self.ctx.get_block_epoch().map_err(Error::NativeVpError)?, + self.ctx.get_block_height().map_err(Error::NativeVpError)?, + self.ctx.get_tx_index().map_err(Error::NativeVpError)?, + shielded.transfer.clone(), + shielded.masp_tx.clone(), + ); + self.write( + ¤t_tx_key, + record.try_to_vec().expect("encoding shouldn't failed"), + )?; + self.write( + &head_tx_key, + (current_tx_idx + 1) + .try_to_vec() + .expect("encoding shouldn't failed"), + )?; + // If storage key has been supplied, then pin this transaction to it + if let Some(key) = &shielded.transfer.key { + let pin_key = Key::from(masp_addr.to_db_key()) + .push(&(PIN_KEY_PREFIX.to_owned() + key)) + .expect("Cannot obtain a storage key"); + self.write( + &pin_key, + current_tx_idx + .try_to_vec() + .expect("encoding shouldn't fail"), + )?; + } + Ok(()) + } + fn mint_token( &mut self, target: &Address, @@ -344,6 +398,13 @@ where unimplemented!("Validation doesn't transfer") } + fn handle_masp_tx( + &mut self, + _shielded: &IbcShieldedTransfer, + ) -> Result<(), Self::Error> { + unimplemented!("Validation doesn't handle a masp tx") + } + fn mint_token( &mut self, _target: &Address, diff --git a/shared/src/ledger/native_vp/mod.rs b/shared/src/ledger/native_vp/mod.rs index 31148a1568..002c256eb1 100644 --- a/shared/src/ledger/native_vp/mod.rs +++ b/shared/src/ledger/native_vp/mod.rs @@ -23,6 +23,7 @@ use crate::ledger::storage::{Storage, StorageHasher}; use crate::proto::Tx; use crate::types::address::Address; use crate::types::hash::Hash; +use crate::types::ibc::IbcEvent; use crate::types::storage::{ BlockHash, BlockHeight, Epoch, Header, Key, TxIndex, }; @@ -449,6 +450,18 @@ where .into_storage_result() } + fn get_ibc_event( + &self, + event_type: String, + ) -> Result, storage_api::Error> { + vp_host_fns::get_ibc_event( + &mut self.gas_meter.borrow_mut(), + self.write_log, + event_type, + ) + .into_storage_result() + } + fn iter_prefix<'iter>( &'iter self, prefix: &Key, diff --git a/shared/src/ledger/vp_host_fns.rs b/shared/src/ledger/vp_host_fns.rs index a9aaa7eb16..c8e1bd12d7 100644 --- a/shared/src/ledger/vp_host_fns.rs +++ b/shared/src/ledger/vp_host_fns.rs @@ -15,6 +15,7 @@ use crate::ledger::gas::{GasMetering, VpGasMeter}; use crate::ledger::storage::write_log::WriteLog; use crate::ledger::storage::{self, write_log, Storage, StorageHasher}; use crate::proto::{Section, Tx}; +use crate::types::ibc::IbcEvent; /// These runtime errors will abort VP execution immediately #[allow(missing_docs)] @@ -333,6 +334,20 @@ where Ok(storage.native_token.clone()) } +/// Getting the IBC event. +pub fn get_ibc_event( + _gas_meter: &mut VpGasMeter, + write_log: &WriteLog, + event_type: String, +) -> EnvResult> { + for event in write_log.get_ibc_events() { + if event.event_type == event_type { + return Ok(Some(event.clone())); + } + } + Ok(None) +} + /// Storage prefix iterator for prior state (before tx execution), ordered by /// storage keys. It will try to get an iterator from the storage. pub fn iter_prefix_pre<'a, DB, H>( diff --git a/shared/src/vm/host_env.rs b/shared/src/vm/host_env.rs index 7806d1abef..fd4129501d 100644 --- a/shared/src/vm/host_env.rs +++ b/shared/src/vm/host_env.rs @@ -5,6 +5,7 @@ use std::convert::TryInto; use std::num::TryFromIntError; use borsh::{BorshDeserialize, BorshSerialize}; +use masp_primitives::transaction::Transaction; use namada_core::ledger::gas::{GasMetering, TxGasMeter}; use namada_core::types::internal::KeyVal; use thiserror::Error; @@ -21,11 +22,12 @@ use crate::ledger::vp_host_fns; use crate::proto::Tx; use crate::types::address::{self, Address}; use crate::types::hash::Hash; -use crate::types::ibc::IbcEvent; +use crate::types::ibc::{IbcEvent, IbcShieldedTransfer}; use crate::types::internal::HostEnvResult; -use crate::types::storage::{BlockHeight, Key, TxIndex}; +use crate::types::storage::{BlockHeight, Epoch, Key, KeySeg, TxIndex}; use crate::types::token::{ is_any_minted_balance_key, is_any_minter_key, is_any_token_balance_key, + Transfer, HEAD_TX_KEY, PIN_KEY_PREFIX, TX_KEY_PREFIX, }; use crate::vm::memory::VmMemory; use crate::vm::prefix_iter::{PrefixIteratorId, PrefixIterators}; @@ -1778,6 +1780,44 @@ where Ok(epoch.0) } +/// Getting the IBC event function exposed to the wasm VM VP environment. +pub fn vp_get_ibc_event( + env: &VpVmEnv, + event_type_ptr: u64, + event_type_len: u64, +) -> vp_host_fns::EnvResult +where + MEM: VmMemory, + DB: storage::DB + for<'iter> storage::DBIter<'iter>, + H: StorageHasher, + EVAL: VpEvaluator, + CA: WasmCacheAccess, +{ + let (event_type, gas) = env + .memory + .read_string(event_type_ptr, event_type_len as _) + .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; + let gas_meter = unsafe { env.ctx.gas_meter.get() }; + vp_host_fns::add_gas(gas_meter, gas)?; + + let write_log = unsafe { env.ctx.write_log.get() }; + match vp_host_fns::get_ibc_event(gas_meter, write_log, event_type)? { + Some(event) => { + let value = event + .try_to_vec() + .map_err(vp_host_fns::RuntimeError::EncodingError)?; + let len: i64 = value + .len() + .try_into() + .map_err(vp_host_fns::RuntimeError::NumConversionError)?; + let result_buffer = unsafe { env.ctx.result_buffer.get() }; + result_buffer.replace(value); + Ok(len) + } + None => Ok(HostEnvResult::Fail.to_i64()), + } +} + /// Verify a transaction signature /// TODO: this is just a warkaround to track gas for multiple singature /// verifications. When the runtime gas meter is implemented, this funcion can @@ -1868,8 +1908,6 @@ where EVAL: VpEvaluator, CA: WasmCacheAccess, { - use masp_primitives::transaction::Transaction; - let gas_meter = unsafe { env.ctx.gas_meter.get() }; let (tx_bytes, gas) = env .memory @@ -2247,6 +2285,42 @@ where Ok(()) } + fn handle_masp_tx( + &mut self, + shielded: &IbcShieldedTransfer, + ) -> Result<(), Self::Error> { + let masp_addr = address::masp(); + let head_tx_key = Key::from(masp_addr.to_db_key()) + .push(&HEAD_TX_KEY.to_owned()) + .expect("Cannot obtain a storage key"); + let current_tx_idx: u64 = ibc_read_borsh(self, &head_tx_key) + .unwrap_or(None) + .unwrap_or(0); + let current_tx_key = Key::from(masp_addr.to_db_key()) + .push(&(TX_KEY_PREFIX.to_owned() + ¤t_tx_idx.to_string())) + .expect("Cannot obtain a storage key"); + // Save the Transfer object and its location within the blockchain + // so that clients do not have to separately look these + // up + let record: (Epoch, BlockHeight, TxIndex, Transfer, Transaction) = ( + ibc_get_block_epoch(self)?, + self.get_height()?, + ibc_get_tx_index(self)?, + shielded.transfer.clone(), + shielded.masp_tx.clone(), + ); + ibc_write_borsh(self, ¤t_tx_key, &record)?; + ibc_write_borsh(self, &head_tx_key, &(current_tx_idx + 1))?; + // If storage key has been supplied, then pin this transaction to it + if let Some(key) = &shielded.transfer.key { + let pin_key = Key::from(masp_addr.to_db_key()) + .push(&(PIN_KEY_PREFIX.to_owned() + key)) + .expect("Cannot obtain a storage key"); + ibc_write_borsh(self, &pin_key, ¤t_tx_idx)?; + } + Ok(()) + } + fn mint_token( &mut self, target: &Address, @@ -2396,6 +2470,40 @@ where Ok(()) } +/// Get the current epoch. +// Temp helper for ibc tx workaround. +fn ibc_get_block_epoch<'a, DB, H, CA>( + ctx: &TxCtx<'a, DB, H, CA>, +) -> TxResult +where + DB: storage::DB + for<'iter> storage::DBIter<'iter>, + H: StorageHasher, + CA: WasmCacheAccess, +{ + let storage = unsafe { ctx.storage.get() }; + let (epoch, gas) = storage.get_current_epoch(); + ibc_tx_charge_gas(ctx, gas)?; + Ok(epoch) +} + +/// Get the tx index. +// Temp helper for ibc tx workaround. +fn ibc_get_tx_index<'a, DB, H, CA>( + ctx: &TxCtx<'a, DB, H, CA>, +) -> TxResult +where + DB: storage::DB + for<'iter> storage::DBIter<'iter>, + H: StorageHasher, + CA: WasmCacheAccess, +{ + let tx_index = unsafe { ctx.tx_index.get() }; + ibc_tx_charge_gas( + ctx, + crate::vm::host_env::gas::STORAGE_ACCESS_GAS_PER_BYTE, + )?; + Ok(TxIndex(tx_index.0)) +} + // Temp. workaround for impl<'a, DB, H, CA> namada_core::ledger::ibc::IbcCommonContext for TxCtx<'a, DB, H, CA> diff --git a/shared/src/vm/wasm/host_env.rs b/shared/src/vm/wasm/host_env.rs index 57a8bc6986..e5ebce6ee2 100644 --- a/shared/src/vm/wasm/host_env.rs +++ b/shared/src/vm/wasm/host_env.rs @@ -127,6 +127,7 @@ where "namada_vp_get_block_hash" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_get_block_hash), "namada_vp_get_tx_code_hash" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_get_tx_code_hash), "namada_vp_get_block_epoch" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_get_block_epoch), + "namada_vp_get_ibc_event" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_get_ibc_event), "namada_vp_verify_tx_section_signature" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_verify_tx_section_signature), "namada_vp_verify_masp" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_verify_masp), "namada_vp_eval" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_eval), diff --git a/tx_prelude/src/ibc.rs b/tx_prelude/src/ibc.rs index f7ee8230b1..5e8e9522fa 100644 --- a/tx_prelude/src/ibc.rs +++ b/tx_prelude/src/ibc.rs @@ -10,11 +10,11 @@ pub use namada_core::ledger::ibc::{ use namada_core::ledger::storage_api::{StorageRead, StorageWrite}; use namada_core::ledger::tx_env::TxEnv; use namada_core::types::address::{Address, InternalAddress}; -pub use namada_core::types::ibc::IbcEvent; +pub use namada_core::types::ibc::{IbcEvent, IbcShieldedTransfer}; use namada_core::types::storage::{BlockHeight, Header, Key}; use namada_core::types::token::DenominatedAmount; -use crate::token::{burn, mint, transfer}; +use crate::token::{burn, handle_masp_tx, mint, transfer}; use crate::{Ctx, KeyValIterator}; /// IBC actions to handle an IBC message @@ -89,7 +89,14 @@ impl IbcStorageContext for Ctx { token: &Address, amount: DenominatedAmount, ) -> std::result::Result<(), Self::Error> { - transfer(self, src, dest, token, amount, &None, &None, &None) + transfer(self, src, dest, token, amount) + } + + fn handle_masp_tx( + &mut self, + shielded: &IbcShieldedTransfer, + ) -> Result<(), Self::Error> { + handle_masp_tx(self, &shielded.transfer, &shielded.masp_tx) } fn mint_token( diff --git a/tx_prelude/src/token.rs b/tx_prelude/src/token.rs index e950668d2b..6067c82a46 100644 --- a/tx_prelude/src/token.rs +++ b/tx_prelude/src/token.rs @@ -1,6 +1,5 @@ use masp_primitives::transaction::Transaction; use namada_core::types::address::Address; -use namada_core::types::hash::Hash; use namada_core::types::storage::KeySeg; use namada_core::types::token; pub use namada_core::types::token::*; @@ -15,9 +14,6 @@ pub fn transfer( dest: &Address, token: &Address, amount: DenominatedAmount, - key: &Option, - shielded_hash: &Option, - shielded: &Option, ) -> TxResult { if amount.amount != Amount::default() && src != dest { let src_key = token::balance_key(token, src); @@ -33,47 +29,43 @@ pub fn transfer( ctx.write(&src_key, src_bal)?; ctx.write(&dest_key, dest_bal)?; } + Ok(()) +} - // If this transaction has a shielded component, then handle it - // separately - if let Some(shielded) = shielded { - let masp_addr = address::masp(); - ctx.insert_verifier(&masp_addr)?; - let head_tx_key = storage::Key::from(masp_addr.to_db_key()) - .push(&HEAD_TX_KEY.to_owned()) - .expect("Cannot obtain a storage key"); - let current_tx_idx: u64 = - ctx.read(&head_tx_key).unwrap_or(None).unwrap_or(0); - let current_tx_key = storage::Key::from(masp_addr.to_db_key()) - .push(&(TX_KEY_PREFIX.to_owned() + ¤t_tx_idx.to_string())) +/// Handle a MASP transaction. +pub fn handle_masp_tx( + ctx: &mut Ctx, + transfer: &Transfer, + shielded: &Transaction, +) -> TxResult { + let masp_addr = address::masp(); + ctx.insert_verifier(&masp_addr)?; + let head_tx_key = storage::Key::from(masp_addr.to_db_key()) + .push(&HEAD_TX_KEY.to_owned()) + .expect("Cannot obtain a storage key"); + let current_tx_idx: u64 = + ctx.read(&head_tx_key).unwrap_or(None).unwrap_or(0); + let current_tx_key = storage::Key::from(masp_addr.to_db_key()) + .push(&(TX_KEY_PREFIX.to_owned() + ¤t_tx_idx.to_string())) + .expect("Cannot obtain a storage key"); + // Save the Transfer object and its location within the blockchain + // so that clients do not have to separately look these + // up + let record: (Epoch, BlockHeight, TxIndex, Transfer, Transaction) = ( + ctx.get_block_epoch()?, + ctx.get_block_height()?, + ctx.get_tx_index()?, + transfer.clone(), + shielded.clone(), + ); + ctx.write(¤t_tx_key, record)?; + ctx.write(&head_tx_key, current_tx_idx + 1)?; + // If storage key has been supplied, then pin this transaction to it + if let Some(key) = &transfer.key { + let pin_key = storage::Key::from(masp_addr.to_db_key()) + .push(&(PIN_KEY_PREFIX.to_owned() + key)) .expect("Cannot obtain a storage key"); - // Save the Transfer object and its location within the blockchain - // so that clients do not have to separately look these - // up - let transfer = Transfer { - source: src.clone(), - target: dest.clone(), - token: token.clone(), - amount, - key: key.clone(), - shielded: *shielded_hash, - }; - let record: (Epoch, BlockHeight, TxIndex, Transfer, Transaction) = ( - ctx.get_block_epoch()?, - ctx.get_block_height()?, - ctx.get_tx_index()?, - transfer, - shielded.clone(), - ); - ctx.write(¤t_tx_key, record)?; - ctx.write(&head_tx_key, current_tx_idx + 1)?; - // If storage key has been supplied, then pin this transaction to it - if let Some(key) = key { - let pin_key = storage::Key::from(masp_addr.to_db_key()) - .push(&(PIN_KEY_PREFIX.to_owned() + key)) - .expect("Cannot obtain a storage key"); - ctx.write(&pin_key, current_tx_idx)?; - } + ctx.write(&pin_key, current_tx_idx)?; } Ok(()) } diff --git a/vm_env/src/lib.rs b/vm_env/src/lib.rs index 6a630f9349..edf00f847c 100644 --- a/vm_env/src/lib.rs +++ b/vm_env/src/lib.rs @@ -197,6 +197,12 @@ pub mod vp { // Get the native token address pub fn namada_vp_get_native_token(result_ptr: u64); + // Get the IBC event + pub fn namada_vp_get_ibc_event( + event_type_ptr: u64, + event_type_len: u64, + ) -> i64; + // Requires a node running with "Info" log level pub fn namada_vp_log_string(str_ptr: u64, str_len: u64); diff --git a/vp_prelude/src/lib.rs b/vp_prelude/src/lib.rs index 0962628363..9857574acd 100644 --- a/vp_prelude/src/lib.rs +++ b/vp_prelude/src/lib.rs @@ -297,6 +297,20 @@ impl<'view> VpEnv<'view> for Ctx { get_native_token() } + fn get_ibc_event( + &self, + event_type: String, + ) -> Result, Error> { + let read_result = unsafe { + namada_vp_get_ibc_event( + event_type.as_ptr() as _, + event_type.len() as _, + ) + }; + Ok(read_from_buffer(read_result, namada_vp_result_buffer) + .and_then(|t| ibc::IbcEvent::try_from_slice(&t[..]).ok())) + } + fn iter_prefix<'iter>( &'iter self, prefix: &storage::Key, diff --git a/wasm/wasm_source/src/tx_bridge_pool.rs b/wasm/wasm_source/src/tx_bridge_pool.rs index 88d757998f..d93b983943 100644 --- a/wasm/wasm_source/src/tx_bridge_pool.rs +++ b/wasm/wasm_source/src/tx_bridge_pool.rs @@ -23,9 +23,6 @@ fn apply_tx(ctx: &mut Ctx, signed: Tx) -> TxResult { &bridge_pool::BRIDGE_POOL_ADDRESS, fee_token_addr, amount.native_denominated(), - &None, - &None, - &None, )?; log_string("Token transfer succeeded."); let TransferToEthereum { @@ -43,9 +40,6 @@ fn apply_tx(ctx: &mut Ctx, signed: Tx) -> TxResult { ð_bridge::ADDRESS, &nam_addr, amount.native_denominated(), - &None, - &None, - &None, )?; } else { // Otherwise we escrow ERC20 tokens. @@ -56,9 +50,6 @@ fn apply_tx(ctx: &mut Ctx, signed: Tx) -> TxResult { &bridge_pool::BRIDGE_POOL_ADDRESS, &token, amount.native_denominated(), - &None, - &None, - &None, )?; } log_string("Escrow succeeded"); diff --git a/wasm/wasm_source/src/tx_transfer.rs b/wasm/wasm_source/src/tx_transfer.rs index bdc683c339..f36f52c74d 100644 --- a/wasm/wasm_source/src/tx_transfer.rs +++ b/wasm/wasm_source/src/tx_transfer.rs @@ -11,15 +11,17 @@ fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { let transfer = token::Transfer::try_from_slice(&data[..]) .wrap_err("failed to decode token::Transfer")?; debug_log!("apply_tx called with transfer: {:#?}", transfer); - let token::Transfer { - source, - target, - token, - amount, - key, - shielded: shielded_hash, - } = transfer; - let shielded = shielded_hash + + token::transfer( + ctx, + &transfer.source, + &transfer.target, + &transfer.token, + transfer.amount, + )?; + + let shielded = transfer + .shielded .as_ref() .map(|hash| { signed @@ -28,14 +30,8 @@ fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult { .ok_or_err_msg("unable to find shielded section") }) .transpose()?; - token::transfer( - ctx, - &source, - &target, - &token, - amount, - &key, - &shielded_hash, - &shielded, - ) + if let Some(shielded) = shielded { + token::handle_masp_tx(ctx, &transfer, &shielded)?; + } + Ok(()) } diff --git a/wasm/wasm_source/src/vp_implicit.rs b/wasm/wasm_source/src/vp_implicit.rs index 215dccf421..68db2d8408 100644 --- a/wasm/wasm_source/src/vp_implicit.rs +++ b/wasm/wasm_source/src/vp_implicit.rs @@ -362,9 +362,6 @@ mod tests { address, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); @@ -595,9 +592,6 @@ mod tests { &target, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); @@ -658,9 +652,6 @@ mod tests { &target, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); @@ -734,9 +725,6 @@ mod tests { &target, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); diff --git a/wasm/wasm_source/src/vp_masp.rs b/wasm/wasm_source/src/vp_masp.rs index cb66211118..794ddec2ee 100644 --- a/wasm/wasm_source/src/vp_masp.rs +++ b/wasm/wasm_source/src/vp_masp.rs @@ -90,185 +90,167 @@ fn validate_tx( verifiers, ); - let signed = tx_data; - let transfer = - token::Transfer::try_from_slice(&signed.data().unwrap()[..]).unwrap(); + let (transfer, shielded_tx) = ctx.get_shielded_action(tx_data)?; + let mut transparent_tx_pool = I128Sum::zero(); + // The Sapling value balance adds to the transparent tx pool + transparent_tx_pool += shielded_tx.sapling_value_balance(); - let shielded = transfer - .shielded - .as_ref() - .map(|hash| { - signed - .get_section(hash) - .and_then(|x| x.as_ref().masp_tx()) - .ok_or_err_msg("unable to find shielded section") - }) - .transpose()?; - if let Some(shielded_tx) = shielded { - let mut transparent_tx_pool = I128Sum::zero(); - // The Sapling value balance adds to the transparent tx pool - transparent_tx_pool += shielded_tx.sapling_value_balance(); + if transfer.source != masp() { + // Handle transparent input + // Note that the asset type is timestamped so shields + // where the shielded value has an incorrect timestamp + // are automatically rejected + for denom in token::MaspDenom::iter() { + let (_transp_asset, transp_amt) = convert_amount( + ctx.get_block_epoch().unwrap(), + &transfer.token, + transfer.amount.into(), + denom, + ); - if transfer.source != masp() { - // Handle transparent input - // Note that the asset type is timestamped so shields - // where the shielded value has an incorrect timestamp - // are automatically rejected - for denom in token::MaspDenom::iter() { - let (_transp_asset, transp_amt) = convert_amount( - ctx.get_block_epoch().unwrap(), - &transfer.token, - transfer.amount.into(), - denom, - ); - - // Non-masp sources add to transparent tx pool - transparent_tx_pool += transp_amt; - } - } else { - // Handle shielded input - // The following boundary conditions must be satisfied - // 1. Zero transparent input - // 2. the transparent transaction value pool's amount must equal the - // containing wrapper transaction's fee amount - // Satisfies 1. - if let Some(transp_bundle) = shielded_tx.transparent_bundle() { - if !transp_bundle.vin.is_empty() { - debug_log!( - "Transparent input to a transaction from the masp \ - must be 0 but is {}", - transp_bundle.vin.len() - ); - return reject(); - } - } + // Non-masp sources add to transparent tx pool + transparent_tx_pool += transp_amt; } - - if transfer.target != masp() { - // Handle transparent output - // The following boundary conditions must be satisfied - // 1. One to 4 transparent outputs - // 2. Asset type must be properly derived - // 3. Value from the output must be the same as the containing - // transfer - // 4. Public key must be the hash of the target - - // Satisfies 1. - let transp_bundle = - shielded_tx.transparent_bundle().ok_or_err_msg( - "Expected transparent outputs in unshielding transaction", - )?; - - let out_length = transp_bundle.vout.len(); - if !(1..=4).contains(&out_length) { + } else { + // Handle shielded input + // The following boundary conditions must be satisfied + // 1. Zero transparent input + // 2. the transparent transaction value pool's amount must equal the + // containing wrapper transaction's fee amount + // Satisfies 1. + if let Some(transp_bundle) = shielded_tx.transparent_bundle() { + if !transp_bundle.vin.is_empty() { debug_log!( - "Transparent output to a transaction to the masp must be \ - beteween 1 and 4 but is {}", - transp_bundle.vout.len() + "Transparent input to a transaction from the masp must be \ + 0 but is {}", + transp_bundle.vin.len() ); - return reject(); } - let mut outs = transp_bundle.vout.iter(); - let mut valid_count = 0; - for denom in token::MaspDenom::iter() { - let out = match outs.next() { - Some(out) => out, - None => continue, - }; + } + } + + if transfer.target != masp() { + // Handle transparent output + // The following boundary conditions must be satisfied + // 1. One to 4 transparent outputs + // 2. Asset type must be properly derived + // 3. Value from the output must be the same as the containing + // transfer + // 4. Public key must be the hash of the target - let expected_asset_type: AssetType = - asset_type_from_epoched_address( - ctx.get_block_epoch().unwrap(), - &transfer.token, - denom, - ); + // Satisfies 1. + let transp_bundle = shielded_tx.transparent_bundle().ok_or_err_msg( + "Expected transparent outputs in unshielding transaction", + )?; - // Satisfies 2. and 3. - if !valid_asset_type(&expected_asset_type, &out.asset_type) { - // we don't know which masp denoms are necessary apriori. - // This is encoded via the asset types. - continue; - } - if !valid_transfer_amount( - out.value, - denom.denominate(&transfer.amount.amount), - ) { - return reject(); - } + let out_length = transp_bundle.vout.len(); + if !(1..=4).contains(&out_length) { + debug_log!( + "Transparent output to a transaction to the masp must be \ + beteween 1 and 4 but is {}", + transp_bundle.vout.len() + ); + + return reject(); + } + let mut outs = transp_bundle.vout.iter(); + let mut valid_count = 0; + for denom in token::MaspDenom::iter() { + let out = match outs.next() { + Some(out) => out, + None => continue, + }; - let (_transp_asset, transp_amt) = convert_amount( + let expected_asset_type: AssetType = + asset_type_from_epoched_address( ctx.get_block_epoch().unwrap(), &transfer.token, - transfer.amount.amount, denom, ); - // Non-masp destinations subtract from transparent tx pool - transparent_tx_pool -= transp_amt; - - // Satisfies 4. - let target_enc = transfer - .target - .try_to_vec() - .expect("target address encoding"); - - let hash = Ripemd160::digest(sha256(&target_enc).0.as_slice()); - - if <[u8; 20]>::from(hash) != out.address.0 { - debug_log!( - "the public key of the output account does not match \ - the transfer target" - ); - return reject(); - } - valid_count += 1; + // Satisfies 2. and 3. + if !valid_asset_type(&expected_asset_type, &out.asset_type) { + // we don't know which masp denoms are necessary apriori. + // This is encoded via the asset types. + continue; } - // one or more of the denoms in the batch failed to verify - // the asset derivation. - if valid_count != out_length { + if !valid_transfer_amount( + out.value, + denom.denominate(&transfer.amount.amount), + ) { return reject(); } - } else { - // Handle shielded output - // The following boundary conditions must be satisfied - // 1. Zero transparent output - // Satisfies 1. - if let Some(transp_bundle) = shielded_tx.transparent_bundle() { - if !transp_bundle.vout.is_empty() { - debug_log!( - "Transparent output to a transaction from the masp \ - must be 0 but is {}", - transp_bundle.vout.len() - ); - return reject(); - } - } - } + let (_transp_asset, transp_amt) = convert_amount( + ctx.get_block_epoch().unwrap(), + &transfer.token, + transfer.amount.amount, + denom, + ); - match transparent_tx_pool.partial_cmp(&I128Sum::zero()) { - None | Some(Ordering::Less) => { + // Non-masp destinations subtract from transparent tx pool + transparent_tx_pool -= transp_amt; + + // Satisfies 4. + let target_enc = transfer + .target + .try_to_vec() + .expect("target address encoding"); + + let hash = Ripemd160::digest(sha256(&target_enc).0.as_slice()); + + if <[u8; 20]>::from(hash) != out.address.0 { debug_log!( - "Transparent transaction value pool must be nonnegative. \ - Violation may be caused by transaction being constructed \ - in previous epoch. Maybe try again." + "the public key of the output account does not match the \ + transfer target" ); - // Section 3.4: The remaining value in the transparent - // transaction value pool MUST be nonnegative. return reject(); } - Some(Ordering::Greater) => { + valid_count += 1; + } + // one or more of the denoms in the batch failed to verify + // the asset derivation. + if valid_count != out_length { + return reject(); + } + } else { + // Handle shielded output + // The following boundary conditions must be satisfied + // 1. Zero transparent output + + // Satisfies 1. + if let Some(transp_bundle) = shielded_tx.transparent_bundle() { + if !transp_bundle.vout.is_empty() { debug_log!( - "Transaction fees cannot be paid inside MASP transaction." + "Transparent output to a transaction from the masp must \ + be 0 but is {}", + transp_bundle.vout.len() ); return reject(); } - _ => {} } - // Do the expensive proof verification in the VM at the end. - ctx.verify_masp(shielded_tx.try_to_vec().unwrap()) - } else { - reject() } + + match transparent_tx_pool.partial_cmp(&I128Sum::zero()) { + None | Some(Ordering::Less) => { + debug_log!( + "Transparent transaction value pool must be nonnegative. \ + Violation may be caused by transaction being constructed in \ + previous epoch. Maybe try again." + ); + // Section 3.4: The remaining value in the transparent + // transaction value pool MUST be nonnegative. + return reject(); + } + Some(Ordering::Greater) => { + debug_log!( + "Transaction fees cannot be paid inside MASP transaction." + ); + return reject(); + } + _ => {} + } + // Do the expensive proof verification in the VM at the end. + ctx.verify_masp(shielded_tx.try_to_vec().unwrap()) } diff --git a/wasm/wasm_source/src/vp_testnet_faucet.rs b/wasm/wasm_source/src/vp_testnet_faucet.rs index 7298c0b126..2f85740028 100644 --- a/wasm/wasm_source/src/vp_testnet_faucet.rs +++ b/wasm/wasm_source/src/vp_testnet_faucet.rs @@ -176,9 +176,6 @@ mod tests { address, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); @@ -331,7 +328,7 @@ mod tests { // Initialize VP environment from a transaction vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| { // Apply transfer in a transaction - tx_host_env::token::transfer(tx::ctx(), address, &target, &token, amount, &None, &None, &None).unwrap(); + tx_host_env::token::transfer(tx::ctx(), address, &target, &token, amount).unwrap(); }); let vp_env = vp_host_env::take(); @@ -390,7 +387,7 @@ mod tests { let valid = solution.validate(tx::ctx(), address, target.clone()).unwrap(); assert!(valid); // Apply transfer in a transaction - tx_host_env::token::transfer(tx::ctx(), address, &target, &token, amount, &None, &None, &None).unwrap(); + tx_host_env::token::transfer(tx::ctx(), address, &target, &token, amount).unwrap(); }); let mut vp_env = vp_host_env::take(); diff --git a/wasm/wasm_source/src/vp_user.rs b/wasm/wasm_source/src/vp_user.rs index a334576b53..25b17a14b4 100644 --- a/wasm/wasm_source/src/vp_user.rs +++ b/wasm/wasm_source/src/vp_user.rs @@ -259,9 +259,6 @@ mod tests { address, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); @@ -317,9 +314,6 @@ mod tests { &target, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); @@ -379,9 +373,6 @@ mod tests { &target, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); @@ -612,9 +603,6 @@ mod tests { &target, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); diff --git a/wasm/wasm_source/src/vp_validator.rs b/wasm/wasm_source/src/vp_validator.rs index f929a8a0d1..9c32d81717 100644 --- a/wasm/wasm_source/src/vp_validator.rs +++ b/wasm/wasm_source/src/vp_validator.rs @@ -266,9 +266,6 @@ mod tests { address, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); @@ -324,9 +321,6 @@ mod tests { &target, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); @@ -386,9 +380,6 @@ mod tests { &target, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); @@ -629,9 +620,6 @@ mod tests { &target, &token, amount, - &None, - &None, - &None, ) .unwrap(); }); From f35b779fe2a0907883f2b224d0f3a41325e1ecfd Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 20 Sep 2023 16:01:30 +0200 Subject: [PATCH 02/10] add gen_ibc_shielded_transfer --- apps/src/lib/cli.rs | 97 +++++++++++++++++++++ apps/src/lib/cli/client.rs | 18 ++++ apps/src/lib/client/tx.rs | 33 ++++++++ benches/lib.rs | 53 +++--------- core/src/ledger/ibc/mod.rs | 43 +++++++++- core/src/ledger/storage_api/account.rs | 1 + core/src/ledger/storage_api/token.rs | 13 +-- core/src/types/address.rs | 31 +++++-- shared/src/ledger/protocol/mod.rs | 1 + shared/src/sdk/args.rs | 23 +++++ shared/src/sdk/masp.rs | 40 +++++---- shared/src/sdk/rpc.rs | 1 + shared/src/sdk/signing.rs | 47 ++++++----- shared/src/sdk/tx.rs | 111 ++++++++++++++++++++++++- 14 files changed, 410 insertions(+), 102 deletions(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 135ff1e3c5..9981cb06e9 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -255,6 +255,7 @@ pub mod cmds { .subcommand(QueryValidatorState::def().display_order(5)) // Actions .subcommand(SignTx::def().display_order(6)) + .subcommand(GenIbcShieldedTransafer::def().display_order(6)) // Utils .subcommand(Utils::def().display_order(7)) } @@ -313,6 +314,8 @@ pub mod cmds { let add_to_eth_bridge_pool = Self::parse_with_ctx(matches, AddToEthBridgePool); let sign_tx = Self::parse_with_ctx(matches, SignTx); + let gen_ibc_shielded = + Self::parse_with_ctx(matches, GenIbcShieldedTransafer); let utils = SubCmd::parse(matches).map(Self::WithoutContext); tx_custom .or(tx_transfer) @@ -350,6 +353,7 @@ pub mod cmds { .or(query_validator_state) .or(query_account) .or(sign_tx) + .or(gen_ibc_shielded) .or(utils) } } @@ -424,6 +428,7 @@ pub mod cmds { QueryPgf(QueryPgf), QueryValidatorState(QueryValidatorState), SignTx(SignTx), + GenIbcShieldedTransafer(GenIbcShieldedTransafer), } #[allow(clippy::large_enum_variant)] @@ -1874,6 +1879,29 @@ pub mod cmds { } } + #[derive(Clone, Debug)] + pub struct GenIbcShieldedTransafer( + pub args::GenIbcShieldedTransafer, + ); + + impl SubCmd for GenIbcShieldedTransafer { + const CMD: &'static str = "ibc-gen-shielded"; + + fn parse(matches: &ArgMatches) -> Option { + matches.subcommand_matches(Self::CMD).map(|matches| { + GenIbcShieldedTransafer(args::GenIbcShieldedTransafer::parse( + matches, + )) + }) + } + + fn def() -> App { + App::new(Self::CMD) + .about("Generate shielded transfer for IBC.") + .add_args::>() + } + } + #[derive(Clone, Debug)] pub struct EpochSleep(pub args::Query); @@ -2713,6 +2741,7 @@ pub mod args { pub const SAFE_MODE: ArgFlag = flag("safe-mode"); pub const SCHEME: ArgDefault = arg_default("scheme", DefaultFn(|| SchemeType::Ed25519)); + pub const SENDER: Arg = arg("sender"); pub const SIGNING_KEYS: ArgMulti = arg_multi("signing-keys"); pub const SIGNATURES: ArgMulti = arg_multi("signatures"); pub const SOURCE: Arg = arg("source"); @@ -4731,6 +4760,74 @@ pub mod args { } } + impl CliToSdk> + for GenIbcShieldedTransafer + { + fn to_sdk( + self, + ctx: &mut Context, + ) -> GenIbcShieldedTransafer { + GenIbcShieldedTransafer:: { + query: self.query.to_sdk(ctx), + output_folder: self.output_folder, + sender: self.sender, + target: ctx.get(&self.target), + token: ctx.get(&self.token), + trace_path: self.trace_path, + amount: self.amount, + port_id: self.port_id, + channel_id: self.channel_id, + } + } + } + + impl Args for GenIbcShieldedTransafer { + fn parse(matches: &ArgMatches) -> Self { + let query = Query::parse(matches); + let output_folder = OUTPUT_FOLDER_PATH.parse(matches); + let sender = SENDER.parse(matches); + let target = TRANSFER_TARGET.parse(matches); + let token = TOKEN.parse(matches); + let trace_path = TRACE_PATH.parse(matches); + let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); + let port_id = PORT_ID.parse(matches); + let channel_id = CHANNEL_ID.parse(matches); + Self { + query, + output_folder, + sender, + target, + token, + trace_path, + amount, + port_id, + channel_id, + } + } + + fn def(app: App) -> App { + app.add_args::>() + .arg(OUTPUT_FOLDER_PATH.def().help( + "The output folder path where the artifact will be stored.", + )) + .arg(SENDER.def().help("The foreign sender address.")) + .arg(TRANSFER_TARGET.def().help("The target address.")) + .arg(TOKEN.def().help("The transfer token.")) + .arg(TRACE_PATH.def().help("The IBC trace path of the token.")) + .arg(AMOUNT.def().help("The amount to transfer in decimal.")) + .arg( + PORT_ID + .def() + .help("The port ID via which the token is received."), + ) + .arg( + CHANNEL_ID.def().help( + "The channel ID via which the token is received.", + ), + ) + } + } + impl CliToSdk> for QueryCommissionRate { fn to_sdk(self, ctx: &mut Context) -> QueryCommissionRate { QueryCommissionRate:: { diff --git a/apps/src/lib/cli/client.rs b/apps/src/lib/cli/client.rs index 1a7d9f534a..4ccfb438e0 100644 --- a/apps/src/lib/cli/client.rs +++ b/apps/src/lib/cli/client.rs @@ -660,6 +660,24 @@ impl CliApi { let args = args.to_sdk(&mut ctx); tx::sign_tx::<_, IO>(&client, &mut ctx, args).await?; } + Sub::GenIbcShieldedTransafer(GenIbcShieldedTransafer( + mut args, + )) => { + let client = client.unwrap_or_else(|| { + C::from_tendermint_address( + &mut args.query.ledger_address, + ) + }); + client + .wait_until_node_is_synced::() + .await + .proceed_or_else(error)?; + let args = args.to_sdk(&mut ctx); + tx::gen_ibc_shielded_transfer::<_, IO>( + &client, &mut ctx, args, + ) + .await?; + } } } cli::NamadaClient::WithoutContext(cmd, global_args) => match cmd { diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index c8c42b190b..81ff927b7f 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -12,6 +12,7 @@ use namada::core::ledger::governance::cli::offline::{ use namada::core::ledger::governance::cli::onchain::{ DefaultProposal, PgfFundingProposal, PgfStewardProposal, ProposalVote, }; +use namada::ibc::applications::transfer::Memo; use namada::ledger::pos; use namada::proof_of_stake::parameters::PosParams; use namada::proto::Tx; @@ -1606,3 +1607,35 @@ pub async fn submit_tx( ) -> Result { tx::submit_tx::<_, IO>(client, to_broadcast).await } + +pub async fn gen_ibc_shielded_transfer( + client: &C, + ctx: &mut Context, + args: args::GenIbcShieldedTransafer, +) -> Result<(), error::Error> +where + C: namada::ledger::queries::Client + Sync, + C::Error: std::fmt::Display, +{ + if let Some(shielded_transfer) = tx::gen_ibc_shielded_transfer::<_, _, IO>( + client, + &mut ctx.shielded, + args.clone(), + ) + .await? + { + let tx_id = shielded_transfer.masp_tx.txid().to_string(); + let filename = format!("ibc_shielded_transfer_{}.memo", tx_id,); + let output_path = match &args.output_folder { + Some(path) => path.join(filename), + None => filename.into(), + }; + let out = File::create(&output_path) + .expect("Should be able to create the out file."); + serde_json::to_writer_pretty(out, &Memo::from(shielded_transfer)) + .expect("IBC memo should be deserializable."); + } else { + eprintln!("No shielded transfer for this IBC transfer.") + } + Ok(()) +} diff --git a/benches/lib.rs b/benches/lib.rs index 47645abdf4..d48e45cb4a 100644 --- a/benches/lib.rs +++ b/benches/lib.rs @@ -89,9 +89,7 @@ use namada::types::time::DateTimeUtc; use namada::types::token::DenominatedAmount; use namada::types::transaction::governance::InitProposalData; use namada::types::transaction::pos::Bond; -use namada::types::transaction::GasLimit; use namada::vm::wasm::run; -use namada_apps::cli::args::{Tx as TxArgs, TxTransfer}; use namada_apps::cli::context::FromContext; use namada_apps::cli::Context; use namada_apps::config::TendermintMode; @@ -753,44 +751,10 @@ impl BenchShieldedCtx { source: TransferSource, target: TransferTarget, ) -> Tx { - let mock_args = TxArgs { - dry_run: false, - dry_run_wrapper: false, - dump_tx: false, - force: false, - broadcast_only: false, - ledger_address: (), - initialized_account_alias: None, - fee_amount: None, - fee_token: address::nam(), - fee_unshield: None, - gas_limit: GasLimit::from(u64::MAX), - expiration: None, - disposable_signing_key: false, - signing_keys: vec![defaults::albert_keypair()], - signatures: vec![], - wallet_alias_force: true, - chain_id: None, - tx_reveal_code_path: TX_REVEAL_PK_WASM.into(), - verification_key: None, - password: None, - wrapper_fee_payer: None, - output_folder: None, + let denominated_amount = DenominatedAmount { + amount, + denom: 0.into(), }; - - let args = TxTransfer { - tx: mock_args, - source: source.clone(), - target: target.clone(), - token: address::nam(), - amount: InputAmount::Validated(DenominatedAmount { - amount, - denom: 0.into(), - }), - native_token: self.shell.wl_storage.storage.native_token.clone(), - tx_code_path: TX_TRANSFER_WASM.into(), - }; - let async_runtime = tokio::runtime::Runtime::new().unwrap(); let spending_key = self .wallet @@ -804,10 +768,13 @@ impl BenchShieldedCtx { )) .unwrap(); let shielded = async_runtime - .block_on( - self.shielded - .gen_shielded_transfer::<_, DefaultIo>(&self.shell, args), - ) + .block_on(self.shielded.gen_shielded_transfer::<_, DefaultIo>( + &self.shell, + &source, + &target, + &address::nam(), + denominated_amount, + )) .unwrap() .map( |ShieldedTransfer { diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 05dec84b99..3b8edcc271 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -7,6 +7,7 @@ use std::cell::RefCell; use std::collections::HashMap; use std::fmt::Debug; use std::rc::Rc; +use std::str::FromStr; use std::time::Duration; pub use context::common::IbcCommonContext; @@ -18,11 +19,14 @@ use thiserror::Error; use crate::ibc::applications::transfer::error::TokenTransferError; use crate::ibc::applications::transfer::msgs::transfer::MsgTransfer; use crate::ibc::applications::transfer::{ - send_transfer_execute, send_transfer_validate, + is_receiver_chain_source, send_transfer_execute, send_transfer_validate, + BaseDenom, PrefixedDenom, TracePath, TracePrefix, }; use crate::ibc::core::ics04_channel::msgs::PacketMsg; use crate::ibc::core::ics23_commitment::specs::ProofSpecs; -use crate::ibc::core::ics24_host::identifier::{ChainId as IbcChainId, PortId}; +use crate::ibc::core::ics24_host::identifier::{ + ChainId as IbcChainId, ChannelId, PortId, +}; use crate::ibc::core::router::{Module, ModuleId, Router}; use crate::ibc::core::{execute, validate, MsgEnvelope, RouterError}; use crate::ibc_proto::google::protobuf::Any; @@ -299,3 +303,38 @@ pub struct ValidationParams { /// Upgrade path pub upgrade_path: Vec, } + +/// Get the IbcToken from the source/destination ports and channels +pub fn received_ibc_token( + token: &Address, + trace_path: Option, + src_port_id: &PortId, + src_channel_id: &ChannelId, + dest_port_id: &PortId, + dest_channel_id: &ChannelId, +) -> Result { + if let Some(trace_path) = trace_path { + let mut ibc_denom = PrefixedDenom { + trace_path, + base_denom: BaseDenom::from_str(&token.to_string()).map_err( + |e| Error::Denom(format!("Trace path is invalid: error {e}")), + )?, + }; + if is_receiver_chain_source( + src_port_id.clone(), + src_channel_id.clone(), + &ibc_denom, + ) { + let prefix = + TracePrefix::new(src_port_id.clone(), src_channel_id.clone()); + ibc_denom.remove_trace_prefix(&prefix); + } else { + let prefix = + TracePrefix::new(dest_port_id.clone(), dest_channel_id.clone()); + ibc_denom.add_trace_prefix(prefix); + } + Ok(storage::ibc_token(&ibc_denom.to_string())) + } else { + Ok(token.clone()) + } +} diff --git a/core/src/ledger/storage_api/account.rs b/core/src/ledger/storage_api/account.rs index 5fa0abb8f9..b3cdd5fb67 100644 --- a/core/src/ledger/storage_api/account.rs +++ b/core/src/ledger/storage_api/account.rs @@ -77,6 +77,7 @@ where } Address::Implicit(_) => Ok(true), Address::Internal(_) => Ok(false), + Address::Foreign(_) => Ok(false), } } diff --git a/core/src/ledger/storage_api/token.rs b/core/src/ledger/storage_api/token.rs index 02adcc32be..f22ca99280 100644 --- a/core/src/ledger/storage_api/token.rs +++ b/core/src/ledger/storage_api/token.rs @@ -46,19 +46,22 @@ pub fn read_denom( where S: StorageRead, { - let (key, nut) = match token { + let (key, is_default_zero) = match token { Address::Internal(InternalAddress::Nut(erc20)) => { let token = Address::Internal(InternalAddress::Erc20(*erc20)); + // NB: always use the equivalent ERC20's smallest + // denomination to specify amounts, if we cannot + // find a denom in storage (token::denom_key(&token), true) } + Address::Internal(InternalAddress::IbcToken(_)) => { + (token::denom_key(token), true) + } token => (token::denom_key(token), false), }; storage.read(&key).map(|opt_denom| { Some(opt_denom.unwrap_or_else(|| { - if nut { - // NB: always use the equivalent ERC20's smallest - // denomination to specify amounts, if we cannot - // find a denom in storage + if is_default_zero { 0u8.into() } else { // FIXME: perhaps when we take this branch, we should diff --git a/core/src/types/address.rs b/core/src/types/address.rs index 1a18118e83..32cc7de99b 100644 --- a/core/src/types/address.rs +++ b/core/src/types/address.rs @@ -96,6 +96,8 @@ const PREFIX_ESTABLISHED: &str = "est"; const PREFIX_IMPLICIT: &str = "imp"; /// Fixed-length address strings prefix for internal addresses. const PREFIX_INTERNAL: &str = "ano"; +/// Fixed-length address strings prefix for foreign addresses. +const PREFIX_FOREIGN: &str = "for"; /// Fixed-length address strings prefix for IBC addresses. const PREFIX_IBC: &str = "ibc"; /// Fixed-length address strings prefix for Ethereum addresses. @@ -134,6 +136,8 @@ pub enum Address { Implicit(ImplicitAddress), /// An internal address represents a module with a native VP Internal(InternalAddress), + /// An foreign address is provided from other chains + Foreign(String), } // We're using the string format of addresses (bech32m) for ordering to ensure @@ -196,6 +200,7 @@ impl Address { Some(hash_hex) } Address::Internal(_) => None, + Address::Foreign(_) => None, } } @@ -254,6 +259,9 @@ impl Address { debug_assert_eq!(string.len(), FIXED_LEN_STRING_BYTES); string } + Address::Foreign(addr) => { + format!("{}::{}", PREFIX_FOREIGN, addr) + } } .into_bytes(); string.resize(FIXED_LEN_STRING_BYTES, b' '); @@ -374,6 +382,9 @@ impl Address { "Invalid ERC20 internal address".to_string(), )), }, + Some((PREFIX_FOREIGN, raw)) => { + Ok(Address::Foreign(raw.to_string())) + } _ => Err(DecodeError::InvalidInnerEncoding( ErrorKind::InvalidData, "Invalid address prefix".to_string(), @@ -397,6 +408,9 @@ impl Address { Address::Internal(kind) => { format!("Internal {}: {}", kind, self.encode()) } + Address::Foreign(_) => { + format!("Foreign: {}", self.encode()) + } } } } @@ -445,17 +459,22 @@ impl FromStr for Address { } } -/// Convert the IBC signer to an address for the IBC transfer +/// for IBC signer impl TryFrom for Address { type Error = DecodeError; fn try_from(signer: Signer) -> Result { - // When IBC transfer, the address in this signer should be an address or - // a payment address. If it's a payment address, this returns - // the masp address. + // The given address should be an address or payment address. When + // sending a token from a spending key, it has been already + // replaced with the MASP address. Address::decode(signer.as_ref()).or( - crate::types::masp::PaymentAddress::from_str(signer.as_ref()) - .and(Ok(masp())), + match crate::types::masp::PaymentAddress::from_str(signer.as_ref()) + { + Ok(_) => Ok(masp()), + Err(_) => Err(DecodeError::InvalidInnerEncodingStr(format!( + "Invalid address for IBC transfer: {signer}" + ))), + }, ) } } diff --git a/shared/src/ledger/protocol/mod.rs b/shared/src/ledger/protocol/mod.rs index a23b026eea..c1a518e023 100644 --- a/shared/src/ledger/protocol/mod.rs +++ b/shared/src/ledger/protocol/mod.rs @@ -973,6 +973,7 @@ where accepted } + Address::Foreign(_) => Ok(true), }; // Returning error from here will short-circuit the VP parallel diff --git a/shared/src/sdk/args.rs b/shared/src/sdk/args.rs index 0b87317530..87f01d2e90 100644 --- a/shared/src/sdk/args.rs +++ b/shared/src/sdk/args.rs @@ -885,3 +885,26 @@ pub struct ValidatorSetUpdateRelay { /// Ethereum transfers aren't canceled midway through. pub safe_mode: bool, } + +/// IBC shielded transfer generation arguments +#[derive(Clone, Debug)] +pub struct GenIbcShieldedTransafer { + /// The query parameters. + pub query: Query, + /// The output directory path to where serialize the data + pub output_folder: Option, + /// The foreign sender address + pub sender: String, + /// The target address + pub target: C::TransferTarget, + /// The token address + pub token: C::Address, + /// The trace path of the token + pub trace_path: Option, + /// Transferred token amount + pub amount: InputAmount, + /// Port ID via which the token is received + pub port_id: PortId, + /// Channel ID via which the token is received + pub channel_id: ChannelId, +} diff --git a/shared/src/sdk/masp.rs b/shared/src/sdk/masp.rs index 739f941b9a..08cd0844a7 100644 --- a/shared/src/sdk/masp.rs +++ b/shared/src/sdk/masp.rs @@ -59,17 +59,19 @@ use sha2::Digest; use thiserror::Error; use crate::proto::Tx; -use crate::sdk::args::InputAmount; use crate::sdk::error::{EncodingError, Error, PinnedBalanceError, QueryError}; use crate::sdk::queries::Client; +use crate::sdk::rpc; use crate::sdk::rpc::{query_conversion, query_storage_value}; use crate::sdk::tx::decode_component; -use crate::sdk::{args, rpc}; use crate::tendermint_rpc::query::Query; use crate::tendermint_rpc::Order; use crate::types::address::{masp, Address}; use crate::types::io::Io; -use crate::types::masp::{BalanceOwner, ExtendedViewingKey, PaymentAddress}; +use crate::types::masp::{ + BalanceOwner, ExtendedViewingKey, PaymentAddress, TransferSource, + TransferTarget, +}; use crate::types::storage::{BlockHeight, Epoch, Key, KeySeg, TxIndex}; use crate::types::token; use crate::types::token::{ @@ -1486,7 +1488,10 @@ impl ShieldedContext { pub async fn gen_shielded_transfer( &mut self, client: &C, - args: args::TxTransfer, + source: &TransferSource, + target: &TransferTarget, + token: &Address, + amount: token::DenominatedAmount, ) -> Result, TransferErr> { // No shielded components are needed when neither source nor destination // are shielded @@ -1496,8 +1501,8 @@ impl ShieldedContext { use rand::rngs::StdRng; use rand_core::SeedableRng; - let spending_key = args.source.spending_key(); - let payment_address = args.target.payment_address(); + let spending_key = source.spending_key(); + let payment_address = target.payment_address(); // No shielded components are needed when neither source nor // destination are shielded if spending_key.is_none() && payment_address.is_none() { @@ -1540,14 +1545,9 @@ impl ShieldedContext { let mut builder = Builder::::new_with_rng(NETWORK, 1.into(), rng); - // break up a transfer into a number of transfers with suitable - // denominations - let InputAmount::Validated(amt) = args.amount else { - unreachable!("The function `gen_shielded_transfer` is only called by `submit_tx` which validates amounts.") - }; // Convert transaction amount into MASP types - let (asset_types, amount) = - convert_amount(epoch, &args.token, amt.amount)?; + let (asset_types, masp_amount) = + convert_amount(epoch, token, amount.amount)?; // If there are shielded inputs if let Some(sk) = spending_key { @@ -1556,7 +1556,7 @@ impl ShieldedContext { .collect_unspent_notes::<_, IO>( client, &to_viewing_key(&sk).vk, - I128Sum::from_sum(amount), + I128Sum::from_sum(masp_amount), epoch, ) .await?; @@ -1582,8 +1582,7 @@ impl ShieldedContext { // We add a dummy UTXO to our transaction, but only the source of // the parent Transfer object is used to validate fund // availability - let source_enc = args - .source + let source_enc = source .address() .ok_or_else(|| { Error::Other( @@ -1605,7 +1604,7 @@ impl ShieldedContext { builder .add_transparent_input(TxOut { asset_type: *asset_type, - value: denom.denominate(&amt), + value: denom.denominate(&amount), address: script, }) .map_err(builder::Error::TransparentBuild)?; @@ -1623,7 +1622,7 @@ impl ShieldedContext { ovk_opt, pa.into(), *asset_type, - denom.denominate(&amt), + denom.denominate(&amount), memo.clone(), ) .map_err(builder::Error::SaplingBuild)?; @@ -1631,8 +1630,7 @@ impl ShieldedContext { } else { // Embed the transparent target address into the shielded // transaction so that it can be signed - let target_enc = args - .target + let target_enc = target .address() .ok_or_else(|| { Error::Other( @@ -1650,7 +1648,7 @@ impl ShieldedContext { )); for (denom, asset_type) in MaspDenom::iter().zip(asset_types.iter()) { - let vout = denom.denominate(&amt); + let vout = denom.denominate(&amount); if vout != 0 { builder .add_transparent_output( diff --git a/shared/src/sdk/rpc.rs b/shared/src/sdk/rpc.rs index d98e0b61ec..5c69d743d9 100644 --- a/shared/src/sdk/rpc.rs +++ b/shared/src/sdk/rpc.rs @@ -217,6 +217,7 @@ pub async fn known_address( query_has_storage_key(client, &key).await } Address::Implicit(_) | Address::Internal(_) => Ok(true), + Address::Foreign(_) => Ok(false), } } diff --git a/shared/src/sdk/signing.rs b/shared/src/sdk/signing.rs index 042be03a63..48695e9069 100644 --- a/shared/src/sdk/signing.rs +++ b/shared/src/sdk/signing.rs @@ -1,6 +1,5 @@ //! Functions to sign transactions use std::collections::{BTreeMap, HashMap}; -use std::path::PathBuf; use borsh::{BorshDeserialize, BorshSerialize}; use data_encoding::HEXLOWER; @@ -121,6 +120,10 @@ pub async fn find_pk< "Internal address {} doesn't have any signing keys.", addr )), + Address::Foreign(_) => other_err(format!( + "Foreign address {} doesn't have any signing keys.", + addr + )), } } @@ -275,7 +278,8 @@ pub async fn aux_signing_data< Some(AccountPublicKeysMap::from_iter(public_keys.clone())), 1u8, ), - Some(owner @ Address::Internal(_)) => { + Some(owner @ Address::Internal(_)) + | Some(owner @ Address::Foreign(_)) => { return Err(Error::from(TxError::InvalidAccount(owner.encode()))); } None => (None, 0u8), @@ -419,30 +423,27 @@ pub async fn wrap_tx< Some(diff) if !diff.is_zero() => { if let Some(spending_key) = args.fee_unshield.clone() { // Unshield funds for fee payment - let transfer_args = args::TxTransfer { - tx: args.to_owned(), - source: spending_key, - target: namada_core::types::masp::TransferTarget::Address( - fee_payer_address.clone(), - ), - token: args.fee_token.clone(), - amount: args::InputAmount::Validated(DenominatedAmount { - // NOTE: must unshield the total fee amount, not the - // diff, because the ledger evaluates the transaction in - // reverse (wrapper first, inner second) and cannot know - // ahead of time if the inner will modify the balance of - // the gas payer - amount: total_fee, - denom: 0.into(), - }), - // These last two fields are not used in the function, mock - // them - native_token: args.fee_token.clone(), - tx_code_path: PathBuf::new(), + let target = namada_core::types::masp::TransferTarget::Address( + fee_payer_address.clone(), + ); + let fee_amount = DenominatedAmount { + // NOTE: must unshield the total fee amount, not the + // diff, because the ledger evaluates the transaction in + // reverse (wrapper first, inner second) and cannot know + // ahead of time if the inner will modify the balance of + // the gas payer + amount: total_fee, + denom: 0.into(), }; match shielded - .gen_shielded_transfer::<_, IO>(client, transfer_args) + .gen_shielded_transfer::<_, IO>( + client, + &spending_key, + &target, + &args.fee_token, + fee_amount, + ) .await { Ok(Some(ShieldedTransfer { diff --git a/shared/src/sdk/tx.rs b/shared/src/sdk/tx.rs index e95ab58fe1..c9c61991c7 100644 --- a/shared/src/sdk/tx.rs +++ b/shared/src/sdk/tx.rs @@ -22,6 +22,7 @@ use namada_core::ledger::governance::cli::onchain::{ }; use namada_core::ledger::governance::storage::proposal::ProposalType; use namada_core::ledger::governance::storage::vote::StorageProposalVote; +use namada_core::ledger::ibc::storage::channel_key; use namada_core::ledger::pgf::cli::steward::Commission; use namada_core::types::address::{masp, Address}; use namada_core::types::dec::Dec; @@ -38,6 +39,7 @@ use crate::ibc::applications::transfer::msgs::transfer::MsgTransfer; use crate::ibc::applications::transfer::packet::PacketData; use crate::ibc::applications::transfer::PrefixedCoin; use crate::ibc::core::ics04_channel::timeout::TimeoutHeight; +use crate::ibc::core::ics24_host::identifier::{ChannelId, PortId}; use crate::ibc::core::timestamp::Timestamp as IbcTimestamp; use crate::ibc::core::Msg; use crate::ibc::Height as IbcHeight; @@ -55,9 +57,10 @@ use crate::sdk::wallet::{Wallet, WalletUtils}; use crate::tendermint_rpc::endpoint::broadcast::tx_sync::Response; use crate::tendermint_rpc::error::Error as RpcError; use crate::types::control_flow::{time, ProceedOrElse}; +use crate::types::ibc::IbcShieldedTransfer; use crate::types::io::Io; use crate::types::key::*; -use crate::types::masp::TransferTarget; +use crate::types::masp::{TransferSource, TransferTarget}; use crate::types::storage::Epoch; use crate::types::time::DateTimeUtc; use crate::types::transaction::account::{InitAccount, UpdateAccount}; @@ -1714,7 +1717,13 @@ pub async fn build_transfer< // Construct the shielded part of the transaction, if any let stx_result = shielded - .gen_shielded_transfer::<_, IO>(client, args.clone()) + .gen_shielded_transfer::<_, IO>( + client, + &args.source, + &args.target, + &args.token, + validated_amount, + ) .await; let shielded_parts = match stx_result { @@ -1997,6 +2006,104 @@ pub async fn build_custom< Ok((tx, epoch)) } +/// Generate IBC shielded transfer +pub async fn gen_ibc_shielded_transfer< + C: crate::ledger::queries::Client + Sync, + V: ShieldedUtils, + IO: Io, +>( + client: &C, + shielded: &mut ShieldedContext, + args: args::GenIbcShieldedTransafer, +) -> Result> { + let key = match args.target.payment_address() { + Some(pa) if pa.is_pinned() => Some(pa.hash()), + Some(_) => None, + None => return Ok(None), + }; + let source = Address::Foreign(args.sender.clone()); + let (src_port_id, src_channel_id) = + get_ibc_src_port_channel(client, &args.port_id, &args.channel_id) + .await?; + let token = namada_core::ledger::ibc::received_ibc_token( + &args.token, + args.trace_path, + &src_port_id, + &src_channel_id, + &args.port_id, + &args.channel_id, + ) + .map_err(|e| { + Error::Other(format!("Getting IBC Token failed: error {e}")) + })?; + let validated_amount = + validate_amount::<_, IO>(client, args.amount, &token, false) + .await + .expect("expected to validate amount"); + + let shielded = shielded + .gen_shielded_transfer::<_, IO>( + client, + &TransferSource::Address(source.clone()), + &args.target, + &token, + validated_amount, + ) + .await + .map_err(|err| TxError::MaspError(err.to_string()))?; + let transfer = token::Transfer { + source: source.clone(), + target: masp(), + token, + amount: validated_amount, + key, + shielded: None, + }; + if let Some(shielded) = shielded { + Ok(Some(IbcShieldedTransfer { + transfer, + masp_tx: shielded.masp_tx, + })) + } else { + Ok(None) + } +} + +async fn get_ibc_src_port_channel( + client: &C, + dest_port_id: &PortId, + dest_channel_id: &ChannelId, +) -> Result<(PortId, ChannelId)> { + use crate::ibc::core::ics04_channel::channel::ChannelEnd; + use crate::ibc_proto::protobuf::Protobuf; + + let channel_key = channel_key(dest_port_id, dest_channel_id); + match rpc::query_storage_value_bytes::(client, &channel_key, None, false) + .await + { + Ok((Some(bytes), _)) => { + let channel = ChannelEnd::decode_vec(&bytes).map_err(|_| { + Error::Other(format!( + "Decoding channel end failed: port {}, channel {}", + dest_port_id, dest_channel_id + )) + })?; + if let Some(src_channel) = channel.remote.channel_id() { + Ok((channel.remote.port_id.clone(), src_channel.clone())) + } else { + Err(Error::Other(format!( + "The source channel doesn't exist: port {dest_port_id}, \ + channel {dest_channel_id}" + ))) + } + } + _ => Err(Error::Other(format!( + "Reading channel end failed: port {dest_port_id}, channel \ + {dest_channel_id}" + ))), + } +} + async fn expect_dry_broadcast< C: crate::ledger::queries::Client + Sync, IO: Io, From 0e1850739aaf2fc8562d02863b59f82dee13fcbc Mon Sep 17 00:00:00 2001 From: yito88 Date: Wed, 20 Sep 2023 23:41:23 +0200 Subject: [PATCH 03/10] WIP: add e2e test for receiving to payment address --- apps/src/lib/cli.rs | 17 ++- apps/src/lib/client/tx.rs | 10 +- benches/lib.rs | 1 - core/src/ledger/ibc/mod.rs | 65 ++++++------ core/src/types/ibc.rs | 8 +- shared/src/sdk/args.rs | 2 - shared/src/sdk/tx.rs | 5 +- tests/src/e2e/ibc_tests.rs | 212 ++++++++++++++++++++++++++++++++----- 8 files changed, 239 insertions(+), 81 deletions(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 9981cb06e9..2980b21e49 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -2687,7 +2687,7 @@ pub mod args { pub const HD_WALLET_DERIVATION_PATH_OPT: ArgOpt = HD_WALLET_DERIVATION_PATH.opt(); pub const HISTORIC: ArgFlag = flag("historic"); - pub const IBC_TRANSFER_MEMO: ArgOpt = arg_opt("memo"); + pub const IBC_TRANSFER_MEMO_PATH: ArgOpt = arg_opt("memo-path"); pub const LEDGER_ADDRESS_ABOUT: &str = "Address of a ledger node as \"{scheme}://{host}:{port}\". If the \ scheme is not supplied, it is assumed to be TCP."; @@ -3603,7 +3603,10 @@ pub mod args { let channel_id = CHANNEL_ID.parse(matches); let timeout_height = TIMEOUT_HEIGHT.parse(matches); let timeout_sec_offset = TIMEOUT_SEC_OFFSET.parse(matches); - let memo = IBC_TRANSFER_MEMO.parse(matches); + let memo = IBC_TRANSFER_MEMO_PATH.parse(matches).map(|path| { + std::fs::read_to_string(path) + .expect("Expected a file at given path") + }); let tx_code_path = PathBuf::from(TX_IBC_WASM); Self { tx, @@ -3640,9 +3643,9 @@ pub mod args { ) .arg(TIMEOUT_SEC_OFFSET.def().help("The timeout as seconds.")) .arg( - IBC_TRANSFER_MEMO + IBC_TRANSFER_MEMO_PATH .def() - .help("Memo field of ICS20 transfer."), + .help("The path for the memo field of ICS20 transfer."), ) } } @@ -4773,7 +4776,6 @@ pub mod args { sender: self.sender, target: ctx.get(&self.target), token: ctx.get(&self.token), - trace_path: self.trace_path, amount: self.amount, port_id: self.port_id, channel_id: self.channel_id, @@ -4788,7 +4790,6 @@ pub mod args { let sender = SENDER.parse(matches); let target = TRANSFER_TARGET.parse(matches); let token = TOKEN.parse(matches); - let trace_path = TRACE_PATH.parse(matches); let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); let port_id = PORT_ID.parse(matches); let channel_id = CHANNEL_ID.parse(matches); @@ -4798,7 +4799,6 @@ pub mod args { sender, target, token, - trace_path, amount, port_id, channel_id, @@ -4806,14 +4806,13 @@ pub mod args { } fn def(app: App) -> App { - app.add_args::>() + app.add_args::>() .arg(OUTPUT_FOLDER_PATH.def().help( "The output folder path where the artifact will be stored.", )) .arg(SENDER.def().help("The foreign sender address.")) .arg(TRANSFER_TARGET.def().help("The target address.")) .arg(TOKEN.def().help("The transfer token.")) - .arg(TRACE_PATH.def().help("The IBC trace path of the token.")) .arg(AMOUNT.def().help("The amount to transfer in decimal.")) .arg( PORT_ID diff --git a/apps/src/lib/client/tx.rs b/apps/src/lib/client/tx.rs index 81ff927b7f..9e2ae749b0 100644 --- a/apps/src/lib/client/tx.rs +++ b/apps/src/lib/client/tx.rs @@ -1625,15 +1625,19 @@ where .await? { let tx_id = shielded_transfer.masp_tx.txid().to_string(); - let filename = format!("ibc_shielded_transfer_{}.memo", tx_id,); + let filename = format!("ibc_shielded_transfer_{}.memo", tx_id); let output_path = match &args.output_folder { Some(path) => path.join(filename), None => filename.into(), }; - let out = File::create(&output_path) + let mut out = File::create(&output_path) .expect("Should be able to create the out file."); - serde_json::to_writer_pretty(out, &Memo::from(shielded_transfer)) + out.write_all(Memo::from(shielded_transfer).as_ref().as_bytes()) .expect("IBC memo should be deserializable."); + println!( + "Output IBC shielded transfer for {tx_id} to {}", + output_path.to_string_lossy() + ); } else { eprintln!("No shielded transfer for this IBC transfer.") } diff --git a/benches/lib.rs b/benches/lib.rs index d48e45cb4a..287d6683b0 100644 --- a/benches/lib.rs +++ b/benches/lib.rs @@ -71,7 +71,6 @@ use namada::ledger::queries::{ use namada::ledger::storage_api::StorageRead; use namada::proof_of_stake; use namada::proto::{Code, Data, Section, Signature, Tx}; -use namada::sdk::args::InputAmount; use namada::sdk::masp::{ self, ShieldedContext, ShieldedTransfer, ShieldedUtils, }; diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 3b8edcc271..277a19011b 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -20,7 +20,7 @@ use crate::ibc::applications::transfer::error::TokenTransferError; use crate::ibc::applications::transfer::msgs::transfer::MsgTransfer; use crate::ibc::applications::transfer::{ is_receiver_chain_source, send_transfer_execute, send_transfer_validate, - BaseDenom, PrefixedDenom, TracePath, TracePrefix, + PrefixedDenom, TracePrefix, }; use crate::ibc::core::ics04_channel::msgs::PacketMsg; use crate::ibc::core::ics23_commitment::specs::ProofSpecs; @@ -30,12 +30,13 @@ use crate::ibc::core::ics24_host::identifier::{ use crate::ibc::core::router::{Module, ModuleId, Router}; use crate::ibc::core::{execute, validate, MsgEnvelope, RouterError}; use crate::ibc_proto::google::protobuf::Any; -use crate::types::address::Address; +use crate::types::address::{masp, Address}; use crate::types::chain::ChainId; use crate::types::ibc::{ get_shielded_transfer, is_ibc_denom, EVENT_TYPE_DENOM_TRACE, EVENT_TYPE_PACKET, }; +use crate::types::masp::PaymentAddress; #[allow(missing_docs)] #[derive(Error, Debug)] @@ -211,14 +212,20 @@ where .as_ref() .and_then(|event| event.attributes.get("receiver")) { - Some(receiver) => { - Some(Address::decode(receiver).map_err(|_| { - Error::Denom(format!( - "Decoding the receiver address failed: {:?}", - receive_event - )) - })?) - } + Some(receiver) => Some( + Address::decode(receiver) + .or_else(|_| { + // Replace it with MASP address when the receiver is a + // payment address + PaymentAddress::from_str(receiver).map(|_| masp()) + }) + .map_err(|_| { + Error::Denom(format!( + "Decoding the receiver address failed: {:?}", + receive_event + )) + })?, + ), None => None, }; let denom_event = self @@ -306,35 +313,25 @@ pub struct ValidationParams { /// Get the IbcToken from the source/destination ports and channels pub fn received_ibc_token( - token: &Address, - trace_path: Option, + ibc_denom: &PrefixedDenom, src_port_id: &PortId, src_channel_id: &ChannelId, dest_port_id: &PortId, dest_channel_id: &ChannelId, ) -> Result { - if let Some(trace_path) = trace_path { - let mut ibc_denom = PrefixedDenom { - trace_path, - base_denom: BaseDenom::from_str(&token.to_string()).map_err( - |e| Error::Denom(format!("Trace path is invalid: error {e}")), - )?, - }; - if is_receiver_chain_source( - src_port_id.clone(), - src_channel_id.clone(), - &ibc_denom, - ) { - let prefix = - TracePrefix::new(src_port_id.clone(), src_channel_id.clone()); - ibc_denom.remove_trace_prefix(&prefix); - } else { - let prefix = - TracePrefix::new(dest_port_id.clone(), dest_channel_id.clone()); - ibc_denom.add_trace_prefix(prefix); - } - Ok(storage::ibc_token(&ibc_denom.to_string())) + let mut ibc_denom = ibc_denom.clone(); + if is_receiver_chain_source( + src_port_id.clone(), + src_channel_id.clone(), + &ibc_denom, + ) { + let prefix = + TracePrefix::new(src_port_id.clone(), src_channel_id.clone()); + ibc_denom.remove_trace_prefix(&prefix); } else { - Ok(token.clone()) + let prefix = + TracePrefix::new(dest_port_id.clone(), dest_channel_id.clone()); + ibc_denom.add_trace_prefix(prefix); } + Ok(storage::ibc_token(ibc_denom.to_string())) } diff --git a/core/src/types/ibc.rs b/core/src/types/ibc.rs index 3e7ae3c4fc..6ff3c911f2 100644 --- a/core/src/types/ibc.rs +++ b/core/src/types/ibc.rs @@ -65,7 +65,7 @@ mod ibc_rs_conversion { use std::str::FromStr; use borsh::{BorshDeserialize, BorshSerialize}; - use data_encoding::HEXLOWER; + use data_encoding::HEXUPPER; use thiserror::Error; use super::{IbcEvent, IbcShieldedTransfer}; @@ -126,7 +126,7 @@ mod ibc_rs_conversion { fn from(shielded: IbcShieldedTransfer) -> Self { let bytes = shielded.try_to_vec().expect("Encoding shouldn't failed"); - HEXLOWER.encode(&bytes).into() + HEXUPPER.encode(&bytes).into() } } @@ -134,7 +134,7 @@ mod ibc_rs_conversion { type Error = Error; fn try_from(memo: Memo) -> Result { - let bytes = HEXLOWER + let bytes = HEXUPPER .decode(memo.as_ref().as_bytes()) .map_err(Error::DecodingHex)?; Self::try_from_slice(&bytes) @@ -154,7 +154,7 @@ mod ibc_rs_conversion { event.attributes.get("success") == Some(&"true".to_string()); let receiver = event.attributes.get("receiver"); let is_shielded = if let Some(receiver) = receiver { - PaymentAddress::from_str(&receiver).is_ok() + PaymentAddress::from_str(receiver).is_ok() } else { false }; diff --git a/shared/src/sdk/args.rs b/shared/src/sdk/args.rs index 87f01d2e90..ff031d8f62 100644 --- a/shared/src/sdk/args.rs +++ b/shared/src/sdk/args.rs @@ -899,8 +899,6 @@ pub struct GenIbcShieldedTransafer { pub target: C::TransferTarget, /// The token address pub token: C::Address, - /// The trace path of the token - pub trace_path: Option, /// Transferred token amount pub amount: InputAmount, /// Port ID via which the token is received diff --git a/shared/src/sdk/tx.rs b/shared/src/sdk/tx.rs index c9c61991c7..56b05395a6 100644 --- a/shared/src/sdk/tx.rs +++ b/shared/src/sdk/tx.rs @@ -2025,9 +2025,10 @@ pub async fn gen_ibc_shielded_transfer< let (src_port_id, src_channel_id) = get_ibc_src_port_channel(client, &args.port_id, &args.channel_id) .await?; + let ibc_denom = + rpc::query_ibc_denom::<_, IO>(client, &args.token, Some(&source)).await; let token = namada_core::ledger::ibc::received_ibc_token( - &args.token, - args.trace_path, + &ibc_denom.parse().expect("Invalid IBC denom"), &src_port_id, &src_channel_id, &args.port_id, diff --git a/tests/src/e2e/ibc_tests.rs b/tests/src/e2e/ibc_tests.rs index 3cd2ba48a3..e5f91b8c7c 100644 --- a/tests/src/e2e/ibc_tests.rs +++ b/tests/src/e2e/ibc_tests.rs @@ -13,6 +13,7 @@ use core::convert::TryFrom; use core::str::FromStr; use core::time::Duration; use std::collections::HashMap; +use std::path::PathBuf; use color_eyre::eyre::Result; use eyre::eyre; @@ -60,12 +61,14 @@ use namada::ledger::storage::traits::Sha256Hasher; use namada::tendermint::abci::Event as AbciEvent; use namada::tendermint::block::Height as TmHeight; use namada::types::address::{Address, InternalAddress}; +use namada::types::io::DefaultIo; use namada::types::key::PublicKey; use namada::types::storage::{BlockHeight, Key}; use namada::types::token::Amount; use namada_apps::client::rpc::{ query_storage_value, query_storage_value_bytes, }; +use namada_apps::client::tx::CLIShieldedUtils; use namada_apps::client::utils::id_from_pk; use namada_apps::config::ethereum_bridge; use namada_apps::config::genesis::genesis_config::GenesisConfig; @@ -77,9 +80,10 @@ use prost::Message; use setup::constants::*; use tendermint_light_client::components::io::{Io, ProdIo as TmLightClientIo}; -use super::helpers::wait_for_wasm_pre_compile; use super::setup::set_ethereum_bridge_mode; -use crate::e2e::helpers::{find_address, get_actor_rpc, get_validator_pk}; +use crate::e2e::helpers::{ + find_address, get_actor_rpc, get_validator_pk, wait_for_wasm_pre_compile, +}; use crate::e2e::setup::{self, sleep, Bin, NamadaCmd, Test, Who}; use crate::{run, run_as}; @@ -125,10 +129,6 @@ fn run_ledger_ibc() -> Result<()> { wait_for_wasm_pre_compile(&mut ledger_a)?; wait_for_wasm_pre_compile(&mut ledger_b)?; - // Wait for a first block - ledger_a.exp_string("Committed block hash")?; - ledger_b.exp_string("Committed block hash")?; - let _bg_ledger_a = ledger_a.background(); let _bg_ledger_b = ledger_b.background(); @@ -190,6 +190,18 @@ fn run_ledger_ibc() -> Result<()> { // The balance should not be changed check_balances_after_back(&port_id_b, &channel_id_b, &test_a, &test_b)?; + shielded_transfer( + &test_a, + &test_b, + &client_id_a, + &client_id_b, + &port_id_a, + &channel_id_a, + &port_id_b, + &channel_id_b, + )?; + check_shielded_balances(&port_id_b, &channel_id_b, &test_b)?; + // Skip tests for closing a channel and timeout_on_close since the transfer // channel cannot be closed @@ -197,13 +209,18 @@ fn run_ledger_ibc() -> Result<()> { } fn setup_two_single_node_nets() -> Result<(Test, Test)> { + // Download the shielded pool parameters before starting node + let _ = CLIShieldedUtils::new::(PathBuf::new()); + // epoch per 100 seconds let update_genesis = |mut genesis: GenesisConfig| { - genesis.parameters.epochs_per_year = 315_360; + genesis.parameters.epochs_per_year = 31536; + genesis.parameters.min_num_of_blocks = 1; genesis }; let update_genesis_b = |mut genesis: GenesisConfig| { - genesis.parameters.epochs_per_year = 315_360; + genesis.parameters.epochs_per_year = 31536; + genesis.parameters.min_num_of_blocks = 1; setup::set_validators(1, genesis, |_| setup::ANOTHER_CHAIN_PORT_OFFSET) }; Ok(( @@ -635,7 +652,7 @@ fn transfer_token( let height = transfer( test_a, ALBERT, - &receiver, + receiver.to_string(), NAM, "100000", ALBERT_KEY, @@ -643,6 +660,7 @@ fn transfer_token( channel_id_a, None, None, + None, false, )?; let events = get_events(test_a, height)?; @@ -704,13 +722,14 @@ fn try_invalid_transfers( transfer( test_a, ALBERT, - &receiver, + receiver.to_string(), NAM, "10.1", ALBERT_KEY, port_id_a, channel_id_a, None, + None, Some("The amount for the IBC transfer should be an integer"), false, )?; @@ -719,13 +738,14 @@ fn try_invalid_transfers( transfer( test_a, ALBERT, - &receiver, + receiver.to_string(), NAM, "10", ALBERT_KEY, &"port".parse().unwrap(), channel_id_a, None, + None, Some("Error trying to apply a transaction"), false, )?; @@ -734,13 +754,14 @@ fn try_invalid_transfers( transfer( test_a, ALBERT, - &receiver, + receiver.to_string(), NAM, "10", ALBERT_KEY, port_id_a, &"channel-42".parse().unwrap(), None, + None, Some("Error trying to apply a transaction"), false, )?; @@ -795,7 +816,7 @@ fn transfer_back( let height = transfer( test_b, BERTHA, - &receiver, + receiver.to_string(), ibc_denom, "50000", BERTHA_KEY, @@ -803,6 +824,7 @@ fn transfer_back( channel_id_b, None, None, + None, false, )?; let events = get_events(test_b, height)?; @@ -858,12 +880,13 @@ fn transfer_timeout( let height = transfer( test_a, ALBERT, - &receiver, + receiver.to_string(), NAM, "100000", ALBERT_KEY, port_id_a, channel_id_a, + None, Some(Duration::new(5, 0)), None, false, @@ -893,6 +916,116 @@ fn transfer_timeout( Ok(()) } +#[allow(clippy::too_many_arguments)] +fn shielded_transfer( + test_a: &Test, + test_b: &Test, + client_id_a: &ClientId, + client_id_b: &ClientId, + port_id_a: &PortId, + channel_id_a: &ChannelId, + port_id_b: &PortId, + channel_id_b: &ChannelId, +) -> Result<()> { + // Get masp proof for the following IBC transfer from the destination chain + // It will send 10 BTC from Chain A to PA(B) on Chain B + let sender = find_address(test_a, ALBERT)?; + let rpc_b = get_actor_rpc(test_b, &Who::Validator(0)); + let output_folder = test_b.test_dir.path().to_string_lossy(); + let amount = Amount::native_whole(10).to_string_native(); + let args = [ + "ibc-gen-shielded", + "--output-folder-path", + &output_folder, + "--sender", + &sender.to_string(), + "--target", + AB_PAYMENT_ADDRESS, + "--token", + BTC, + "--amount", + &amount, + "--port-id", + port_id_b.as_ref(), + "--channel-id", + channel_id_b.as_ref(), + "--node", + &rpc_b, + ]; + let mut client = run!(test_b, Bin::Client, args, Some(120))?; + let file_path = get_shielded_transfer_path(&mut client)?; + client.assert_success(); + + // Send a token from Chain A to PA(B) on Chain B + let amount = Amount::native_whole(10).to_string_native(); + let height = transfer( + test_a, + ALBERT, + AB_PAYMENT_ADDRESS, + BTC, + amount, + ALBERT_KEY, + port_id_a, + channel_id_a, + Some(&file_path.to_string_lossy()), + None, + None, + false, + )?; + let events = get_events(test_a, height)?; + let packet = + get_packet_from_events(&events).ok_or(eyre!("Transaction failed"))?; + check_ibc_packet_query(test_a, &"send_packet".parse().unwrap(), &packet)?; + + let height_a = query_height(test_a)?; + let proof_commitment_on_a = + get_commitment_proof(test_a, &packet, height_a)?; + let msg = MsgRecvPacket { + packet, + proof_commitment_on_a, + proof_height_on_a: height_a, + signer: signer(), + }; + // Update the client state of Chain A on Chain B + update_client_with_height(test_a, test_b, client_id_b, height_a)?; + // Receive the token on Chain B + let height = submit_ibc_tx(test_b, msg, ALBERT, ALBERT_KEY, false)?; + let events = get_events(test_b, height)?; + let packet = + get_packet_from_events(&events).ok_or(eyre!("Transaction failed"))?; + let ack = + get_ack_from_events(&events).ok_or(eyre!("Transaction failed"))?; + check_ibc_packet_query( + test_b, + &"write_acknowledgement".parse().unwrap(), + &packet, + )?; + + // get the proof on Chain B + let height_b = query_height(test_b)?; + let proof_acked_on_b = get_ack_proof(test_b, &packet, height_b)?; + let msg = MsgAcknowledgement { + packet, + acknowledgement: ack.try_into().expect("invalid ack"), + proof_acked_on_b, + proof_height_on_b: height_b, + signer: signer(), + }; + // Update the client state of Chain B on Chain A + update_client_with_height(test_b, test_a, client_id_a, height_b)?; + // Acknowledge on Chain A + submit_ibc_tx(test_a, msg, ALBERT, ALBERT_KEY, false)?; + + Ok(()) +} + +fn get_shielded_transfer_path(client: &mut NamadaCmd) -> Result { + let (_unread, matched) = + client.exp_regex("Output IBC shielded transfer .*")?; + let file_path = matched.trim().split(' ').last().expect("invalid output"); + Ok(PathBuf::from_str(file_path).expect("invalid file path")) +} + fn get_commitment_proof( test: &Test, packet: &Packet, @@ -987,19 +1120,19 @@ fn submit_ibc_tx( fn transfer( test: &Test, sender: impl AsRef, - receiver: &Address, + receiver: impl AsRef, token: impl AsRef, amount: impl AsRef, signer: impl AsRef, port_id: &PortId, channel_id: &ChannelId, + memo: Option<&str>, timeout_sec: Option, expected_err: Option<&str>, wait_reveal_pk: bool, ) -> Result { let rpc = get_actor_rpc(test, &Who::Validator(0)); - let receiver = receiver.to_string(); let channel_id = channel_id.to_string(); let port_id = port_id.to_string(); let mut tx_args = vec![ @@ -1007,7 +1140,7 @@ fn transfer( "--source", sender.as_ref(), "--receiver", - &receiver, + receiver.as_ref(), "--signing-keys", signer.as_ref(), "--token", @@ -1022,13 +1155,19 @@ fn transfer( &rpc, ]; + let memo_path = memo.unwrap_or_default(); + if memo.is_some() { + tx_args.push("--memo-path"); + tx_args.push(memo_path); + } + let timeout = timeout_sec.unwrap_or_default().as_secs().to_string(); if timeout_sec.is_some() { tx_args.push("--timeout-sec-offset"); tx_args.push(&timeout); } - let mut client = run!(test, Bin::Client, tx_args, Some(40))?; + let mut client = run!(test, Bin::Client, tx_args, Some(300))?; match expected_err { Some(err) => { client.exp_string(err)?; @@ -1124,10 +1263,7 @@ fn check_ibc_update_query( client_id, &consensus_height, )) { - Ok(Some(event)) => { - println!("Found the update event: {:?}", event); - Ok(()) - } + Ok(Some(_)) => Ok(()), Ok(None) => Err(eyre!("No update event for the client {}", client_id)), Err(e) => Err(eyre!("IBC update event query failed: {}", e)), } @@ -1150,10 +1286,7 @@ fn check_ibc_packet_query( &packet.chan_id_on_b, &packet.seq_on_a, )) { - Ok(Some(event)) => { - println!("Found the packet event: {:?}", event); - Ok(()) - } + Ok(Some(_)) => Ok(()), Ok(None) => Err(eyre!("No packet event for the packet {}", packet)), Err(e) => Err(eyre!("IBC packet event query failed: {}", e)), } @@ -1297,6 +1430,33 @@ fn check_balances_after_back( Ok(()) } +/// Check balances after IBC shielded transfer +fn check_shielded_balances( + dest_port_id: &PortId, + dest_channel_id: &ChannelId, + test_b: &Test, +) -> Result<()> { + // Check the balance on Chain B + let token = find_address(test_b, BTC)?; + let denom = format!("{}/{}/{}", dest_port_id, dest_channel_id, &token); + let ibc_token = ibc_token(denom).to_string(); + let rpc_b = get_actor_rpc(test_b, &Who::Validator(0)); + let query_args = vec![ + "balance", + "--owner", + AB_VIEWING_KEY, + "--token", + &ibc_token, + "--node", + &rpc_b, + ]; + let expected = format!("{}: 10", ibc_token); + let mut client = run!(test_b, Bin::Client, query_args, Some(40))?; + client.exp_string(&expected)?; + client.assert_success(); + Ok(()) +} + fn signer() -> Signer { "signer".to_string().into() } From 94e6c2d53a09e17005b984c0b4dfe623a1ba5357 Mon Sep 17 00:00:00 2001 From: yito88 Date: Fri, 22 Sep 2023 22:18:41 +0200 Subject: [PATCH 04/10] workaround for decoding asset types for IbcToken --- shared/src/sdk/tx.rs | 25 ++++++++++++++++++++----- tests/src/e2e/ibc_tests.rs | 1 + 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/shared/src/sdk/tx.rs b/shared/src/sdk/tx.rs index 56b05395a6..773ccbd635 100644 --- a/shared/src/sdk/tx.rs +++ b/shared/src/sdk/tx.rs @@ -47,7 +47,9 @@ use crate::proto::{MaspBuilder, Tx}; use crate::sdk::args::{self, InputAmount}; use crate::sdk::error::{EncodingError, Error, QueryError, Result, TxError}; use crate::sdk::masp::TransferErr::Build; -use crate::sdk::masp::{ShieldedContext, ShieldedTransfer, ShieldedUtils}; +use crate::sdk::masp::{ + make_asset_type, ShieldedContext, ShieldedTransfer, ShieldedUtils, +}; use crate::sdk::rpc::{ self, format_denominated_amount, query_wasm_code_hash, validate_amount, TxBroadcastData, TxResponse, @@ -2042,7 +2044,7 @@ pub async fn gen_ibc_shielded_transfer< .await .expect("expected to validate amount"); - let shielded = shielded + let shielded_transfer = shielded .gen_shielded_transfer::<_, IO>( client, &TransferSource::Address(source.clone()), @@ -2052,18 +2054,31 @@ pub async fn gen_ibc_shielded_transfer< ) .await .map_err(|err| TxError::MaspError(err.to_string()))?; + let transfer = token::Transfer { source: source.clone(), target: masp(), - token, + token: token.clone(), amount: validated_amount, key, shielded: None, }; - if let Some(shielded) = shielded { + if let Some(shielded_transfer) = shielded_transfer { + // TODO: Workaround for decoding the asset_type later + let mut asset_types = Vec::new(); + for denom in MaspDenom::iter() { + let epoch = shielded_transfer.epoch; + let asset_type = make_asset_type(Some(epoch), &token, denom)?; + shielded + .asset_types + .insert(asset_type, (token.clone(), denom, epoch)); + asset_types.push(asset_type); + } + let _ = shielded.save().await; + Ok(Some(IbcShieldedTransfer { transfer, - masp_tx: shielded.masp_tx, + masp_tx: shielded_transfer.masp_tx, })) } else { Ok(None) diff --git a/tests/src/e2e/ibc_tests.rs b/tests/src/e2e/ibc_tests.rs index e5f91b8c7c..01f71cb953 100644 --- a/tests/src/e2e/ibc_tests.rs +++ b/tests/src/e2e/ibc_tests.rs @@ -1447,6 +1447,7 @@ fn check_shielded_balances( AB_VIEWING_KEY, "--token", &ibc_token, + "--no-conversions", "--node", &rpc_b, ]; From c8f84261b1f839345046e10d498845eb9e624546 Mon Sep 17 00:00:00 2001 From: yito88 Date: Fri, 29 Sep 2023 15:14:29 +0200 Subject: [PATCH 05/10] fix after merge --- core/src/ledger/ibc/mod.rs | 12 +++++------- tests/src/e2e/ibc_tests.rs | 7 ++----- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 277a19011b..c997e88c3b 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -159,11 +159,7 @@ where // denomination is also set for the minting. self.ctx .borrow_mut() - .store_ibc_denom( - &receiver.to_string(), - &trace_hash, - &ibc_denom, - ) + .store_ibc_denom(&receiver, &trace_hash, &ibc_denom) .map_err(|e| { Error::Denom(format!( "Writing the IBC denom failed: {}", @@ -203,7 +199,7 @@ where /// events fn get_minted_token_info( &self, - ) -> Result, Error> { + ) -> Result, Error> { let receive_event = self.ctx.borrow().get_ibc_event(EVENT_TYPE_PACKET).map_err( |_| Error::Denom("Reading the IBC event failed".to_string()), @@ -212,6 +208,7 @@ where .as_ref() .and_then(|event| event.attributes.get("receiver")) { + // Check the receiver address Some(receiver) => Some( Address::decode(receiver) .or_else(|_| { @@ -224,7 +221,8 @@ where "Decoding the receiver address failed: {:?}", receive_event )) - })?, + })? + .to_string(), ), None => None, }; diff --git a/tests/src/e2e/ibc_tests.rs b/tests/src/e2e/ibc_tests.rs index 01f71cb953..7eb99c7e44 100644 --- a/tests/src/e2e/ibc_tests.rs +++ b/tests/src/e2e/ibc_tests.rs @@ -1437,21 +1437,18 @@ fn check_shielded_balances( test_b: &Test, ) -> Result<()> { // Check the balance on Chain B - let token = find_address(test_b, BTC)?; - let denom = format!("{}/{}/{}", dest_port_id, dest_channel_id, &token); - let ibc_token = ibc_token(denom).to_string(); let rpc_b = get_actor_rpc(test_b, &Who::Validator(0)); let query_args = vec![ "balance", "--owner", AB_VIEWING_KEY, "--token", - &ibc_token, + BTC, "--no-conversions", "--node", &rpc_b, ]; - let expected = format!("{}: 10", ibc_token); + let expected = format!("{}/{}/btc: 10", dest_port_id, dest_channel_id); let mut client = run!(test_b, Bin::Client, query_args, Some(40))?; client.exp_string(&expected)?; client.assert_success(); From 27f4e0a177923d1da3233e53bf897910538f4c4a Mon Sep 17 00:00:00 2001 From: yito88 Date: Fri, 29 Sep 2023 21:08:20 +0200 Subject: [PATCH 06/10] fix get_shielded_action --- core/src/ledger/ibc/mod.rs | 2 +- core/src/ledger/vp_env.rs | 60 ++++++++++++++++++-------------------- core/src/types/ibc.rs | 4 +-- 3 files changed, 32 insertions(+), 34 deletions(-) diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index c997e88c3b..c4db5fae54 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -270,7 +270,7 @@ where let event = self .ctx .borrow() - .get_ibc_event("fungible_token_packet") + .get_ibc_event(EVENT_TYPE_PACKET) .map_err(|_| { Error::MaspTx( "Reading the IBC event failed".to_string(), diff --git a/core/src/ledger/vp_env.rs b/core/src/ledger/vp_env.rs index c62cf2982d..5bac346b48 100644 --- a/core/src/ledger/vp_env.rs +++ b/core/src/ledger/vp_env.rs @@ -8,7 +8,7 @@ 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}; +use crate::types::ibc::{get_shielded_transfer, IbcEvent, EVENT_TYPE_PACKET}; use crate::types::storage::{ BlockHash, BlockHeight, Epoch, Header, Key, TxIndex, }; @@ -112,37 +112,35 @@ where tx_data: Tx, ) -> Result<(Transfer, Transaction), storage_api::Error> { let signed = tx_data; - match Transfer::try_from_slice(&signed.data().unwrap()[..]) { - Ok(transfer) => { - let shielded_hash = transfer - .shielded - .ok_or_err_msg("unable to find shielded hash")?; - let masp_tx = signed - .get_section(&shielded_hash) - .and_then(|x| x.as_ref().masp_tx()) - .ok_or_err_msg("unable to find shielded section")?; - Ok((transfer, masp_tx)) - } - Err(_) => { - if let Some(event) = - self.get_ibc_event("fungible_token_packet".to_string())? - { - if let Some(shielded) = - get_shielded_transfer(&event).into_storage_result()? - { - Ok((shielded.transfer, shielded.masp_tx)) - } else { - Err(storage_api::Error::new_const( - "No shielded transfer in the IBC event", - )) - } - } else { - Err(storage_api::Error::new_const( - "No IBC event for the shielded action", - )) - } - } + if let Ok(transfer) = + Transfer::try_from_slice(&signed.data().unwrap()[..]) + { + let shielded_hash = transfer + .shielded + .ok_or_err_msg("unable to find shielded hash")?; + let masp_tx = signed + .get_section(&shielded_hash) + .and_then(|x| x.as_ref().masp_tx()) + .ok_or_err_msg("unable to find shielded section")?; + return Ok((transfer, masp_tx)); } + + // Shielded transfer over IBC + let event = self + .get_ibc_event(EVENT_TYPE_PACKET.to_string())? + .ok_or_else(|| { + storage_api::Error::new_const( + "No IBC event for the shielded action", + ) + })?; + get_shielded_transfer(&event) + .into_storage_result()? + .map(|shielded| (shielded.transfer, shielded.masp_tx)) + .ok_or_else(|| { + storage_api::Error::new_const( + "No shielded transfer in the IBC event", + ) + }) } /// Verify a MASP transaction diff --git a/core/src/types/ibc.rs b/core/src/types/ibc.rs index 6ff3c911f2..daf736df46 100644 --- a/core/src/types/ibc.rs +++ b/core/src/types/ibc.rs @@ -68,7 +68,7 @@ mod ibc_rs_conversion { use data_encoding::HEXUPPER; use thiserror::Error; - use super::{IbcEvent, IbcShieldedTransfer}; + use super::{IbcEvent, IbcShieldedTransfer, EVENT_TYPE_PACKET}; use crate::ibc::applications::transfer::{Memo, PrefixedDenom, TracePath}; use crate::ibc::core::events::{ Error as IbcEventError, IbcEvent as RawIbcEvent, @@ -146,7 +146,7 @@ mod ibc_rs_conversion { pub fn get_shielded_transfer( event: &IbcEvent, ) -> Result> { - if event.event_type != "fungible_token_packet" { + if event.event_type != EVENT_TYPE_PACKET { // This event is not for receiving a token return Ok(None); } From 68064109a072f1f3b91a8b0f940a2ef2f1dd534f Mon Sep 17 00:00:00 2001 From: yito88 Date: Fri, 29 Sep 2023 23:29:43 +0200 Subject: [PATCH 07/10] remove Address::Foreign --- apps/src/lib/cli.rs | 4 --- core/src/ledger/storage_api/account.rs | 1 - core/src/types/address.rs | 14 -------- shared/src/ledger/protocol/mod.rs | 1 - shared/src/sdk/args.rs | 2 -- shared/src/sdk/rpc.rs | 1 - shared/src/sdk/signing.rs | 7 +--- shared/src/sdk/tx.rs | 50 ++++++++++++++------------ tests/src/e2e/ibc_tests.rs | 3 -- 9 files changed, 28 insertions(+), 55 deletions(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 2980b21e49..66c1f73c3a 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -4773,7 +4773,6 @@ pub mod args { GenIbcShieldedTransafer:: { query: self.query.to_sdk(ctx), output_folder: self.output_folder, - sender: self.sender, target: ctx.get(&self.target), token: ctx.get(&self.token), amount: self.amount, @@ -4787,7 +4786,6 @@ pub mod args { fn parse(matches: &ArgMatches) -> Self { let query = Query::parse(matches); let output_folder = OUTPUT_FOLDER_PATH.parse(matches); - let sender = SENDER.parse(matches); let target = TRANSFER_TARGET.parse(matches); let token = TOKEN.parse(matches); let amount = InputAmount::Unvalidated(AMOUNT.parse(matches)); @@ -4796,7 +4794,6 @@ pub mod args { Self { query, output_folder, - sender, target, token, amount, @@ -4810,7 +4807,6 @@ pub mod args { .arg(OUTPUT_FOLDER_PATH.def().help( "The output folder path where the artifact will be stored.", )) - .arg(SENDER.def().help("The foreign sender address.")) .arg(TRANSFER_TARGET.def().help("The target address.")) .arg(TOKEN.def().help("The transfer token.")) .arg(AMOUNT.def().help("The amount to transfer in decimal.")) diff --git a/core/src/ledger/storage_api/account.rs b/core/src/ledger/storage_api/account.rs index b3cdd5fb67..5fa0abb8f9 100644 --- a/core/src/ledger/storage_api/account.rs +++ b/core/src/ledger/storage_api/account.rs @@ -77,7 +77,6 @@ where } Address::Implicit(_) => Ok(true), Address::Internal(_) => Ok(false), - Address::Foreign(_) => Ok(false), } } diff --git a/core/src/types/address.rs b/core/src/types/address.rs index 32cc7de99b..ecb0670904 100644 --- a/core/src/types/address.rs +++ b/core/src/types/address.rs @@ -96,8 +96,6 @@ const PREFIX_ESTABLISHED: &str = "est"; const PREFIX_IMPLICIT: &str = "imp"; /// Fixed-length address strings prefix for internal addresses. const PREFIX_INTERNAL: &str = "ano"; -/// Fixed-length address strings prefix for foreign addresses. -const PREFIX_FOREIGN: &str = "for"; /// Fixed-length address strings prefix for IBC addresses. const PREFIX_IBC: &str = "ibc"; /// Fixed-length address strings prefix for Ethereum addresses. @@ -136,8 +134,6 @@ pub enum Address { Implicit(ImplicitAddress), /// An internal address represents a module with a native VP Internal(InternalAddress), - /// An foreign address is provided from other chains - Foreign(String), } // We're using the string format of addresses (bech32m) for ordering to ensure @@ -200,7 +196,6 @@ impl Address { Some(hash_hex) } Address::Internal(_) => None, - Address::Foreign(_) => None, } } @@ -259,9 +254,6 @@ impl Address { debug_assert_eq!(string.len(), FIXED_LEN_STRING_BYTES); string } - Address::Foreign(addr) => { - format!("{}::{}", PREFIX_FOREIGN, addr) - } } .into_bytes(); string.resize(FIXED_LEN_STRING_BYTES, b' '); @@ -382,9 +374,6 @@ impl Address { "Invalid ERC20 internal address".to_string(), )), }, - Some((PREFIX_FOREIGN, raw)) => { - Ok(Address::Foreign(raw.to_string())) - } _ => Err(DecodeError::InvalidInnerEncoding( ErrorKind::InvalidData, "Invalid address prefix".to_string(), @@ -408,9 +397,6 @@ impl Address { Address::Internal(kind) => { format!("Internal {}: {}", kind, self.encode()) } - Address::Foreign(_) => { - format!("Foreign: {}", self.encode()) - } } } } diff --git a/shared/src/ledger/protocol/mod.rs b/shared/src/ledger/protocol/mod.rs index c1a518e023..a23b026eea 100644 --- a/shared/src/ledger/protocol/mod.rs +++ b/shared/src/ledger/protocol/mod.rs @@ -973,7 +973,6 @@ where accepted } - Address::Foreign(_) => Ok(true), }; // Returning error from here will short-circuit the VP parallel diff --git a/shared/src/sdk/args.rs b/shared/src/sdk/args.rs index ff031d8f62..df782ca497 100644 --- a/shared/src/sdk/args.rs +++ b/shared/src/sdk/args.rs @@ -893,8 +893,6 @@ pub struct GenIbcShieldedTransafer { pub query: Query, /// The output directory path to where serialize the data pub output_folder: Option, - /// The foreign sender address - pub sender: String, /// The target address pub target: C::TransferTarget, /// The token address diff --git a/shared/src/sdk/rpc.rs b/shared/src/sdk/rpc.rs index 5c69d743d9..d98e0b61ec 100644 --- a/shared/src/sdk/rpc.rs +++ b/shared/src/sdk/rpc.rs @@ -217,7 +217,6 @@ pub async fn known_address( query_has_storage_key(client, &key).await } Address::Implicit(_) | Address::Internal(_) => Ok(true), - Address::Foreign(_) => Ok(false), } } diff --git a/shared/src/sdk/signing.rs b/shared/src/sdk/signing.rs index 48695e9069..5ff3376c23 100644 --- a/shared/src/sdk/signing.rs +++ b/shared/src/sdk/signing.rs @@ -120,10 +120,6 @@ pub async fn find_pk< "Internal address {} doesn't have any signing keys.", addr )), - Address::Foreign(_) => other_err(format!( - "Foreign address {} doesn't have any signing keys.", - addr - )), } } @@ -278,8 +274,7 @@ pub async fn aux_signing_data< Some(AccountPublicKeysMap::from_iter(public_keys.clone())), 1u8, ), - Some(owner @ Address::Internal(_)) - | Some(owner @ Address::Foreign(_)) => { + Some(owner @ Address::Internal(_)) => { return Err(Error::from(TxError::InvalidAccount(owner.encode()))); } None => (None, 0u8), diff --git a/shared/src/sdk/tx.rs b/shared/src/sdk/tx.rs index 773ccbd635..16aa79170a 100644 --- a/shared/src/sdk/tx.rs +++ b/shared/src/sdk/tx.rs @@ -24,7 +24,7 @@ use namada_core::ledger::governance::storage::proposal::ProposalType; use namada_core::ledger::governance::storage::vote::StorageProposalVote; use namada_core::ledger::ibc::storage::channel_key; use namada_core::ledger::pgf::cli::steward::Commission; -use namada_core::types::address::{masp, Address}; +use namada_core::types::address::{masp, Address, InternalAddress}; use namada_core::types::dec::Dec; use namada_core::types::hash::Hash; use namada_core::types::token::MaspDenom; @@ -2023,7 +2023,7 @@ pub async fn gen_ibc_shielded_transfer< Some(_) => None, None => return Ok(None), }; - let source = Address::Foreign(args.sender.clone()); + let source = Address::Internal(InternalAddress::Ibc); let (src_port_id, src_channel_id) = get_ibc_src_port_channel(client, &args.port_id, &args.channel_id) .await?; @@ -2094,30 +2094,34 @@ async fn get_ibc_src_port_channel( use crate::ibc_proto::protobuf::Protobuf; let channel_key = channel_key(dest_port_id, dest_channel_id); - match rpc::query_storage_value_bytes::(client, &channel_key, None, false) - .await - { - Ok((Some(bytes), _)) => { - let channel = ChannelEnd::decode_vec(&bytes).map_err(|_| { + let bytes = + rpc::query_storage_value_bytes::(client, &channel_key, None, false) + .await? + .0 + .ok_or_else(|| { Error::Other(format!( - "Decoding channel end failed: port {}, channel {}", - dest_port_id, dest_channel_id + "No channel end: port {dest_port_id}, channel \ + {dest_channel_id}" )) })?; - if let Some(src_channel) = channel.remote.channel_id() { - Ok((channel.remote.port_id.clone(), src_channel.clone())) - } else { - Err(Error::Other(format!( - "The source channel doesn't exist: port {dest_port_id}, \ - channel {dest_channel_id}" - ))) - } - } - _ => Err(Error::Other(format!( - "Reading channel end failed: port {dest_port_id}, channel \ - {dest_channel_id}" - ))), - } + let channel = ChannelEnd::decode_vec(&bytes).map_err(|_| { + Error::Other(format!( + "Decoding channel end failed: port {dest_port_id}, channel \ + {dest_channel_id}", + )) + })?; + channel + .remote + .channel_id() + .map(|src_channel| { + (channel.remote.port_id.clone(), src_channel.clone()) + }) + .ok_or_else(|| { + Error::Other(format!( + "The source channel doesn't exist: port {dest_port_id}, \ + channel {dest_channel_id}" + )) + }) } async fn expect_dry_broadcast< diff --git a/tests/src/e2e/ibc_tests.rs b/tests/src/e2e/ibc_tests.rs index 7eb99c7e44..584974c61a 100644 --- a/tests/src/e2e/ibc_tests.rs +++ b/tests/src/e2e/ibc_tests.rs @@ -929,7 +929,6 @@ fn shielded_transfer( ) -> Result<()> { // Get masp proof for the following IBC transfer from the destination chain // It will send 10 BTC from Chain A to PA(B) on Chain B - let sender = find_address(test_a, ALBERT)?; let rpc_b = get_actor_rpc(test_b, &Who::Validator(0)); let output_folder = test_b.test_dir.path().to_string_lossy(); let amount = Amount::native_whole(10).to_string_native(); @@ -937,8 +936,6 @@ fn shielded_transfer( "ibc-gen-shielded", "--output-folder-path", &output_folder, - "--sender", - &sender.to_string(), "--target", AB_PAYMENT_ADDRESS, "--token", From 6656f1b276563998475d511e36ff6b8619ea7e57 Mon Sep 17 00:00:00 2001 From: yito88 Date: Mon, 2 Oct 2023 22:41:16 +0200 Subject: [PATCH 08/10] fix get_ibc_event --- core/src/ledger/ibc/context/storage.rs | 6 +- core/src/ledger/ibc/mod.rs | 26 ++++++--- core/src/ledger/tx_env.rs | 6 +- core/src/ledger/vp_env.rs | 22 +++---- shared/src/ledger/native_vp/ibc/context.rs | 20 +++---- shared/src/ledger/native_vp/mod.rs | 6 +- shared/src/ledger/vp_host_fns.rs | 16 +++--- shared/src/vm/host_env.rs | 67 +++++++++++----------- shared/src/vm/wasm/host_env.rs | 4 +- tests/src/vm_host_env/tx.rs | 2 +- tx_prelude/src/ibc.rs | 6 +- tx_prelude/src/lib.rs | 14 ++--- vm_env/src/lib.rs | 6 +- vp_prelude/src/lib.rs | 13 +++-- 14 files changed, 110 insertions(+), 104 deletions(-) diff --git a/core/src/ledger/ibc/context/storage.rs b/core/src/ledger/ibc/context/storage.rs index ef8b61116d..ef24cd94f2 100644 --- a/core/src/ledger/ibc/context/storage.rs +++ b/core/src/ledger/ibc/context/storage.rs @@ -55,11 +55,11 @@ pub trait IbcStorageContext { /// Emit an IBC event fn emit_ibc_event(&mut self, event: IbcEvent) -> Result<(), Self::Error>; - /// Get an IBC event - fn get_ibc_event( + /// Get IBC events + fn get_ibc_events( &self, event_type: impl AsRef, - ) -> Result, Self::Error>; + ) -> Result, Self::Error>; /// Transfer token fn transfer_token( diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index c4db5fae54..c78afeabd6 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -200,11 +200,16 @@ where fn get_minted_token_info( &self, ) -> Result, Error> { - let receive_event = - self.ctx.borrow().get_ibc_event(EVENT_TYPE_PACKET).map_err( - |_| Error::Denom("Reading the IBC event failed".to_string()), - )?; + let receive_event = self + .ctx + .borrow() + .get_ibc_events(EVENT_TYPE_PACKET) + .map_err(|_| { + Error::Denom("Reading the IBC event failed".to_string()) + })?; + // The receiving event should be only one in the single IBC transaction let receiver = match receive_event + .first() .as_ref() .and_then(|event| event.attributes.get("receiver")) { @@ -229,11 +234,12 @@ where let denom_event = self .ctx .borrow() - .get_ibc_event(EVENT_TYPE_DENOM_TRACE) + .get_ibc_events(EVENT_TYPE_DENOM_TRACE) .map_err(|_| { Error::Denom("Reading the IBC event failed".to_string()) })?; - Ok(denom_event.as_ref().and_then(|event| { + // The denom event should be only one in the single IBC transaction + Ok(denom_event.first().as_ref().and_then(|event| { let trace_hash = event.attributes.get("trace_hash").cloned()?; let denom = event.attributes.get("denom").cloned()?; Some((trace_hash, denom, receiver?)) @@ -270,14 +276,16 @@ where let event = self .ctx .borrow() - .get_ibc_event(EVENT_TYPE_PACKET) + .get_ibc_events(EVENT_TYPE_PACKET) .map_err(|_| { Error::MaspTx( "Reading the IBC event failed".to_string(), ) })?; - match event { - Some(event) => get_shielded_transfer(&event) + // The receiving event should be only one in the single IBC + // transaction + match event.first() { + Some(event) => get_shielded_transfer(event) .map_err(|e| Error::MaspTx(e.to_string()))?, None => return Ok(()), } diff --git a/core/src/ledger/tx_env.rs b/core/src/ledger/tx_env.rs index 61b7824e64..ad8b23e60c 100644 --- a/core/src/ledger/tx_env.rs +++ b/core/src/ledger/tx_env.rs @@ -59,9 +59,9 @@ pub trait TxEnv: StorageRead + StorageWrite { /// Request to charge the provided amount of gas for the current transaction fn charge_gas(&mut self, used_gas: u64) -> Result<(), storage_api::Error>; - /// Get an IBC event with a event type - fn get_ibc_event( + /// Get IBC events with a event type + fn get_ibc_events( &self, event_type: impl AsRef, - ) -> Result, storage_api::Error>; + ) -> Result, storage_api::Error>; } diff --git a/core/src/ledger/vp_env.rs b/core/src/ledger/vp_env.rs index 5bac346b48..1241adfed7 100644 --- a/core/src/ledger/vp_env.rs +++ b/core/src/ledger/vp_env.rs @@ -78,11 +78,11 @@ where /// Get the address of the native token. fn get_native_token(&self) -> Result; - /// Get the IBC event. - fn get_ibc_event( + /// Get the IBC events. + fn get_ibc_events( &self, event_type: String, - ) -> Result, storage_api::Error>; + ) -> Result, storage_api::Error>; /// Storage prefix iterator, ordered by storage keys. It will try to get an /// iterator from the storage. @@ -126,14 +126,14 @@ where } // Shielded transfer over IBC - let event = self - .get_ibc_event(EVENT_TYPE_PACKET.to_string())? - .ok_or_else(|| { - storage_api::Error::new_const( - "No IBC event for the shielded action", - ) - })?; - get_shielded_transfer(&event) + let events = self.get_ibc_events(EVENT_TYPE_PACKET.to_string())?; + // The receiving event should be only one in the single IBC transaction + let event = events.first().ok_or_else(|| { + storage_api::Error::new_const( + "No IBC event for the shielded action", + ) + })?; + get_shielded_transfer(event) .into_storage_result()? .map(|shielded| (shielded.transfer, shielded.masp_tx)) .ok_or_else(|| { diff --git a/shared/src/ledger/native_vp/ibc/context.rs b/shared/src/ledger/native_vp/ibc/context.rs index 93784b8054..0043737818 100644 --- a/shared/src/ledger/native_vp/ibc/context.rs +++ b/shared/src/ledger/native_vp/ibc/context.rs @@ -124,16 +124,16 @@ where Ok(()) } - fn get_ibc_event( + fn get_ibc_events( &self, event_type: impl AsRef, - ) -> Result, Self::Error> { - for event in &self.event { - if event.event_type == *event_type.as_ref() { - return Ok(Some(event.clone())); - } - } - Ok(None) + ) -> Result, Self::Error> { + Ok(self + .event + .iter() + .filter(|event| event.event_type == *event_type.as_ref()) + .cloned() + .collect()) } fn transfer_token( @@ -381,10 +381,10 @@ where unimplemented!("Validation doesn't emit an event") } - fn get_ibc_event( + fn get_ibc_events( &self, _event_type: impl AsRef, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { unimplemented!("Validation doesn't get an event") } diff --git a/shared/src/ledger/native_vp/mod.rs b/shared/src/ledger/native_vp/mod.rs index 002c256eb1..a75543718c 100644 --- a/shared/src/ledger/native_vp/mod.rs +++ b/shared/src/ledger/native_vp/mod.rs @@ -450,11 +450,11 @@ where .into_storage_result() } - fn get_ibc_event( + fn get_ibc_events( &self, event_type: String, - ) -> Result, storage_api::Error> { - vp_host_fns::get_ibc_event( + ) -> Result, storage_api::Error> { + vp_host_fns::get_ibc_events( &mut self.gas_meter.borrow_mut(), self.write_log, event_type, diff --git a/shared/src/ledger/vp_host_fns.rs b/shared/src/ledger/vp_host_fns.rs index c8e1bd12d7..ebe32d4f72 100644 --- a/shared/src/ledger/vp_host_fns.rs +++ b/shared/src/ledger/vp_host_fns.rs @@ -335,17 +335,17 @@ where } /// Getting the IBC event. -pub fn get_ibc_event( +pub fn get_ibc_events( _gas_meter: &mut VpGasMeter, write_log: &WriteLog, event_type: String, -) -> EnvResult> { - for event in write_log.get_ibc_events() { - if event.event_type == event_type { - return Ok(Some(event.clone())); - } - } - Ok(None) +) -> EnvResult> { + Ok(write_log + .get_ibc_events() + .iter() + .filter(|event| event.event_type == event_type) + .cloned() + .collect()) } /// Storage prefix iterator for prior state (before tx execution), ordered by diff --git a/shared/src/vm/host_env.rs b/shared/src/vm/host_env.rs index fd4129501d..65bcce69b0 100644 --- a/shared/src/vm/host_env.rs +++ b/shared/src/vm/host_env.rs @@ -977,7 +977,7 @@ where } /// Getting an IBC event function exposed to the wasm VM Tx environment. -pub fn tx_get_ibc_event( +pub fn tx_get_ibc_events( env: &TxVmEnv, event_type_ptr: u64, event_type_len: u64, @@ -994,20 +994,20 @@ where .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas(env, gas)?; let write_log = unsafe { env.ctx.write_log.get() }; - for event in write_log.get_ibc_events() { - if event.event_type == event_type { - let value = - event.try_to_vec().map_err(TxRuntimeError::EncodingError)?; - let len: i64 = value - .len() - .try_into() - .map_err(TxRuntimeError::NumConversionError)?; - let result_buffer = unsafe { env.ctx.result_buffer.get() }; - result_buffer.replace(value); - return Ok(len); - } - } - Ok(HostEnvResult::Fail.to_i64()) + let events: Vec = write_log + .get_ibc_events() + .iter() + .filter(|event| event.event_type == event_type) + .cloned() + .collect(); + let value = events.try_to_vec().map_err(TxRuntimeError::EncodingError)?; + let len: i64 = value + .len() + .try_into() + .map_err(TxRuntimeError::NumConversionError)?; + let result_buffer = unsafe { env.ctx.result_buffer.get() }; + result_buffer.replace(value); + Ok(len) } /// Storage read prior state (before tx execution) function exposed to the wasm @@ -1781,7 +1781,7 @@ where } /// Getting the IBC event function exposed to the wasm VM VP environment. -pub fn vp_get_ibc_event( +pub fn vp_get_ibc_events( env: &VpVmEnv, event_type_ptr: u64, event_type_len: u64, @@ -1801,21 +1801,17 @@ where vp_host_fns::add_gas(gas_meter, gas)?; let write_log = unsafe { env.ctx.write_log.get() }; - match vp_host_fns::get_ibc_event(gas_meter, write_log, event_type)? { - Some(event) => { - let value = event - .try_to_vec() - .map_err(vp_host_fns::RuntimeError::EncodingError)?; - let len: i64 = value - .len() - .try_into() - .map_err(vp_host_fns::RuntimeError::NumConversionError)?; - let result_buffer = unsafe { env.ctx.result_buffer.get() }; - result_buffer.replace(value); - Ok(len) - } - None => Ok(HostEnvResult::Fail.to_i64()), - } + let events = vp_host_fns::get_ibc_events(gas_meter, write_log, event_type)?; + let value = events + .try_to_vec() + .map_err(vp_host_fns::RuntimeError::EncodingError)?; + let len: i64 = value + .len() + .try_into() + .map_err(vp_host_fns::RuntimeError::NumConversionError)?; + let result_buffer = unsafe { env.ctx.result_buffer.get() }; + result_buffer.replace(value); + Ok(len) } /// Verify a transaction signature @@ -2245,16 +2241,17 @@ where ibc_tx_charge_gas(self, gas) } - fn get_ibc_event( + fn get_ibc_events( &self, event_type: impl AsRef, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { let write_log = unsafe { self.write_log.get() }; Ok(write_log .get_ibc_events() .iter() - .find(|event| event.event_type == event_type.as_ref()) - .cloned()) + .filter(|event| event.event_type == event_type.as_ref()) + .cloned() + .collect()) } fn transfer_token( diff --git a/shared/src/vm/wasm/host_env.rs b/shared/src/vm/wasm/host_env.rs index e5ebce6ee2..e9a63e631e 100644 --- a/shared/src/vm/wasm/host_env.rs +++ b/shared/src/vm/wasm/host_env.rs @@ -76,7 +76,7 @@ where "namada_tx_update_validity_predicate" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_update_validity_predicate), "namada_tx_init_account" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_init_account), "namada_tx_emit_ibc_event" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_emit_ibc_event), - "namada_tx_get_ibc_event" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_ibc_event), + "namada_tx_get_ibc_events" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_ibc_events), "namada_tx_get_chain_id" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_chain_id), "namada_tx_get_tx_index" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_tx_index), "namada_tx_get_block_height" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_block_height), @@ -127,7 +127,7 @@ where "namada_vp_get_block_hash" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_get_block_hash), "namada_vp_get_tx_code_hash" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_get_tx_code_hash), "namada_vp_get_block_epoch" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_get_block_epoch), - "namada_vp_get_ibc_event" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_get_ibc_event), + "namada_vp_get_ibc_events" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_get_ibc_events), "namada_vp_verify_tx_section_signature" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_verify_tx_section_signature), "namada_vp_verify_masp" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_verify_masp), "namada_vp_eval" => Function::new_native_with_env(wasm_store, env.clone(), host_env::vp_eval), diff --git a/tests/src/vm_host_env/tx.rs b/tests/src/vm_host_env/tx.rs index 582b668b79..ad425d09fd 100644 --- a/tests/src/vm_host_env/tx.rs +++ b/tests/src/vm_host_env/tx.rs @@ -447,7 +447,7 @@ mod native_tx_host_env { result_ptr: u64 )); native_host_fn!(tx_emit_ibc_event(event_ptr: u64, event_len: u64)); - native_host_fn!(tx_get_ibc_event(event_type_ptr: u64, event_type_len: u64) -> i64); + native_host_fn!(tx_get_ibc_events(event_type_ptr: u64, event_type_len: u64) -> i64); native_host_fn!(tx_get_chain_id(result_ptr: u64)); native_host_fn!(tx_get_block_height() -> u64); native_host_fn!(tx_get_tx_index() -> u32); diff --git a/tx_prelude/src/ibc.rs b/tx_prelude/src/ibc.rs index 5e8e9522fa..2f19c24a82 100644 --- a/tx_prelude/src/ibc.rs +++ b/tx_prelude/src/ibc.rs @@ -75,11 +75,11 @@ impl IbcStorageContext for Ctx { ::emit_ibc_event(self, &event) } - fn get_ibc_event( + fn get_ibc_events( &self, event_type: impl AsRef, - ) -> Result, Self::Error> { - ::get_ibc_event(self, &event_type) + ) -> Result, Self::Error> { + ::get_ibc_events(self, &event_type) } fn transfer_token( diff --git a/tx_prelude/src/lib.rs b/tx_prelude/src/lib.rs index 87d91e07a3..8ec36a7b30 100644 --- a/tx_prelude/src/lib.rs +++ b/tx_prelude/src/lib.rs @@ -331,23 +331,21 @@ impl TxEnv for Ctx { Ok(()) } - fn get_ibc_event( + fn get_ibc_events( &self, event_type: impl AsRef, - ) -> Result, Error> { + ) -> Result, Error> { let event_type = event_type.as_ref().to_string(); let read_result = unsafe { - namada_tx_get_ibc_event( + namada_tx_get_ibc_events( event_type.as_ptr() as _, event_type.len() as _, ) }; match read_from_buffer(read_result, namada_tx_result_buffer) { - Some(value) => Ok(Some( - ibc::IbcEvent::try_from_slice(&value[..]) - .expect("The conversion shouldn't fail"), - )), - None => Ok(None), + Some(value) => Ok(Vec::::try_from_slice(&value[..]) + .expect("The conversion shouldn't fail")), + None => Ok(Vec::new()), } } } diff --git a/vm_env/src/lib.rs b/vm_env/src/lib.rs index edf00f847c..baadd56182 100644 --- a/vm_env/src/lib.rs +++ b/vm_env/src/lib.rs @@ -78,8 +78,8 @@ pub mod tx { // Emit an IBC event pub fn namada_tx_emit_ibc_event(event_ptr: u64, event_len: u64); - // Get an IBC event - pub fn namada_tx_get_ibc_event( + // Get IBC events + pub fn namada_tx_get_ibc_events( event_type_ptr: u64, event_type_len: u64, ) -> i64; @@ -198,7 +198,7 @@ pub mod vp { pub fn namada_vp_get_native_token(result_ptr: u64); // Get the IBC event - pub fn namada_vp_get_ibc_event( + pub fn namada_vp_get_ibc_events( event_type_ptr: u64, event_type_len: u64, ) -> i64; diff --git a/vp_prelude/src/lib.rs b/vp_prelude/src/lib.rs index 9857574acd..60618cfacf 100644 --- a/vp_prelude/src/lib.rs +++ b/vp_prelude/src/lib.rs @@ -297,18 +297,21 @@ impl<'view> VpEnv<'view> for Ctx { get_native_token() } - fn get_ibc_event( + fn get_ibc_events( &self, event_type: String, - ) -> Result, Error> { + ) -> Result, Error> { let read_result = unsafe { - namada_vp_get_ibc_event( + namada_vp_get_ibc_events( event_type.as_ptr() as _, event_type.len() as _, ) }; - Ok(read_from_buffer(read_result, namada_vp_result_buffer) - .and_then(|t| ibc::IbcEvent::try_from_slice(&t[..]).ok())) + match read_from_buffer(read_result, namada_vp_result_buffer) { + Some(value) => Ok(Vec::::try_from_slice(&value[..]) + .expect("The conversion shouldn't fail")), + None => Ok(Vec::new()), + } } fn iter_prefix<'iter>( From ac2535a4b148e5a248b179a0bbb19496bc1ec030 Mon Sep 17 00:00:00 2001 From: yito88 Date: Fri, 6 Oct 2023 14:13:35 +0200 Subject: [PATCH 09/10] add changelog --- .changelog/unreleased/improvements/1917-ibc-shielded-actions.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/1917-ibc-shielded-actions.md diff --git a/.changelog/unreleased/improvements/1917-ibc-shielded-actions.md b/.changelog/unreleased/improvements/1917-ibc-shielded-actions.md new file mode 100644 index 0000000000..d9ec6743d0 --- /dev/null +++ b/.changelog/unreleased/improvements/1917-ibc-shielded-actions.md @@ -0,0 +1,2 @@ +- IBC transfer to a payment address + ([\#1917](https://github.com/anoma/namada/issues/1917)) \ No newline at end of file From af011f6fb2f44452a7beccb97047e6f02d253798 Mon Sep 17 00:00:00 2001 From: yito88 Date: Mon, 9 Oct 2023 17:40:52 +0200 Subject: [PATCH 10/10] fix error handling --- shared/src/sdk/tx.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/shared/src/sdk/tx.rs b/shared/src/sdk/tx.rs index 16aa79170a..2e506d6a63 100644 --- a/shared/src/sdk/tx.rs +++ b/shared/src/sdk/tx.rs @@ -2029,8 +2029,11 @@ pub async fn gen_ibc_shielded_transfer< .await?; let ibc_denom = rpc::query_ibc_denom::<_, IO>(client, &args.token, Some(&source)).await; + let prefixed_denom = ibc_denom + .parse() + .map_err(|_| Error::Other(format!("Invalid IBC denom: {ibc_denom}")))?; let token = namada_core::ledger::ibc::received_ibc_token( - &ibc_denom.parse().expect("Invalid IBC denom"), + &prefixed_denom, &src_port_id, &src_channel_id, &args.port_id, @@ -2040,9 +2043,7 @@ pub async fn gen_ibc_shielded_transfer< Error::Other(format!("Getting IBC Token failed: error {e}")) })?; let validated_amount = - validate_amount::<_, IO>(client, args.amount, &token, false) - .await - .expect("expected to validate amount"); + validate_amount::<_, IO>(client, args.amount, &token, false).await?; let shielded_transfer = shielded .gen_shielded_transfer::<_, IO>(