From fbc328f9c4bcc90cc771c315a6800b421aeb7fda Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 10 Sep 2024 19:24:21 +0200 Subject: [PATCH 01/26] Transfers trigger internal vps of destinations --- crates/tx_prelude/src/token.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/tx_prelude/src/token.rs b/crates/tx_prelude/src/token.rs index 6718ebdfd3..e11c3f622f 100644 --- a/crates/tx_prelude/src/token.rs +++ b/crates/tx_prelude/src/token.rs @@ -26,6 +26,11 @@ pub fn transfer( ) -> TxResult { // The tx must be authorized by the source address ctx.insert_verifier(src)?; + if dest.is_internal() { + // If the destination address is an internal address we trigger its vp + // for extra validation + ctx.insert_verifier(dest)?; + } if token.is_internal() { // Established address tokens do not have VPs themselves, their // validation is handled by the `Multitoken` internal address, but @@ -84,6 +89,11 @@ pub fn multi_transfer( } for ((dest, token), amount) in dests { + if dest.is_internal() { + // If the destination address is an internal address we trigger its + // vp for extra validation + ctx.insert_verifier(dest)?; + } if token.is_internal() { // Established address tokens do not have VPs themselves, their // validation is handled by the `Multitoken` internal address, but From a62a2bc8c3dac51952b0a9598668a63bf0e3139d Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 10 Sep 2024 19:35:06 +0200 Subject: [PATCH 02/26] Account for gas errors in pgf vp --- crates/governance/src/vp/pgf.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/crates/governance/src/vp/pgf.rs b/crates/governance/src/vp/pgf.rs index 6f2c656652..c11bedc428 100644 --- a/crates/governance/src/vp/pgf.rs +++ b/crates/governance/src/vp/pgf.rs @@ -119,15 +119,10 @@ where match key_type { KeyType::Stewards(steward_address) => { let stewards_have_increased = { - // TODO(namada#3238): we should check errors here, which - // could be out-of-gas related - let total_stewards_pre = pgf_storage::stewards_handle() - .len(&ctx.pre()) - .unwrap_or_default(); + let total_stewards_pre = + pgf_storage::stewards_handle().len(&ctx.pre())?; let total_stewards_post = - pgf_storage::stewards_handle() - .len(&ctx.post()) - .unwrap_or_default(); + pgf_storage::stewards_handle().len(&ctx.post())?; total_stewards_pre < total_stewards_post }; From ccbb457617c25eb68dcaeafdfbd9e0767665b451 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 10 Sep 2024 19:58:09 +0200 Subject: [PATCH 03/26] Multitoken vp ensures internal addresses verify the transaction --- crates/trans_token/src/vp.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/trans_token/src/vp.rs b/crates/trans_token/src/vp.rs index 471ea24ae3..6035810093 100644 --- a/crates/trans_token/src/vp.rs +++ b/crates/trans_token/src/vp.rs @@ -98,6 +98,16 @@ where let mut dec_mints: HashMap = HashMap::new(); for key in keys_changed { if let Some([token, owner]) = is_any_token_balance_key(key) { + if owner.is_internal() && !verifiers.contains(owner) { + // Internal addresses need to always verify the transaction + // (credit and debit) + return Err(Error::new_alloc(format!( + "The vp of debited internal address {} has not been \ + triggered", + owner + ))); + } + let pre: Amount = ctx.read_pre(key)?.unwrap_or_default(); let post: Amount = ctx.read_post(key)?.unwrap_or_default(); match post.checked_sub(pre) { From fc393086b9aeef9c1bdabc638d9ae81f567ce766 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 10 Sep 2024 21:24:22 +0200 Subject: [PATCH 04/26] Adds clippy lint to avoid `unwrap_or_default` on `Result`s --- clippy.toml | 1 + crates/apps_lib/src/client/utils.rs | 1 + crates/governance/src/finalize_block.rs | 1 + crates/governance/src/utils.rs | 1 + crates/node/src/shell/mod.rs | 1 + crates/node/src/shell/snapshots.rs | 1 + crates/node/src/shell/testing/node.rs | 1 + crates/node/src/shims/abcipp_shim_types.rs | 1 + crates/proof_of_stake/src/rewards.rs | 4 +++- crates/proof_of_stake/src/tests/test_pos.rs | 1 + .../proof_of_stake/src/tests/test_slash_and_redel.rs | 2 ++ crates/sdk/src/queries/vp/token.rs | 2 ++ crates/sdk/src/rpc.rs | 4 ++++ crates/sdk/src/signing.rs | 1 + crates/sdk/src/tx.rs | 2 ++ crates/shielded_token/src/masp.rs | 1 + .../src/masp/shielded_sync/dispatcher.rs | 1 + crates/tests/src/e2e/ibc_tests.rs | 12 +++++++++--- 18 files changed, 34 insertions(+), 4 deletions(-) diff --git a/clippy.toml b/clippy.toml index 4ae91bc7ca..2fc50cebd1 100644 --- a/clippy.toml +++ b/clippy.toml @@ -8,6 +8,7 @@ disallowed-methods = [ { path = "chrono::Utc::now", reason = "Do not use current date/time in code that must be deterministic" }, { path = "namada_core::time::DateTimeUtc::now", reason = "Do not use current date/time in code that must be deterministic" }, { path = "wasmtimer::std::Instant", reason = "Do not use current date/time in code that must be deterministic" }, + { path = "std::result::Result::unwrap_or_default", reason = "Do not silently ignore an error that could be caused by gas" }, ] allow-dbg-in-tests = true allow-print-in-tests = true diff --git a/crates/apps_lib/src/client/utils.rs b/crates/apps_lib/src/client/utils.rs index 6a227ea362..8e8c7c6fc5 100644 --- a/crates/apps_lib/src/client/utils.rs +++ b/crates/apps_lib/src/client/utils.rs @@ -991,6 +991,7 @@ pub async fn sign_genesis_tx( if let Some(output_path) = output.as_ref() { // If the output path contains existing signed txs, we append // the newly signed txs to the file + #[allow(clippy::disallowed_methods)] let mut prev_txs = genesis::templates::read_transactions(output_path) .unwrap_or_default(); diff --git a/crates/governance/src/finalize_block.rs b/crates/governance/src/finalize_block.rs index 6eea729bc9..6dc6b8d012 100644 --- a/crates/governance/src/finalize_block.rs +++ b/crates/governance/src/finalize_block.rs @@ -324,6 +324,7 @@ where if vote.is_validator() { let vote_data = vote.data.clone(); + #[allow(clippy::disallowed_methods)] let validator_stake = PoS::read_validator_stake::>( storage, validator, epoch, ) diff --git a/crates/governance/src/utils.rs b/crates/governance/src/utils.rs index 9612a166be..dd677b6e17 100644 --- a/crates/governance/src/utils.rs +++ b/crates/governance/src/utils.rs @@ -221,6 +221,7 @@ impl ProposalResult { /// Return true if at least 2/3 of the total voting power voted and at least /// two third of the non-abstained voting power voted nay. /// Returns `false` if any arithmetic fails. + #[allow(clippy::disallowed_methods)] pub fn two_thirds_nay_over_two_thirds_total(&self) -> bool { (|| { let two_thirds_power = diff --git a/crates/node/src/shell/mod.rs b/crates/node/src/shell/mod.rs index 4911a4c6f8..ea27241312 100644 --- a/crates/node/src/shell/mod.rs +++ b/crates/node/src/shell/mod.rs @@ -633,6 +633,7 @@ where }; if let Some(schedule_migration) = scheduled_migration.as_ref() { + #[allow(clippy::disallowed_methods)] let current = state.get_block_height().unwrap_or_default(); if schedule_migration.height < current { panic!( diff --git a/crates/node/src/shell/snapshots.rs b/crates/node/src/shell/snapshots.rs index 40572677f6..227fdd5daa 100644 --- a/crates/node/src/shell/snapshots.rs +++ b/crates/node/src/shell/snapshots.rs @@ -112,6 +112,7 @@ impl Shell { } match self.syncing.as_ref() { None => { + #[allow(clippy::disallowed_methods)] if self.state.get_block_height().unwrap_or_default().0 < u64::from(req.snapshot.height) { diff --git a/crates/node/src/shell/testing/node.rs b/crates/node/src/shell/testing/node.rs index abebd5cdd6..4a213c6b05 100644 --- a/crates/node/src/shell/testing/node.rs +++ b/crates/node/src/shell/testing/node.rs @@ -361,6 +361,7 @@ impl MockNode { } pub fn block_height(&self) -> BlockHeight { + #[allow(clippy::disallowed_methods)] self.shell .lock() .unwrap() diff --git a/crates/node/src/shims/abcipp_shim_types.rs b/crates/node/src/shims/abcipp_shim_types.rs index c4b7dad9b4..3e266d60b5 100644 --- a/crates/node/src/shims/abcipp_shim_types.rs +++ b/crates/node/src/shims/abcipp_shim_types.rs @@ -223,6 +223,7 @@ pub mod shim { let header = req.header; FinalizeBlock { header: BlockHeader { + #[allow(clippy::disallowed_methods)] hash: Hash::try_from(header.app_hash.as_bytes()) .unwrap_or_default(), time: DateTimeUtc::try_from(header.time).unwrap(), diff --git a/crates/proof_of_stake/src/rewards.rs b/crates/proof_of_stake/src/rewards.rs index 1efb6040a4..faa65c3c0d 100644 --- a/crates/proof_of_stake/src/rewards.rs +++ b/crates/proof_of_stake/src/rewards.rs @@ -261,12 +261,14 @@ where // Ensure TM stake updates properly with a debug_assert if cfg!(debug_assertions) { + #[allow(clippy::disallowed_methods)] + let validator_vp = i64::try_from(validator_vp).unwrap_or_default(); debug_assert_eq!( into_tm_voting_power( params.tm_votes_per_token, stake_from_deltas, ), - i64::try_from(validator_vp).unwrap_or_default(), + validator_vp ); } diff --git a/crates/proof_of_stake/src/tests/test_pos.rs b/crates/proof_of_stake/src/tests/test_pos.rs index 2e1816fab1..aa5fee2cfd 100644 --- a/crates/proof_of_stake/src/tests/test_pos.rs +++ b/crates/proof_of_stake/src/tests/test_pos.rs @@ -1159,6 +1159,7 @@ fn test_unslashed_bond_amount_aux(validators: Vec) { Epoch(0), current_epoch + params.pipeline_len, ) { + #[allow(clippy::disallowed_methods)] let amount = bond_amount( &storage, &BondId { diff --git a/crates/proof_of_stake/src/tests/test_slash_and_redel.rs b/crates/proof_of_stake/src/tests/test_slash_and_redel.rs index ab404cad4d..f0aad33032 100644 --- a/crates/proof_of_stake/src/tests/test_slash_and_redel.rs +++ b/crates/proof_of_stake/src/tests/test_slash_and_redel.rs @@ -1638,6 +1638,7 @@ fn test_slashed_bond_amount_aux(validators: Vec) { let pipeline_epoch = current_epoch + params.pipeline_len; + #[allow(clippy::disallowed_methods)] let del_bond_amount = bond_amount( &storage, &BondId { @@ -1648,6 +1649,7 @@ fn test_slashed_bond_amount_aux(validators: Vec) { ) .unwrap_or_default(); + #[allow(clippy::disallowed_methods)] let self_bond_amount = bond_amount( &storage, &BondId { diff --git a/crates/sdk/src/queries/vp/token.rs b/crates/sdk/src/queries/vp/token.rs index b0f151eaa1..a0c6e33314 100644 --- a/crates/sdk/src/queries/vp/token.rs +++ b/crates/sdk/src/queries/vp/token.rs @@ -84,6 +84,7 @@ pub mod client_only_methods { let balance = if response.data.is_empty() { token::Amount::zero() } else { + #[allow(clippy::disallowed_methods)] token::Amount::try_from_slice(&response.data) .unwrap_or_default() }; @@ -107,6 +108,7 @@ pub mod client_only_methods { let tokens = if response.data.is_empty() { token::Amount::zero() } else { + #[allow(clippy::disallowed_methods)] token::Amount::try_from_slice(&response.data) .unwrap_or_default() }; diff --git a/crates/sdk/src/rpc.rs b/crates/sdk/src/rpc.rs index 781ecc41fa..961bf0a356 100644 --- a/crates/sdk/src/rpc.rs +++ b/crates/sdk/src/rpc.rs @@ -984,10 +984,12 @@ pub async fn query_proposal_result( let is_author_pgf_steward = is_steward(client, &proposal.author).await; + #[allow(clippy::disallowed_methods)] let votes = query_proposal_votes(client, proposal_id) .await .unwrap_or_default(); let tally_type = proposal.get_tally_type(is_author_pgf_steward); + #[allow(clippy::disallowed_methods)] let total_active_voting_power = get_total_active_voting_power(client, tally_epoch) .await @@ -998,6 +1000,7 @@ pub async fn query_proposal_result( for vote in votes { match vote.is_validator() { true => { + #[allow(clippy::disallowed_methods)] let voting_power = get_validator_stake( client, tally_epoch, @@ -1013,6 +1016,7 @@ pub async fn query_proposal_result( ); } false => { + #[allow(clippy::disallowed_methods)] let voting_power = get_bond_amount_at( client, &vote.delegator, diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index f8c70ab659..8d1c058489 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -483,6 +483,7 @@ pub async fn validate_transparent_fee( let fee_payer_address = Address::from(fee_payer); let balance_key = balance_key(&args.fee_token, &fee_payer_address); + #[allow(clippy::disallowed_methods)] let balance = rpc::query_storage_value::<_, token::Amount>( context.client(), &balance_key, diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 116ed815e7..9e74fd3cc5 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -3151,6 +3151,7 @@ async fn get_masp_fee_payment_amount( ) -> Result> { let fee_payer_address = Address::from(fee_payer); let balance_key = balance_key(&args.fee_token, &fee_payer_address); + #[allow(clippy::disallowed_methods)] let balance = rpc::query_storage_value::<_, token::Amount>( context.client(), &balance_key, @@ -3492,6 +3493,7 @@ async fn construct_shielded_parts( // Get the decoded asset types used in the transaction to give offline // wallet users more information + #[allow(clippy::disallowed_methods)] let asset_types = used_asset_types(context, &shielded_parts.builder) .await .unwrap_or_default(); diff --git a/crates/shielded_token/src/masp.rs b/crates/shielded_token/src/masp.rs index d2cc14a60f..cc1fc79fbb 100644 --- a/crates/shielded_token/src/masp.rs +++ b/crates/shielded_token/src/masp.rs @@ -259,6 +259,7 @@ pub fn is_amount_required( for (asset_type, value) in gap.components() { if *value > 0 && normed_delta[asset_type] > 0 { + #[allow(clippy::disallowed_methods)] let signed_change_amt = checked!(normed_delta[asset_type] - *value).unwrap_or_default(); let unsigned_change_amt = if signed_change_amt > 0 { diff --git a/crates/shielded_token/src/masp/shielded_sync/dispatcher.rs b/crates/shielded_token/src/masp/shielded_sync/dispatcher.rs index 3f5a3ec416..27675a3838 100644 --- a/crates/shielded_token/src/masp/shielded_sync/dispatcher.rs +++ b/crates/shielded_token/src/masp/shielded_sync/dispatcher.rs @@ -291,6 +291,7 @@ where panic_flag: PanicFlag::default(), }; + #[allow(clippy::disallowed_methods)] let cache = ctx.utils.cache_load().await.unwrap_or_default(); Dispatcher { diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 275c971794..a3487cc7bf 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -508,7 +508,9 @@ fn ibc_token_inflation() -> Result<()> { let delegated = epoch + PIPELINE_LEN; while epoch < delegated { sleep(10); - epoch = get_epoch(&test, &rpc).unwrap_or_default(); + #[allow(clippy::disallowed_methods)] + let new_epoch = get_epoch(&test, &rpc).unwrap_or_default(); + epoch = new_epoch; } // inflation proposal on Namada let start_epoch = propose_inflation(&test)?; @@ -516,7 +518,9 @@ fn ibc_token_inflation() -> Result<()> { // Vote while epoch < start_epoch { sleep(10); - epoch = get_epoch(&test, &rpc).unwrap_or_default(); + #[allow(clippy::disallowed_methods)] + let new_epoch = get_epoch(&test, &rpc).unwrap_or_default(); + epoch = new_epoch; } submit_votes(&test)?; @@ -566,7 +570,9 @@ fn ibc_token_inflation() -> Result<()> { let new_epoch = epoch + MASP_EPOCH_MULTIPLIER; while epoch < new_epoch { sleep(10); - epoch = get_epoch(&test, &rpc).unwrap_or_default(); + #[allow(clippy::disallowed_methods)] + let new_epoch = get_epoch(&test, &rpc).unwrap_or_default(); + epoch = new_epoch; } // Check balances From cd3f5a132696f496d7196b3083a334188af4f23f Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 10 Sep 2024 21:57:47 +0200 Subject: [PATCH 05/26] Multitoken vp ensures debited accounts' vps are triggered --- crates/trans_token/src/vp.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/trans_token/src/vp.rs b/crates/trans_token/src/vp.rs index 6035810093..0d22fe646f 100644 --- a/crates/trans_token/src/vp.rs +++ b/crates/trans_token/src/vp.rs @@ -102,8 +102,7 @@ where // Internal addresses need to always verify the transaction // (credit and debit) return Err(Error::new_alloc(format!( - "The vp of debited internal address {} has not been \ - triggered", + "The vp of internal address {} has not been triggered", owner ))); } @@ -136,6 +135,14 @@ where "Native token deposit isn't allowed", )); } + if !verifiers.contains(owner) { + // Debit must be verified by the controlling vp + return Err(Error::new_alloc(format!( + "The vp of debited address {} has not been \ + triggered", + owner + ))); + } let diff = pre .checked_sub(post) .expect("Underflow shouldn't happen here"); From 77ad90e4c0978051dad893c027222b7904cbfb33 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 10 Sep 2024 23:01:25 +0200 Subject: [PATCH 06/26] Fixes broken ibc unit test --- crates/tests/src/vm_host_env/mod.rs | 4 +++- crates/trans_token/src/vp.rs | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/tests/src/vm_host_env/mod.rs b/crates/tests/src/vm_host_env/mod.rs index 4e6aa6fd64..cf944d6ca4 100644 --- a/crates/tests/src/vm_host_env/mod.rs +++ b/crates/tests/src/vm_host_env/mod.rs @@ -1344,8 +1344,10 @@ mod tests { // Check let mut env = tx_host_env::take(); - // The token must be part of the verifier set (checked by MultitokenVp) + // The token and sender must be part of the verifier set (checked by + // MultitokenVp) env.verifiers.insert(ibc_token); + env.verifiers.insert(sender); let result = ibc::validate_ibc_vp_from_tx( &env, &tx.batch_ref_first_tx().unwrap(), diff --git a/crates/trans_token/src/vp.rs b/crates/trans_token/src/vp.rs index 0d22fe646f..87a68772b3 100644 --- a/crates/trans_token/src/vp.rs +++ b/crates/trans_token/src/vp.rs @@ -949,6 +949,7 @@ mod tests { let (vp_vp_cache, _vp_cache_dir) = vp_cache(); let mut verifiers = BTreeSet::new(); verifiers.insert(src); + verifiers.insert(dest); let ctx = Ctx::new( &ADDRESS, &state, @@ -1043,6 +1044,7 @@ mod tests { let (vp_vp_cache, _vp_cache_dir) = vp_cache(); let mut verifiers = BTreeSet::new(); verifiers.insert(src); + verifiers.insert(dest); let ctx = Ctx::new( &ADDRESS, &state, From f8cfe7bfda0523e7fb718242a9be57ffde334f50 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 11 Sep 2024 19:21:52 +0200 Subject: [PATCH 07/26] Requests verification from both transfer's source and destination --- crates/ibc/src/lib.rs | 26 ++++++++++++++++++++++++++ crates/trans_token/src/vp.rs | 14 ++------------ crates/tx_prelude/src/token.rs | 17 +++++------------ 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index b77fef8da4..c0569a418e 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -32,6 +32,7 @@ use std::collections::BTreeSet; use std::fmt::Debug; use std::marker::PhantomData; use std::rc::Rc; +use std::str::FromStr; pub use actions::transfer_over_ibc; use apps::transfer::types::packet::PacketData; @@ -612,6 +613,18 @@ where self.ctx.inner.clone(), self.verifiers.clone(), ); + // Add the source to the set of verifiers + self.verifiers.borrow_mut().insert( + Address::from_str(msg.message.packet_data.sender.as_ref()) + .map_err(|_| { + Error::TokenTransfer(TokenTransferError::Other( + format!( + "Cannot convert the sender address {}", + msg.message.packet_data.sender + ), + )) + })?, + ); self.insert_verifiers()?; if msg.transfer.is_some() { token_transfer_ctx.enable_shielded_transfer(); @@ -630,6 +643,19 @@ where if msg.transfer.is_some() { nft_transfer_ctx.enable_shielded_transfer(); } + // Add the source to the set of verifiers + self.verifiers.borrow_mut().insert( + Address::from_str(msg.message.packet_data.sender.as_ref()) + .map_err(|_| { + Error::TokenTransfer(TokenTransferError::Other( + format!( + "Cannot convert the sender address {}", + msg.message.packet_data.sender + ), + )) + })?, + ); + self.insert_verifiers()?; send_nft_transfer_execute( &mut self.ctx, &mut nft_transfer_ctx, diff --git a/crates/trans_token/src/vp.rs b/crates/trans_token/src/vp.rs index 87a68772b3..6c49f10727 100644 --- a/crates/trans_token/src/vp.rs +++ b/crates/trans_token/src/vp.rs @@ -98,11 +98,9 @@ where let mut dec_mints: HashMap = HashMap::new(); for key in keys_changed { if let Some([token, owner]) = is_any_token_balance_key(key) { - if owner.is_internal() && !verifiers.contains(owner) { - // Internal addresses need to always verify the transaction - // (credit and debit) + if !verifiers.contains(owner) { return Err(Error::new_alloc(format!( - "The vp of internal address {} has not been triggered", + "The vp of the address {} has not been triggered", owner ))); } @@ -135,14 +133,6 @@ where "Native token deposit isn't allowed", )); } - if !verifiers.contains(owner) { - // Debit must be verified by the controlling vp - return Err(Error::new_alloc(format!( - "The vp of debited address {} has not been \ - triggered", - owner - ))); - } let diff = pre .checked_sub(post) .expect("Underflow shouldn't happen here"); diff --git a/crates/tx_prelude/src/token.rs b/crates/tx_prelude/src/token.rs index e11c3f622f..a1b753e39e 100644 --- a/crates/tx_prelude/src/token.rs +++ b/crates/tx_prelude/src/token.rs @@ -24,13 +24,9 @@ pub fn transfer( token: &Address, amount: Amount, ) -> TxResult { - // The tx must be authorized by the source address + // The tx must be authorized by the source and destination addresses ctx.insert_verifier(src)?; - if dest.is_internal() { - // If the destination address is an internal address we trigger its vp - // for extra validation - ctx.insert_verifier(dest)?; - } + ctx.insert_verifier(dest)?; if token.is_internal() { // Established address tokens do not have VPs themselves, their // validation is handled by the `Multitoken` internal address, but @@ -70,7 +66,7 @@ pub fn multi_transfer( let mut post_balances = BTreeMap::new(); for ((src, token), amount) in sources { - // The tx must be authorized by the source address + // The tx must be authorized by the involved address ctx.insert_verifier(src)?; if token.is_internal() { // Established address tokens do not have VPs themselves, their @@ -89,11 +85,8 @@ pub fn multi_transfer( } for ((dest, token), amount) in dests { - if dest.is_internal() { - // If the destination address is an internal address we trigger its - // vp for extra validation - ctx.insert_verifier(dest)?; - } + // The tx must be authorized by the involved address + ctx.insert_verifier(dest)?; if token.is_internal() { // Established address tokens do not have VPs themselves, their // validation is handled by the `Multitoken` internal address, but From 6e52f37ba356732de73dd83c59987993716bc854 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Wed, 11 Sep 2024 19:57:45 +0200 Subject: [PATCH 08/26] Fixes unit tests --- crates/tests/src/vm_host_env/mod.rs | 4 +++- crates/trans_token/src/vp.rs | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/tests/src/vm_host_env/mod.rs b/crates/tests/src/vm_host_env/mod.rs index cf944d6ca4..0a7a3c858a 100644 --- a/crates/tests/src/vm_host_env/mod.rs +++ b/crates/tests/src/vm_host_env/mod.rs @@ -1436,10 +1436,12 @@ mod tests { ); assert!(result.is_ok()); // Check if the token was minted - // The token must be part of the verifier set (checked by MultitokenVp) + // The token and recevier must be part of the verifier set (checked by + // MultitokenVp) let denom = format!("{}/{}/{}", port_id, channel_id, token); let ibc_token = ibc::ibc_token(&denom); env.verifiers.insert(ibc_token.clone()); + env.verifiers.insert(receiver.clone()); let result = ibc::validate_ibc_vp_from_tx( &env, &tx.batch_ref_first_tx().unwrap(), diff --git a/crates/trans_token/src/vp.rs b/crates/trans_token/src/vp.rs index 6c49f10727..abc9a8c68f 100644 --- a/crates/trans_token/src/vp.rs +++ b/crates/trans_token/src/vp.rs @@ -455,6 +455,7 @@ mod tests { let (vp_vp_cache, _vp_cache_dir) = vp_cache(); let mut verifiers = BTreeSet::new(); verifiers.insert(src); + verifiers.insert(dest); let ctx = Ctx::new( &ADDRESS, &state, @@ -566,8 +567,10 @@ mod tests { let mut verifiers = BTreeSet::new(); // for the minter verifiers.insert(minter); - // The token must be part of the verifier set (checked by MultitokenVp) + // The token and minter must be part of the verifier set (checked by + // MultitokenVp) verifiers.insert(token); + verifiers.insert(target); let ctx = Ctx::new( &ADDRESS, &state, @@ -1087,6 +1090,7 @@ mod tests { let (vp_vp_cache, _vp_cache_dir) = vp_cache(); let mut verifiers = BTreeSet::new(); verifiers.insert(src); + verifiers.insert(dest); let ctx = Ctx::new( &ADDRESS, &state, From 07035e04e94b6ab10bfe53cacb3e584d72959114 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 12 Sep 2024 09:52:00 +0200 Subject: [PATCH 09/26] Updates gas limit and fixes masp tests --- crates/sdk/src/lib.rs | 2 +- crates/tests/src/integration/masp.rs | 24 +++++++++++------------- genesis/hardware/parameters.toml | 4 ++-- genesis/localnet/parameters.toml | 4 ++-- genesis/starter/parameters.toml | 4 ++-- 5 files changed, 18 insertions(+), 20 deletions(-) diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 773d94017f..f3f0e2284b 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -82,7 +82,7 @@ pub use {namada_io as io, namada_wallet as wallet}; use crate::masp::ShieldedContext; /// Default gas-limit -pub const DEFAULT_GAS_LIMIT: u64 = 200_000; +pub const DEFAULT_GAS_LIMIT: u64 = 250_000; /// An interface for high-level interaction with the Namada SDK #[cfg_attr(feature = "async-send", async_trait::async_trait)] diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 71b287cfe5..4bc8292bc1 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -687,7 +687,7 @@ fn masp_incentives() -> Result<()> { "--token", NAM, "--gas-limit", - "250000", + "300000", "--amount", "1.451732", "--signing-keys", @@ -2582,8 +2582,6 @@ fn masp_fee_payment() -> Result<()> { NAM, "--amount", "10000", - "--gas-limit", - "200000", "--gas-price", "1", "--disposable-gas-payer", @@ -2617,7 +2615,7 @@ fn masp_fee_payment() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 290000")); + assert!(captured.contains("nam: 240000")); let captured = CapturedOutput::of(|| { run( &node, @@ -2750,7 +2748,7 @@ fn masp_fee_payment_gas_limit() -> Result<()> { "--amount", "1", "--gas-limit", - "300000", + "500000", "--gas-price", "1", "--disposable-gas-payer", @@ -2920,7 +2918,7 @@ fn masp_fee_payment_with_non_disposable() -> Result<()> { "--amount", "1", "--gas-limit", - "200000", + "300000", "--gas-price", "1", "--gas-payer", @@ -2959,7 +2957,7 @@ fn masp_fee_payment_with_non_disposable() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 1799999")); + assert!(captured.contains("nam: 1699999")); let captured = CapturedOutput::of(|| { run( @@ -3067,7 +3065,7 @@ fn masp_fee_payment_with_custom_spending_key() -> Result<()> { "--token", NAM, "--amount", - "250000", + "300000", "--ledger-address", validator_one_rpc, ], @@ -3117,7 +3115,7 @@ fn masp_fee_payment_with_custom_spending_key() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 250000")); + assert!(captured.contains("nam: 300000")); // Masp fee payment with custom gas payer let captured = CapturedOutput::of(|| { @@ -3135,7 +3133,7 @@ fn masp_fee_payment_with_custom_spending_key() -> Result<()> { "--amount", "9000", "--gas-limit", - "250000", + "300000", "--gas-price", "1", "--gas-spending-key", @@ -3314,7 +3312,7 @@ fn masp_fee_payment_with_different_token() -> Result<()> { "--token", BTC, "--amount", - "250000", + "300000", "--gas-payer", ALBERT_KEY, "--ledger-address", @@ -3383,7 +3381,7 @@ fn masp_fee_payment_with_different_token() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("btc: 250000")); + assert!(captured.contains("btc: 300000")); // Masp fee payment with custom token and gas payer let captured = CapturedOutput::of(|| { @@ -3403,7 +3401,7 @@ fn masp_fee_payment_with_different_token() -> Result<()> { "--gas-token", BTC, "--gas-limit", - "250000", + "300000", "--gas-price", "1", "--gas-spending-key", diff --git a/genesis/hardware/parameters.toml b/genesis/hardware/parameters.toml index 584a4b5258..cda15160aa 100644 --- a/genesis/hardware/parameters.toml +++ b/genesis/hardware/parameters.toml @@ -19,9 +19,9 @@ epochs_per_year = 31_536_000 # The multiplier for masp epochs masp_epoch_multiplier = 2 # Max gas for block -max_block_gas = 20_000_000 +max_block_gas = 25_000_000 # Masp fee payment gas limit -masp_fee_payment_gas_limit = 200_000 +masp_fee_payment_gas_limit = 250_000 # Gas scale gas_scale = 10_000 diff --git a/genesis/localnet/parameters.toml b/genesis/localnet/parameters.toml index 60f780cc1b..161e4da0ab 100644 --- a/genesis/localnet/parameters.toml +++ b/genesis/localnet/parameters.toml @@ -19,9 +19,9 @@ epochs_per_year = 31_536_000 # The multiplier for masp epochs masp_epoch_multiplier = 2 # Max gas for block -max_block_gas = 20_000_000 +max_block_gas = 25_000_000 # Masp fee payment gas limit -masp_fee_payment_gas_limit = 200_000 +masp_fee_payment_gas_limit = 250_000 # Gas scale gas_scale = 10_000 diff --git a/genesis/starter/parameters.toml b/genesis/starter/parameters.toml index ad8f91eaf4..79b53b6b1c 100644 --- a/genesis/starter/parameters.toml +++ b/genesis/starter/parameters.toml @@ -19,9 +19,9 @@ epochs_per_year = 31_536_000 # The multiplier for masp epochs masp_epoch_multiplier = 2 # Max gas for block -max_block_gas = 20_000_000 +max_block_gas = 25_000_000 # Masp fee payment gas limit -masp_fee_payment_gas_limit = 200_000 +masp_fee_payment_gas_limit = 250_000 # Gas scale gas_scale = 10_000 From 664ee169be445584236cd5a0ee99757fa94445d1 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 12 Sep 2024 12:20:05 +0200 Subject: [PATCH 10/26] Extracts receiver from ibc envelope and requests verification --- crates/ibc/src/lib.rs | 77 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index c0569a418e..acacbb23d0 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -55,13 +55,16 @@ use ibc::apps::nft_transfer::types::msgs::transfer::MsgTransfer as IbcMsgNftTran use ibc::apps::nft_transfer::types::{ ack_success_b64, is_receiver_chain_source as is_nft_receiver_chain_source, PrefixedClassId, TokenId, TracePrefix as NftTracePrefix, + PORT_ID_STR as NFT_PORT_ID_STR, }; use ibc::apps::transfer::handler::{ send_transfer_execute, send_transfer_validate, }; use ibc::apps::transfer::types::error::TokenTransferError; use ibc::apps::transfer::types::msgs::transfer::MsgTransfer as IbcMsgTransfer; -use ibc::apps::transfer::types::{is_receiver_chain_source, TracePrefix}; +use ibc::apps::transfer::types::{ + is_receiver_chain_source, TracePrefix, PORT_ID_STR as FT_PORT_ID_STR, +}; use ibc::core::channel::types::acknowledgement::AcknowledgementStatus; use ibc::core::channel::types::commitment::compute_ack_commitment; use ibc::core::channel::types::msgs::{ @@ -647,7 +650,7 @@ where self.verifiers.borrow_mut().insert( Address::from_str(msg.message.packet_data.sender.as_ref()) .map_err(|_| { - Error::TokenTransfer(TokenTransferError::Other( + Error::NftTransfer(NftTransferError::Other( format!( "Cannot convert the sender address {}", msg.message.packet_data.sender @@ -667,6 +670,76 @@ where IbcMessage::Envelope(envelope) => { execute(&mut self.ctx, &mut self.router, *envelope.clone()) .map_err(|e| Error::Context(Box::new(e)))?; + + // Extract destination address from the packet to trigger its + // address FIXME: refactor, maybe can share with + // the extract_memofrom_packet function? Maybe extract both at + // the same time + if let MsgEnvelope::Packet(PacketMsg::Recv(ref msg)) = *envelope + { + if self.is_receiving_success(msg)? { + match msg.packet.port_id_on_b.as_str() { + FT_PORT_ID_STR => { + let packet_data = + serde_json::from_slice::( + &msg.packet.data, + ).map_err(|_| Error::TokenTransfer(TokenTransferError::PacketDataDeserialization))?; + tracing::error!( + "ADDING {} TO THE VERIFIER", + packet_data.receiver + ); //FIXME: remove + self.verifiers.borrow_mut().insert( + Address::from_str( + packet_data.receiver.as_ref(), + ) + .map_err(|_| { + Error::TokenTransfer( + TokenTransferError::Other(format!( + "Cannot convert the receiver \ + address {}", + packet_data.receiver + )), + ) + })?, + ); + } + NFT_PORT_ID_STR => { + let packet_data = + serde_json::from_slice::( + &msg.packet.data, + ) + .map_err(|_| { + Error::NftTransfer( + NftTransferError::PacketDataDeserialization, + ) + })?; + self.verifiers.borrow_mut().insert( + Address::from_str( + packet_data.receiver.as_ref(), + ) + .map_err(|_| { + Error::NftTransfer( + NftTransferError::Other(format!( + "Cannot convert the receiver \ + address {}", + packet_data.receiver + )), + ) + })?, + ); + } + _ => { + tracing::warn!( + "Sender couldn't be extracted from the \ + unsupported IBC packet data for Port ID \ + {}", + msg.packet.port_id_on_b + ); + } + } + } + } + // Extract MASP tx from the memo in the packet if needed let masp_tx = match &*envelope { MsgEnvelope::Packet(PacketMsg::Recv(msg)) From 439a3a6e8e46bf28d5db627723f83f7e53ac822a Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 12 Sep 2024 19:59:43 +0200 Subject: [PATCH 11/26] IBC verifiers also for failed acks and timeouts --- crates/ibc/src/lib.rs | 191 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 172 insertions(+), 19 deletions(-) diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index acacbb23d0..0a7bb176ce 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -672,36 +672,187 @@ where .map_err(|e| Error::Context(Box::new(e)))?; // Extract destination address from the packet to trigger its - // address FIXME: refactor, maybe can share with - // the extract_memofrom_packet function? Maybe extract both at - // the same time - if let MsgEnvelope::Packet(PacketMsg::Recv(ref msg)) = *envelope - { - if self.is_receiving_success(msg)? { - match msg.packet.port_id_on_b.as_str() { + // address FIXME: refactor, + match *envelope { + MsgEnvelope::Packet(PacketMsg::Recv(ref msg)) => { + if self.is_receiving_success(msg)? { + match msg.packet.port_id_on_b.as_str() { + FT_PORT_ID_STR => { + let packet_data = + serde_json::from_slice::( + &msg.packet.data, + ).map_err(|_| Error::TokenTransfer(TokenTransferError::PacketDataDeserialization))?; + self.verifiers.borrow_mut().insert( + Address::from_str( + packet_data.receiver.as_ref(), + ) + .map_err(|_| { + Error::TokenTransfer( + TokenTransferError::Other( + format!( + "Cannot convert the \ + receiver address {}", + packet_data.receiver + ), + ), + ) + })?, + ); + self.insert_verifiers()?; + } + NFT_PORT_ID_STR => { + let packet_data = serde_json::from_slice::< + NftPacketData, + >( + &msg.packet.data + ) + .map_err(|_| { + Error::NftTransfer( + NftTransferError::PacketDataDeserialization, + ) + })?; + self.verifiers.borrow_mut().insert( + Address::from_str( + packet_data.receiver.as_ref(), + ) + .map_err(|_| { + Error::NftTransfer( + NftTransferError::Other( + format!( + "Cannot convert the \ + receiver address {}", + packet_data.receiver + ), + ), + ) + })?, + ); + self.insert_verifiers()?; + } + _ => { + tracing::warn!( + "Receiver couldn't be extracted from \ + the unsupported IBC packet data for \ + Port ID {}", + msg.packet.port_id_on_b + ); + } + } + } + } + MsgEnvelope::Packet(PacketMsg::Ack(ref msg)) => { + match msg.packet.port_id_on_a.as_str() { + FT_PORT_ID_STR => { + let packet_data = + serde_json::from_slice::( + &msg.packet.data, + ).map_err(|_| Error::TokenTransfer(TokenTransferError::PacketDataDeserialization))?; + + let ack = serde_json::from_slice::< + AcknowledgementStatus, + >( + msg.acknowledgement.as_ref() + ) + .map_err(|_| { + Error::TokenTransfer( + TokenTransferError::AckDeserialization, + ) + })?; + + if !ack.is_successful() { + self.verifiers.borrow_mut().insert( + Address::from_str( + packet_data.sender.as_ref(), + ) + .map_err(|_| { + Error::TokenTransfer( + TokenTransferError::Other( + format!( + "Cannot convert the \ + sender address {}", + packet_data.sender + ), + ), + ) + })?, + ); + self.insert_verifiers()?; + } + } + NFT_PORT_ID_STR => { + let packet_data = + serde_json::from_slice::( + &msg.packet.data, + ) + .map_err(|_| { + Error::NftTransfer( + NftTransferError::PacketDataDeserialization, + ) + })?; + + let ack = serde_json::from_slice::< + AcknowledgementStatus, + >( + msg.acknowledgement.as_ref() + ) + .map_err(|_| { + Error::NftTransfer( + NftTransferError::AckDeserialization, + ) + })?; + + if !ack.is_successful() { + self.verifiers.borrow_mut().insert( + Address::from_str( + packet_data.sender.as_ref(), + ) + .map_err(|_| { + Error::TokenTransfer( + TokenTransferError::Other( + format!( + "Cannot convert the \ + sender address {}", + packet_data.sender + ), + ), + ) + })?, + ); + self.insert_verifiers()?; + } + } + _ => { + tracing::warn!( + "Receiver couldn't be extracted from the \ + unsupported IBC packet data for Port ID \ + {}", + msg.packet.port_id_on_a + ); + } + }; + } + MsgEnvelope::Packet(PacketMsg::Timeout(ref msg)) => { + match msg.packet.port_id_on_a.as_str() { FT_PORT_ID_STR => { let packet_data = serde_json::from_slice::( &msg.packet.data, ).map_err(|_| Error::TokenTransfer(TokenTransferError::PacketDataDeserialization))?; - tracing::error!( - "ADDING {} TO THE VERIFIER", - packet_data.receiver - ); //FIXME: remove self.verifiers.borrow_mut().insert( Address::from_str( - packet_data.receiver.as_ref(), + packet_data.sender.as_ref(), ) .map_err(|_| { Error::TokenTransfer( TokenTransferError::Other(format!( - "Cannot convert the receiver \ + "Cannot convert the sender \ address {}", - packet_data.receiver + packet_data.sender )), ) })?, ); + self.insert_verifiers()?; } NFT_PORT_ID_STR => { let packet_data = @@ -715,29 +866,31 @@ where })?; self.verifiers.borrow_mut().insert( Address::from_str( - packet_data.receiver.as_ref(), + packet_data.sender.as_ref(), ) .map_err(|_| { Error::NftTransfer( NftTransferError::Other(format!( - "Cannot convert the receiver \ + "Cannot convert the sender \ address {}", - packet_data.receiver + packet_data.sender )), ) })?, ); + self.insert_verifiers()?; } _ => { tracing::warn!( - "Sender couldn't be extracted from the \ + "Receiver couldn't be extracted from the \ unsupported IBC packet data for Port ID \ {}", - msg.packet.port_id_on_b + msg.packet.port_id_on_a ); } } } + _ => (), } // Extract MASP tx from the memo in the packet if needed From c82a4fdd77ae74d749d50a1d0c634389965ed076 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 12 Sep 2024 20:00:09 +0200 Subject: [PATCH 12/26] Adds some timeouts in ibc e2e test --- crates/tests/src/e2e/ibc_tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index a3487cc7bf..632e453e23 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -156,7 +156,7 @@ fn ibc_transfers() -> Result<()> { &port_id_gaia, &channel_id_gaia, None, - None, + Some(Duration::new(10, 0)), )?; wait_for_packet_relay(&port_id_gaia, &channel_id_gaia, &test)?; @@ -273,7 +273,7 @@ fn ibc_transfers() -> Result<()> { &port_id_gaia, &channel_id_gaia, Some(memo_path), - None, + Some(Duration::new(10, 0)), )?; wait_for_packet_relay(&port_id_gaia, &channel_id_gaia, &test)?; // Check the token on Namada From 578eb0eb857514509953387c2ff0bb50a6efdd9e Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 12 Sep 2024 20:13:17 +0200 Subject: [PATCH 13/26] Extracts ibc envelope verifiers to a separate function --- crates/ibc/src/lib.rs | 410 +++++++++++++++++++++--------------------- 1 file changed, 205 insertions(+), 205 deletions(-) diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index 0a7bb176ce..3c08273b4d 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -668,180 +668,129 @@ where Ok((msg.transfer, None)) } IbcMessage::Envelope(envelope) => { + self.trigger_envelope_verifier(envelope.as_ref())?; + execute(&mut self.ctx, &mut self.router, *envelope.clone()) .map_err(|e| Error::Context(Box::new(e)))?; - // Extract destination address from the packet to trigger its - // address FIXME: refactor, - match *envelope { - MsgEnvelope::Packet(PacketMsg::Recv(ref msg)) => { - if self.is_receiving_success(msg)? { - match msg.packet.port_id_on_b.as_str() { - FT_PORT_ID_STR => { - let packet_data = + // Extract MASP tx from the memo in the packet if needed + let masp_tx = match &*envelope { + MsgEnvelope::Packet(PacketMsg::Recv(msg)) + if self.is_receiving_success(msg)? => + { + extract_masp_tx_from_packet(&msg.packet) + } + #[cfg(is_apple_silicon)] + MsgEnvelope::Packet(PacketMsg::Ack(msg)) => { + // NOTE: This is unneeded but wasm compilation error + // happened if deleted on macOS with Apple Silicon + let _ = extract_masp_tx_from_packet(&msg.packet); + None + } + _ => None, + }; + Ok((None, masp_tx)) + } + } + } + + // Extract the relevant namada address from the packet (either sender or + // receiver) and trigger its vp + fn trigger_envelope_verifier( + &mut self, + envelope: &MsgEnvelope, + ) -> Result<(), Error> { + match envelope { + MsgEnvelope::Packet(PacketMsg::Recv(msg)) => { + if self.is_receiving_success(msg)? { + match msg.packet.port_id_on_b.as_str() { + FT_PORT_ID_STR => { + let packet_data = serde_json::from_slice::( &msg.packet.data, ).map_err(|_| Error::TokenTransfer(TokenTransferError::PacketDataDeserialization))?; - self.verifiers.borrow_mut().insert( - Address::from_str( - packet_data.receiver.as_ref(), + self.verifiers.borrow_mut().insert( + Address::from_str( + packet_data.receiver.as_ref(), + ) + .map_err( + |_| { + Error::TokenTransfer( + TokenTransferError::Other(format!( + "Cannot convert the receiver \ + address {}", + packet_data.receiver + )), ) - .map_err(|_| { - Error::TokenTransfer( - TokenTransferError::Other( - format!( - "Cannot convert the \ - receiver address {}", - packet_data.receiver - ), - ), - ) - })?, - ); - self.insert_verifiers()?; - } - NFT_PORT_ID_STR => { - let packet_data = serde_json::from_slice::< - NftPacketData, - >( - &msg.packet.data - ) - .map_err(|_| { - Error::NftTransfer( + }, + )?, + ); + self.insert_verifiers()?; + } + NFT_PORT_ID_STR => { + let packet_data = serde_json::from_slice::< + NftPacketData, + >( + &msg.packet.data + ) + .map_err(|_| { + Error::NftTransfer( NftTransferError::PacketDataDeserialization, ) - })?; - self.verifiers.borrow_mut().insert( - Address::from_str( - packet_data.receiver.as_ref(), + })?; + self.verifiers.borrow_mut().insert( + Address::from_str( + packet_data.receiver.as_ref(), + ) + .map_err( + |_| { + Error::NftTransfer( + NftTransferError::Other(format!( + "Cannot convert the receiver \ + address {}", + packet_data.receiver + )), ) - .map_err(|_| { - Error::NftTransfer( - NftTransferError::Other( - format!( - "Cannot convert the \ - receiver address {}", - packet_data.receiver - ), - ), - ) - })?, - ); - self.insert_verifiers()?; - } - _ => { - tracing::warn!( - "Receiver couldn't be extracted from \ - the unsupported IBC packet data for \ - Port ID {}", - msg.packet.port_id_on_b - ); - } - } + }, + )?, + ); + self.insert_verifiers()?; + } + _ => { + tracing::warn!( + "Receiver couldn't be extracted from the \ + unsupported IBC packet data for Port ID {}", + msg.packet.port_id_on_b + ); } } - MsgEnvelope::Packet(PacketMsg::Ack(ref msg)) => { - match msg.packet.port_id_on_a.as_str() { - FT_PORT_ID_STR => { - let packet_data = - serde_json::from_slice::( - &msg.packet.data, - ).map_err(|_| Error::TokenTransfer(TokenTransferError::PacketDataDeserialization))?; - - let ack = serde_json::from_slice::< - AcknowledgementStatus, - >( - msg.acknowledgement.as_ref() - ) - .map_err(|_| { - Error::TokenTransfer( - TokenTransferError::AckDeserialization, - ) - })?; - - if !ack.is_successful() { - self.verifiers.borrow_mut().insert( - Address::from_str( - packet_data.sender.as_ref(), - ) - .map_err(|_| { - Error::TokenTransfer( - TokenTransferError::Other( - format!( - "Cannot convert the \ - sender address {}", - packet_data.sender - ), - ), - ) - })?, - ); - self.insert_verifiers()?; - } - } - NFT_PORT_ID_STR => { - let packet_data = - serde_json::from_slice::( - &msg.packet.data, - ) - .map_err(|_| { - Error::NftTransfer( - NftTransferError::PacketDataDeserialization, - ) - })?; + } + } + MsgEnvelope::Packet(PacketMsg::Ack(msg)) => { + match msg.packet.port_id_on_a.as_str() { + FT_PORT_ID_STR => { + let packet_data = serde_json::from_slice::( + &msg.packet.data, + ) + .map_err(|_| { + Error::TokenTransfer( + TokenTransferError::PacketDataDeserialization, + ) + })?; - let ack = serde_json::from_slice::< - AcknowledgementStatus, - >( - msg.acknowledgement.as_ref() + let ack = + serde_json::from_slice::( + msg.acknowledgement.as_ref(), + ) + .map_err(|_| { + Error::TokenTransfer( + TokenTransferError::AckDeserialization, ) - .map_err(|_| { - Error::NftTransfer( - NftTransferError::AckDeserialization, - ) - })?; + })?; - if !ack.is_successful() { - self.verifiers.borrow_mut().insert( - Address::from_str( - packet_data.sender.as_ref(), - ) - .map_err(|_| { - Error::TokenTransfer( - TokenTransferError::Other( - format!( - "Cannot convert the \ - sender address {}", - packet_data.sender - ), - ), - ) - })?, - ); - self.insert_verifiers()?; - } - } - _ => { - tracing::warn!( - "Receiver couldn't be extracted from the \ - unsupported IBC packet data for Port ID \ - {}", - msg.packet.port_id_on_a - ); - } - }; - } - MsgEnvelope::Packet(PacketMsg::Timeout(ref msg)) => { - match msg.packet.port_id_on_a.as_str() { - FT_PORT_ID_STR => { - let packet_data = - serde_json::from_slice::( - &msg.packet.data, - ).map_err(|_| Error::TokenTransfer(TokenTransferError::PacketDataDeserialization))?; - self.verifiers.borrow_mut().insert( - Address::from_str( - packet_data.sender.as_ref(), - ) + if !ack.is_successful() { + self.verifiers.borrow_mut().insert( + Address::from_str(packet_data.sender.as_ref()) .map_err(|_| { Error::TokenTransfer( TokenTransferError::Other(format!( @@ -851,67 +800,118 @@ where )), ) })?, - ); - self.insert_verifiers()?; - } - NFT_PORT_ID_STR => { - let packet_data = - serde_json::from_slice::( - &msg.packet.data, - ) - .map_err(|_| { - Error::NftTransfer( + ); + self.insert_verifiers()?; + } + } + NFT_PORT_ID_STR => { + let packet_data = + serde_json::from_slice::( + &msg.packet.data, + ) + .map_err(|_| { + Error::NftTransfer( NftTransferError::PacketDataDeserialization, ) - })?; - self.verifiers.borrow_mut().insert( - Address::from_str( - packet_data.sender.as_ref(), - ) + })?; + + let ack = + serde_json::from_slice::( + msg.acknowledgement.as_ref(), + ) + .map_err(|_| { + Error::NftTransfer( + NftTransferError::AckDeserialization, + ) + })?; + + if !ack.is_successful() { + self.verifiers.borrow_mut().insert( + Address::from_str(packet_data.sender.as_ref()) .map_err(|_| { - Error::NftTransfer( - NftTransferError::Other(format!( + Error::TokenTransfer( + TokenTransferError::Other(format!( "Cannot convert the sender \ address {}", packet_data.sender )), ) })?, - ); - self.insert_verifiers()?; - } - _ => { - tracing::warn!( - "Receiver couldn't be extracted from the \ - unsupported IBC packet data for Port ID \ - {}", - msg.packet.port_id_on_a - ); - } + ); + self.insert_verifiers()?; } } - _ => (), - } - - // Extract MASP tx from the memo in the packet if needed - let masp_tx = match &*envelope { - MsgEnvelope::Packet(PacketMsg::Recv(msg)) - if self.is_receiving_success(msg)? => - { - extract_masp_tx_from_packet(&msg.packet) - } - #[cfg(is_apple_silicon)] - MsgEnvelope::Packet(PacketMsg::Ack(msg)) => { - // NOTE: This is unneeded but wasm compilation error - // happened if deleted on macOS with Apple Silicon - let _ = extract_masp_tx_from_packet(&msg.packet); - None + _ => { + tracing::warn!( + "Receiver couldn't be extracted from the \ + unsupported IBC packet data for Port ID {}", + msg.packet.port_id_on_a + ); } - _ => None, }; - Ok((None, masp_tx)) } + MsgEnvelope::Packet(PacketMsg::Timeout(msg)) => { + match msg.packet.port_id_on_a.as_str() { + FT_PORT_ID_STR => { + let packet_data = serde_json::from_slice::( + &msg.packet.data, + ) + .map_err(|_| { + Error::TokenTransfer( + TokenTransferError::PacketDataDeserialization, + ) + })?; + self.verifiers.borrow_mut().insert( + Address::from_str(packet_data.sender.as_ref()) + .map_err(|_| { + Error::TokenTransfer( + TokenTransferError::Other(format!( + "Cannot convert the sender \ + address {}", + packet_data.sender + )), + ) + })?, + ); + self.insert_verifiers()?; + } + NFT_PORT_ID_STR => { + let packet_data = + serde_json::from_slice::( + &msg.packet.data, + ) + .map_err(|_| { + Error::NftTransfer( + NftTransferError::PacketDataDeserialization, + ) + })?; + self.verifiers.borrow_mut().insert( + Address::from_str(packet_data.sender.as_ref()) + .map_err(|_| { + Error::NftTransfer(NftTransferError::Other( + format!( + "Cannot convert the sender \ + address {}", + packet_data.sender + ), + )) + })?, + ); + self.insert_verifiers()?; + } + _ => { + tracing::warn!( + "Receiver couldn't be extracted from the \ + unsupported IBC packet data for Port ID {}", + msg.packet.port_id_on_a + ); + } + } + } + _ => (), } + + Ok(()) } /// Check the result of receiving the packet by checking the packet From 8799b269c74f00c3f89f04e9b676f3b2e095b37b Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 12 Sep 2024 20:26:21 +0200 Subject: [PATCH 14/26] Refactors IBC envelope verifier --- crates/ibc/src/lib.rs | 187 +++++++++++++----------------------------- 1 file changed, 59 insertions(+), 128 deletions(-) diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index 3c08273b4d..069d5b6eaf 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -700,7 +700,7 @@ where &mut self, envelope: &MsgEnvelope, ) -> Result<(), Error> { - match envelope { + let opt_verifier = match envelope { MsgEnvelope::Packet(PacketMsg::Recv(msg)) => { if self.is_receiving_success(msg)? { match msg.packet.port_id_on_b.as_str() { @@ -709,23 +709,7 @@ where serde_json::from_slice::( &msg.packet.data, ).map_err(|_| Error::TokenTransfer(TokenTransferError::PacketDataDeserialization))?; - self.verifiers.borrow_mut().insert( - Address::from_str( - packet_data.receiver.as_ref(), - ) - .map_err( - |_| { - Error::TokenTransfer( - TokenTransferError::Other(format!( - "Cannot convert the receiver \ - address {}", - packet_data.receiver - )), - ) - }, - )?, - ); - self.insert_verifiers()?; + Some(packet_data.receiver) } NFT_PORT_ID_STR => { let packet_data = serde_json::from_slice::< @@ -738,23 +722,7 @@ where NftTransferError::PacketDataDeserialization, ) })?; - self.verifiers.borrow_mut().insert( - Address::from_str( - packet_data.receiver.as_ref(), - ) - .map_err( - |_| { - Error::NftTransfer( - NftTransferError::Other(format!( - "Cannot convert the receiver \ - address {}", - packet_data.receiver - )), - ) - }, - )?, - ); - self.insert_verifiers()?; + Some(packet_data.receiver) } _ => { tracing::warn!( @@ -762,52 +730,44 @@ where unsupported IBC packet data for Port ID {}", msg.packet.port_id_on_b ); + None } } + } else { + None } } MsgEnvelope::Packet(PacketMsg::Ack(msg)) => { - match msg.packet.port_id_on_a.as_str() { - FT_PORT_ID_STR => { - let packet_data = serde_json::from_slice::( - &msg.packet.data, - ) - .map_err(|_| { - Error::TokenTransfer( - TokenTransferError::PacketDataDeserialization, - ) - })?; - - let ack = - serde_json::from_slice::( - msg.acknowledgement.as_ref(), + let ack = serde_json::from_slice::( + msg.acknowledgement.as_ref(), + ) + .map_err(|_| { + Error::TokenTransfer(TokenTransferError::AckDeserialization) + })?; + + if ack.is_successful() { + None + } else { + match msg.packet.port_id_on_a.as_str() { + FT_PORT_ID_STR => { + let packet_data = serde_json::from_slice::< + PacketData, + >( + &msg.packet.data ) .map_err(|_| { Error::TokenTransfer( - TokenTransferError::AckDeserialization, - ) + TokenTransferError::PacketDataDeserialization, + ) })?; - if !ack.is_successful() { - self.verifiers.borrow_mut().insert( - Address::from_str(packet_data.sender.as_ref()) - .map_err(|_| { - Error::TokenTransfer( - TokenTransferError::Other(format!( - "Cannot convert the sender \ - address {}", - packet_data.sender - )), - ) - })?, - ); - self.insert_verifiers()?; + Some(packet_data.sender) } - } - NFT_PORT_ID_STR => { - let packet_data = - serde_json::from_slice::( - &msg.packet.data, + NFT_PORT_ID_STR => { + let packet_data = serde_json::from_slice::< + NftPacketData, + >( + &msg.packet.data ) .map_err(|_| { Error::NftTransfer( @@ -815,40 +775,19 @@ where ) })?; - let ack = - serde_json::from_slice::( - msg.acknowledgement.as_ref(), - ) - .map_err(|_| { - Error::NftTransfer( - NftTransferError::AckDeserialization, - ) - })?; - - if !ack.is_successful() { - self.verifiers.borrow_mut().insert( - Address::from_str(packet_data.sender.as_ref()) - .map_err(|_| { - Error::TokenTransfer( - TokenTransferError::Other(format!( - "Cannot convert the sender \ - address {}", - packet_data.sender - )), - ) - })?, + Some(packet_data.sender) + } + _ => { + tracing::warn!( + "Receiver couldn't be extracted from the \ + unsupported IBC packet data for Port ID {}", + msg.packet.port_id_on_a ); - self.insert_verifiers()?; + + None } } - _ => { - tracing::warn!( - "Receiver couldn't be extracted from the \ - unsupported IBC packet data for Port ID {}", - msg.packet.port_id_on_a - ); - } - }; + } } MsgEnvelope::Packet(PacketMsg::Timeout(msg)) => { match msg.packet.port_id_on_a.as_str() { @@ -861,19 +800,8 @@ where TokenTransferError::PacketDataDeserialization, ) })?; - self.verifiers.borrow_mut().insert( - Address::from_str(packet_data.sender.as_ref()) - .map_err(|_| { - Error::TokenTransfer( - TokenTransferError::Other(format!( - "Cannot convert the sender \ - address {}", - packet_data.sender - )), - ) - })?, - ); - self.insert_verifiers()?; + + Some(packet_data.sender) } NFT_PORT_ID_STR => { let packet_data = @@ -885,19 +813,8 @@ where NftTransferError::PacketDataDeserialization, ) })?; - self.verifiers.borrow_mut().insert( - Address::from_str(packet_data.sender.as_ref()) - .map_err(|_| { - Error::NftTransfer(NftTransferError::Other( - format!( - "Cannot convert the sender \ - address {}", - packet_data.sender - ), - )) - })?, - ); - self.insert_verifiers()?; + + Some(packet_data.sender) } _ => { tracing::warn!( @@ -905,10 +822,24 @@ where unsupported IBC packet data for Port ID {}", msg.packet.port_id_on_a ); + + None } } } - _ => (), + _ => None, + }; + + if let Some(verifier) = opt_verifier { + self.verifiers.borrow_mut().insert( + Address::from_str(verifier.as_ref()).map_err(|_| { + Error::TokenTransfer(TokenTransferError::Other(format!( + "Cannot convert the address {}", + verifier + ))) + })?, + ); + self.insert_verifiers()?; } Ok(()) From c74695c11dd3db37ed529b603e067f42f94799b1 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 12 Sep 2024 22:17:28 +0200 Subject: [PATCH 15/26] Inserts verifier before executing IBC enevelope and does not fail on errors --- crates/ibc/src/lib.rs | 231 ++++++++++++++---------------------------- 1 file changed, 78 insertions(+), 153 deletions(-) diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index 069d5b6eaf..b8e1588d1d 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -668,8 +668,20 @@ where Ok((msg.transfer, None)) } IbcMessage::Envelope(envelope) => { - self.trigger_envelope_verifier(envelope.as_ref())?; - + if let Some(verifier) = get_envelope_verifier(envelope.as_ref()) + { + self.verifiers.borrow_mut().insert( + Address::from_str(verifier.as_ref()).map_err(|_| { + Error::NftTransfer(NftTransferError::Other( + format!( + "Cannot convert the address {}", + verifier + ), + )) + })?, + ); + self.insert_verifiers()?; + } execute(&mut self.ctx, &mut self.router, *envelope.clone()) .map_err(|e| Error::Context(Box::new(e)))?; @@ -694,157 +706,6 @@ where } } - // Extract the relevant namada address from the packet (either sender or - // receiver) and trigger its vp - fn trigger_envelope_verifier( - &mut self, - envelope: &MsgEnvelope, - ) -> Result<(), Error> { - let opt_verifier = match envelope { - MsgEnvelope::Packet(PacketMsg::Recv(msg)) => { - if self.is_receiving_success(msg)? { - match msg.packet.port_id_on_b.as_str() { - FT_PORT_ID_STR => { - let packet_data = - serde_json::from_slice::( - &msg.packet.data, - ).map_err(|_| Error::TokenTransfer(TokenTransferError::PacketDataDeserialization))?; - Some(packet_data.receiver) - } - NFT_PORT_ID_STR => { - let packet_data = serde_json::from_slice::< - NftPacketData, - >( - &msg.packet.data - ) - .map_err(|_| { - Error::NftTransfer( - NftTransferError::PacketDataDeserialization, - ) - })?; - Some(packet_data.receiver) - } - _ => { - tracing::warn!( - "Receiver couldn't be extracted from the \ - unsupported IBC packet data for Port ID {}", - msg.packet.port_id_on_b - ); - None - } - } - } else { - None - } - } - MsgEnvelope::Packet(PacketMsg::Ack(msg)) => { - let ack = serde_json::from_slice::( - msg.acknowledgement.as_ref(), - ) - .map_err(|_| { - Error::TokenTransfer(TokenTransferError::AckDeserialization) - })?; - - if ack.is_successful() { - None - } else { - match msg.packet.port_id_on_a.as_str() { - FT_PORT_ID_STR => { - let packet_data = serde_json::from_slice::< - PacketData, - >( - &msg.packet.data - ) - .map_err(|_| { - Error::TokenTransfer( - TokenTransferError::PacketDataDeserialization, - ) - })?; - - Some(packet_data.sender) - } - NFT_PORT_ID_STR => { - let packet_data = serde_json::from_slice::< - NftPacketData, - >( - &msg.packet.data - ) - .map_err(|_| { - Error::NftTransfer( - NftTransferError::PacketDataDeserialization, - ) - })?; - - Some(packet_data.sender) - } - _ => { - tracing::warn!( - "Receiver couldn't be extracted from the \ - unsupported IBC packet data for Port ID {}", - msg.packet.port_id_on_a - ); - - None - } - } - } - } - MsgEnvelope::Packet(PacketMsg::Timeout(msg)) => { - match msg.packet.port_id_on_a.as_str() { - FT_PORT_ID_STR => { - let packet_data = serde_json::from_slice::( - &msg.packet.data, - ) - .map_err(|_| { - Error::TokenTransfer( - TokenTransferError::PacketDataDeserialization, - ) - })?; - - Some(packet_data.sender) - } - NFT_PORT_ID_STR => { - let packet_data = - serde_json::from_slice::( - &msg.packet.data, - ) - .map_err(|_| { - Error::NftTransfer( - NftTransferError::PacketDataDeserialization, - ) - })?; - - Some(packet_data.sender) - } - _ => { - tracing::warn!( - "Receiver couldn't be extracted from the \ - unsupported IBC packet data for Port ID {}", - msg.packet.port_id_on_a - ); - - None - } - } - } - _ => None, - }; - - if let Some(verifier) = opt_verifier { - self.verifiers.borrow_mut().insert( - Address::from_str(verifier.as_ref()).map_err(|_| { - Error::TokenTransfer(TokenTransferError::Other(format!( - "Cannot convert the address {}", - verifier - ))) - })?, - ); - self.insert_verifiers()?; - } - - Ok(()) - } - /// Check the result of receiving the packet by checking the packet /// acknowledgement pub fn is_receiving_success( @@ -917,6 +778,70 @@ where } } +// Extract the involved namada address from the packet (either sender or +// receiver) to trigger its vp. Returns None if an address could not be found +fn get_envelope_verifier( + envelope: &MsgEnvelope, +) -> Option { + match envelope { + MsgEnvelope::Packet(PacketMsg::Recv(msg)) => { + match msg.packet.port_id_on_b.as_str() { + FT_PORT_ID_STR => { + serde_json::from_slice::(&msg.packet.data) + .ok() + .map(|packet_data| packet_data.receiver) + } + NFT_PORT_ID_STR => { + serde_json::from_slice::(&msg.packet.data) + .ok() + .map(|packet_data| packet_data.receiver) + } + _ => None, + } + } + MsgEnvelope::Packet(PacketMsg::Ack(msg)) => serde_json::from_slice::< + AcknowledgementStatus, + >( + msg.acknowledgement.as_ref(), + ) + .map_or(None, |ack| { + if ack.is_successful() { + None + } else { + match msg.packet.port_id_on_a.as_str() { + FT_PORT_ID_STR => { + serde_json::from_slice::(&msg.packet.data) + .ok() + .map(|packet_data| packet_data.sender) + } + NFT_PORT_ID_STR => serde_json::from_slice::( + &msg.packet.data, + ) + .ok() + .map(|packet_data| packet_data.sender), + _ => None, + } + } + }), + MsgEnvelope::Packet(PacketMsg::Timeout(msg)) => { + match msg.packet.port_id_on_a.as_str() { + FT_PORT_ID_STR => { + serde_json::from_slice::(&msg.packet.data) + .ok() + .map(|packet_data| packet_data.sender) + } + NFT_PORT_ID_STR => { + serde_json::from_slice::(&msg.packet.data) + .ok() + .map(|packet_data| packet_data.sender) + } + _ => None, + } + } + _ => None, + } +} + /// Tries to decode transaction data to an `IbcMessage` pub fn decode_message( tx_data: &[u8], From 1651cadfd0079958e40b391c115851fcacc426c5 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 13 Sep 2024 16:52:37 +0200 Subject: [PATCH 16/26] Unit tests verifiers in multitoken vp --- crates/trans_token/src/vp.rs | 74 ++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/crates/trans_token/src/vp.rs b/crates/trans_token/src/vp.rs index abc9a8c68f..1f29a10a9a 100644 --- a/crates/trans_token/src/vp.rs +++ b/crates/trans_token/src/vp.rs @@ -1113,4 +1113,78 @@ mod tests { Ok(_) ); } + + // The multitoken vps ensures that all the involved parties have their vp + // triggered + #[test] + fn test_verifiers() { + let mut state = init_state(); + let src1 = established_address_1(); + let dest1 = namada_core::address::MASP; + let src2 = namada_core::address::IBC; + let dest2 = established_address_2(); + let mut keys_changed = transfer(&mut state, &src1, &dest1); + keys_changed.append(&mut transfer(&mut state, &src2, &dest2)); + + let tx_index = TxIndex::default(); + let BatchedTx { tx, cmt } = dummy_tx(&state); + let gas_meter = RefCell::new(VpGasMeter::new_from_tx_meter( + &TxGasMeter::new(u64::MAX), + )); + let (vp_vp_cache, _vp_cache_dir) = vp_cache(); + + let parties = BTreeSet::from([src1, dest1, src2, dest2]); + + // One at a time remove one of the expected verifiers of this + // transaction and check that the multitoken vp rejects the tx + for verifier in &parties { + let mut verifiers = parties.clone(); + verifiers.remove(verifier); + + let ctx = Ctx::new( + &ADDRESS, + &state, + &tx, + &cmt, + &tx_index, + &gas_meter, + &keys_changed, + &verifiers, + vp_vp_cache.clone(), + ); + + let vp = MultitokenVp::new(ctx); + let err_msg = format!( + "The vp of the address {} has not been triggered", + verifier + ); + + match vp + .validate_tx(&tx.batch_ref_tx(&cmt), &keys_changed, &verifiers) + .unwrap_err() + { + Error::AllocMessage(msg) if msg == err_msg => (), + _ => panic!("Test failed with an unexpected error"), + } + } + + // Fnally run the validation with all the required verifiers + let ctx = Ctx::new( + &ADDRESS, + &state, + &tx, + &cmt, + &tx_index, + &gas_meter, + &keys_changed, + &parties, + vp_vp_cache, + ); + + let vp = MultitokenVp::new(ctx); + assert!( + vp.validate_tx(&tx.batch_ref_tx(&cmt), &keys_changed, &parties) + .is_ok() + ); + } } From 2fb578d6440a8067420c87bd2659fc7b5f27b3ea Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 13 Sep 2024 18:19:19 +0200 Subject: [PATCH 17/26] MASP VP evaluates its balance. Adds unit test --- Cargo.lock | 4 + crates/shielded_token/Cargo.toml | 6 +- crates/shielded_token/src/storage_key.rs | 18 ++-- crates/shielded_token/src/utils.rs | 8 +- crates/shielded_token/src/vp.rs | 101 +++++++++++++++++++++++ crates/trans_token/src/vp.rs | 21 +++-- wasm/Cargo.lock | 1 + wasm_for_tests/Cargo.lock | 1 + 8 files changed, 142 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index db39370be6..ffd306a156 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5377,6 +5377,8 @@ dependencies = [ "namada_core", "namada_events", "namada_gas", + "namada_governance", + "namada_ibc", "namada_io", "namada_macros", "namada_migrations", @@ -5385,6 +5387,8 @@ dependencies = [ "namada_systems", "namada_trans_token", "namada_tx", + "namada_vm", + "namada_vp", "namada_vp_env", "namada_wallet", "proptest", diff --git a/crates/shielded_token/Cargo.toml b/crates/shielded_token/Cargo.toml index 4243999b25..1740853767 100644 --- a/crates/shielded_token/Cargo.toml +++ b/crates/shielded_token/Cargo.toml @@ -46,6 +46,7 @@ namada_macros = {path = "../macros" } namada_migrations = { path = "../migrations", optional = true } namada_state = { path = "../state" } namada_systems = { path = "../systems" } +namada_trans_token = { path = "../trans_token" } namada_tx = { path = "../tx" } namada_vp_env = { path = "../vp_env" } namada_wallet = { path = "../wallet", optional = true } @@ -78,9 +79,12 @@ xorf.workspace = true [dev-dependencies] namada_gas = { path = "../gas" } +namada_governance = { path = "../governance", features = ["testing"] } +namada_ibc = { path = "../ibc", features = ["testing"] } namada_parameters = { path = "../parameters", features = ["testing"] } namada_state = { path = "../state", features = ["testing"] } -namada_trans_token = { path = "../trans_token" } +namada_vm = { path = "../vm", features = ["testing"] } +namada_vp = { path = "../vp" } lazy_static.workspace = true masp_primitives = { workspace = true, features = ["test-dependencies"] } diff --git a/crates/shielded_token/src/storage_key.rs b/crates/shielded_token/src/storage_key.rs index e7a76a71e1..67bf21f9cd 100644 --- a/crates/shielded_token/src/storage_key.rs +++ b/crates/shielded_token/src/storage_key.rs @@ -6,6 +6,7 @@ use namada_core::address::{self, Address}; use namada_core::hash::Hash; use namada_core::storage::{self, DbKeySeg, KeySeg}; use namada_systems::trans_token; +use namada_trans_token::storage_key::is_any_token_balance_key; /// Key segment prefix for the nullifiers pub const MASP_NULLIFIERS_KEY: &str = "nullifiers"; @@ -83,12 +84,20 @@ pub fn masp_last_inflation_key( .with_segment(MASP_LAST_INFLATION_KEY.to_owned()) } +fn is_masp_balance_key(key: &storage::Key) -> bool { + is_any_token_balance_key(key) + .map_or(false, |[_token, owner]| *owner == address::MASP) +} + /// Check if the given storage key is a masp key pub fn is_masp_key(key: &storage::Key) -> bool { - matches!(&key.segments[..], - [DbKeySeg::AddressSeg(addr), ..] if *addr == address::MASP - ) + match &key.segments[..] { + [DbKeySeg::AddressSeg(addr), ..] if *addr == address::MASP => true, + // The balance key of the MASP is also considered a MASP key + _ => is_masp_balance_key(key), + } } + /// Check if the given storage key is allowed to be touched by a masp transfer pub fn is_masp_transfer_key(key: &storage::Key) -> bool { match &key.segments[..] { @@ -98,13 +107,12 @@ pub fn is_masp_transfer_key(key: &storage::Key) -> bool { { true } - [ DbKeySeg::AddressSeg(addr), DbKeySeg::StringSeg(key), DbKeySeg::StringSeg(_nullifier), ] if *addr == address::MASP && key == MASP_NULLIFIERS_KEY => true, - _ => false, + _ => is_masp_balance_key(key), } } diff --git a/crates/shielded_token/src/utils.rs b/crates/shielded_token/src/utils.rs index 11c0fe9fc7..46b50e66b8 100644 --- a/crates/shielded_token/src/utils.rs +++ b/crates/shielded_token/src/utils.rs @@ -73,11 +73,9 @@ pub fn handle_masp_tx( Ok(()) } -/// Check if a transaction was a MASP transaction. -/// -/// This means that at least one key owned by MASP was changed. We cannot simply -/// check that the MASP VP was triggered, as this can be manually requested to -/// be triggered by users. +/// Check if a transaction was a MASP transfer transaction by looking at the +/// changed keys. We cannot simply check that the MASP VP was triggered, as this +/// can be manually requested to be triggered by users. pub fn is_masp_transfer(changed_keys: &BTreeSet) -> bool { changed_keys.iter().any(is_masp_transfer_key) } diff --git a/crates/shielded_token/src/vp.rs b/crates/shielded_token/src/vp.rs index 1851cd6fcb..ef8cdc2781 100644 --- a/crates/shielded_token/src/vp.rs +++ b/crates/shielded_token/src/vp.rs @@ -878,3 +878,104 @@ fn verify_sapling_balancing_value( Err(error) } } + +#[cfg(test)] +mod shielded_token_tests { + use std::cell::RefCell; + use std::collections::BTreeSet; + + use namada_core::address::testing::nam; + use namada_core::address::MASP; + use namada_core::borsh::BorshSerializeExt; + use namada_gas::{TxGasMeter, VpGasMeter}; + use namada_state::testing::TestState; + use namada_state::{StateRead, TxIndex}; + use namada_trans_token::storage_key::balance_key; + use namada_trans_token::Amount; + use namada_tx::{BatchedTx, Tx}; + use namada_vm::wasm::compilation_cache::common::testing::vp_cache; + use namada_vm::wasm::run::VpEvalWasm; + use namada_vm::wasm::VpCache; + use namada_vm::WasmCacheRwAccess; + use namada_vp::native_vp::{self, CtxPostStorageRead, CtxPreStorageRead}; + + type CA = WasmCacheRwAccess; + type Eval = VpEvalWasm<::D, ::H, CA>; + type Ctx<'ctx, S> = native_vp::Ctx<'ctx, S, VpCache, Eval>; + type MaspVp<'ctx, S> = super::MaspVp< + 'ctx, + Ctx<'ctx, S>, + namada_parameters::Store< + CtxPreStorageRead<'ctx, 'ctx, S, VpCache, Eval>, + >, + namada_governance::Store< + CtxPreStorageRead<'ctx, 'ctx, S, VpCache, Eval>, + >, + namada_ibc::Store< + CtxPostStorageRead<'ctx, 'ctx, S, VpCache, Eval>, + >, + namada_trans_token::Store< + CtxPreStorageRead<'ctx, 'ctx, S, VpCache, Eval>, + >, + (), + >; + + // Changing only the balance key of the MASP alone is invalid + #[test] + fn test_balance_change() { + let mut state = TestState::default(); + namada_parameters::init_test_storage(&mut state).unwrap(); + let src_key = balance_key(&nam(), &MASP); + let amount = Amount::native_whole(100); + let keys_changed = BTreeSet::from([src_key.clone()]); + let verifiers = Default::default(); + + // Initialize MASP balance + state + .db_write(&src_key, Amount::native_whole(100).serialize_to_vec()) + .unwrap(); + + state.db_write(&src_key, amount.serialize_to_vec()).unwrap(); + + let tx_index = TxIndex::default(); + let mut tx = Tx::from_type(namada_tx::data::TxType::Raw); + tx.push_default_inner_tx(); + let BatchedTx { tx, cmt } = tx.batch_first_tx(); + + // Test both credit and debit + for new_amount in [150, 1] { + // Update the balance key + let new_amount = Amount::native_whole(new_amount); + let _ = state + .write_log_mut() + .write(&src_key, new_amount.serialize_to_vec()) + .unwrap(); + + let gas_meter = RefCell::new(VpGasMeter::new_from_tx_meter( + &TxGasMeter::new(u64::MAX), + )); + let (vp_vp_cache, _vp_cache_dir) = vp_cache(); + let ctx = Ctx::new( + &MASP, + &state, + &tx, + &cmt, + &tx_index, + &gas_meter, + &keys_changed, + &verifiers, + vp_vp_cache, + ); + + assert!( + MaspVp::validate_tx( + &ctx, + &tx.batch_ref_tx(&cmt), + &keys_changed, + &verifiers + ) + .is_err() + ); + } + } +} diff --git a/crates/trans_token/src/vp.rs b/crates/trans_token/src/vp.rs index 1f29a10a9a..64b7aae644 100644 --- a/crates/trans_token/src/vp.rs +++ b/crates/trans_token/src/vp.rs @@ -1153,15 +1153,18 @@ mod tests { vp_vp_cache.clone(), ); - let vp = MultitokenVp::new(ctx); let err_msg = format!( "The vp of the address {} has not been triggered", verifier ); - match vp - .validate_tx(&tx.batch_ref_tx(&cmt), &keys_changed, &verifiers) - .unwrap_err() + match MultitokenVp::validate_tx( + &ctx, + &tx.batch_ref_tx(&cmt), + &keys_changed, + &verifiers, + ) + .unwrap_err() { Error::AllocMessage(msg) if msg == err_msg => (), _ => panic!("Test failed with an unexpected error"), @@ -1181,10 +1184,14 @@ mod tests { vp_vp_cache, ); - let vp = MultitokenVp::new(ctx); assert!( - vp.validate_tx(&tx.batch_ref_tx(&cmt), &keys_changed, &parties) - .is_ok() + MultitokenVp::validate_tx( + &ctx, + &tx.batch_ref_tx(&cmt), + &keys_changed, + &parties + ) + .is_ok() ); } } diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 70b07f5340..c94f746054 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -3978,6 +3978,7 @@ dependencies = [ "namada_macros", "namada_state", "namada_systems", + "namada_trans_token", "namada_tx", "namada_vp_env", "namada_wallet", diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index b854ddb998..c45117e756 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -2177,6 +2177,7 @@ dependencies = [ "namada_macros", "namada_state", "namada_systems", + "namada_trans_token", "namada_tx", "namada_vp_env", "rand", From 160f8b507e55ad06f4101ffd3666fb86336afd87 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 13 Sep 2024 20:30:36 +0200 Subject: [PATCH 18/26] Improves ibc e2e test --- crates/tests/src/e2e/ibc_tests.rs | 52 +++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 632e453e23..a007dd18d9 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -73,6 +73,9 @@ const UPGRADED_CHAIN_ID: &str = "upgraded-chain"; /// - When unshielding transfer failure, /// - Mint the IBC token for the refund /// - Unescrow the token for the refund +/// 6. Malformed shielded actions +/// - Missing memo +/// - Wrong memo #[test] fn ibc_transfers() -> Result<()> { let update_genesis = @@ -264,6 +267,8 @@ fn ibc_transfers() -> Result<()> { &port_id_namada, &channel_id_namada, )?; + // FIXME: try to remove the timeouts where we technically don't need them. I + // believe we only need them when the transaction fails because of vps transfer_from_gaia( &test_gaia, GAIA_USER, @@ -384,6 +389,53 @@ fn ibc_transfers() -> Result<()> { check_balance(&test, AA_VIEWING_KEY, APFEL, 0)?; check_balance(&test, IBC_REFUND_TARGET_ALIAS, APFEL, 1)?; + // 6. Malformed shielded actions + + // Check initial balance + check_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 40)?; + + // Missing memo + transfer_from_gaia( + &test_gaia, + GAIA_USER, + AA_PAYMENT_ADDRESS, + GAIA_COIN, + 100, + &port_id_gaia, + &channel_id_gaia, + None, + // MASP VP shall reject it, make it timeout + Some(Duration::new(10, 0)), + )?; + wait_for_packet_relay(&port_id_namada, &channel_id_namada, &test)?; + // Check the balance didn't change + check_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 40)?; + + // Wrong memo (different amount) + let shielding_data_path = gen_ibc_shielding_data( + &test, + AA_PAYMENT_ADDRESS, + GAIA_COIN, + 100, + &port_id_namada, + &channel_id_namada, + )?; + transfer_from_gaia( + &test_gaia, + GAIA_USER, + AA_PAYMENT_ADDRESS, + GAIA_COIN, + 101, + &port_id_gaia, + &channel_id_gaia, + Some(shielding_data_path), + // MASP VP shall reject it, make it timeout + Some(Duration::new(10, 0)), + )?; + wait_for_packet_relay(&port_id_gaia, &channel_id_gaia, &test_gaia)?; + // Check the balances didn't change + check_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 40)?; + Ok(()) } From 1ae39190ea73e8d531b4de138966c76fd57b8bce Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 13 Sep 2024 20:54:18 +0200 Subject: [PATCH 19/26] Removes useless timeouts in ibc test --- crates/tests/src/e2e/ibc_tests.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index a007dd18d9..9a37fe2251 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -159,7 +159,7 @@ fn ibc_transfers() -> Result<()> { &port_id_gaia, &channel_id_gaia, None, - Some(Duration::new(10, 0)), + None, )?; wait_for_packet_relay(&port_id_gaia, &channel_id_gaia, &test)?; @@ -267,8 +267,6 @@ fn ibc_transfers() -> Result<()> { &port_id_namada, &channel_id_namada, )?; - // FIXME: try to remove the timeouts where we technically don't need them. I - // believe we only need them when the transaction fails because of vps transfer_from_gaia( &test_gaia, GAIA_USER, @@ -278,7 +276,7 @@ fn ibc_transfers() -> Result<()> { &port_id_gaia, &channel_id_gaia, Some(memo_path), - Some(Duration::new(10, 0)), + None, )?; wait_for_packet_relay(&port_id_gaia, &channel_id_gaia, &test)?; // Check the token on Namada From 902b06ae46aa879cb45446f3bc9ce56e6d902ded Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 13 Sep 2024 21:31:57 +0200 Subject: [PATCH 20/26] Removes system cross dep --- crates/shielded_token/Cargo.toml | 2 +- crates/shielded_token/src/storage_key.rs | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/shielded_token/Cargo.toml b/crates/shielded_token/Cargo.toml index 1740853767..126c543462 100644 --- a/crates/shielded_token/Cargo.toml +++ b/crates/shielded_token/Cargo.toml @@ -46,7 +46,6 @@ namada_macros = {path = "../macros" } namada_migrations = { path = "../migrations", optional = true } namada_state = { path = "../state" } namada_systems = { path = "../systems" } -namada_trans_token = { path = "../trans_token" } namada_tx = { path = "../tx" } namada_vp_env = { path = "../vp_env" } namada_wallet = { path = "../wallet", optional = true } @@ -83,6 +82,7 @@ namada_governance = { path = "../governance", features = ["testing"] } namada_ibc = { path = "../ibc", features = ["testing"] } namada_parameters = { path = "../parameters", features = ["testing"] } namada_state = { path = "../state", features = ["testing"] } +namada_trans_token = { path = "../trans_token" } namada_vm = { path = "../vm", features = ["testing"] } namada_vp = { path = "../vp" } diff --git a/crates/shielded_token/src/storage_key.rs b/crates/shielded_token/src/storage_key.rs index 67bf21f9cd..4eb14ea9c7 100644 --- a/crates/shielded_token/src/storage_key.rs +++ b/crates/shielded_token/src/storage_key.rs @@ -6,8 +6,9 @@ use namada_core::address::{self, Address}; use namada_core::hash::Hash; use namada_core::storage::{self, DbKeySeg, KeySeg}; use namada_systems::trans_token; -use namada_trans_token::storage_key::is_any_token_balance_key; +// Key segment for a balance key +const BALANCE_STORAGE_KEY: &str = "balance"; /// Key segment prefix for the nullifiers pub const MASP_NULLIFIERS_KEY: &str = "nullifiers"; /// Key segment prefix for the note commitment merkle tree @@ -85,8 +86,14 @@ pub fn masp_last_inflation_key( } fn is_masp_balance_key(key: &storage::Key) -> bool { - is_any_token_balance_key(key) - .map_or(false, |[_token, owner]| *owner == address::MASP) + matches!( + &key.segments[..], + [DbKeySeg::AddressSeg(addr), DbKeySeg::AddressSeg(_token), DbKeySeg::StringSeg(balance), DbKeySeg::AddressSeg(owner)] + if *addr + == Address::Internal(address::InternalAddress::Multitoken) + && balance == BALANCE_STORAGE_KEY + && *owner == address::MASP + ) } /// Check if the given storage key is a masp key From 5c1902719a1514c0e69501024df667942fc87083 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 13 Sep 2024 22:19:25 +0200 Subject: [PATCH 21/26] Updates lock files for wasm --- wasm/Cargo.lock | 1 - wasm_for_tests/Cargo.lock | 1 - 2 files changed, 2 deletions(-) diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index c94f746054..70b07f5340 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -3978,7 +3978,6 @@ dependencies = [ "namada_macros", "namada_state", "namada_systems", - "namada_trans_token", "namada_tx", "namada_vp_env", "namada_wallet", diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index c45117e756..b854ddb998 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -2177,7 +2177,6 @@ dependencies = [ "namada_macros", "namada_state", "namada_systems", - "namada_trans_token", "namada_tx", "namada_vp_env", "rand", From e5973473edbbf06f3a8aa1d340128b1b44f5b363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Sat, 14 Sep 2024 15:07:09 +0200 Subject: [PATCH 22/26] add a test for masp balance key --- crates/shielded_token/src/storage_key.rs | 3 +- crates/token/src/lib.rs | 36 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/crates/shielded_token/src/storage_key.rs b/crates/shielded_token/src/storage_key.rs index 4eb14ea9c7..f79d5865f0 100644 --- a/crates/shielded_token/src/storage_key.rs +++ b/crates/shielded_token/src/storage_key.rs @@ -85,7 +85,8 @@ pub fn masp_last_inflation_key( .with_segment(MASP_LAST_INFLATION_KEY.to_owned()) } -fn is_masp_balance_key(key: &storage::Key) -> bool { +/// Check if the given storage key is MASP transparent balance key +pub fn is_masp_balance_key(key: &storage::Key) -> bool { matches!( &key.segments[..], [DbKeySeg::AddressSeg(addr), DbKeySeg::AddressSeg(_token), DbKeySeg::StringSeg(balance), DbKeySeg::AddressSeg(owner)] diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index 69460ff73e..dff43909bf 100644 --- a/crates/token/src/lib.rs +++ b/crates/token/src/lib.rs @@ -474,6 +474,9 @@ pub mod testing { #[cfg(test)] mod test_token_transfer_actions { use namada_core::address::testing::{established_address_1, nam}; + use namada_core::address::{self}; + use namada_core::storage::DbKeySeg; + use namada_shielded_token::storage_key::is_masp_balance_key; use super::*; @@ -599,4 +602,37 @@ mod test_token_transfer_actions { transfer2(BTreeMap::from([(account.clone(), amount_80)])), ); } + + /// Check that the MASP balance key is a transparent balance key. + #[test] + fn test_masp_trans_balance_key() { + let token = nam(); + let key = namada_trans_token::storage_key::balance_key( + &token, + &address::MASP, + ); + assert!(is_masp_balance_key(&key)); + + // Replace the token address and check that it still matches + let mut another_token_key = key.clone(); + another_token_key.segments[1] = + DbKeySeg::AddressSeg(address::testing::gen_established_address()); + assert!(is_masp_balance_key(&another_token_key)); + + // Replace one of the non-token segments with some random string or + // address and check that it no longer matches. + // Skip index 1 which is the token address. + for segment_num in [0, 2, 3] { + let mut key = key.clone(); + key.segments[segment_num] = match &key.segments[segment_num] { + DbKeySeg::AddressSeg(_) => DbKeySeg::AddressSeg( + address::testing::gen_established_address(), + ), + DbKeySeg::StringSeg(_) => { + DbKeySeg::StringSeg("Dangus".to_string()) + } + }; + assert!(!is_masp_balance_key(&key)); + } + } } From b634d91444e30af89b2f0921d834c3f821afbcb8 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 13 Sep 2024 21:34:53 +0200 Subject: [PATCH 23/26] Changelog #3804 --- .../unreleased/bug-fixes/3804-fix-transfer-dest-vp-trigger.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/bug-fixes/3804-fix-transfer-dest-vp-trigger.md diff --git a/.changelog/unreleased/bug-fixes/3804-fix-transfer-dest-vp-trigger.md b/.changelog/unreleased/bug-fixes/3804-fix-transfer-dest-vp-trigger.md new file mode 100644 index 0000000000..262d4934bd --- /dev/null +++ b/.changelog/unreleased/bug-fixes/3804-fix-transfer-dest-vp-trigger.md @@ -0,0 +1,3 @@ +- The multitoken vp now checks that the involved parties + validate the transaction. Improved tests and transfer code. + ([\#3804](https://github.com/anoma/namada/pull/3804)) \ No newline at end of file From 5d5e9db23c4172e401baf985551fc16d2aa13c8a Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sun, 15 Sep 2024 10:58:32 +0200 Subject: [PATCH 24/26] Fixes doc lint --- crates/shielded_token/src/utils.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/shielded_token/src/utils.rs b/crates/shielded_token/src/utils.rs index 46b50e66b8..1cdd51b18e 100644 --- a/crates/shielded_token/src/utils.rs +++ b/crates/shielded_token/src/utils.rs @@ -73,9 +73,11 @@ pub fn handle_masp_tx( Ok(()) } -/// Check if a transaction was a MASP transfer transaction by looking at the -/// changed keys. We cannot simply check that the MASP VP was triggered, as this -/// can be manually requested to be triggered by users. +/// Check if a transaction is a MASP transfer transaction. +/// +/// We do that by looking at the changed keys. We cannot simply check that the +/// MASP VP was triggered, as this can be manually requested to be triggered by +/// users. pub fn is_masp_transfer(changed_keys: &BTreeSet) -> bool { changed_keys.iter().any(is_masp_transfer_key) } From b11c556ff8e70214e4db6298ec1d97e77770d48c Mon Sep 17 00:00:00 2001 From: yito88 Date: Mon, 16 Sep 2024 10:14:59 +0200 Subject: [PATCH 25/26] check gaia balance --- crates/tests/src/e2e/ibc_tests.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 9a37fe2251..cf0f4c8747 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -391,6 +391,7 @@ fn ibc_transfers() -> Result<()> { // Check initial balance check_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 40)?; + check_gaia_balance(&test_gaia, GAIA_USER, GAIA_COIN, 810)?; // Missing memo transfer_from_gaia( @@ -408,6 +409,7 @@ fn ibc_transfers() -> Result<()> { wait_for_packet_relay(&port_id_namada, &channel_id_namada, &test)?; // Check the balance didn't change check_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 40)?; + check_gaia_balance(&test_gaia, GAIA_USER, GAIA_COIN, 810)?; // Wrong memo (different amount) let shielding_data_path = gen_ibc_shielding_data( @@ -433,6 +435,7 @@ fn ibc_transfers() -> Result<()> { wait_for_packet_relay(&port_id_gaia, &channel_id_gaia, &test_gaia)?; // Check the balances didn't change check_balance(&test, AA_VIEWING_KEY, &ibc_denom_on_namada, 40)?; + check_gaia_balance(&test_gaia, GAIA_USER, GAIA_COIN, 810)?; Ok(()) } From 41583ebe8613b33af8ed9d700758c2ecefdc7ebe Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 16 Sep 2024 16:39:42 +0200 Subject: [PATCH 26/26] Fixes wrong error type for ibc envelope --- crates/ibc/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index b8e1588d1d..ff5d8d34f4 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -143,6 +143,8 @@ pub enum Error { ChainId(IdentifierError), #[error("Verifier insertion error: {0}")] Verifier(StorageError), + #[error("IBC error: {0}")] + Other(String), } impl From for StorageError { @@ -672,11 +674,9 @@ where { self.verifiers.borrow_mut().insert( Address::from_str(verifier.as_ref()).map_err(|_| { - Error::NftTransfer(NftTransferError::Other( - format!( - "Cannot convert the address {}", - verifier - ), + Error::Other(format!( + "Cannot convert the address {}", + verifier, )) })?, );