From b874ffaf17be6e61a4af43769ee1733ada3fbce1 Mon Sep 17 00:00:00 2001 From: yito88 Date: Mon, 2 Oct 2023 17:23:42 +0200 Subject: [PATCH] remove trace-path --- apps/src/lib/cli.rs | 10 --------- apps/src/lib/cli/context.rs | 16 +++++++++++++ apps/src/lib/client/rpc.rs | 7 +++--- benches/lib.rs | 1 - core/src/ledger/ibc/mod.rs | 4 ++-- core/src/types/ibc.rs | 7 +++--- shared/src/sdk/args.rs | 5 ----- shared/src/sdk/rpc.rs | 45 ++++++++++++++++++++++++++++++++++++- shared/src/sdk/signing.rs | 1 - shared/src/sdk/tx.rs | 39 +++++++++++++------------------- tests/src/e2e/ibc_tests.rs | 24 ++++---------------- 11 files changed, 89 insertions(+), 70 deletions(-) diff --git a/apps/src/lib/cli.rs b/apps/src/lib/cli.rs index 79db0b3824..135ff1e3c5 100644 --- a/apps/src/lib/cli.rs +++ b/apps/src/lib/cli.rs @@ -2516,7 +2516,6 @@ pub mod args { use std::path::PathBuf; use std::str::FromStr; - use namada::ibc::applications::transfer::TracePath; use namada::ibc::core::ics24_host::identifier::{ChannelId, PortId}; pub use namada::sdk::args::*; use namada::types::address::Address; @@ -2726,7 +2725,6 @@ pub mod args { pub const TM_ADDRESS: Arg = arg("tm-address"); pub const TOKEN_OPT: ArgOpt = TOKEN.opt(); pub const TOKEN: Arg = arg("token"); - pub const TRACE_PATH: ArgOpt = arg_opt("trace-path"); pub const TRANSFER_SOURCE: Arg = arg("source"); pub const TRANSFER_TARGET: Arg = arg("target"); pub const TX_HASH: Arg = arg("tx-hash"); @@ -3506,7 +3504,6 @@ pub mod args { source: ctx.get_cached(&self.source), target: ctx.get(&self.target), token: ctx.get(&self.token), - trace_path: self.trace_path, amount: self.amount, native_token: ctx.native_token.clone(), tx_code_path: self.tx_code_path.to_path_buf(), @@ -3520,7 +3517,6 @@ pub mod args { let source = TRANSFER_SOURCE.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 tx_code_path = PathBuf::from(TX_TRANSFER_WASM); Self { @@ -3528,7 +3524,6 @@ pub mod args { source, target, token, - trace_path, amount, native_token: (), tx_code_path, @@ -3546,7 +3541,6 @@ pub mod args { to produce the signature.", )) .arg(TOKEN.def().help("The transfer token.")) - .arg(TRACE_PATH.def().help("The transfer token's trace path.")) .arg(AMOUNT.def().help("The amount to transfer in decimal.")) } } @@ -3558,7 +3552,6 @@ pub mod args { source: ctx.get(&self.source), receiver: self.receiver, token: ctx.get(&self.token), - trace_path: self.trace_path, amount: self.amount, port_id: self.port_id, channel_id: self.channel_id, @@ -3576,7 +3569,6 @@ pub mod args { let source = SOURCE.parse(matches); let receiver = RECEIVER.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); @@ -3589,7 +3581,6 @@ pub mod args { source, receiver, token, - trace_path, amount, port_id, channel_id, @@ -3610,7 +3601,6 @@ pub mod args { "The receiver address on the destination chain as string.", )) .arg(TOKEN.def().help("The transfer token.")) - .arg(TRACE_PATH.def().help("The transfer token's trace path.")) .arg(AMOUNT.def().help("The amount to transfer in decimal.")) .arg(PORT_ID.def().help("The port ID.")) .arg(CHANNEL_ID.def().help("The channel ID.")) diff --git a/apps/src/lib/cli/context.rs b/apps/src/lib/cli/context.rs index 4aac8b1026..520e999189 100644 --- a/apps/src/lib/cli/context.rs +++ b/apps/src/lib/cli/context.rs @@ -6,11 +6,13 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use color_eyre::eyre::Result; +use namada::ledger::ibc::storage::ibc_token; use namada::sdk::masp::ShieldedContext; use namada::sdk::wallet::Wallet; use namada::types::address::{Address, InternalAddress}; use namada::types::chain::ChainId; use namada::types::ethereum_events::EthAddress; +use namada::types::ibc::is_ibc_denom; use namada::types::io::Io; use namada::types::key::*; use namada::types::masp::*; @@ -367,6 +369,20 @@ impl ArgFromContext for Address { }) .unwrap_or(Err(Skip)) }) + // An IBC token + .or_else(|_| { + is_ibc_denom(raw) + .map(|(trace_path, base_denom)| { + let base_token = ctx + .wallet + .find_address(&base_denom) + .map(|addr| addr.to_string()) + .unwrap_or(base_denom); + let ibc_denom = format!("{trace_path}/{base_token}"); + ibc_token(ibc_denom) + }) + .ok_or(Skip) + }) // Or it can be an alias that may be found in the wallet .or_else(|_| ctx.wallet.find_address(raw).cloned().ok_or(Skip)) .map_err(|_| format!("Unknown address {raw}")) diff --git a/apps/src/lib/client/rpc.rs b/apps/src/lib/client/rpc.rs index d3363d17a7..cfd13b42e2 100644 --- a/apps/src/lib/client/rpc.rs +++ b/apps/src/lib/client/rpc.rs @@ -50,7 +50,7 @@ use namada::sdk::wallet::{AddressVpType, Wallet}; use namada::types::address::{masp, Address, InternalAddress}; use namada::types::control_flow::ProceedOrElse; use namada::types::hash::Hash; -use namada::types::ibc::split_ibc_denom; +use namada::types::ibc::is_ibc_denom; use namada::types::io::Io; use namada::types::key::*; use namada::types::masp::{BalanceOwner, ExtendedViewingKey, PaymentAddress}; @@ -385,7 +385,7 @@ pub async fn query_transparent_balance< display_line!(IO, "{}: {}", token_alias, balance); } Err(e) => { - display_line!(IO, "Eror in querying: {e}"); + display_line!(IO, "Querying error: {e}"); display_line!( IO, "No {} balance found for {}", @@ -761,7 +761,7 @@ fn get_ibc_denom_alias( wallet: &Wallet, ibc_denom: impl AsRef, ) -> String { - split_ibc_denom(&ibc_denom) + is_ibc_denom(&ibc_denom) .map(|(trace_path, base_token)| { let base_token_alias = match Address::decode(&base_token) { Ok(base_token) => wallet.lookup_alias(&base_token), @@ -776,7 +776,6 @@ fn get_ibc_denom_alias( .unwrap_or(ibc_denom.as_ref().to_string()) } - /// Query Proposals pub async fn query_proposal< C: namada::ledger::queries::Client + Sync, diff --git a/benches/lib.rs b/benches/lib.rs index 30765d8beb..47645abdf4 100644 --- a/benches/lib.rs +++ b/benches/lib.rs @@ -783,7 +783,6 @@ impl BenchShieldedCtx { source: source.clone(), target: target.clone(), token: address::nam(), - trace_path: None, amount: InputAmount::Validated(DenominatedAmount { amount, denom: 0.into(), diff --git a/core/src/ledger/ibc/mod.rs b/core/src/ledger/ibc/mod.rs index 4900312a7f..40382282cd 100644 --- a/core/src/ledger/ibc/mod.rs +++ b/core/src/ledger/ibc/mod.rs @@ -29,7 +29,7 @@ use crate::ibc_proto::google::protobuf::Any; use crate::types::address::Address; use crate::types::chain::ChainId; use crate::types::ibc::{ - split_ibc_denom, EVENT_TYPE_DENOM_TRACE, EVENT_TYPE_PACKET, + is_ibc_denom, EVENT_TYPE_DENOM_TRACE, EVENT_TYPE_PACKET, }; #[allow(missing_docs)] @@ -160,7 +160,7 @@ where e )) })?; - if let Some((_, base_token)) = split_ibc_denom(&ibc_denom) { + if let Some((_, base_token)) = is_ibc_denom(&ibc_denom) { self.ctx .borrow_mut() .store_ibc_denom(base_token, trace_hash, &ibc_denom) diff --git a/core/src/types/ibc.rs b/core/src/types/ibc.rs index 4ee504fb4a..e7cb5f745f 100644 --- a/core/src/types/ibc.rs +++ b/core/src/types/ibc.rs @@ -94,10 +94,11 @@ mod ibc_rs_conversion { /// Returns the trace path and the token string if the denom is an IBC /// denom. - pub fn split_ibc_denom( - denom: impl AsRef, - ) -> Option<(TracePath, String)> { + pub fn is_ibc_denom(denom: impl AsRef) -> Option<(TracePath, String)> { let prefixed_denom = PrefixedDenom::from_str(denom.as_ref()).ok()?; + if prefixed_denom.trace_path.is_empty() { + return None; + } // The base token isn't decoded because it could be non Namada token Some(( prefixed_denom.trace_path, diff --git a/shared/src/sdk/args.rs b/shared/src/sdk/args.rs index af65abcad2..0b87317530 100644 --- a/shared/src/sdk/args.rs +++ b/shared/src/sdk/args.rs @@ -11,7 +11,6 @@ use namada_core::types::time::DateTimeUtc; use serde::{Deserialize, Serialize}; use zeroize::Zeroizing; -use crate::ibc::applications::transfer::TracePath; use crate::ibc::core::ics24_host::identifier::{ChannelId, PortId}; use crate::types::address::Address; use crate::types::keccak::KeccakHash; @@ -136,8 +135,6 @@ pub struct TxTransfer { pub target: C::TransferTarget, /// Transferred token address pub token: C::Address, - /// Transferred token's trace path - pub trace_path: Option, /// Transferred token amount pub amount: InputAmount, /// Native token address @@ -168,8 +165,6 @@ pub struct TxIbcTransfer { pub receiver: String, /// Transferred token address pub token: C::Address, - /// Transferred token's trace path - pub trace_path: Option, /// Transferred token amount pub amount: InputAmount, /// Port ID diff --git a/shared/src/sdk/rpc.rs b/shared/src/sdk/rpc.rs index 58609bed42..d98e0b61ec 100644 --- a/shared/src/sdk/rpc.rs +++ b/shared/src/sdk/rpc.rs @@ -13,7 +13,7 @@ use namada_core::ledger::governance::storage::proposal::StorageProposal; use namada_core::ledger::governance::utils::Vote; use namada_core::ledger::storage::LastBlock; use namada_core::types::account::Account; -use namada_core::types::address::Address; +use namada_core::types::address::{Address, InternalAddress}; use namada_core::types::storage::Key; use namada_core::types::token::{ Amount, DenominatedAmount, Denomination, MaspDenom, @@ -25,6 +25,9 @@ use namada_proof_of_stake::types::{ use serde::Serialize; use crate::ledger::events::Event; +use crate::ledger::ibc::storage::{ + ibc_denom_key, ibc_denom_key_prefix, is_ibc_denom_key, +}; use crate::ledger::queries::vp::pos::EnrichedBondsAndUnbondsDetails; use crate::ledger::queries::RPC; use crate::proto::Tx; @@ -1088,3 +1091,43 @@ pub async fn format_denominated_amount< }); DenominatedAmount { amount, denom }.to_string() } + +/// Look up the IBC denomination from a IbcToken. +pub async fn query_ibc_denom< + C: crate::ledger::queries::Client + Sync, + IO: Io, +>( + client: &C, + token: &Address, + owner: Option<&Address>, +) -> String { + let hash = match token { + Address::Internal(InternalAddress::IbcToken(hash)) => hash, + _ => return token.to_string(), + }; + + if let Some(owner) = owner { + let ibc_denom_key = ibc_denom_key(owner.to_string(), hash); + if let Ok(ibc_denom) = + query_storage_value::(client, &ibc_denom_key).await + { + return ibc_denom; + } + } + + // No owner is specified or the owner doesn't have the token + let ibc_denom_prefix = ibc_denom_key_prefix(None); + if let Ok(Some(ibc_denoms)) = + query_storage_prefix::(client, &ibc_denom_prefix).await + { + for (key, ibc_denom) in ibc_denoms { + if let Some((_, token_hash)) = is_ibc_denom_key(&key) { + if token_hash == *hash { + return ibc_denom; + } + } + } + } + + token.to_string() +} diff --git a/shared/src/sdk/signing.rs b/shared/src/sdk/signing.rs index 9f1d541faf..042be03a63 100644 --- a/shared/src/sdk/signing.rs +++ b/shared/src/sdk/signing.rs @@ -426,7 +426,6 @@ pub async fn wrap_tx< fee_payer_address.clone(), ), token: args.fee_token.clone(), - trace_path: None, amount: args::InputAmount::Validated(DenominatedAmount { // NOTE: must unshield the total fee amount, not the // diff, because the ledger evaluates the transaction in diff --git a/shared/src/sdk/tx.rs b/shared/src/sdk/tx.rs index 5b477de47e..e95ab58fe1 100644 --- a/shared/src/sdk/tx.rs +++ b/shared/src/sdk/tx.rs @@ -36,12 +36,11 @@ use namada_proof_of_stake::types::{CommissionPair, ValidatorState}; use crate::ibc::applications::transfer::msgs::transfer::MsgTransfer; use crate::ibc::applications::transfer::packet::PacketData; -use crate::ibc::applications::transfer::{PrefixedCoin, PrefixedDenom}; +use crate::ibc::applications::transfer::PrefixedCoin; use crate::ibc::core::ics04_channel::timeout::TimeoutHeight; use crate::ibc::core::timestamp::Timestamp as IbcTimestamp; use crate::ibc::core::Msg; use crate::ibc::Height as IbcHeight; -use crate::ledger::ibc::storage::ibc_token; use crate::proto::{MaspBuilder, Tx}; use crate::sdk::args::{self, InputAmount}; use crate::sdk::error::{EncodingError, Error, QueryError, Result, TxError}; @@ -1414,16 +1413,10 @@ pub async fn build_ibc_transfer< .await .map_err(|e| Error::from(QueryError::Wasm(e.to_string())))?; - let ibc_denom = PrefixedDenom { - trace_path: args.trace_path.unwrap_or_default(), - base_denom: args - .token - .to_string() - .parse() - .expect("Conversion from the token shouldn't fail"), - }; + let ibc_denom = + rpc::query_ibc_denom::<_, IO>(client, &args.token, Some(&source)).await; let token = PrefixedCoin { - denom: ibc_denom, + denom: ibc_denom.parse().expect("Invalid IBC denom"), // Set the IBC amount as an integer amount: validated_amount.into(), }; @@ -1666,11 +1659,6 @@ pub async fn build_transfer< ) -> Result<(Tx, Option)> { let source = args.source.effective_address(); let target = args.target.effective_address(); - let token = if let Some(trace_path) = &args.trace_path { - ibc_token(format!("{}/{}", trace_path.clone(), args.token)) - } else { - args.token.clone() - }; // Check that the source address exists on chain source_exists_or_err::<_, IO>(source.clone(), args.tx.force, client) @@ -1679,16 +1667,20 @@ pub async fn build_transfer< target_exists_or_err::<_, IO>(target.clone(), args.tx.force, client) .await?; // Check source balance - let balance_key = token::balance_key(&token, &source); + let balance_key = token::balance_key(&args.token, &source); // validate the amount given - let validated_amount = - validate_amount::<_, IO>(client, args.amount, &token, args.tx.force) - .await?; + let validated_amount = validate_amount::<_, IO>( + client, + args.amount, + &args.token, + args.tx.force, + ) + .await?; args.amount = InputAmount::Validated(validated_amount); let post_balance = check_balance_too_low_err::( - &token, + &args.token, &source, validated_amount.amount, balance_key, @@ -1699,7 +1691,7 @@ pub async fn build_transfer< let tx_source_balance = Some(TxSourcePostBalance { post_balance, source: source.clone(), - token: token.clone(), + token: args.token.clone(), }); let masp_addr = masp(); @@ -1712,7 +1704,7 @@ pub async fn build_transfer< // TODO Refactor me, we shouldn't rely on any specific token here. (token::Amount::default(), args.native_token.clone()) } else { - (validated_amount.amount, token) + (validated_amount.amount, args.token.clone()) }; // Determine whether to pin this transaction to a storage key let key = match &args.target { @@ -2215,6 +2207,7 @@ fn validate_untrusted_code_err( Ok(()) } } + async fn query_wasm_code_hash_buf< C: crate::ledger::queries::Client + Sync, IO: Io, diff --git a/tests/src/e2e/ibc_tests.rs b/tests/src/e2e/ibc_tests.rs index 27261a233d..3cd2ba48a3 100644 --- a/tests/src/e2e/ibc_tests.rs +++ b/tests/src/e2e/ibc_tests.rs @@ -637,7 +637,6 @@ fn transfer_token( ALBERT, &receiver, NAM, - None, "100000", ALBERT_KEY, port_id_a, @@ -707,7 +706,6 @@ fn try_invalid_transfers( ALBERT, &receiver, NAM, - None, "10.1", ALBERT_KEY, port_id_a, @@ -723,7 +721,6 @@ fn try_invalid_transfers( ALBERT, &receiver, NAM, - None, "10", ALBERT_KEY, &"port".parse().unwrap(), @@ -739,7 +736,6 @@ fn try_invalid_transfers( ALBERT, &receiver, NAM, - None, "10", ALBERT_KEY, port_id_a, @@ -757,10 +753,8 @@ fn transfer_received_token( channel_id: &ChannelId, test: &Test, ) -> Result<()> { - // token received via the port and channel - let trace_path = format!("{port_id}/{channel_id}"); - let rpc = get_actor_rpc(test, &Who::Validator(0)); + let ibc_denom = format!("{port_id}/{channel_id}/nam"); let amount = Amount::native_whole(50000).to_string_native(); let tx_args = [ "transfer", @@ -769,9 +763,7 @@ fn transfer_received_token( "--target", ALBERT, "--token", - NAM, - "--trace-path", - &trace_path, + &ibc_denom, "--amount", &amount, "--gas-token", @@ -798,14 +790,13 @@ fn transfer_back( let receiver = find_address(test_a, ALBERT)?; // Chain A was the source for the sent token - let trace_path = format!("{}/{}", port_id_b, channel_id_b); + let ibc_denom = format!("{port_id_b}/{channel_id_b}/nam"); // Send a token from Chain B let height = transfer( test_b, BERTHA, &receiver, - NAM, - Some(&trace_path), + ibc_denom, "50000", BERTHA_KEY, port_id_b, @@ -869,7 +860,6 @@ fn transfer_timeout( ALBERT, &receiver, NAM, - None, "100000", ALBERT_KEY, port_id_a, @@ -999,7 +989,6 @@ fn transfer( sender: impl AsRef, receiver: &Address, token: impl AsRef, - trace_path: Option<&str>, amount: impl AsRef, signer: impl AsRef, port_id: &PortId, @@ -1033,11 +1022,6 @@ fn transfer( &rpc, ]; - if let Some(trace_path) = trace_path { - tx_args.push("--trace-path"); - tx_args.push(trace_path.clone()); - } - let timeout = timeout_sec.unwrap_or_default().as_secs().to_string(); if timeout_sec.is_some() { tx_args.push("--timeout-sec-offset");