From 99f356649ae520af7233235763e526c96d9996b8 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 10 Oct 2025 11:18:41 +0200 Subject: [PATCH 01/17] Removes speculative shielded context --- crates/apps_lib/src/client/masp.rs | 2 +- crates/apps_lib/src/client/tx.rs | 78 +------------------ crates/node/src/bench_utils.rs | 40 ++-------- crates/shielded_token/src/masp.rs | 75 ++++-------------- .../src/masp/shielded_sync/dispatcher.rs | 2 +- .../src/masp/shielded_wallet.rs | 52 ++----------- .../src/masp/wallet_migrations.rs | 4 +- 7 files changed, 33 insertions(+), 220 deletions(-) diff --git a/crates/apps_lib/src/client/masp.rs b/crates/apps_lib/src/client/masp.rs index 45b1953fa5d..dbd8a354007 100644 --- a/crates/apps_lib/src/client/masp.rs +++ b/crates/apps_lib/src/client/masp.rs @@ -126,7 +126,7 @@ pub async fn syncing< // for the shielded history. This is needed for integration // tests only as the cli wallet is not supposed to compile the // history of shielded transactions - shielded.load_confirmed().await; + shielded.load().await; let current_masp_epoch = namada_sdk::rpc::query_masp_epoch(&client).await?; let epochs: Vec<_> = MaspEpoch::iter_bounds_inclusive( diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 70eb38ebcd7..650efa74090 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -14,7 +14,6 @@ use masp_primitives::transaction::components::sapling::fees::InputView; use masp_primitives::zip32::{ ExtendedFullViewingKey, ExtendedKey, PseudoExtendedKey, }; -use namada_core::masp::MaspTransaction; use namada_sdk::address::{Address, ImplicitAddress, MASP}; use namada_sdk::args::TxBecomeValidator; use namada_sdk::borsh::{BorshDeserialize, BorshSerializeExt}; @@ -1262,15 +1261,6 @@ pub async fn submit_shielded_transfer( ) .await?; - let masp_section = tx - .sections - .iter() - .find_map(|section| section.masp_tx()) - .ok_or_else(|| { - error::Error::Other( - "Missing MASP section in shielded transaction".to_string(), - ) - })?; if let Some(dump_tx) = args.tx.dump_tx { if let Some(true) = disposable_fee_payer { display_line!( @@ -1280,7 +1270,6 @@ pub async fn submit_shielded_transfer( ) } tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; - pre_cache_masp_data(namada, &masp_section).await; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; // Store the generated disposable signing key to wallet in case of need @@ -1291,11 +1280,11 @@ pub async fn submit_shielded_transfer( ) })?; } - let res = namada.submit(tx, &args.tx).await?; - pre_cache_masp_data_on_tx_result(namada, &res, &masp_section).await; + namada.submit(tx, &args.tx).await?; } Ok(()) } +// FIXME: add calls to shielded sync pub async fn submit_shielding_transfer( namada: &impl Namada, @@ -1436,15 +1425,6 @@ pub async fn submit_unshielding_transfer( ) .await?; - let masp_section = tx - .sections - .iter() - .find_map(|section| section.masp_tx()) - .ok_or_else(|| { - error::Error::Other( - "Missing MASP section in shielded transaction".to_string(), - ) - })?; if let Some(dump_tx) = args.tx.dump_tx { if let Some(true) = disposable_fee_payer { display_line!( @@ -1454,7 +1434,6 @@ pub async fn submit_unshielding_transfer( ) } tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; - pre_cache_masp_data(namada, &masp_section).await; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; // Store the generated disposable signing key to wallet in case of need @@ -1465,8 +1444,7 @@ pub async fn submit_unshielding_transfer( ) })?; } - let res = namada.submit(tx, &args.tx).await?; - pre_cache_masp_data_on_tx_result(namada, &res, &masp_section).await; + namada.submit(tx, &args.tx).await?; } Ok(()) } @@ -1536,8 +1514,6 @@ where ) .await?; - let opt_masp_section = - tx.sections.iter().find_map(|section| section.masp_tx()); if let Some(dump_tx) = args.tx.dump_tx { tx::dump_tx(namada.io(), dump_tx, args.tx.output_folder, tx)?; if let Some(true) = disposable_fee_payer { @@ -1547,9 +1523,6 @@ where saved to wallet." ) } - if let Some(masp_section) = opt_masp_section { - pre_cache_masp_data(namada, &masp_section).await; - } } else { // Store the generated disposable signing key to wallet in case of need if let Some(true) = disposable_fee_payer { @@ -1559,17 +1532,13 @@ where ) })?; } - let res = batch_opt_reveal_pk_and_submit( + batch_opt_reveal_pk_and_submit( namada, &args.tx, &[&args.source.effective_address()], (tx, signing_data), ) .await?; - - if let Some(masp_section) = opt_masp_section { - pre_cache_masp_data_on_tx_result(namada, &res, &masp_section).await; - } } // NOTE that the tx could fail when its submission epoch doesn't match // construction epoch @@ -2074,42 +2043,3 @@ pub async fn gen_ibc_shielding_transfer( } Ok(()) } - -// Pre-cache the data for the provided MASP transaction. Log an error on -// failure. -async fn pre_cache_masp_data(namada: &impl Namada, masp_tx: &MaspTransaction) { - if let Err(e) = namada - .shielded_mut() - .await - .pre_cache_transaction(masp_tx) - .await - { - // Just display the error but do not propagate it - edisplay_line!(namada.io(), "Failed to pre-cache masp data: {}.", e); - } -} - -// Check the result of a transaction and pre-cache the masp data accordingly -async fn pre_cache_masp_data_on_tx_result( - namada: &impl Namada, - tx_result: &ProcessTxResponse, - masp_tx: &MaspTransaction, -) { - match tx_result { - ProcessTxResponse::Applied(resp) => { - if let Some(InnerTxResult::Success(_)) = - // If we have the masp data in an ibc transfer it - // means we are unshielding, so there's no reveal pk - // tx in the batch which contains only the ibc tx - resp.batch_result().first().map(|(_, res)| res) - { - pre_cache_masp_data(namada, masp_tx).await; - } - } - ProcessTxResponse::Broadcast(_) => { - pre_cache_masp_data(namada, masp_tx).await; - } - // Do not pre-cache when dry-running - ProcessTxResponse::DryRun(_) => {} - } -} diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index 2314eb0d4b9..0b3a537fe91 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -80,9 +80,8 @@ use namada_sdk::key::common::SecretKey; use namada_sdk::masp::shielded_wallet::ShieldedApi; use namada_sdk::masp::utils::RetryStrategy; use namada_sdk::masp::{ - self, ContextSyncStatus, DispatcherCache, MaspTransferData, - ShieldedContext, ShieldedUtils, ShieldedWallet, VersionedWallet, - VersionedWalletRef, + self, DispatcherCache, MaspTransferData, ShieldedContext, ShieldedUtils, + ShieldedWallet, VersionedWallet, VersionedWalletRef, }; use namada_sdk::queries::{ EncodedResponseQuery, RPC, RequestCtx, RequestQuery, Router, @@ -143,8 +142,6 @@ const BERTHA_SPENDING_KEY: &str = "bertha_spending"; const FILE_NAME: &str = "shielded.dat"; const TMP_FILE_NAME: &str = "shielded.tmp"; -const SPECULATIVE_FILE_NAME: &str = "speculative_shielded.dat"; -const SPECULATIVE_TMP_FILE_NAME: &str = "speculative_shielded.tmp"; const CACHE_FILE_NAME: &str = "shielded_sync.cache"; const CACHE_FILE_TMP_PREFIX: &str = "shielded_sync.cache.tmp"; @@ -801,19 +798,10 @@ impl ShieldedUtils for BenchShieldedUtils { async fn load( &self, ctx: &mut ShieldedWallet, - force_confirmed: bool, ) -> std::io::Result<()> { // Try to load shielded context from file - let file_name = if force_confirmed { - FILE_NAME - } else { - match ctx.sync_status { - ContextSyncStatus::Confirmed => FILE_NAME, - ContextSyncStatus::Speculative => SPECULATIVE_FILE_NAME, - } - }; let mut ctx_file = match File::open( - self.context_dir.0.path().to_path_buf().join(file_name), + self.context_dir.0.path().to_path_buf().join(FILE_NAME), ) { Ok(file) => file, Err(err) if err.kind() == std::io::ErrorKind::NotFound => { @@ -840,16 +828,9 @@ impl ShieldedUtils for BenchShieldedUtils { async fn save<'a, U: ShieldedUtils>( &'a self, ctx: VersionedWalletRef<'a, U>, - sync_status: ContextSyncStatus, ) -> std::io::Result<()> { - let (tmp_file_name, file_name) = match sync_status { - ContextSyncStatus::Confirmed => (TMP_FILE_NAME, FILE_NAME), - ContextSyncStatus::Speculative => { - (SPECULATIVE_TMP_FILE_NAME, SPECULATIVE_FILE_NAME) - } - }; let tmp_path = - self.context_dir.0.path().to_path_buf().join(tmp_file_name); + self.context_dir.0.path().to_path_buf().join(TMP_FILE_NAME); { // First serialize the shielded context into a temporary file. // Inability to create this file implies a simultaneuous write is in @@ -870,20 +851,9 @@ impl ShieldedUtils for BenchShieldedUtils { // corrupt data. std::fs::rename( tmp_path, - self.context_dir.0.path().to_path_buf().join(file_name), + self.context_dir.0.path().to_path_buf().join(FILE_NAME), )?; - // Remove the speculative file if present since it's state is - // overwritten by the confirmed one we just saved - if let ContextSyncStatus::Confirmed = sync_status { - let _ = std::fs::remove_file( - self.context_dir - .0 - .path() - .to_path_buf() - .join(SPECULATIVE_FILE_NAME), - ); - } Ok(()) } diff --git a/crates/shielded_token/src/masp.rs b/crates/shielded_token/src/masp.rs index 0a6d6381fe2..5d657ea46fb 100644 --- a/crates/shielded_token/src/masp.rs +++ b/crates/shielded_token/src/masp.rs @@ -228,14 +228,12 @@ pub trait ShieldedUtils: async fn load( &self, ctx: &mut ShieldedWallet, - force_confirmed: bool, ) -> std::io::Result<()>; /// Save the given ShieldedContext for future loads async fn save<'a, U: ShieldedUtils + MaybeSync>( &'a self, ctx: VersionedWalletRef<'a, U>, - sync_status: ContextSyncStatus, ) -> std::io::Result<()>; /// Save a cache of data as part of shielded sync if that @@ -306,8 +304,13 @@ pub type NoteIndex = BTreeMap; /// Maps the note index (in the commitment tree) to a witness pub type WitnessMap = HashMap>; +// FIXME: actually, we need this to deserialize the wallet from file, can we +// avoid deserializing that field so that we can remove this type? #[derive(Copy, Clone, BorshSerialize, BorshDeserialize, Debug, Default)] /// The possible sync states of the shielded context +/// WARNING: this is deprecated in favor of explicit calls to shielded-sync +/// before MASP operations. Users of the SDK should do the same or construct a +/// local cache akin to the speculative context on a use-case basis. pub enum ContextSyncStatus { /// The context contains data that has been confirmed by the protocol #[default] @@ -983,8 +986,6 @@ pub mod fs { /// Shielded context file name const FILE_NAME: &str = "shielded.dat"; const TMP_FILE_PREFIX: &str = "shielded.tmp"; - const SPECULATIVE_FILE_NAME: &str = "speculative_shielded.dat"; - const SPECULATIVE_TMP_FILE_PREFIX: &str = "speculative_shielded.tmp"; const CACHE_FILE_NAME: &str = "shielded_sync.cache"; const CACHE_FILE_TMP_PREFIX: &str = "shielded_sync.cache.tmp"; @@ -1024,21 +1025,9 @@ pub mod fs { } } // Finally initialize a shielded context with the supplied directory - - let sync_status = - if std::fs::read(context_dir.join(SPECULATIVE_FILE_NAME)) - .is_ok() - { - // Load speculative state - ContextSyncStatus::Speculative - } else { - ContextSyncStatus::Confirmed - }; - let utils = Self { context_dir }; ShieldedWallet { utils, - sync_status, ..Default::default() } } @@ -1117,19 +1106,10 @@ pub mod fs { async fn load( &self, ctx: &mut ShieldedWallet, - force_confirmed: bool, ) -> std::io::Result<()> { // Try to load shielded context from file - let file_name = if force_confirmed { - FILE_NAME - } else { - match ctx.sync_status { - ContextSyncStatus::Confirmed => FILE_NAME, - ContextSyncStatus::Speculative => SPECULATIVE_FILE_NAME, - } - }; let mut ctx_file = - match File::open(self.context_dir.join(file_name)) { + match File::open(self.context_dir.join(FILE_NAME)) { Ok(file) => file, Err(err) if err.kind() == std::io::ErrorKind::NotFound => { // a missing file means there is nothing to load. @@ -1157,33 +1137,18 @@ pub mod fs { } /// Save this confirmed shielded context into its associated context - /// directory. At the same time, delete the speculative file if present + /// directory. async fn save<'a, U: ShieldedUtils + MaybeSync>( &'a self, ctx: VersionedWalletRef<'a, U>, - sync_status: ContextSyncStatus, ) -> std::io::Result<()> { - let (tmp_file_pref, file_name) = match sync_status { - ContextSyncStatus::Confirmed => (TMP_FILE_PREFIX, FILE_NAME), - ContextSyncStatus::Speculative => { - (SPECULATIVE_TMP_FILE_PREFIX, SPECULATIVE_FILE_NAME) - } - }; let tmp_file_name = { let t = tempfile::Builder::new() - .prefix(tmp_file_pref) + .prefix(TMP_FILE_PREFIX) .tempfile()?; t.path().file_name().unwrap().to_owned() }; - self.atomic_file_write(tmp_file_name, file_name, ctx)?; - - // Remove the speculative file if present since it's state is - // overruled by the confirmed one we just saved - if let ContextSyncStatus::Confirmed = sync_status { - let _ = std::fs::remove_file( - self.context_dir.join(SPECULATIVE_FILE_NAME), - ); - } + self.atomic_file_write(tmp_file_name, FILE_NAME, ctx)?; Ok(()) } @@ -1225,14 +1190,7 @@ pub mod fs { utils, ..Default::default() }; - assert!( - shielded - .utils - .clone() - .load(&mut shielded, false) - .await - .is_ok() - ); + assert!(shielded.utils.clone().load(&mut shielded).await.is_ok()); } /// Test that if the backing file isn't versioned but contains V0 data, @@ -1266,7 +1224,7 @@ pub mod fs { shielded .utils .clone() - .load(&mut shielded, true) + .load(&mut shielded) .await .expect("Test failed"); assert_eq!(shielded.spents, HashSet::from([NotePosition(42)])); @@ -1304,7 +1262,7 @@ pub mod fs { shielded .utils .clone() - .load(&mut shielded, true) + .load(&mut shielded) .await .expect("Test failed"); assert_eq!(shielded.spents, HashSet::from([NotePosition(42)])); @@ -1333,14 +1291,7 @@ pub mod fs { std::fs::write(temp.path().join(FILE_NAME), &serialized) .expect("Test failed"); - assert!( - shielded - .utils - .clone() - .load(&mut shielded, true) - .await - .is_err() - ); + assert!(shielded.utils.clone().load(&mut shielded,).await.is_err()); } } } diff --git a/crates/shielded_token/src/masp/shielded_sync/dispatcher.rs b/crates/shielded_token/src/masp/shielded_sync/dispatcher.rs index a9bfde07d79..9a55bdaa822 100644 --- a/crates/shielded_token/src/masp/shielded_sync/dispatcher.rs +++ b/crates/shielded_token/src/masp/shielded_sync/dispatcher.rs @@ -304,7 +304,7 @@ where ..Default::default() }; - ctx.load_confirmed().await; + ctx.load().await; ctx }; diff --git a/crates/shielded_token/src/masp/shielded_wallet.rs b/crates/shielded_token/src/masp/shielded_wallet.rs index 618595db34c..e2578095e24 100644 --- a/crates/shielded_token/src/masp/shielded_wallet.rs +++ b/crates/shielded_token/src/masp/shielded_wallet.rs @@ -57,10 +57,9 @@ use crate::masp::bridge_tree::BridgeTree; use crate::masp::utils::MaspClient; use crate::masp::wallet_migrations::VersionedWalletRef; use crate::masp::{ - ContextSyncStatus, Conversions, MaspAmount, MaspDataLogEntry, MaspFeeData, - MaspTransferData, MaspTxCombinedData, NETWORK, NoteIndex, - ShieldedSyncConfig, ShieldedTransfer, ShieldedUtils, SpentNotesTracker, - TransferErr, WalletMap, + Conversions, MaspAmount, MaspDataLogEntry, MaspFeeData, MaspTransferData, + MaspTxCombinedData, NETWORK, NoteIndex, ShieldedSyncConfig, + ShieldedTransfer, ShieldedUtils, SpentNotesTracker, TransferErr, WalletMap, }; #[cfg(any(test, feature = "testing"))] use crate::masp::{ENV_VAR_MASP_TEST_SEED, testing}; @@ -311,8 +310,6 @@ pub struct ShieldedWallet { pub conversions: EpochedConversions, /// Maps a shielded tx to the index of its first output note. pub note_index: NoteIndex, - /// The sync state of the context - pub sync_status: ContextSyncStatus, /// Maps note positions to their corresponding viewing keys #[cfg(feature = "historic")] pub vk_map: HashMap, @@ -341,7 +338,7 @@ impl ShieldedWallet { /// Otherwise, we load the wallet and attempt to migrate it to the /// latest version. This function panics if that fails. pub async fn load(&mut self) { - self.utils.clone().load(self, false).await.unwrap() + self.utils.clone().load(self).await.unwrap() } /// Same as the load function, but is provided an async closure @@ -352,26 +349,14 @@ impl ShieldedWallet { F: Fn(std::io::Error) -> X, X: std::future::Future, { - if let Err(e) = self.utils.clone().load(self, false).await { + if let Err(e) = self.utils.clone().load(self).await { on_error(e).await; } } - /// Try to load the last saved confirmed shielded context from the given - /// context directory.If the file is missing, an empty wallet is created. - /// Otherwise, we load the wallet and attempt to migrate it to the - /// latest version. This function panics if that fails. - pub async fn load_confirmed(&mut self) { - self.utils.clone().load(self, true).await.unwrap() - } - - /// Save this shielded context into its associated context directory. If the - /// state to be saved is confirmed than also delete the speculative one (if - /// available) + /// Save this shielded context into its associated context directory. pub async fn save(&self) -> std::io::Result<()> { - self.utils - .save(VersionedWalletRef::V2(self), self.sync_status) - .await + self.utils.save(VersionedWalletRef::V2(self)).await } /// Update the merkle tree of witnesses the first time we @@ -719,29 +704,6 @@ impl ShieldedWallet { } Ok(()) } - - /// Updates the internal state with the data of the newly generated - /// transaction. More specifically invalidate the spent notes, but do not - /// cache the newly produced output descriptions and therefore the merkle - /// tree (this is because we don't know the exact position in the tree where - /// the new notes will be appended) - pub async fn pre_cache_transaction( - &mut self, - masp_tx: &Transaction, - ) -> Result<(), eyre::Error> { - self.save_shielded_spends( - masp_tx, - true, - #[cfg(feature = "historic")] - None, - )?; - - // Save the speculative state for future usage - self.sync_status = ContextSyncStatus::Speculative; - self.save().await.map_err(|e| eyre!(e.to_string()))?; - - Ok(()) - } } /// A trait that allows downstream types specify how a shielded wallet diff --git a/crates/shielded_token/src/masp/wallet_migrations.rs b/crates/shielded_token/src/masp/wallet_migrations.rs index 2f9d700beca..f69dd080ebc 100644 --- a/crates/shielded_token/src/masp/wallet_migrations.rs +++ b/crates/shielded_token/src/masp/wallet_migrations.rs @@ -18,6 +18,8 @@ pub enum VersionedWallet { V1(v1::ShieldedWallet), /// Version 2 V2(ShieldedWallet), + // FIXME: need a new version without the sync state, also mention what the + // change was } impl VersionedWallet { @@ -301,7 +303,6 @@ pub mod v0 { asset_types: wallet.asset_types, conversions: Default::default(), note_index: wallet.note_index, - sync_status: wallet.sync_status, } } #[cfg(feature = "historic")] @@ -441,7 +442,6 @@ pub mod v1 { asset_types: wallet.asset_types, conversions: wallet.conversions, note_index: wallet.note_index, - sync_status: wallet.sync_status, } } #[cfg(feature = "historic")] From ecb60d820d6acf398b849ffff4867de50323ccd6 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 13 Oct 2025 14:16:56 +0200 Subject: [PATCH 02/17] Adds calls to shielded sync in MASP client commands --- crates/apps_lib/src/cli.rs | 4 +- crates/apps_lib/src/cli/client.rs | 2 +- crates/apps_lib/src/client/masp.rs | 40 ++- crates/apps_lib/src/client/rpc.rs | 36 ++- crates/apps_lib/src/client/tx.rs | 46 +-- crates/node/src/bench_utils.rs | 4 +- crates/tests/src/integration/masp.rs | 428 +-------------------------- 7 files changed, 99 insertions(+), 461 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 32ec3e07b05..296b275589a 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -1912,11 +1912,11 @@ pub mod cmds { } fn def() -> App { + // FIXME: remove the cli shielded-sync command App::new(Self::CMD) .about(wrap!( "Estimate the amount of MASP rewards for the \ - next MASP epoch. Please run shielded-sync first for best \ - results." + next MASP epoch." )) .add_args::>() } diff --git a/crates/apps_lib/src/cli/client.rs b/crates/apps_lib/src/cli/client.rs index c55dac91df3..3ee7f5b8001 100644 --- a/crates/apps_lib/src/cli/client.rs +++ b/crates/apps_lib/src/cli/client.rs @@ -419,7 +419,7 @@ impl CliApi { ); crate::client::masp::syncing( - ShieldedContext::new(chain_ctx.shielded), + &mut ShieldedContext::new(chain_ctx.shielded), client, args, &io, diff --git a/crates/apps_lib/src/client/masp.rs b/crates/apps_lib/src/client/masp.rs index dbd8a354007..795979f26a2 100644 --- a/crates/apps_lib/src/client/masp.rs +++ b/crates/apps_lib/src/client/masp.rs @@ -13,6 +13,9 @@ use namada_sdk::masp::{ IndexerMaspClient, LedgerMaspClient, LinearBackoffSleepMaspClient, MaspLocalTaskEnv, ShieldedContext, ShieldedSyncConfig, ShieldedUtils, }; +use tendermint_rpc::HttpClient; + +use crate::cli::api::{CliClient, CliIo}; const MASP_INDEXER_CLIENT_USER_AGENT: &str = { const TOKENS: &[&str] = @@ -26,11 +29,11 @@ pub async fn syncing< C: Client + Send + Sync + 'static, IO: Io + Send + Sync, >( - mut shielded: ShieldedContext, + shielded: &mut ShieldedContext, client: C, args: ShieldedSync, io: &IO, -) -> Result, Error> { +) -> Result<(), Error> { let (fetched_bar, scanned_bar, applied_bar) = { #[cfg(any(test, feature = "testing"))] { @@ -97,7 +100,7 @@ pub async fn syncing< let env = MaspLocalTaskEnv::new(500) .map_err(|e| Error::Other(e.to_string()))?; - let ctx = shielded + let res = shielded .sync( env, config, @@ -106,12 +109,11 @@ pub async fn syncing< &vks, ) .await - .map(|_| shielded) .map_err(|e| Error::Other(e.to_string())); display!(io, "\nSyncing finished\n"); - ctx + res }}; } @@ -163,7 +165,7 @@ pub async fn syncing< .map_err(|e| Error::Other(e.to_string()))?; } - let shielded = if let Some(endpoint) = args.with_indexer { + if let Some(endpoint) = args.with_indexer { display_line!( io, "{}\n", @@ -203,7 +205,31 @@ pub async fn syncing< LedgerMaspClient::new(client, args.max_concurrent_fetches,), Duration::from_millis(5) ))? + } + + Ok(()) +} + +// FIXME: rename? +pub async fn sync_shielded_context( + ledger_address: tendermint_rpc::Url, + shielded_ctx: &mut ShieldedContext, +) -> Result<(), Error> { + let sync_client = HttpClient::from_tendermint_address(&ledger_address); + // FIXME: review these args + let sync_args = namada_sdk::args::ShieldedSync { + ledger_address, + last_query_height: None, + spending_keys: vec![], + viewing_keys: vec![], + with_indexer: None, + wait_for_last_query_height: false, + max_concurrent_fetches: 100, + block_batch_size: 10, + retry_strategy: namada_sdk::masp::utils::RetryStrategy::Forever, }; - Ok(shielded) + // FIXME: is CliIo correct? + crate::client::masp::syncing(shielded_ctx, sync_client, sync_args, &CliIo) + .await } diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index 4f416e60113..70eb066bc2a 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -3,7 +3,6 @@ use std::collections::{BTreeMap, BTreeSet}; use std::io; -use color_eyre::owo_colors::OwoColorize; use data_encoding::HEXLOWER; use either::Either; use futures::StreamExt; @@ -60,6 +59,7 @@ use namada_sdk::wallet::AddressVpType; use namada_sdk::{Namada, error, state as storage, token}; use crate::cli::{self, args}; +use crate::client::masp::sync_shielded_context; use crate::tendermint::merkle::proof::ProofOps; /// Query the status of a given transaction. @@ -428,6 +428,17 @@ pub async fn query_rewards_estimate( edisplay_line!(context.io(), "Failed to load shielded context: {}", e); cli::safe_exit(1); } + // Sync the shielded context before estimating the rewards + if let Err(e) = + sync_shielded_context(args.query.ledger_address, &mut *shielded).await + { + edisplay_line!( + context.io(), + "Failed to sync the shielded context: {}", + e + ); + cli::safe_exit(1); + } let raw_balance = match shielded .compute_shielded_balance(&args.owner.as_viewing_key()) .await @@ -476,16 +487,9 @@ async fn query_shielded_balance( context: &impl Namada, args: args::QueryBalance, ) { - display_line!( - context.io(), - "{}: {}\n", - "WARNING".bold().underline().yellow(), - "The resulting balance could be outdated, make sure to run `namadac \ - shielded-sync` before querying the balance to get the most recent \ - value." - ); - let args::QueryBalance { + // Need the ledger address for sync purposes + query, // Token owner (needs to be a viewing key) owner, // The token to query @@ -538,6 +542,18 @@ async fn query_shielded_balance( display_line!(context.io(), "{token_alias}: 0"); }; + // Sync the shielded context before computing the balance + if let Err(e) = + sync_shielded_context(query.ledger_address, &mut *shielded).await + { + edisplay_line!( + context.io(), + "Failed to sync the shielded context: {}", + e + ); + cli::safe_exit(1); + } + let balance = if no_conversions || token != context.native_token() { let Some(bal) = shielded .compute_shielded_balance(&viewing_key) diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 650efa74090..fd803da5a6e 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -35,6 +35,7 @@ use namada_sdk::{ExtendedViewingKey, Namada, error, signing, tx}; use rand::rngs::OsRng; use tokio::sync::RwLock; +use super::masp::sync_shielded_context; use super::rpc; use crate::cli::{args, safe_exit}; use crate::client::tx::signing::{SigningTxData, default_sign}; @@ -1209,15 +1210,6 @@ pub async fn submit_shielded_transfer( namada: &impl Namada, mut args: args::TxShieldedTransfer, ) -> Result<(), error::Error> { - display_line!( - namada.io(), - "{}: {}\n", - "WARNING".bold().underline().yellow(), - "Some information might be leaked if your shielded wallet is not up \ - to date, make sure to run `namadac shielded-sync` before running \ - this command.", - ); - let sources = args .sources .iter_mut() @@ -1232,6 +1224,14 @@ pub async fn submit_shielded_transfer( &args.tx, ) .await?; + + // Sync the shielded context before building the transaction + sync_shielded_context( + args.tx.ledger_address.clone(), + &mut *namada.shielded_mut().await, + ) + .await?; + let (mut tx, signing_data) = args.clone().build(namada, &mut bparams).await?; let disposable_fee_payer = match signing_data { @@ -1284,7 +1284,6 @@ pub async fn submit_shielded_transfer( } Ok(()) } -// FIXME: add calls to shielded sync pub async fn submit_shielding_transfer( namada: &impl Namada, @@ -1373,15 +1372,6 @@ pub async fn submit_unshielding_transfer( namada: &impl Namada, mut args: args::TxUnshieldingTransfer, ) -> Result<(), error::Error> { - display_line!( - namada.io(), - "{}: {}\n", - "WARNING".bold().underline().yellow(), - "Some information might be leaked if your shielded wallet is not up \ - to date, make sure to run `namadac shielded-sync` before running \ - this command.", - ); - let sources = args .sources .iter_mut() @@ -1396,6 +1386,14 @@ pub async fn submit_unshielding_transfer( &args.tx, ) .await?; + + // Sync the shielded context before building the transaction + sync_shielded_context( + args.tx.ledger_address.clone(), + &mut *namada.shielded_mut().await, + ) + .await?; + let (mut tx, signing_data) = args.clone().build(namada, &mut bparams).await?; let disposable_fee_payer = match signing_data { @@ -1463,6 +1461,7 @@ where .chain(args.gas_spending_key.iter_mut()); let shielded_hw_keys = augment_masp_hardware_keys(namada, &args.tx, sources).await?; + // FIXME: don't we need this only if masp is involved? // Try to generate MASP build parameters. This might fail when using a // hardware wallet if it does not support MASP operations. let bparams_result = generate_masp_build_params( @@ -1478,6 +1477,15 @@ where |e| (Box::new(StoredBuildParams::default()) as _, Some(e)), |bparams| (bparams, None), ); + + // If the ibc sources are MASP, sync the shielded context before building + // the transaction FIXME: only if source is masp + sync_shielded_context( + args.tx.ledger_address.clone(), + &mut *namada.shielded_mut().await, + ) + .await?; + // If transaction building fails for any reason, then abort the process // blaming MASP build parameter generation if that had also failed. let (mut tx, signing_data, _) = args diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index 0b3a537fe91..090495ab26e 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -1243,9 +1243,9 @@ impl BenchShieldedCtx { spending_key, self.wallet.find_birthday(ALBERT_SPENDING_KEY).copied(), ); - self.shielded = async_runtime + async_runtime .block_on(namada_apps_lib::client::masp::syncing( - self.shielded, + &mut self.shielded, self.shell.clone(), ShieldedSync { ledger_address: FromStr::from_str("http://127.0.0.1:1337") diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index a9d88bb761c..ede0ce51de2 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -50,6 +50,8 @@ use crate::e2e::setup::constants::{ use crate::integration::helpers::make_temp_account; use crate::strings::TX_APPLIED_SUCCESS; +// FIXME: avoid explicitly calling shielded-sync in the tests + /// Enable masp rewards before some token is shielded, /// but the max reward rate is null. #[test] @@ -253,18 +255,6 @@ fn init_null_rewards() -> Result<()> { .shielded_converts .is_empty() ); - // Invalidate the speculative shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; // Now, let us increase the max reward rate token::write_params( @@ -8341,408 +8331,6 @@ fn tricky_masp_txs() -> Result<()> { Ok(()) } -// Test generation of transactions and querying balance with the speculative -// context. Also checks that the shielded history is not updated when in a -// speculative context. -#[cfg(feature = "historic-masp")] -#[test] -fn speculative_context() -> Result<()> { - let rt = tokio::runtime::Runtime::new().unwrap(); - - // This address doesn't matter for tests. But an argument is required. - let validator_one_rpc = "http://127.0.0.1:26567"; - // Download the shielded pool parameters before starting node - let _ = FsShieldedUtils::new(PathBuf::new()); - let (mut node, _services) = setup::setup()?; - // Generate the shielded wallet in a fixed path to retrieve it later - let mut shielded_wallet = FsShieldedUtils::new(node.genesis_dir()); - // Assert that the shielded history is empty - assert!(shielded_wallet.history.is_empty()); - - _ = node.next_masp_epoch(); - - // 1. Shield some tokens in two steps two generate two different output - // notes - for _ in 0..2 { - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "shield", - "--source", - ALBERT, - "--target", - AA_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "100", - "--node", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains(TX_APPLIED_SUCCESS)); - } - - // 2. Sync the shielded context and check the balance - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - vec![ - "balance", - "--owner", - AA_VIEWING_KEY, - "--token", - NAM, - "--node", - validator_one_rpc, - ], - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 200")); - - let native_token = node - .shell - .lock() - .unwrap() - .state - .in_mem() - .native_token - .clone(); - // Load the updated shielded wallet - rt.block_on(shielded_wallet.load_confirmed()); - // Assert that the shielded history has been updated with the corresponding - // entries - assert_eq!(shielded_wallet.history.len(), 1); - let aa_history = shielded_wallet.history.first().unwrap().1; - assert_eq!(aa_history.len(), 2); - let first_aa_history_entry = aa_history - .get(&IndexedTx { - block_height: 11.into(), - block_index: 0.into(), - batch_index: 0.into(), - }) - .unwrap(); - assert!(first_aa_history_entry.inputs.is_empty()); - let expected_outputs: HashMap = - [(native_token.clone(), Amount::from(100_000_000))] - .into_iter() - .collect(); - assert_eq!(expected_outputs, first_aa_history_entry.outputs); - let second_aa_history_entry = aa_history - .get(&IndexedTx { - block_height: 13.into(), - block_index: 0.into(), - batch_index: 0.into(), - }) - .unwrap(); - assert!(second_aa_history_entry.inputs.is_empty()); - let expected_outputs: HashMap = - [(native_token.clone(), Amount::from(100_000_000))] - .into_iter() - .collect(); - assert_eq!(expected_outputs, second_aa_history_entry.outputs); - - // 3. Spend an amount of tokens which is less than the amount of every - // single note - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "transfer", - "--source", - A_SPENDING_KEY, - "--target", - AB_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "90", - "--gas-payer", - ALBERT_KEY, - "--node", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains(TX_APPLIED_SUCCESS)); - - // 4. Check the balance without calling shielded-sync to check the response - // of the speculative context - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - vec![ - "balance", - "--owner", - AA_VIEWING_KEY, - "--token", - NAM, - "--node", - validator_one_rpc, - ], - ) - }); - assert!(captured.result.is_ok()); - // The speculative context invalidates the entire note spent so we expect to - // see the balance coming only from the second unspent note - assert!(captured.contains("nam: 100")); - // Load the updated shielded wallet - rt.block_on(shielded_wallet.load_confirmed()); - // Assert that the shielded history has not been updated - assert_eq!(shielded_wallet.history.len(), 1); - let aa_history = shielded_wallet.history.first().unwrap().1; - assert_eq!(aa_history.len(), 2); - - // 5. Try to spend some amount from the remaining note with a tx that will - // fail - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "transfer", - "--source", - A_SPENDING_KEY, - "--target", - AB_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "90", - "--gas-payer", - ALBERT_KEY, - // Force failure with low gas limit - "--gas-limit", - "10000", - "--node", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains( - "Gas error: Transaction gas exceeded the limit of 10000 gas units" - )); - - // 6. Check that the speculative context was not updated - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - vec![ - "balance", - "--owner", - AA_VIEWING_KEY, - "--token", - NAM, - "--node", - validator_one_rpc, - ], - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 100")); - // Load the updated shielded wallet - rt.block_on(shielded_wallet.load_confirmed()); - // Assert that the shielded history has not been updated - assert_eq!(shielded_wallet.history.len(), 1); - let aa_history = shielded_wallet.history.first().unwrap().1; - assert_eq!(aa_history.len(), 2); - - // 7. Try to spend some amount from the remaining note - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "transfer", - "--source", - A_SPENDING_KEY, - "--target", - AB_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "90", - "--gas-payer", - ALBERT_KEY, - "--node", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains(TX_APPLIED_SUCCESS)); - - // 8. Check the balance without calling shielded-sync to check the response - // of the speculative context - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - vec![ - "balance", - "--owner", - AA_VIEWING_KEY, - "--token", - NAM, - "--node", - validator_one_rpc, - ], - ) - }); - assert!(captured.result.is_ok()); - // The speculative context invalidates the entire note spent so we expect to - // see an empty balance - assert!(captured.contains("nam: 0")); - // Load the updated shielded wallet - rt.block_on(shielded_wallet.load_confirmed()); - // Assert that the shielded history has not been updated - assert_eq!(shielded_wallet.history.len(), 1); - let aa_history = shielded_wallet.history.first().unwrap().1; - assert_eq!(aa_history.len(), 2); - - // 9. Finally, sync the shielded context and check the confirmed balances - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - vec![ - "balance", - "--owner", - AA_VIEWING_KEY, - "--token", - NAM, - "--node", - validator_one_rpc, - ], - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 20")); - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - vec![ - "balance", - "--owner", - AB_VIEWING_KEY, - "--token", - NAM, - "--node", - validator_one_rpc, - ], - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 180")); - - // Load the updated shielded wallet - rt.block_on(shielded_wallet.load_confirmed()); - // Assert that the shielded history has been updated with the corresponding - // entries - assert_eq!(shielded_wallet.history.len(), 2); - let aa_history = shielded_wallet.history.first().unwrap().1; - assert_eq!(aa_history.len(), 4); - let third_aa_history_entry = aa_history - .get(&IndexedTx { - block_height: 15.into(), - block_index: 0.into(), - batch_index: 0.into(), - }) - .unwrap(); - let expected_inputs: HashMap = - [(native_token.clone(), Amount::from(100_000_000))] - .into_iter() - .collect(); - assert_eq!(expected_inputs, third_aa_history_entry.inputs); - let expected_outputs: HashMap = - [(native_token.clone(), Amount::from(10_000_000))] - .into_iter() - .collect(); - assert_eq!(expected_outputs, third_aa_history_entry.outputs); - let fourth_aa_history_entry = aa_history - .get(&IndexedTx { - block_height: 19.into(), - block_index: 0.into(), - batch_index: 0.into(), - }) - .unwrap(); - let expected_inputs: HashMap = - [(native_token.clone(), Amount::from(100_000_000))] - .into_iter() - .collect(); - assert_eq!(expected_inputs, fourth_aa_history_entry.inputs); - let expected_outputs: HashMap = - [(native_token.clone(), Amount::from(10_000_000))] - .into_iter() - .collect(); - assert_eq!(expected_outputs, fourth_aa_history_entry.outputs); - let ab_history = shielded_wallet.history.get_index(1).unwrap().1; - assert_eq!(ab_history.len(), 2); - let first_ab_history_entry = ab_history - .get(&IndexedTx { - block_height: 15.into(), - block_index: 0.into(), - batch_index: 0.into(), - }) - .unwrap(); - assert!(first_ab_history_entry.inputs.is_empty()); - let expected_outputs: HashMap = - [(native_token.clone(), Amount::from(90_000_000))] - .into_iter() - .collect(); - assert_eq!(expected_outputs, first_ab_history_entry.outputs); - let second_ab_history_entry = ab_history - .get(&IndexedTx { - block_height: 19.into(), - block_index: 0.into(), - batch_index: 0.into(), - }) - .unwrap(); - assert!(second_ab_history_entry.inputs.is_empty()); - let expected_outputs: HashMap = - [(native_token.clone(), Amount::from(90_000_000))] - .into_iter() - .collect(); - assert_eq!(expected_outputs, second_ab_history_entry.outputs); - - Ok(()) -} - // Test that mixed masp tranfers and fee payments are correctly labeld by the // protocol (by means of events) and reconstructed in the correct order by the // client @@ -9673,7 +9261,7 @@ fn history() -> Result<()> { // Load the updated shielded wallet and check that the history is empty // since we haven't run shielded-sync yet - rt.block_on(shielded_wallet.load_confirmed()); + rt.block_on(shielded_wallet.load()); assert!(shielded_wallet.history.is_empty()); // sync the shielded context @@ -9746,7 +9334,7 @@ fn history() -> Result<()> { .unwrap_or_default(); let btc_addr = tokens[&BTC.to_lowercase()].clone(); // Load the updated shielded wallet - rt.block_on(shielded_wallet.load_confirmed()); + rt.block_on(shielded_wallet.load()); // Assert that the shielded history has been updated with the corresponding // entries assert_eq!(shielded_wallet.history.len(), 1); @@ -9897,7 +9485,7 @@ fn history() -> Result<()> { assert!(captured.contains("btc: 0")); // Load the updated shielded wallet - rt.block_on(shielded_wallet.load_confirmed()); + rt.block_on(shielded_wallet.load()); // Assert that the shielded history has been updated with the corresponding // entries assert_eq!(shielded_wallet.history.len(), 2); @@ -10015,7 +9603,7 @@ fn history_with_conversions() -> Result<()> { // Load the updated shielded wallet and check that the history is empty // since we haven't run shielded-sync yet - rt.block_on(shielded_wallet.load_confirmed()); + rt.block_on(shielded_wallet.load()); assert!(shielded_wallet.history.is_empty()); // sync the shielded context @@ -10051,7 +9639,7 @@ fn history_with_conversions() -> Result<()> { assert!(captured.contains("btc: 10")); // Load the updated shielded wallet - rt.block_on(shielded_wallet.load_confirmed()); + rt.block_on(shielded_wallet.load()); // Assert that the shielded history has been updated with the corresponding // entries assert_eq!(shielded_wallet.history.len(), 1); @@ -10199,7 +9787,7 @@ fn history_with_conversions() -> Result<()> { .native_token .clone(); // Load the updated shielded wallet - rt.block_on(shielded_wallet.load_confirmed()); + rt.block_on(shielded_wallet.load()); // Assert that the shielded history has been updated with the corresponding // entries assert_eq!(shielded_wallet.history.len(), 2); From b5b26398f8b5589812cf87c609d28a86441c15f9 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 23 Oct 2025 12:09:59 +0200 Subject: [PATCH 03/17] Adds shielded-sync arguments to the other cli commands --- crates/apps_lib/src/cli.rs | 123 ++++++++++++++++++----------- crates/apps_lib/src/cli/client.rs | 4 +- crates/apps_lib/src/client/masp.rs | 38 ++++----- crates/apps_lib/src/client/rpc.rs | 18 ++++- crates/apps_lib/src/client/tx.rs | 21 ++++- crates/node/src/bench_utils.rs | 2 - crates/sdk/src/args.rs | 19 ++++- crates/sdk/src/lib.rs | 3 + 8 files changed, 148 insertions(+), 80 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index 296b275589a..a5afbc61546 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -4864,6 +4864,8 @@ pub mod args { let gas_spending_key = self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); + let shielded_sync = + self.shielded_sync.map(|sync| sync.to_sdk(ctx).unwrap()); Ok(TxShieldedTransfer:: { tx, @@ -4871,6 +4873,7 @@ pub mod args { targets, gas_spending_key, tx_code_path: self.tx_code_path.to_path_buf(), + shielded_sync, }) } } @@ -4894,6 +4897,7 @@ pub mod args { amount, }]; let gas_spending_key = GAS_SPENDING_KEY.parse(matches); + let shielded_sync = Some(ShieldedSync::parse(matches)); Self { tx, @@ -4901,11 +4905,13 @@ pub mod args { targets, gas_spending_key, tx_code_path, + shielded_sync, } } fn def(app: App) -> App { app.add_args::>() + .add_args::>() .arg( SPENDING_KEY_SOURCE .def() @@ -5072,6 +5078,8 @@ pub mod args { let gas_spending_key = self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); + let shielded_sync = + self.shielded_sync.map(|sync| sync.to_sdk(ctx).unwrap()); Ok(TxUnshieldingTransfer:: { tx, @@ -5080,6 +5088,7 @@ pub mod args { sources, tx_code_path: self.tx_code_path.to_path_buf(), frontend_sus_fee, + shielded_sync, }) } } @@ -5110,6 +5119,7 @@ pub mod args { // Take a constant fee of 10% on top of the input amount (target, Dec::new(1, 1).unwrap()) }); + let shielded_sync = Some(ShieldedSync::parse(matches)); Self { tx, @@ -5118,11 +5128,13 @@ pub mod args { gas_spending_key, tx_code_path, frontend_sus_fee, + shielded_sync, } } fn def(app: App) -> App { app.add_args::>() + .add_args::>() .arg( SPENDING_KEY_SOURCE .def() @@ -5164,6 +5176,8 @@ pub mod args { ctx: &mut Context, ) -> Result, Self::Error> { let tx = self.tx.to_sdk(ctx)?; + let shielded_sync = + self.shielded_sync.map(|sync| sync.to_sdk(ctx).unwrap()); let chain_ctx = ctx.borrow_mut_chain_or_exit(); let gas_spending_key = self.gas_spending_key.map(|key| chain_ctx.get_cached(&key)); @@ -5188,6 +5202,7 @@ pub mod args { gas_spending_key, tx_code_path: self.tx_code_path.to_path_buf(), frontend_sus_fee, + shielded_sync, }) } } @@ -5221,6 +5236,7 @@ pub mod args { // Take a constant fee of 10% on top of the input amount (target, Dec::new(1, 1).unwrap()) }); + let shielded_sync = Some(ShieldedSync::parse(matches)); Self { tx, @@ -5238,11 +5254,13 @@ pub mod args { gas_spending_key, tx_code_path, frontend_sus_fee, + shielded_sync, } } fn def(app: App) -> App { app.add_args::>() + .add_args::>() .arg(SOURCE.def().help(wrap!( "The source account address. The source's key is used to \ produce the signature." @@ -6665,6 +6683,8 @@ pub mod args { ctx: &mut Context, ) -> Result, Self::Error> { let query = self.query.to_sdk(ctx)?; + let shielded_sync = + self.shielded_sync.map(|sync| sync.to_sdk(ctx).unwrap()); let chain_ctx = ctx.borrow_mut_chain_or_exit(); Ok(QueryBalance:: { @@ -6673,6 +6693,7 @@ pub mod args { token: chain_ctx.get(&self.token), no_conversions: self.no_conversions, height: self.height, + shielded_sync, }) } } @@ -6684,17 +6705,21 @@ pub mod args { let token = TOKEN.parse(matches); let no_conversions = NO_CONVERSIONS.parse(matches); let height = BLOCK_HEIGHT_OPT.parse(matches); + let shielded_sync = Some(ShieldedSync::parse(matches)); + Self { query, owner, token, no_conversions, height, + shielded_sync, } } fn def(app: App) -> App { app.add_args::>() + .add_args::>() .arg( BALANCE_OWNER.def().help(wrap!( "The account address whose balance to query." @@ -6726,11 +6751,14 @@ pub mod args { ) -> Result, Self::Error> { let query = self.query.to_sdk(ctx)?; + let shielded_sync = + self.shielded_sync.map(|sync| sync.to_sdk(ctx).unwrap()); let chain_ctx = ctx.borrow_mut_chain_or_exit(); Ok(QueryShieldingRewardsEstimate:: { query, owner: chain_ctx.get_cached(&self.owner), + shielded_sync, }) } } @@ -6739,15 +6767,23 @@ pub mod args { fn parse(matches: &ArgMatches) -> Self { let query = Query::parse(matches); let owner = VIEWING_KEY.parse(matches); - Self { query, owner } + let shielded_sync = Some(ShieldedSync::parse(matches)); + + Self { + query, + owner, + shielded_sync, + } } fn def(app: App) -> App { - app.add_args::>().arg( - VIEWING_KEY - .def() - .help(wrap!("The viewing key whose rewards to estimate.")), - ) + app.add_args::>() + .add_args::>() + .arg( + VIEWING_KEY.def().help(wrap!( + "The viewing key whose rewards to estimate." + )), + ) } } @@ -7181,7 +7217,6 @@ pub mod args { impl Args for ShieldedSync { fn parse(matches: &ArgMatches) -> Self { - let ledger_address = CONFIG_RPC_LEDGER_ADDRESS.parse(matches); let last_query_height = BLOCK_HEIGHT_TO_OPT.parse(matches); let spending_keys = DATED_SPENDING_KEYS.parse(matches); let viewing_keys = DATED_VIEWING_KEYS.parse(matches); @@ -7195,7 +7230,6 @@ pub mod args { }; let block_batch_size = BLOCK_BATCH.parse(matches); Self { - ledger_address, last_query_height, spending_keys, viewing_keys, @@ -7208,43 +7242,41 @@ pub mod args { } fn def(app: App) -> App { - app.arg(CONFIG_RPC_LEDGER_ADDRESS.def().help(LEDGER_ADDRESS_ABOUT)) - .arg(BLOCK_HEIGHT_TO_OPT.def().help(wrap!( - "Option block height to sync up to. Default is latest." - ))) - .arg(DATED_SPENDING_KEYS.def().help(wrap!( - "List of new spending keys with which to check note \ - ownership. These will be added to the shielded context. \ - Appending \"<<$BLOCKHEIGHT\" to the end of each key adds \ - a birthday." - ))) - .arg(DATED_VIEWING_KEYS.def().help(wrap!( - "List of new viewing keys with which to check note \ - ownership. These will be added to the shielded context. \ - Appending \"<<$BLOCKHEIGHT\" to the end of each key adds \ - a birthday." - ))) - .arg(WITH_INDEXER.def().help(wrap!( - "Address of a `namada-masp-indexer` live instance. If \ - present, the shielded sync will be performed using data \ - retrieved from the given indexer." - ))) - .arg(WAIT_FOR_LAST_QUERY_HEIGHT.def().help(wrap!( - "Wait until the last height to sync is available instead \ - of returning early from the shielded sync." - ))) - .arg(MAX_CONCURRENT_FETCHES.def().help(wrap!( - "Maximum number of fetch jobs that will ever execute \ - concurrently during the shielded sync." - ))) - .arg(RETRIES.def().help(wrap!( - "Maximum number of times to retry fetching. If no \ - argument is provided, defaults to retrying forever." - ))) - .arg(BLOCK_BATCH.def().help(wrap!( - "Number of blocks fetched per concurrent fetch job. The \ - default is 10." - ))) + app.arg(BLOCK_HEIGHT_TO_OPT.def().help(wrap!( + "Option block height to sync up to. Default is latest." + ))) + .arg(DATED_SPENDING_KEYS.def().help(wrap!( + "List of new spending keys with which to check note \ + ownership. These will be added to the shielded context. \ + Appending \"<<$BLOCKHEIGHT\" to the end of each key adds a \ + birthday." + ))) + .arg(DATED_VIEWING_KEYS.def().help(wrap!( + "List of new viewing keys with which to check note ownership. \ + These will be added to the shielded context. Appending \ + \"<<$BLOCKHEIGHT\" to the end of each key adds a birthday." + ))) + .arg(WITH_INDEXER.def().help(wrap!( + "Address of a `namada-masp-indexer` live instance. If \ + present, the shielded sync will be performed using data \ + retrieved from the given indexer." + ))) + .arg(WAIT_FOR_LAST_QUERY_HEIGHT.def().help(wrap!( + "Wait until the last height to sync is available instead of \ + returning early from the shielded sync." + ))) + .arg(MAX_CONCURRENT_FETCHES.def().help(wrap!( + "Maximum number of fetch jobs that will ever execute \ + concurrently during the shielded sync." + ))) + .arg(RETRIES.def().help(wrap!( + "Maximum number of times to retry fetching. If no argument is \ + provided, defaults to retrying forever." + ))) + .arg(BLOCK_BATCH.def().help(wrap!( + "Number of blocks fetched per concurrent fetch job. The \ + default is 10." + ))) } } @@ -7261,7 +7293,6 @@ pub mod args { block_batch_size: self.block_batch_size, max_concurrent_fetches: self.max_concurrent_fetches, wait_for_last_query_height: self.wait_for_last_query_height, - ledger_address: chain_ctx.get(&self.ledger_address), last_query_height: self.last_query_height, spending_keys: self .spending_keys diff --git a/crates/apps_lib/src/cli/client.rs b/crates/apps_lib/src/cli/client.rs index 3ee7f5b8001..13222fa69be 100644 --- a/crates/apps_lib/src/cli/client.rs +++ b/crates/apps_lib/src/cli/client.rs @@ -396,9 +396,7 @@ impl CliApi { Sub::ShieldedSync(ShieldedSync(args)) => { let mut args = args.to_sdk(&mut ctx)?; let chain_ctx = ctx.take_chain_or_exit(); - let client = client.unwrap_or_else(|| { - C::from_tendermint_address(&args.ledger_address) - }); + let client = client.unwrap(); if args.with_indexer.is_none() { client.wait_until_node_is_synced(&io).await?; } diff --git a/crates/apps_lib/src/client/masp.rs b/crates/apps_lib/src/client/masp.rs index 795979f26a2..d655af53ea2 100644 --- a/crates/apps_lib/src/client/masp.rs +++ b/crates/apps_lib/src/client/masp.rs @@ -3,6 +3,7 @@ use std::time::Duration; use color_eyre::owo_colors::OwoColorize; #[cfg(feature = "testing")] use namada_core::masp::AssetData; +use namada_sdk::Namada; use namada_sdk::args::ShieldedSync; use namada_sdk::control_flow::install_shutdown_signal; use namada_sdk::error::Error; @@ -13,6 +14,7 @@ use namada_sdk::masp::{ IndexerMaspClient, LedgerMaspClient, LinearBackoffSleepMaspClient, MaspLocalTaskEnv, ShieldedContext, ShieldedSyncConfig, ShieldedUtils, }; +use namada_wallet::DatedViewingKey; use tendermint_rpc::HttpClient; use crate::cli::api::{CliClient, CliIo}; @@ -210,26 +212,26 @@ pub async fn syncing< Ok(()) } -// FIXME: rename? -pub async fn sync_shielded_context( +pub async fn sync_shielded_context( + ctx: &impl Namada, ledger_address: tendermint_rpc::Url, - shielded_ctx: &mut ShieldedContext, + mut sync_args: namada_sdk::args::ShieldedSync, ) -> Result<(), Error> { let sync_client = HttpClient::from_tendermint_address(&ledger_address); - // FIXME: review these args - let sync_args = namada_sdk::args::ShieldedSync { - ledger_address, - last_query_height: None, - spending_keys: vec![], - viewing_keys: vec![], - with_indexer: None, - wait_for_last_query_height: false, - max_concurrent_fetches: 100, - block_batch_size: 10, - retry_strategy: namada_sdk::masp::utils::RetryStrategy::Forever, - }; - // FIXME: is CliIo correct? - crate::client::masp::syncing(shielded_ctx, sync_client, sync_args, &CliIo) - .await + // Extend the keys we are interested into + let wallet = ctx.wallet().await; + sync_args + .viewing_keys + .extend(wallet.get_viewing_keys().into_iter().map(|(k, v)| { + DatedViewingKey::new(v, wallet.find_birthday(k).copied()) + })); + + crate::client::masp::syncing( + &mut *ctx.shielded_mut().await, + sync_client, + sync_args, + &CliIo, + ) + .await } diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index 70eb066bc2a..220a949317d 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -429,8 +429,13 @@ pub async fn query_rewards_estimate( cli::safe_exit(1); } // Sync the shielded context before estimating the rewards - if let Err(e) = - sync_shielded_context(args.query.ledger_address, &mut *shielded).await + if let Err(e) = args + .shielded_sync + .ok_or("Missing arguments for shielded-sync") + .map(async |sync_args| { + sync_shielded_context(context, args.query.ledger_address, sync_args) + .await + }) { edisplay_line!( context.io(), @@ -543,8 +548,13 @@ async fn query_shielded_balance( }; // Sync the shielded context before computing the balance - if let Err(e) = - sync_shielded_context(query.ledger_address, &mut *shielded).await + if let Err(e) = args + .shielded_sync + .ok_or("Missing arguments for shielded-sync") + .map(async |sync_args| { + sync_shielded_context(context, query.ledger_address, sync_args) + .await + }) { edisplay_line!( context.io(), diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index fd803da5a6e..931700ee060 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -1227,8 +1227,13 @@ pub async fn submit_shielded_transfer( // Sync the shielded context before building the transaction sync_shielded_context( + namada, args.tx.ledger_address.clone(), - &mut *namada.shielded_mut().await, + args.shielded_sync.clone().ok_or_else(|| { + error::Error::Other( + "Missing arguments for shielded sync".to_string(), + ) + })?, ) .await?; @@ -1389,8 +1394,13 @@ pub async fn submit_unshielding_transfer( // Sync the shielded context before building the transaction sync_shielded_context( + namada, args.tx.ledger_address.clone(), - &mut *namada.shielded_mut().await, + args.shielded_sync.clone().ok_or_else(|| { + error::Error::Other( + "Missing arguments for shielded sync".to_string(), + ) + })?, ) .await?; @@ -1481,8 +1491,13 @@ where // If the ibc sources are MASP, sync the shielded context before building // the transaction FIXME: only if source is masp sync_shielded_context( + namada, args.tx.ledger_address.clone(), - &mut *namada.shielded_mut().await, + args.shielded_sync.clone().ok_or_else(|| { + error::Error::Other( + "Missing arguments for shielded sync".to_string(), + ) + })?, ) .await?; diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index 090495ab26e..c626a356192 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -1248,8 +1248,6 @@ impl BenchShieldedCtx { &mut self.shielded, self.shell.clone(), ShieldedSync { - ledger_address: FromStr::from_str("http://127.0.0.1:1337") - .unwrap(), last_query_height: None, spending_keys: vec![spending_key], viewing_keys: vec![], diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index d70e035bc58..5f68e474fbc 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -342,6 +342,11 @@ pub struct TxShieldedTransfer { pub gas_spending_key: Option, /// Path to the TX WASM code file pub tx_code_path: PathBuf, + /// Optional data for the implicit shielded-sync + // FIXME: not sure about this, the sdk won't call shielded-sync implicitly + // so this arg is useless FIXME: maybe better to read a configuration + // file? + pub shielded_sync: Option>, } impl TxBuilder for TxShieldedTransfer { @@ -442,6 +447,8 @@ pub struct TxUnshieldingTransfer { pub frontend_sus_fee: Option<(C::TransferTarget, Dec)>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, + /// Optional data for the implicit shielded-sync + pub shielded_sync: Option>, } impl TxBuilder for TxUnshieldingTransfer { @@ -821,6 +828,8 @@ pub struct TxIbcTransfer { pub frontend_sus_fee: Option<(C::TransferTarget, Dec)>, /// Path to the TX WASM code file pub tx_code_path: PathBuf, + /// Optional data for the implicit shielded-sync + pub shielded_sync: Option>, } impl TxBuilder for TxIbcTransfer { @@ -1927,16 +1936,19 @@ pub struct QueryBalance { pub no_conversions: bool, /// Optional height to query balances at pub height: Option, + /// Optional data for the implicit shielded-sync + pub shielded_sync: Option>, } -/// Get an estimate for the MASP rewards for the next -/// MASP epoch. +/// Get an estimate for the MASP rewards for the next MASP epoch. #[derive(Clone, Debug)] pub struct QueryShieldingRewardsEstimate { /// Common query args pub query: Query, /// Viewing key pub owner: C::ViewingKey, + /// Optional data for the implicit shielded-sync + pub shielded_sync: Option>, } /// Query historical transfer(s) @@ -2468,10 +2480,9 @@ impl TxReactivateValidator { /// viewing keys. Syncing can be told to stop at a given /// block height. pub struct ShieldedSync { - /// The ledger address - pub ledger_address: C::ConfigRpcTendermintAddress, /// Height to sync up to. Defaults to most recent pub last_query_height: Option, + // FIXME: this seems to never be used /// Spending keys used to determine note ownership pub spending_keys: Vec, /// Viewing keys used to determine note ownership diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 677f0bdffe3..1902a527864 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -174,6 +174,7 @@ pub trait Namada: NamadaIo { gas_spending_key, tx_code_path: PathBuf::from(TX_TRANSFER_WASM), tx: self.tx_builder(), + shielded_sync: None, } } @@ -210,6 +211,7 @@ pub trait Namada: NamadaIo { tx_code_path: PathBuf::from(TX_TRANSFER_WASM), tx: self.tx_builder(), frontend_sus_fee, + shielded_sync: None, } } @@ -315,6 +317,7 @@ pub trait Namada: NamadaIo { tx: self.tx_builder(), tx_code_path: PathBuf::from(TX_IBC_WASM), frontend_sus_fee, + shielded_sync: None, } } From 9dff8c6bc97c205f04ca22fff4a414b754657542 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 23 Oct 2025 15:06:07 +0200 Subject: [PATCH 04/17] Shielded sync in ibc tx only when shielded sources --- crates/apps_lib/src/client/tx.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 931700ee060..db2eac08c54 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -1471,7 +1471,6 @@ where .chain(args.gas_spending_key.iter_mut()); let shielded_hw_keys = augment_masp_hardware_keys(namada, &args.tx, sources).await?; - // FIXME: don't we need this only if masp is involved? // Try to generate MASP build parameters. This might fail when using a // hardware wallet if it does not support MASP operations. let bparams_result = generate_masp_build_params( @@ -1489,17 +1488,19 @@ where ); // If the ibc sources are MASP, sync the shielded context before building - // the transaction FIXME: only if source is masp - sync_shielded_context( - namada, - args.tx.ledger_address.clone(), - args.shielded_sync.clone().ok_or_else(|| { - error::Error::Other( - "Missing arguments for shielded sync".to_string(), - ) - })?, - ) - .await?; + // the transaction + if args.source.spending_key().is_some() || args.gas_spending_key.is_some() { + sync_shielded_context( + namada, + args.tx.ledger_address.clone(), + args.shielded_sync.clone().ok_or_else(|| { + error::Error::Other( + "Missing arguments for shielded sync".to_string(), + ) + })?, + ) + .await?; + } // If transaction building fails for any reason, then abort the process // blaming MASP build parameter generation if that had also failed. From 211429a4c4dddab4b818fea3d4bd5b63e9dfe04e Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 31 Oct 2025 18:36:41 +0100 Subject: [PATCH 05/17] Removes `shielded-sync` and the speculative context test from integration tests --- crates/tests/src/integration/masp.rs | 1217 +------------------------- 1 file changed, 18 insertions(+), 1199 deletions(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index ede0ce51de2..2e9f3d7f7d4 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -18,6 +18,7 @@ use namada_node::shell::ResultCode; use namada_node::shell::testing::client::run; use namada_node::shell::testing::node::NodeResults; use namada_node::shell::testing::utils::{Bin, CapturedOutput}; +use namada_sdk::DEFAULT_GAS_LIMIT; use namada_sdk::account::AccountPublicKeysMap; #[cfg(feature = "historic-masp")] use namada_sdk::collections::HashMap; @@ -36,7 +37,6 @@ use namada_sdk::token::{self, Amount, DenominatedAmount, MaspEpoch}; #[cfg(feature = "historic-masp")] use namada_sdk::tx::IndexedTx; use namada_sdk::tx::{Section, Tx}; -use namada_sdk::{DEFAULT_GAS_LIMIT, tx}; use test_log::test; use super::{helpers, setup}; @@ -50,8 +50,6 @@ use crate::e2e::setup::constants::{ use crate::integration::helpers::make_temp_account; use crate::strings::TX_APPLIED_SUCCESS; -// FIXME: avoid explicitly calling shielded-sync in the tests - /// Enable masp rewards before some token is shielded, /// but the max reward rate is null. #[test] @@ -142,19 +140,6 @@ fn init_null_rewards() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch the latest test token notes - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check that we have some shielded test tokens let captured = CapturedOutput::of(|| { run( @@ -341,19 +326,6 @@ fn init_null_rewards() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch latest shielded state - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check that we now have half of the rewards let captured = CapturedOutput::of(|| { run( @@ -398,19 +370,6 @@ fn init_null_rewards() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch latest shielded state - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check that we now a null NAM balance let captured = CapturedOutput::of(|| { run( @@ -455,19 +414,6 @@ fn init_null_rewards() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch latest shielded state - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check test token balance let captured = CapturedOutput::of(|| { run( @@ -564,19 +510,6 @@ fn values_spanning_multiple_masp_digits() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch the note we just created containing test tokens - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check the test token balance corresponds 1:1 to what // we shielded let captured = CapturedOutput::of(|| { @@ -648,19 +581,6 @@ fn values_spanning_multiple_masp_digits() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Run shielded sync - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check test token balance is null let captured = CapturedOutput::of(|| { run( @@ -733,19 +653,6 @@ fn values_spanning_multiple_masp_digits() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch the note we just created containing test tokens - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check the test token balance let captured = CapturedOutput::of(|| { run( @@ -822,19 +729,6 @@ fn values_spanning_multiple_masp_digits() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch latest shielded state - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check that we now have half of the rewards let captured = CapturedOutput::of(|| { run( @@ -879,20 +773,6 @@ fn values_spanning_multiple_masp_digits() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch latest shielded state - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check the shielded NAM balance let captured = CapturedOutput::of(|| { run( @@ -940,20 +820,6 @@ fn values_spanning_multiple_masp_digits() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch latest shielded state - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check that we now have a null NAM balance let captured = CapturedOutput::of(|| { run( @@ -1041,19 +907,6 @@ fn enable_rewards_after_shielding() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch the note we just created containing test tokens - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check the test token balance corresponds 1:1 to what // we shielded let captured = CapturedOutput::of(|| { @@ -1206,19 +1059,6 @@ fn enable_rewards_after_shielding() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch the latest test token notes - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check that the balance is now 0 let captured = CapturedOutput::of(|| { run( @@ -1264,19 +1104,6 @@ fn enable_rewards_after_shielding() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch the latest test token notes - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check that we have some shielded test tokens once more let captured = CapturedOutput::of(|| { run( @@ -1368,19 +1195,6 @@ fn enable_rewards_after_shielding() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch latest shielded state - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check that we now have half of the rewards let captured = CapturedOutput::of(|| { run( @@ -1425,19 +1239,6 @@ fn enable_rewards_after_shielding() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch latest shielded state - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check that we now a null NAM balance let captured = CapturedOutput::of(|| { run( @@ -1482,19 +1283,6 @@ fn enable_rewards_after_shielding() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // Fetch latest shielded state - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - RPC, - ], - )?; - // Check test token balance let captured = CapturedOutput::of(|| { run( @@ -1579,20 +1367,6 @@ fn auto_compounding() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert that the actual and estimated balances are equal to the parameters // of this closure. Also assert that the total MASP balance is equal to the // last parameter. Then unshield, reshield, synchronize, and jump to the @@ -1766,20 +1540,6 @@ fn auto_compounding() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Send bal_b NAM from Albert to Bertha's shielded key let captured = CapturedOutput::of(|| { run( @@ -1805,20 +1565,6 @@ fn auto_compounding() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Wait till epoch boundary node.next_masp_epoch(); @@ -1909,20 +1655,6 @@ fn base_precision_effective() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert NAM balance at Albert's viewing key is 0.1 let captured = CapturedOutput::of(|| { run( @@ -2075,20 +1807,6 @@ fn reset_conversions() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert BTC balance at VK(A) is 1 let captured = CapturedOutput::of(|| { run( @@ -2151,13 +1869,6 @@ fn reset_conversions() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is still 1 let captured = CapturedOutput::of(|| { run( @@ -2239,13 +1950,6 @@ fn reset_conversions() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is still 1 let captured = CapturedOutput::of(|| { run( @@ -2552,20 +2256,6 @@ fn dynamic_precision() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert BTC balance at VK(A) is 1 let captured = CapturedOutput::of(|| { run( @@ -2628,13 +2318,6 @@ fn dynamic_precision() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is still 1 let captured = CapturedOutput::of(|| { run( @@ -2725,13 +2408,6 @@ fn dynamic_precision() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is still 1 let captured = CapturedOutput::of(|| { run( @@ -2840,13 +2516,6 @@ fn dynamic_precision() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert NAM balance at VK(A) is now 0 let captured = CapturedOutput::of(|| { run( @@ -2910,20 +2579,6 @@ fn dynamic_precision() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert BTC balance at VK(A) is 1 let captured = CapturedOutput::of(|| { run( @@ -2965,13 +2620,6 @@ fn dynamic_precision() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is 1 let captured = CapturedOutput::of(|| { run( @@ -3037,13 +2685,6 @@ fn dynamic_precision() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Check that unshielding the 0.25316 NAM reward also works let captured = CapturedOutput::of(|| { run( @@ -3069,13 +2710,6 @@ fn dynamic_precision() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Unfortunately, changing the precision after non-zero rewards have already // been distributed leaves unclaimable NAM in the pool let captured = CapturedOutput::of(|| { @@ -3135,20 +2769,6 @@ fn masp_incentives() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert BTC balance at VK(A) is 1 let captured = CapturedOutput::of(|| { run( @@ -3211,13 +2831,6 @@ fn masp_incentives() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is still 1 let captured = CapturedOutput::of(|| { run( @@ -3299,13 +2912,6 @@ fn masp_incentives() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is still 1 let captured = CapturedOutput::of(|| { run( @@ -3415,13 +3021,6 @@ fn masp_incentives() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert ETH balance at VK(B) is 0.001 let captured = CapturedOutput::of(|| { run( @@ -3463,13 +3062,6 @@ fn masp_incentives() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert ETH balance at VK(B) is still 0.001 let captured = CapturedOutput::of(|| { run( @@ -3556,13 +3148,6 @@ fn masp_incentives() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert ETH balance at VK(B) is 0 let captured = CapturedOutput::of(|| { run( @@ -3583,12 +3168,6 @@ fn masp_incentives() -> Result<()> { assert!(captured.contains("eth: 0")); node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Assert VK(B) retains the NAM rewards dispensed in the correct // amount. @@ -3611,12 +3190,6 @@ fn masp_incentives() -> Result<()> { assert!(captured.contains("nam: 1.502496")); node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Assert NAM balance at MASP pool is // the accumulation of rewards from the shielded assets (BTC and ETH) @@ -3666,13 +3239,6 @@ fn masp_incentives() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is 0 let captured = CapturedOutput::of(|| { run( @@ -3733,12 +3299,6 @@ fn masp_incentives() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Assert NAM balance at VK(A) is the rewards dispensed earlier // (since VK(A) has no shielded assets, no further rewards should @@ -3805,12 +3365,6 @@ fn masp_incentives() -> Result<()> { // Wait till epoch boundary to prevent conversion expiry during transaction // construction node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Send all NAM rewards from SK(B) to Christel let captured = CapturedOutput::of(|| { run( @@ -3840,12 +3394,6 @@ fn masp_incentives() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Send all NAM rewards from SK(A) to Bertha let captured = CapturedOutput::of(|| { run( @@ -3871,13 +3419,6 @@ fn masp_incentives() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert NAM balance at VK(A) is 0 let captured = CapturedOutput::of(|| { run( @@ -3897,12 +3438,6 @@ fn masp_incentives() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains("nam: 0")); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Assert NAM balance at VK(B) is 0 let captured = CapturedOutput::of(|| { run( @@ -4014,19 +3549,6 @@ fn spend_unconverted_asset_type() -> Result<()> { for _ in 0..5 { node.next_epoch(); } - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; // 4. Check the shielded balance let captured = CapturedOutput::of(|| { run( @@ -4107,20 +3629,6 @@ fn masp_txs_and_queries() -> Result<()> { let (mut node, _services) = setup::setup()?; _ = node.next_epoch(); - // add necessary viewing keys to shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - let txs_args = vec![ // 0. Attempt to spend 10 BTC at SK(A) to PA(B) ( @@ -4347,12 +3855,6 @@ fn masp_txs_and_queries() -> Result<()> { vec![false] }; for &dry_run in &dry_run_args { - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; let tx_args = if dry_run && is_use_device() { continue; } else if dry_run { @@ -4413,251 +3915,10 @@ fn masp_txs_and_queries() -> Result<()> { msg, captured.output ); - } - } - } - } - - Ok(()) -} - -/// Tests that multiple transactions can be constructed (without fetching) from -/// the shielded context and executed in the same block -#[test] -fn multiple_unfetched_txs_same_block() -> Result<()> { - // This address doesn't matter for tests. But an argument is required. - let validator_one_rpc = "http://127.0.0.1:26567"; - // Download the shielded pool parameters before starting node - let _ = FsShieldedUtils::new(PathBuf::new()); - let (mut node, _services) = setup::setup()?; - _ = node.next_epoch(); - - // Initialize accounts we can access the secret keys of - let (cooper_alias, cooper_key) = - make_temp_account(&node, validator_one_rpc, "Cooper", NAM, 500_000)?; - - // 1. Shield tokens - _ = node.next_epoch(); - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "shield", - "--source", - ALBERT_KEY, - "--target", - AA_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "100", - "--ledger-address", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains(TX_APPLIED_SUCCESS)); - _ = node.next_epoch(); - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "shield", - "--source", - ALBERT_KEY, - "--target", - AA_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "200", - "--ledger-address", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains(TX_APPLIED_SUCCESS)); - _ = node.next_epoch(); - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "shield", - "--source", - ALBERT_KEY, - "--target", - AB_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "100", - "--ledger-address", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - assert!(captured.contains(TX_APPLIED_SUCCESS)); - - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - - // 2. Shielded operations without fetching. Dump the txs to then reload and - // submit in the same block - let tempdir = tempfile::tempdir().unwrap(); - let mut txs_bytes = vec![]; - - _ = node.next_epoch(); - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "transfer", - "--source", - A_SPENDING_KEY, - "--target", - AC_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "50", - "--output-folder-path", - tempdir.path().to_str().unwrap(), - "--dump-tx", - "--ledger-address", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - - let file_path = tempdir - .path() - .read_dir() - .unwrap() - .next() - .unwrap() - .unwrap() - .path(); - txs_bytes.push(std::fs::read(&file_path).unwrap()); - std::fs::remove_file(&file_path).unwrap(); - - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "transfer", - "--source", - A_SPENDING_KEY, - "--target", - AC_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "50", - "--gas-payer", - cooper_alias, - "--output-folder-path", - tempdir.path().to_str().unwrap(), - "--dump-tx", - "--ledger-address", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - - let file_path = tempdir - .path() - .read_dir() - .unwrap() - .next() - .unwrap() - .unwrap() - .path(); - txs_bytes.push(std::fs::read(&file_path).unwrap()); - std::fs::remove_file(&file_path).unwrap(); - - let captured = CapturedOutput::of(|| { - run( - &node, - Bin::Client, - apply_use_device(vec![ - "transfer", - "--source", - B_SPENDING_KEY, - "--target", - AC_PAYMENT_ADDRESS, - "--token", - NAM, - "--amount", - "50", - "--gas-payer", - cooper_alias, - "--output-folder-path", - tempdir.path().to_str().unwrap(), - "--dump-tx", - "--ledger-address", - validator_one_rpc, - ]), - ) - }); - assert!(captured.result.is_ok()); - - let file_path = tempdir - .path() - .read_dir() - .unwrap() - .next() - .unwrap() - .unwrap() - .path(); - txs_bytes.push(std::fs::read(&file_path).unwrap()); - std::fs::remove_file(&file_path).unwrap(); - - let sk = cooper_key; - let pk = sk.to_public(); - - let native_token = node - .shell - .lock() - .unwrap() - .state - .in_mem() - .native_token - .clone(); - let mut txs = vec![]; - for bytes in txs_bytes { - let mut tx = Tx::try_from_json_bytes(&bytes).unwrap(); - tx.add_wrapper( - tx::data::wrapper::Fee { - amount_per_gas_unit: DenominatedAmount::native(100.into()), - token: native_token.clone(), - }, - pk.clone(), - DEFAULT_GAS_LIMIT.into(), - ); - tx.sign_wrapper(sk.clone()); - - txs.push(tx.to_bytes()); - } - - node.clear_results(); - node.submit_txs(txs); - // If empty then failed in process proposal - assert!(!node.tx_result_codes.lock().unwrap().is_empty()); - node.assert_success(); + } + } + } + } Ok(()) } @@ -4697,12 +3958,6 @@ fn masp_tx_expiration_first_invalid_block_height() -> Result<()> { validator_one_rpc, ]), )?; - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // 2. Shielded operation to avoid the need of a signature on the inner tx. // Dump the tx to then reload and submit @@ -4882,12 +4137,6 @@ fn masp_tx_expiration_first_invalid_block_height_with_fee_payment() -> Result<() validator_one_rpc, ]), )?; - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // 2. Shielded operation to avoid the need of a signature on the inner tx. // Dump the tx to then reload and submit @@ -5047,12 +4296,6 @@ fn masp_tx_expiration_last_valid_block_height() -> Result<()> { validator_one_rpc, ]), )?; - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // 2. Shielded operation to avoid the need of a signature on the inner tx. // Dump the tx to then reload and submit @@ -5224,19 +4467,6 @@ fn cross_epoch_unshield() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // 2. Generate the tx in the current epoch let tempdir = tempfile::tempdir().unwrap(); let captured = CapturedOutput::of(|| { @@ -5329,18 +4559,6 @@ fn dynamic_assets() -> Result<()> { .unwrap(); test_tokens }; - // add necessary viewing keys to shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; // Wait till epoch boundary node.next_masp_epoch(); // Send 1 BTC from Albert to PA @@ -5366,13 +4584,6 @@ fn dynamic_assets() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is 1 let captured = CapturedOutput::of(|| { run( @@ -5433,12 +4644,6 @@ fn dynamic_assets() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Assert BTC balance at VK(A) is still 1 let captured = CapturedOutput::of(|| { @@ -5502,13 +4707,6 @@ fn dynamic_assets() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is now 2 let captured = CapturedOutput::of(|| { run( @@ -5549,12 +4747,6 @@ fn dynamic_assets() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Assert that VK(A) has now received a NAM rewward for second deposit let captured = CapturedOutput::of(|| { @@ -5609,12 +4801,6 @@ fn dynamic_assets() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Assert BTC balance at VK(A) is still 2 let captured = CapturedOutput::of(|| { @@ -5675,12 +4861,6 @@ fn dynamic_assets() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Assert BTC balance at VK(A) is still 2 let captured = CapturedOutput::of(|| { @@ -5722,12 +4902,6 @@ fn dynamic_assets() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Assert BTC balance at VK(A) is still 2 let captured = CapturedOutput::of(|| { run( @@ -5781,12 +4955,6 @@ fn dynamic_assets() -> Result<()> { // Wait till epoch boundary node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Assert BTC balance at VK(A) is still 2 let captured = CapturedOutput::of(|| { run( @@ -5850,13 +5018,6 @@ fn dynamic_assets() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert that the NAM at VK(A) is now null let captured = CapturedOutput::of(|| { run( @@ -5901,13 +5062,6 @@ fn dynamic_assets() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert that the principal's balance is now null let captured = CapturedOutput::of(|| { run( @@ -5971,12 +5125,6 @@ fn masp_fee_payment() -> Result<()> { assert!(captured.contains(TX_APPLIED_SUCCESS)); _ = node.next_masp_epoch(); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; let captured = CapturedOutput::of(|| { run( &node, @@ -6021,12 +5169,6 @@ fn masp_fee_payment() -> Result<()> { }); assert!(captured.result.is_err()); _ = node.next_masp_epoch(); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; let captured = CapturedOutput::of(|| { run( &node, @@ -6159,12 +5301,6 @@ fn masp_fee_payment() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; // Check the exact balance of the tx source to ensure that the masp fee // payment transaction was executed only once let captured = CapturedOutput::of(|| { @@ -6246,13 +5382,6 @@ fn masp_fee_payment_gas_limit() -> Result<()> { _ = node.next_masp_epoch(); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Check that the balance hasn't changed let captured = CapturedOutput::of(|| { run( @@ -6299,13 +5428,6 @@ fn masp_fee_payment_gas_limit() -> Result<()> { _ = node.next_masp_epoch(); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Check that the balance hasn't changed let captured = CapturedOutput::of(|| { run( @@ -6368,13 +5490,6 @@ fn masp_fee_payment_with_non_disposable() -> Result<()> { _ = node.next_masp_epoch(); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - let captured = CapturedOutput::of(|| { run( &node, @@ -6442,13 +5557,6 @@ fn masp_fee_payment_with_non_disposable() -> Result<()> { _ = node.next_masp_epoch(); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - let captured = CapturedOutput::of(|| { run( &node, @@ -6546,13 +5654,6 @@ fn masp_fee_payment_with_custom_spending_key() -> Result<()> { _ = node.next_masp_epoch(); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - let captured = CapturedOutput::of(|| { run( &node, @@ -6619,13 +5720,6 @@ fn masp_fee_payment_with_custom_spending_key() -> Result<()> { _ = node.next_masp_epoch(); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - let captured = CapturedOutput::of(|| { run( &node, @@ -6772,13 +5866,6 @@ fn masp_fee_payment_with_different_token() -> Result<()> { _ = node.next_masp_epoch(); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - let captured = CapturedOutput::of(|| { run( &node, @@ -6864,13 +5951,6 @@ fn masp_fee_payment_with_different_token() -> Result<()> { _ = node.next_masp_epoch(); - // sync shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - let captured = CapturedOutput::of(|| { run( &node, @@ -7084,19 +6164,6 @@ fn identical_output_descriptions() -> Result<()> { } } - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert NAM balance at VK(A) is 2000 let captured = CapturedOutput::of(|| { run( @@ -7455,19 +6522,6 @@ fn masp_batch() -> Result<()> { node.clear_results(); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert NAM balances at VK(A), Bob and Bertha for (owner, balance) in [ (AA_VIEWING_KEY, 2_000), @@ -7719,19 +6773,6 @@ fn masp_atomic_batch() -> Result<()> { node.clear_results(); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert NAM balances at VK(A), Albert and Bertha are unchanged for (owner, balance) in [ (AA_VIEWING_KEY, 0), @@ -7824,19 +6865,7 @@ fn masp_failing_atomic_batch() -> Result<()> { assert!(captured.contains(TX_APPLIED_SUCCESS)); } - // Sync the shielded context and check the balance - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; + // Check the balance for owner in [AA_VIEWING_KEY, AC_VIEWING_KEY] { let captured = CapturedOutput::of(|| { run( @@ -8036,21 +7065,6 @@ fn masp_failing_atomic_batch() -> Result<()> { assert!(inner_tx_result.is_err()); } - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert NAM balances at VK(A), Albert and Bertha are unchanged for (owner, balance) in [ (AA_VIEWING_KEY, 998.5), @@ -8288,19 +7302,6 @@ fn tricky_masp_txs() -> Result<()> { node.submit_txs(txs); node.assert_success(); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert NAM balances at VK(A), Albert, Bertha and Christel for (owner, balance) in [ (AA_VIEWING_KEY, 1_000), @@ -8355,7 +7356,7 @@ fn masp_events() -> Result<()> { // 1. Shield some tokens in two steps two generate two different output // notes - for target in [AA_PAYMENT_ADDRESS, AC_PAYMENT_ADDRESS] { + for target in [AA_PAYMENT_ADDRESS, AB_PAYMENT_ADDRESS, AC_PAYMENT_ADDRESS] { for _ in 0..2 { let captured = CapturedOutput::of(|| { run( @@ -8381,20 +7382,8 @@ fn masp_events() -> Result<()> { } } - // 2. Sync the shielded context and check the balance - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - for owner in [AA_VIEWING_KEY, AC_VIEWING_KEY] { + // 2. Check the balance + for owner in [AA_VIEWING_KEY, AB_VIEWING_KEY, AC_VIEWING_KEY] { let captured = CapturedOutput::of(|| { run( &node, @@ -8432,8 +7421,8 @@ fn masp_events() -> Result<()> { .read(&tree_key) .unwrap() .unwrap(); - // We've produced 4 notes so far from the previous shielding operations - assert_eq!(commitment_tree.size(), 4); + // We've produced 6 notes so far from the previous shielding operations + assert_eq!(commitment_tree.size(), 6); _ = node.next_epoch(); let captured = CapturedOutput::of(|| { @@ -8491,7 +7480,7 @@ fn masp_events() -> Result<()> { apply_use_device(vec![ "unshield", "--source", - C_SPENDING_KEY, + B_SPENDING_KEY, "--target", cooper_alias, "--token", @@ -8505,7 +7494,7 @@ fn masp_events() -> Result<()> { "--gas-payer", cooper_alias, "--gas-spending-key", - C_SPENDING_KEY, + B_SPENDING_KEY, "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-wrapper-tx", @@ -8725,20 +7714,7 @@ fn masp_events() -> Result<()> { .unwrap(); assert_eq!(commitment_tree, storage_commitment_tree); - // 4. Sync the shielded context and check the balances - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; + // 4. Check the balances let captured = CapturedOutput::of(|| { run( &node, @@ -8772,7 +7748,7 @@ fn masp_events() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 2")); + assert!(captured.contains("nam: 1000")); let captured = CapturedOutput::of(|| { run( &node, @@ -8789,7 +7765,7 @@ fn masp_events() -> Result<()> { ) }); assert!(captured.result.is_ok()); - assert!(captured.contains("nam: 997")); + assert!(captured.contains("nam: 999")); // 5. Spend all the tokens in the pool (this verifies that the client // reconstructs the correct shielded state) @@ -8832,7 +7808,7 @@ fn masp_events() -> Result<()> { "--token", NAM, "--amount", - "2", + "1000", "--gas-limit", "100000", "--gas-payer", @@ -8858,7 +7834,7 @@ fn masp_events() -> Result<()> { "--token", NAM, "--amount", - "997", + "999", "--gas-limit", "100000", "--gas-payer", @@ -8872,20 +7848,6 @@ fn masp_events() -> Result<()> { assert!(captured.contains(TX_APPLIED_SUCCESS)); // 6. Check that all the shielded balances are 0 - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - for owner in [AA_VIEWING_KEY, AB_VIEWING_KEY, AC_VIEWING_KEY] { let captured = CapturedOutput::of(|| { run( @@ -8952,20 +7914,6 @@ fn multiple_inputs_from_single_note() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert BTC balance at VK(A) is 10 let captured = CapturedOutput::of(|| { run( @@ -9007,13 +7955,6 @@ fn multiple_inputs_from_single_note() -> Result<()> { // Skip masp epoch for rewards node.next_masp_epoch(); - // sync the shielded context - run( - &node, - Bin::Client, - vec!["shielded-sync", "--node", validator_one_rpc], - )?; - // Assert BTC balance at VK(A) is still 10 let captured = CapturedOutput::of(|| { run( @@ -9114,20 +8055,6 @@ fn multiple_inputs_from_single_note() -> Result<()> { assert!(captured.result.is_ok(), "{:?}", captured.result); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert NAM balance at VK(A) is 0 let captured = CapturedOutput::of(|| { run( @@ -9264,19 +8191,6 @@ fn history() -> Result<()> { rt.block_on(shielded_wallet.load()); assert!(shielded_wallet.history.is_empty()); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert NAM balance at VK(A) is 10 let captured = CapturedOutput::of(|| { run( @@ -9394,20 +8308,6 @@ fn history() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert NAM balance at VK(A) is 5 let captured = CapturedOutput::of(|| { run( @@ -9606,19 +8506,6 @@ fn history_with_conversions() -> Result<()> { rt.block_on(shielded_wallet.load()); assert!(shielded_wallet.history.is_empty()); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert BTC balance at VK(A) is 10 let captured = CapturedOutput::of(|| { run( @@ -9707,20 +8594,6 @@ fn history_with_conversions() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert BTC balance at VK(A) is 0 let captured = CapturedOutput::of(|| { run( @@ -9912,20 +8785,6 @@ fn frontend_sus_fee() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert token balance at VK(A) is 40 let captured = CapturedOutput::of(|| { run( @@ -10048,20 +8907,6 @@ fn frontend_sus_fee() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert token balance at VK(A) is 20.6 let captured = CapturedOutput::of(|| { run( @@ -10152,20 +8997,6 @@ fn frontend_sus_fee() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AC_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; - // Assert BTC balance at VK(A) is 9.6 let captured = CapturedOutput::of(|| { run( @@ -10381,18 +9212,6 @@ fn frontend_sus_fee_client_checks() -> Result<()> { assert!(captured.result.is_ok()); assert!(captured.contains(TX_APPLIED_SUCCESS)); - // sync the shielded context - run( - &node, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - "--node", - validator_one_rpc, - ], - )?; let captured = CapturedOutput::of(|| { run( &node, From c114c5b506dfe957595146549185a9133b7c1cbc Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 31 Oct 2025 18:38:31 +0100 Subject: [PATCH 06/17] Moves `shielded-sync` calls to `handle_client_command` --- crates/apps_lib/src/cli/client.rs | 193 +++++++++++++++++++++++++++-- crates/apps_lib/src/client/masp.rs | 44 ++----- crates/apps_lib/src/client/rpc.rs | 36 +----- crates/apps_lib/src/client/tx.rs | 40 ------ crates/node/src/bench_utils.rs | 4 +- crates/sdk/src/lib.rs | 25 ++++ crates/sdk/src/signing.rs | 11 ++ 7 files changed, 232 insertions(+), 121 deletions(-) diff --git a/crates/apps_lib/src/cli/client.rs b/crates/apps_lib/src/cli/client.rs index 13222fa69be..5338519ce69 100644 --- a/crates/apps_lib/src/cli/client.rs +++ b/crates/apps_lib/src/cli/client.rs @@ -3,8 +3,8 @@ use std::io::Read; use color_eyre::eyre::Result; use namada_sdk::io::{Io, NamadaIo, display_line}; use namada_sdk::masp::ShieldedContext; -use namada_sdk::wallet::DatedViewingKey; use namada_sdk::{Namada, NamadaImpl}; +use namada_wallet::DatedViewingKey; use crate::cli; use crate::cli::api::{CliApi, CliClient}; @@ -19,7 +19,7 @@ impl CliApi { io: IO, ) -> Result<()> where - C: CliClient, + C: CliClient + Clone, { match cmd { cli::NamadaClient::WithContext(cmd_box) => { @@ -65,15 +65,49 @@ impl CliApi { tx::submit_transparent_transfer(&namada, args).await?; } Sub::TxShieldedTransfer(TxShieldedTransfer(args)) => { + let extra_sync_vks = { + let chain_ctx = ctx.borrow_chain_or_exit(); + chain_ctx + .wallet + .get_viewing_keys() + .into_iter() + .map(|(k, v)| { + DatedViewingKey::new( + v, + chain_ctx + .wallet + .find_birthday(k) + .copied(), + ) + }) + .collect::>() + }; let chain_ctx = ctx.borrow_mut_chain_or_exit(); + let shielded = std::mem::take(&mut chain_ctx.shielded); let ledger_address = chain_ctx.get(&args.tx.ledger_address); let client = client.unwrap_or_else(|| { C::from_tendermint_address(&ledger_address) }); client.wait_until_node_is_synced(&io).await?; - let args = args.to_sdk(&mut ctx)?; + + let mut args = args.to_sdk(&mut ctx)?; + let mut sync_args = std::mem::take( + &mut args.shielded_sync, + ) + .expect("Missing required shielded-sync arguments"); + sync_args.viewing_keys.extend(extra_sync_vks); + let shielded = crate::client::masp::syncing( + ShieldedContext::new(shielded), + client.clone(), + sync_args, + &io, + ) + .await?; + let namada = ctx.to_sdk(client, io); + let namada = + namada.update_shielded_context(shielded).await; tx::submit_shielded_transfer(&namada, args).await?; } Sub::TxShieldingTransfer(TxShieldingTransfer(args)) => { @@ -89,28 +123,99 @@ impl CliApi { tx::submit_shielding_transfer(&namada, args).await?; } Sub::TxUnshieldingTransfer(TxUnshieldingTransfer(args)) => { + // FIXME: helper function for this? + let extra_sync_vks = { + let chain_ctx = ctx.borrow_chain_or_exit(); + chain_ctx + .wallet + .get_viewing_keys() + .into_iter() + .map(|(k, v)| { + DatedViewingKey::new( + v, + chain_ctx + .wallet + .find_birthday(k) + .copied(), + ) + }) + .collect::>() + }; let chain_ctx = ctx.borrow_mut_chain_or_exit(); + let shielded = std::mem::take(&mut chain_ctx.shielded); let ledger_address = chain_ctx.get(&args.tx.ledger_address); let client = client.unwrap_or_else(|| { C::from_tendermint_address(&ledger_address) }); client.wait_until_node_is_synced(&io).await?; - let args = args.to_sdk(&mut ctx)?; + + let mut args = args.to_sdk(&mut ctx)?; + let mut sync_args = std::mem::take( + &mut args.shielded_sync, + ) + .expect("Missing required shielded-sync arguments"); + sync_args.viewing_keys.extend(extra_sync_vks); + let shielded = crate::client::masp::syncing( + ShieldedContext::new(shielded), + client.clone(), + sync_args, + &io, + ) + .await?; + let namada = ctx.to_sdk(client, io); + let namada = + namada.update_shielded_context(shielded).await; tx::submit_unshielding_transfer(&namada, args).await?; } Sub::TxIbcTransfer(args) => { + // FIXME: here but only if the source is shielded + let extra_sync_vks = { + let chain_ctx = ctx.borrow_chain_or_exit(); + chain_ctx + .wallet + .get_viewing_keys() + .into_iter() + .map(|(k, v)| { + DatedViewingKey::new( + v, + chain_ctx + .wallet + .find_birthday(k) + .copied(), + ) + }) + .collect::>() + }; let TxIbcTransfer(args) = *args; let chain_ctx = ctx.borrow_mut_chain_or_exit(); + let shielded = std::mem::take(&mut chain_ctx.shielded); let ledger_address = chain_ctx.get(&args.tx.ledger_address); let client = client.unwrap_or_else(|| { C::from_tendermint_address(&ledger_address) }); client.wait_until_node_is_synced(&io).await?; - let args = args.to_sdk(&mut ctx)?; + + let mut args = args.to_sdk(&mut ctx)?; + let mut sync_args = std::mem::take( + &mut args.shielded_sync, + ) + .expect("Missing required shielded-sync arguments"); + sync_args.viewing_keys.extend(extra_sync_vks); + // FIXME: here but only if the source is shielded + let shielded = crate::client::masp::syncing( + ShieldedContext::new(shielded), + client.clone(), + sync_args, + &io, + ) + .await?; + let namada = ctx.to_sdk(client, io); + let namada = + namada.update_shielded_context(shielded).await; tx::submit_ibc_transfer(&namada, args).await?; } Sub::TxOsmosisSwap(args) => { @@ -417,7 +522,7 @@ impl CliApi { ); crate::client::masp::syncing( - &mut ShieldedContext::new(chain_ctx.shielded), + ShieldedContext::new(chain_ctx.shielded), client, args, &io, @@ -604,29 +709,101 @@ impl CliApi { rpc::query_block(&namada).await; } Sub::QueryBalance(QueryBalance(args)) => { + // FIXME: only if query_shielded_balance + let extra_sync_vks = { + let chain_ctx = ctx.borrow_chain_or_exit(); + + chain_ctx + .wallet + .get_viewing_keys() + .into_iter() + .map(|(k, v)| { + DatedViewingKey::new( + v, + chain_ctx + .wallet + .find_birthday(k) + .copied(), + ) + }) + .collect::>() + }; + let chain_ctx = ctx.borrow_mut_chain_or_exit(); + let shielded = std::mem::take(&mut chain_ctx.shielded); let ledger_address = chain_ctx.get(&args.query.ledger_address); let client = client.unwrap_or_else(|| { C::from_tendermint_address(&ledger_address) }); client.wait_until_node_is_synced(&io).await?; - let args = args.to_sdk(&mut ctx)?; + + let mut args = args.to_sdk(&mut ctx)?; + // FIXME: only if query_shielded_balance + let mut sync_args = std::mem::take( + &mut args.shielded_sync, + ) + .expect("Missing required shielded-sync arguments"); + sync_args.viewing_keys.extend(extra_sync_vks); + let shielded = crate::client::masp::syncing( + ShieldedContext::new(shielded), + client.clone(), + sync_args, + &io, + ) + .await?; + let namada = ctx.to_sdk(client, io); + let namada = + namada.update_shielded_context(shielded).await; rpc::query_balance(&namada, args).await; } Sub::QueryShieldingRewardsEstimate( QueryShieldingRewardsEstimate(args), ) => { + let extra_sync_vks = { + let chain_ctx = ctx.borrow_chain_or_exit(); + chain_ctx + .wallet + .get_viewing_keys() + .into_iter() + .map(|(k, v)| { + DatedViewingKey::new( + v, + chain_ctx + .wallet + .find_birthday(k) + .copied(), + ) + }) + .collect::>() + }; let chain_ctx = ctx.borrow_mut_chain_or_exit(); + let shielded = std::mem::take(&mut chain_ctx.shielded); let ledger_address = chain_ctx.get(&args.query.ledger_address); let client = client.unwrap_or_else(|| { C::from_tendermint_address(&ledger_address) }); client.wait_until_node_is_synced(&io).await?; - let args = args.to_sdk(&mut ctx)?; + + let mut args = args.to_sdk(&mut ctx)?; + let mut sync_args = std::mem::take( + &mut args.shielded_sync, + ) + .expect("Missing required shielded-sync arguments"); + sync_args.viewing_keys.extend(extra_sync_vks); + let shielded = crate::client::masp::syncing( + ShieldedContext::new(shielded), + client.clone(), + sync_args, + &io, + ) + .await?; + let namada = ctx.to_sdk(client, io); + let namada = + namada.update_shielded_context(shielded).await; rpc::query_rewards_estimate(&namada, args).await; } Sub::QueryBonds(QueryBonds(args)) => { diff --git a/crates/apps_lib/src/client/masp.rs b/crates/apps_lib/src/client/masp.rs index d655af53ea2..dbd8a354007 100644 --- a/crates/apps_lib/src/client/masp.rs +++ b/crates/apps_lib/src/client/masp.rs @@ -3,7 +3,6 @@ use std::time::Duration; use color_eyre::owo_colors::OwoColorize; #[cfg(feature = "testing")] use namada_core::masp::AssetData; -use namada_sdk::Namada; use namada_sdk::args::ShieldedSync; use namada_sdk::control_flow::install_shutdown_signal; use namada_sdk::error::Error; @@ -14,10 +13,6 @@ use namada_sdk::masp::{ IndexerMaspClient, LedgerMaspClient, LinearBackoffSleepMaspClient, MaspLocalTaskEnv, ShieldedContext, ShieldedSyncConfig, ShieldedUtils, }; -use namada_wallet::DatedViewingKey; -use tendermint_rpc::HttpClient; - -use crate::cli::api::{CliClient, CliIo}; const MASP_INDEXER_CLIENT_USER_AGENT: &str = { const TOKENS: &[&str] = @@ -31,11 +26,11 @@ pub async fn syncing< C: Client + Send + Sync + 'static, IO: Io + Send + Sync, >( - shielded: &mut ShieldedContext, + mut shielded: ShieldedContext, client: C, args: ShieldedSync, io: &IO, -) -> Result<(), Error> { +) -> Result, Error> { let (fetched_bar, scanned_bar, applied_bar) = { #[cfg(any(test, feature = "testing"))] { @@ -102,7 +97,7 @@ pub async fn syncing< let env = MaspLocalTaskEnv::new(500) .map_err(|e| Error::Other(e.to_string()))?; - let res = shielded + let ctx = shielded .sync( env, config, @@ -111,11 +106,12 @@ pub async fn syncing< &vks, ) .await + .map(|_| shielded) .map_err(|e| Error::Other(e.to_string())); display!(io, "\nSyncing finished\n"); - res + ctx }}; } @@ -167,7 +163,7 @@ pub async fn syncing< .map_err(|e| Error::Other(e.to_string()))?; } - if let Some(endpoint) = args.with_indexer { + let shielded = if let Some(endpoint) = args.with_indexer { display_line!( io, "{}\n", @@ -207,31 +203,7 @@ pub async fn syncing< LedgerMaspClient::new(client, args.max_concurrent_fetches,), Duration::from_millis(5) ))? - } - - Ok(()) -} - -pub async fn sync_shielded_context( - ctx: &impl Namada, - ledger_address: tendermint_rpc::Url, - mut sync_args: namada_sdk::args::ShieldedSync, -) -> Result<(), Error> { - let sync_client = HttpClient::from_tendermint_address(&ledger_address); + }; - // Extend the keys we are interested into - let wallet = ctx.wallet().await; - sync_args - .viewing_keys - .extend(wallet.get_viewing_keys().into_iter().map(|(k, v)| { - DatedViewingKey::new(v, wallet.find_birthday(k).copied()) - })); - - crate::client::masp::syncing( - &mut *ctx.shielded_mut().await, - sync_client, - sync_args, - &CliIo, - ) - .await + Ok(shielded) } diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index 220a949317d..7cfd07cc146 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -59,7 +59,6 @@ use namada_sdk::wallet::AddressVpType; use namada_sdk::{Namada, error, state as storage, token}; use crate::cli::{self, args}; -use crate::client::masp::sync_shielded_context; use crate::tendermint::merkle::proof::ProofOps; /// Query the status of a given transaction. @@ -428,22 +427,6 @@ pub async fn query_rewards_estimate( edisplay_line!(context.io(), "Failed to load shielded context: {}", e); cli::safe_exit(1); } - // Sync the shielded context before estimating the rewards - if let Err(e) = args - .shielded_sync - .ok_or("Missing arguments for shielded-sync") - .map(async |sync_args| { - sync_shielded_context(context, args.query.ledger_address, sync_args) - .await - }) - { - edisplay_line!( - context.io(), - "Failed to sync the shielded context: {}", - e - ); - cli::safe_exit(1); - } let raw_balance = match shielded .compute_shielded_balance(&args.owner.as_viewing_key()) .await @@ -494,7 +477,7 @@ async fn query_shielded_balance( ) { let args::QueryBalance { // Need the ledger address for sync purposes - query, + query: _, // Token owner (needs to be a viewing key) owner, // The token to query @@ -547,23 +530,6 @@ async fn query_shielded_balance( display_line!(context.io(), "{token_alias}: 0"); }; - // Sync the shielded context before computing the balance - if let Err(e) = args - .shielded_sync - .ok_or("Missing arguments for shielded-sync") - .map(async |sync_args| { - sync_shielded_context(context, query.ledger_address, sync_args) - .await - }) - { - edisplay_line!( - context.io(), - "Failed to sync the shielded context: {}", - e - ); - cli::safe_exit(1); - } - let balance = if no_conversions || token != context.native_token() { let Some(bal) = shielded .compute_shielded_balance(&viewing_key) diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index db2eac08c54..1aee4e56403 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -35,7 +35,6 @@ use namada_sdk::{ExtendedViewingKey, Namada, error, signing, tx}; use rand::rngs::OsRng; use tokio::sync::RwLock; -use super::masp::sync_shielded_context; use super::rpc; use crate::cli::{args, safe_exit}; use crate::client::tx::signing::{SigningTxData, default_sign}; @@ -1225,18 +1224,6 @@ pub async fn submit_shielded_transfer( ) .await?; - // Sync the shielded context before building the transaction - sync_shielded_context( - namada, - args.tx.ledger_address.clone(), - args.shielded_sync.clone().ok_or_else(|| { - error::Error::Other( - "Missing arguments for shielded sync".to_string(), - ) - })?, - ) - .await?; - let (mut tx, signing_data) = args.clone().build(namada, &mut bparams).await?; let disposable_fee_payer = match signing_data { @@ -1392,18 +1379,6 @@ pub async fn submit_unshielding_transfer( ) .await?; - // Sync the shielded context before building the transaction - sync_shielded_context( - namada, - args.tx.ledger_address.clone(), - args.shielded_sync.clone().ok_or_else(|| { - error::Error::Other( - "Missing arguments for shielded sync".to_string(), - ) - })?, - ) - .await?; - let (mut tx, signing_data) = args.clone().build(namada, &mut bparams).await?; let disposable_fee_payer = match signing_data { @@ -1487,21 +1462,6 @@ where |bparams| (bparams, None), ); - // If the ibc sources are MASP, sync the shielded context before building - // the transaction - if args.source.spending_key().is_some() || args.gas_spending_key.is_some() { - sync_shielded_context( - namada, - args.tx.ledger_address.clone(), - args.shielded_sync.clone().ok_or_else(|| { - error::Error::Other( - "Missing arguments for shielded sync".to_string(), - ) - })?, - ) - .await?; - } - // If transaction building fails for any reason, then abort the process // blaming MASP build parameter generation if that had also failed. let (mut tx, signing_data, _) = args diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index c626a356192..809d24e6962 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -1243,9 +1243,9 @@ impl BenchShieldedCtx { spending_key, self.wallet.find_birthday(ALBERT_SPENDING_KEY).copied(), ); - async_runtime + self.shielded = async_runtime .block_on(namada_apps_lib::client::masp::syncing( - &mut self.shielded, + self.shielded, self.shell.clone(), ShieldedSync { last_query_height: None, diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 1902a527864..68041b7ebdd 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -117,6 +117,14 @@ pub trait Namada: NamadaIo { &self, ) -> RwLockWriteGuard<'_, ShieldedContext>; + /// Update the shielded context with the one provided + async fn update_shielded_context( + self, + context: ShieldedContext, + ) -> impl Namada + where + U: ShieldedUtils + MaybeSend + MaybeSync; + /// Return the native token fn native_token(&self) -> Address; @@ -813,6 +821,23 @@ where self.shielded.write().await } + async fn update_shielded_context( + self, + context: ShieldedContext, + ) -> impl Namada + where + T: ShieldedUtils + MaybeSend + MaybeSync, + { + NamadaImpl:: { + client: self.client, + wallet: self.wallet, + shielded: RwLock::new(context), + io: self.io, + native_token: self.native_token, + prototype: self.prototype, + } + } + fn native_token(&self) -> Address { self.native_token.clone() } diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index f8ca8722fb4..5006c5caeef 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -2402,6 +2402,7 @@ mod test_signing { use namada_core::token::{Denomination, MaspDigitPos}; use namada_governance::storage::proposal::PGFInternalTarget; use namada_io::client::EncodedResponseQuery; + use namada_token::masp::ShieldedUtils; use namada_tx::{Code, Data}; use namada_wallet::test_utils::TestWalletUtils; use tendermint_rpc::SimpleRequest; @@ -2559,6 +2560,16 @@ mod test_signing { unimplemented!() } + async fn update_shielded_context( + self, + _shielded: ShieldedContext, + ) -> Self + where + U: ShieldedUtils + MaybeSend + MaybeSync, + { + unimplemented!() + } + fn native_token(&self) -> Address { unimplemented!() } From 5a2a45498e59a7e9fc605c8f3357dde460a1b4bf Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 3 Nov 2025 11:36:41 +0100 Subject: [PATCH 07/17] Shielded sync only when required --- crates/apps_lib/src/cli/client.rs | 141 ++++++++++++++++-------------- 1 file changed, 73 insertions(+), 68 deletions(-) diff --git a/crates/apps_lib/src/cli/client.rs b/crates/apps_lib/src/cli/client.rs index 5338519ce69..bf59bf63f04 100644 --- a/crates/apps_lib/src/cli/client.rs +++ b/crates/apps_lib/src/cli/client.rs @@ -123,7 +123,6 @@ impl CliApi { tx::submit_shielding_transfer(&namada, args).await?; } Sub::TxUnshieldingTransfer(TxUnshieldingTransfer(args)) => { - // FIXME: helper function for this? let extra_sync_vks = { let chain_ctx = ctx.borrow_chain_or_exit(); chain_ctx @@ -170,10 +169,21 @@ impl CliApi { tx::submit_unshielding_transfer(&namada, args).await?; } Sub::TxIbcTransfer(args) => { - // FIXME: here but only if the source is shielded - let extra_sync_vks = { + let TxIbcTransfer(args) = *args; + let chain_ctx = ctx.borrow_mut_chain_or_exit(); + let shielded = std::mem::take(&mut chain_ctx.shielded); + let ledger_address = + chain_ctx.get(&args.tx.ledger_address); + let client = client.unwrap_or_else(|| { + C::from_tendermint_address(&ledger_address) + }); + client.wait_until_node_is_synced(&io).await?; + + let mut args = args.to_sdk(&mut ctx)?; + if args.source.spending_key().is_some() { + // Sync the shielded context let chain_ctx = ctx.borrow_chain_or_exit(); - chain_ctx + let extra_sync_vks = chain_ctx .wallet .get_viewing_keys() .into_iter() @@ -186,37 +196,30 @@ impl CliApi { .copied(), ) }) - .collect::>() - }; - let TxIbcTransfer(args) = *args; - let chain_ctx = ctx.borrow_mut_chain_or_exit(); - let shielded = std::mem::take(&mut chain_ctx.shielded); - let ledger_address = - chain_ctx.get(&args.tx.ledger_address); - let client = client.unwrap_or_else(|| { - C::from_tendermint_address(&ledger_address) - }); - client.wait_until_node_is_synced(&io).await?; - - let mut args = args.to_sdk(&mut ctx)?; - let mut sync_args = std::mem::take( - &mut args.shielded_sync, - ) - .expect("Missing required shielded-sync arguments"); - sync_args.viewing_keys.extend(extra_sync_vks); - // FIXME: here but only if the source is shielded - let shielded = crate::client::masp::syncing( - ShieldedContext::new(shielded), - client.clone(), - sync_args, - &io, - ) - .await?; + .collect::>(); + let mut sync_args = std::mem::take( + &mut args.shielded_sync, + ) + .expect("Missing required shielded-sync arguments"); + sync_args.viewing_keys.extend(extra_sync_vks); + let synced_shielded_ctx = + crate::client::masp::syncing( + ShieldedContext::new(shielded), + client.clone(), + sync_args, + &io, + ) + .await?; - let namada = ctx.to_sdk(client, io); - let namada = - namada.update_shielded_context(shielded).await; - tx::submit_ibc_transfer(&namada, args).await?; + let namada = ctx + .to_sdk(client, io) + .update_shielded_context(synced_shielded_ctx) + .await; + tx::submit_ibc_transfer(&namada, args).await? + } else { + let namada = ctx.to_sdk(client, io); + tx::submit_ibc_transfer(&namada, args).await? + } } Sub::TxOsmosisSwap(args) => { let TxOsmosisSwap(args) = *args; @@ -709,11 +712,20 @@ impl CliApi { rpc::query_block(&namada).await; } Sub::QueryBalance(QueryBalance(args)) => { - // FIXME: only if query_shielded_balance - let extra_sync_vks = { - let chain_ctx = ctx.borrow_chain_or_exit(); + let chain_ctx = ctx.borrow_mut_chain_or_exit(); + let shielded = std::mem::take(&mut chain_ctx.shielded); + let ledger_address = + chain_ctx.get(&args.query.ledger_address); + let client = client.unwrap_or_else(|| { + C::from_tendermint_address(&ledger_address) + }); + client.wait_until_node_is_synced(&io).await?; - chain_ctx + let mut args = args.to_sdk(&mut ctx)?; + if args.owner.full_viewing_key().is_some() { + // Sync the shielded context + let chain_ctx = ctx.borrow_chain_or_exit(); + let extra_sync_vks = chain_ctx .wallet .get_viewing_keys() .into_iter() @@ -726,37 +738,30 @@ impl CliApi { .copied(), ) }) - .collect::>() - }; - - let chain_ctx = ctx.borrow_mut_chain_or_exit(); - let shielded = std::mem::take(&mut chain_ctx.shielded); - let ledger_address = - chain_ctx.get(&args.query.ledger_address); - let client = client.unwrap_or_else(|| { - C::from_tendermint_address(&ledger_address) - }); - client.wait_until_node_is_synced(&io).await?; - - let mut args = args.to_sdk(&mut ctx)?; - // FIXME: only if query_shielded_balance - let mut sync_args = std::mem::take( - &mut args.shielded_sync, - ) - .expect("Missing required shielded-sync arguments"); - sync_args.viewing_keys.extend(extra_sync_vks); - let shielded = crate::client::masp::syncing( - ShieldedContext::new(shielded), - client.clone(), - sync_args, - &io, - ) - .await?; + .collect::>(); + let mut sync_args = std::mem::take( + &mut args.shielded_sync, + ) + .expect("Missing required shielded-sync arguments"); + sync_args.viewing_keys.extend(extra_sync_vks); + let synced_shielded_ctx = + crate::client::masp::syncing( + ShieldedContext::new(shielded), + client.clone(), + sync_args, + &io, + ) + .await?; - let namada = ctx.to_sdk(client, io); - let namada = - namada.update_shielded_context(shielded).await; - rpc::query_balance(&namada, args).await; + let namada = ctx + .to_sdk(client, io) + .update_shielded_context(synced_shielded_ctx) + .await; + rpc::query_balance(&namada, args).await; + } else { + let namada = ctx.to_sdk(client, io); + rpc::query_balance(&namada, args).await; + } } Sub::QueryShieldingRewardsEstimate( QueryShieldingRewardsEstimate(args), From e62d203f20e4b00f217f32b4e81e0a36058c7f9f Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 3 Nov 2025 15:08:36 +0100 Subject: [PATCH 08/17] Removes `shielded-sync` and the speculative context test from e2e tests --- crates/tests/src/e2e/ibc_tests.rs | 20 -------------------- crates/tests/src/e2e/ledger_tests.rs | 25 ------------------------- crates/tests/src/e2e/setup.rs | 8 +++++++- 3 files changed, 7 insertions(+), 46 deletions(-) diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 2e5873b4dd6..07269af37d9 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -3430,8 +3430,6 @@ fn check_shielded_balance( ) -> Result<()> { let rpc = get_actor_rpc(test, Who::Validator(0)); - shielded_sync(test, owner.as_ref())?; - let query_args = vec![ "balance", "--owner", @@ -3482,8 +3480,6 @@ fn check_inflated_balance( test: &Test, viewing_key: impl AsRef, ) -> Result<()> { - shielded_sync(test, viewing_key.as_ref())?; - let rpc = get_actor_rpc(test, Who::Validator(0)); let query_args = vec![ "balance", @@ -3505,20 +3501,6 @@ fn check_inflated_balance( Ok(()) } -fn shielded_sync(test: &Test, viewing_key: impl AsRef) -> Result<()> { - let rpc = get_actor_rpc(test, Who::Validator(0)); - let tx_args = vec![ - "shielded-sync", - "--viewing-keys", - viewing_key.as_ref(), - "--node", - &rpc, - ]; - let mut client = run!(test, Bin::Client, tx_args, Some(120))?; - client.assert_success(); - Ok(()) -} - /// Get IBC shielding data for the following IBC transfer from the destination /// chain fn gen_ibc_shielding_data( @@ -4583,8 +4565,6 @@ fn osmosis_xcs() -> Result<()> { )? .assert_success(); - shielded_sync(&test_namada, AA_VIEWING_KEY)?; - let query_args = vec![ "balance", "--owner", diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index a2d3360e2ee..9cbe71e452c 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -2522,22 +2522,6 @@ fn masp_txs_and_queries() -> Result<()> { let rpc_address = get_actor_rpc(&test, who); wait_for_block_height(&test, &rpc_address, 1, 30)?; - // add necessary viewing keys to shielded context - let mut sync = run_as!( - test, - who, - Bin::Client, - vec![ - "shielded-sync", - "--viewing-keys", - AA_VIEWING_KEY, - AB_VIEWING_KEY, - "--node", - &rpc_address, - ], - Some(15), - )?; - sync.assert_success(); let txs_args = vec![ // 2. Shield 20 BTC from Albert to PA(A) ( @@ -2601,15 +2585,6 @@ fn masp_txs_and_queries() -> Result<()> { ]; for (tx_args, tx_result) in &txs_args { - // sync shielded context - let mut sync = run_as!( - test, - who, - Bin::Client, - vec!["shielded-sync", "--node", &rpc_address], - Some(15), - )?; - sync.assert_success(); for &dry_run in &[true, false] { if dry_run && is_use_device() { continue; diff --git a/crates/tests/src/e2e/setup.rs b/crates/tests/src/e2e/setup.rs index 13e7f347d90..5b200034036 100644 --- a/crates/tests/src/e2e/setup.rs +++ b/crates/tests/src/e2e/setup.rs @@ -1136,7 +1136,13 @@ where let is_shielded_sync = matches!(bin, Bin::Client) && args .peek() - .map(|fst_arg| fst_arg.as_ref() == "shielded-sync") + .map(|fst_arg| { + fst_arg.as_ref() == "transfer" + || fst_arg.as_ref() == "unshield" + || fst_arg.as_ref() == "ibc-transfer" + || fst_arg.as_ref() == "balance" + || fst_arg.as_ref() == "estimate-shielding-rewards" + }) .unwrap_or_default(); // Root cargo workspace manifest path From b2a37ae6f24ed8b2d13fdd420c33c23f5707db7a Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 3 Nov 2025 15:44:09 +0100 Subject: [PATCH 09/17] Adjusts wallet migrations --- crates/sdk/src/args.rs | 4 - crates/shielded_token/src/masp.rs | 18 +-- .../src/masp/shielded_wallet.rs | 2 +- .../src/masp/wallet_migrations.rs | 126 ++++++++++++++++-- 4 files changed, 117 insertions(+), 33 deletions(-) diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 5f68e474fbc..fbbc8a78299 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -343,9 +343,6 @@ pub struct TxShieldedTransfer { /// Path to the TX WASM code file pub tx_code_path: PathBuf, /// Optional data for the implicit shielded-sync - // FIXME: not sure about this, the sdk won't call shielded-sync implicitly - // so this arg is useless FIXME: maybe better to read a configuration - // file? pub shielded_sync: Option>, } @@ -2482,7 +2479,6 @@ impl TxReactivateValidator { pub struct ShieldedSync { /// Height to sync up to. Defaults to most recent pub last_query_height: Option, - // FIXME: this seems to never be used /// Spending keys used to determine note ownership pub spending_keys: Vec, /// Viewing keys used to determine note ownership diff --git a/crates/shielded_token/src/masp.rs b/crates/shielded_token/src/masp.rs index 5d657ea46fb..e956a7dbaca 100644 --- a/crates/shielded_token/src/masp.rs +++ b/crates/shielded_token/src/masp.rs @@ -304,22 +304,6 @@ pub type NoteIndex = BTreeMap; /// Maps the note index (in the commitment tree) to a witness pub type WitnessMap = HashMap>; -// FIXME: actually, we need this to deserialize the wallet from file, can we -// avoid deserializing that field so that we can remove this type? -#[derive(Copy, Clone, BorshSerialize, BorshDeserialize, Debug, Default)] -/// The possible sync states of the shielded context -/// WARNING: this is deprecated in favor of explicit calls to shielded-sync -/// before MASP operations. Users of the SDK should do the same or construct a -/// local cache akin to the speculative context on a use-case basis. -pub enum ContextSyncStatus { - /// The context contains data that has been confirmed by the protocol - #[default] - Confirmed, - /// The context possibly contains data that has not yet been confirmed by - /// the protocol and could be incomplete or invalid - Speculative, -} - #[cfg(test)] mod tests { use masp_primitives::ff::Field; @@ -1250,7 +1234,7 @@ pub mod fs { ..Default::default() }; BorshSerialize::serialize( - &VersionedWalletRef::V2(&shielded), + &VersionedWalletRef::V3(&shielded), &mut bytes, ) .expect("Test failed"); diff --git a/crates/shielded_token/src/masp/shielded_wallet.rs b/crates/shielded_token/src/masp/shielded_wallet.rs index e2578095e24..af111f06d4d 100644 --- a/crates/shielded_token/src/masp/shielded_wallet.rs +++ b/crates/shielded_token/src/masp/shielded_wallet.rs @@ -356,7 +356,7 @@ impl ShieldedWallet { /// Save this shielded context into its associated context directory. pub async fn save(&self) -> std::io::Result<()> { - self.utils.save(VersionedWalletRef::V2(self)).await + self.utils.save(VersionedWalletRef::V3(self)).await } /// Update the merkle tree of witnesses the first time we diff --git a/crates/shielded_token/src/masp/wallet_migrations.rs b/crates/shielded_token/src/masp/wallet_migrations.rs index f69dd080ebc..424c4376237 100644 --- a/crates/shielded_token/src/masp/wallet_migrations.rs +++ b/crates/shielded_token/src/masp/wallet_migrations.rs @@ -17,9 +17,9 @@ pub enum VersionedWallet { /// Version 1 V1(v1::ShieldedWallet), /// Version 2 - V2(ShieldedWallet), - // FIXME: need a new version without the sync state, also mention what the - // change was + V2(v2::ShieldedWallet), + /// Version 3 + V3(ShieldedWallet), } impl VersionedWallet { @@ -29,7 +29,8 @@ impl VersionedWallet { match self { VersionedWallet::V0(w) => Ok(w.into()), VersionedWallet::V1(w) => Ok(w.into()), - VersionedWallet::V2(w) => Ok(w), + VersionedWallet::V2(w) => Ok(w.into()), + VersionedWallet::V3(w) => Ok(w), } } } @@ -42,10 +43,13 @@ pub enum VersionedWalletRef<'w, U: ShieldedUtils> { /// Version 1 V1(&'w v1::ShieldedWallet), /// Version 2 - V2(&'w ShieldedWallet), + V2(&'w v2::ShieldedWallet), + /// Version 3 + V3(&'w ShieldedWallet), } mod migrations { + use borsh::{BorshDeserialize, BorshSerialize}; use masp_primitives::merkle_tree::CommitmentTree; use masp_primitives::sapling::{Diversifier, Node, Note}; use namada_core::collections::HashMap; @@ -164,6 +168,17 @@ mod migrations { ); } } + + #[derive(Copy, Clone, BorshSerialize, BorshDeserialize, Debug, Default)] + /// The possible sync states of the shielded context + pub enum ContextSyncStatus { + /// The context contains data that has been confirmed by the protocol + #[default] + Confirmed, + /// The context possibly contains data that has not yet been confirmed + /// by the protocol and could be incomplete or invalid + Speculative, + } } pub mod v0 { @@ -181,10 +196,9 @@ pub mod v0 { use namada_core::collections::{HashMap, HashSet}; use namada_core::masp::AssetData; + use super::migrations::ContextSyncStatus; use crate::masp::utils::MaspIndexedTx; - use crate::masp::{ - ContextSyncStatus, NoteIndex, ShieldedUtils, WitnessMap, - }; + use crate::masp::{NoteIndex, ShieldedUtils, WitnessMap}; #[derive(BorshSerialize, BorshDeserialize, Debug)] #[allow(missing_docs)] @@ -334,11 +348,10 @@ pub mod v1 { use namada_core::collections::{HashMap, HashSet}; use namada_core::masp::AssetData; + use super::migrations::ContextSyncStatus; use crate::masp::shielded_wallet::EpochedConversions; use crate::masp::utils::MaspIndexedTx; - use crate::masp::{ - ContextSyncStatus, NoteIndex, ShieldedUtils, WitnessMap, - }; + use crate::masp::{NoteIndex, ShieldedUtils, WitnessMap}; #[derive(BorshSerialize, BorshDeserialize, Debug)] pub struct ShieldedWallet { @@ -455,3 +468,94 @@ pub mod v1 { } } } + +pub mod v2 { + //! Version 2 of the shielded wallet, which is used for migration purposes. + + #![allow(missing_docs)] + + use std::collections::{BTreeMap, BTreeSet}; + + use masp_primitives::asset_type::AssetType; + use masp_primitives::memo::MemoBytes; + use masp_primitives::sapling::{Nullifier, ViewingKey}; + use namada_core::borsh::{BorshDeserialize, BorshSerialize}; + use namada_core::collections::{HashMap, HashSet}; + use namada_core::masp::AssetData; + use namada_state::BlockHeight; + #[cfg(feature = "historic")] + use namada_tx::IndexedTx; + + use super::migrations::ContextSyncStatus; + use crate::masp::bridge_tree::BridgeTree; + #[cfg(feature = "historic")] + use crate::masp::shielded_wallet::TxHistoryEntry; + use crate::masp::shielded_wallet::{CompactNote, EpochedConversions}; + use crate::masp::{NoteIndex, NotePosition, ShieldedUtils}; + + #[derive(BorshSerialize, BorshDeserialize, Debug)] + pub struct ShieldedWallet { + #[borsh(skip)] + pub utils: U, + pub tree: BridgeTree, + pub synced_height: BlockHeight, + pub spents: HashSet, + pub pos_map: HashMap>, + pub nf_map: HashMap, + pub note_map: HashMap, + pub memo_map: HashMap, + pub asset_types: HashMap, + pub conversions: EpochedConversions, + pub note_index: NoteIndex, + #[cfg(feature = "historic")] + pub vk_map: HashMap, + #[cfg(feature = "historic")] + pub history: HashMap>, + pub sync_status: ContextSyncStatus, + } + + impl Default for ShieldedWallet { + fn default() -> ShieldedWallet { + ShieldedWallet:: { + utils: U::default(), + tree: BridgeTree::empty(), + synced_height: Default::default(), + spents: HashSet::default(), + pos_map: HashMap::default(), + nf_map: HashMap::default(), + note_map: HashMap::default(), + memo_map: HashMap::default(), + asset_types: HashMap::default(), + conversions: Default::default(), + note_index: BTreeMap::default(), + #[cfg(feature = "historic")] + vk_map: HashMap::default(), + #[cfg(feature = "historic")] + history: HashMap::default(), + sync_status: Default::default(), + } + } + } + + impl From> for super::ShieldedWallet { + fn from(wallet: ShieldedWallet) -> Self { + Self { + utils: wallet.utils, + tree: wallet.tree, + synced_height: wallet.synced_height, + pos_map: wallet.pos_map, + nf_map: wallet.nf_map, + note_map: wallet.note_map, + memo_map: wallet.memo_map, + spents: wallet.spents, + asset_types: wallet.asset_types, + conversions: wallet.conversions, + note_index: wallet.note_index, + #[cfg(feature = "historic")] + vk_map: wallet.vk_map, + #[cfg(feature = "historic")] + history: wallet.history, + } + } + } +} From da336710ef07353c1324f9acd44981484681a387 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 4 Nov 2025 15:47:21 +0100 Subject: [PATCH 10/17] Stretches expiration time in masp integration tests --- crates/tests/src/integration/masp.rs | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 2e9f3d7f7d4..990c773c0cf 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -3964,6 +3964,12 @@ fn masp_tx_expiration_first_invalid_block_height() -> Result<()> { let tempdir = tempfile::tempdir().unwrap(); _ = node.next_epoch(); + // Expiration a few seconds in the future since we'll need to run + // shielded-sync before building the tx and we must ensure we don't + // overshoot the expiration time in the process + #[allow(clippy::disallowed_methods)] + let expiration = + DateTimeUtc::now().next_second().next_second().next_second(); let captured = CapturedOutput::of(|| { run( &node, @@ -3990,8 +3996,7 @@ fn masp_tx_expiration_first_invalid_block_height() -> Result<()> { // to overwrite the header with one having no // expiration "--expiration", - #[allow(clippy::disallowed_methods)] - &DateTimeUtc::now().to_string(), + &expiration.to_string(), "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-tx", @@ -4143,6 +4148,12 @@ fn masp_tx_expiration_first_invalid_block_height_with_fee_payment() -> Result<() let tempdir = tempfile::tempdir().unwrap(); _ = node.next_epoch(); + // Expiration a few seconds in the future since we'll need to run + // shielded-sync before building the tx and we must ensure we don't + // overshoot the expiration time in the process + #[allow(clippy::disallowed_methods)] + let expiration = + DateTimeUtc::now().next_second().next_second().next_second(); let captured = CapturedOutput::of(|| { run( &node, @@ -4171,8 +4182,7 @@ fn masp_tx_expiration_first_invalid_block_height_with_fee_payment() -> Result<() // to overwrite the header with one having no // expiration "--expiration", - #[allow(clippy::disallowed_methods)] - &DateTimeUtc::now().to_string(), + &expiration.to_string(), "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-tx", @@ -4302,6 +4312,12 @@ fn masp_tx_expiration_last_valid_block_height() -> Result<()> { let tempdir = tempfile::tempdir().unwrap(); _ = node.next_epoch(); + // Expiration a few seconds in the future since we'll need to run + // shielded-sync before building the tx and we must ensure we don't + // overshoot the expiration time in the process + #[allow(clippy::disallowed_methods)] + let expiration = + DateTimeUtc::now().next_second().next_second().next_second(); let captured = CapturedOutput::of(|| { run( &node, @@ -4316,6 +4332,7 @@ fn masp_tx_expiration_last_valid_block_height() -> Result<()> { NAM, "--amount", "50", + // FIXME: can avoid gas payer since dumping? "--gas-payer", cooper_alias, // We want to create an expired masp tx. Doing so will also set @@ -4328,8 +4345,7 @@ fn masp_tx_expiration_last_valid_block_height() -> Result<()> { // to overwrite the header with one having no // expiration "--expiration", - #[allow(clippy::disallowed_methods)] - &DateTimeUtc::now().to_string(), + &expiration.to_string(), "--output-folder-path", tempdir.path().to_str().unwrap(), "--dump-tx", From 0fc256f065bc6e455a8df48de49fb211aa36bead Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 4 Nov 2025 15:56:10 +0100 Subject: [PATCH 11/17] Removes useless gas payer in integration tests --- crates/tests/src/integration/masp.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 990c773c0cf..c767f98469a 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -3936,7 +3936,7 @@ fn masp_tx_expiration_first_invalid_block_height() -> Result<()> { _ = node.next_epoch(); // Initialize accounts we can access the secret keys of - let (cooper_alias, cooper_key) = + let (_, cooper_key) = make_temp_account(&node, validator_one_rpc, "Cooper", NAM, 500_000)?; // 1. Shield tokens @@ -3984,8 +3984,6 @@ fn masp_tx_expiration_first_invalid_block_height() -> Result<()> { NAM, "--amount", "50", - "--gas-payer", - cooper_alias, // We want to create an expired masp tx. Doing so will also set // the expiration field of the header which can // be a problem because this would lead to the @@ -4120,7 +4118,7 @@ fn masp_tx_expiration_first_invalid_block_height_with_fee_payment() -> Result<() // Initialize account we can access the secret keys of. The account must // have no balance to be used as a disposable gas payer - let (cooper_alias, cooper_key) = + let (_, cooper_key) = make_temp_account(&node, validator_one_rpc, "Cooper", NAM, 0)?; // 1. Shield tokens @@ -4168,10 +4166,6 @@ fn masp_tx_expiration_first_invalid_block_height_with_fee_payment() -> Result<() NAM, "--amount", "50", - // This gas payer has no funds so we are going to use it as a - // disposable gas payer via the MASP - "--gas-payer", - cooper_alias, // We want to create an expired masp tx. Doing so will also set // the expiration field of the header which can // be a problem because this would lead to the @@ -4284,7 +4278,7 @@ fn masp_tx_expiration_last_valid_block_height() -> Result<()> { _ = node.next_epoch(); // Initialize accounts we can access the secret keys of - let (cooper_alias, cooper_key) = + let (_, cooper_key) = make_temp_account(&node, validator_one_rpc, "Cooper", NAM, 500_000)?; // 1. Shield tokens @@ -4332,9 +4326,6 @@ fn masp_tx_expiration_last_valid_block_height() -> Result<()> { NAM, "--amount", "50", - // FIXME: can avoid gas payer since dumping? - "--gas-payer", - cooper_alias, // We want to create an expired masp tx. Doing so will also set // the expiration field of the header which can // be a problem because this would lead to the From 47bdd82a589cf313374896647468da5903fb31f3 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 4 Nov 2025 16:52:28 +0100 Subject: [PATCH 12/17] Improves error message for masp expiration --- crates/shielded_token/src/masp/shielded_wallet.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/shielded_token/src/masp/shielded_wallet.rs b/crates/shielded_token/src/masp/shielded_wallet.rs index af111f06d4d..0684ee1d057 100644 --- a/crates/shielded_token/src/masp/shielded_wallet.rs +++ b/crates/shielded_token/src/masp/shielded_wallet.rs @@ -1634,7 +1634,13 @@ pub trait ShieldedApi: delta_time.num_seconds() / i64::try_from(max_block_time.0).unwrap(), ) - .map_err(|e| TransferErr::General(e.to_string()))?; + .map_err(|e| { + TransferErr::General(format!( + "Failed to compute the amount of blocks for the tx \ + expiration, likely due to the provided expiration \ + time being in the past: {e}" + )) + })?; match checked!(last_block_height + delta_blocks) { Ok(height) if height <= u32::MAX - 20 => height, _ => { From fac08f3980f74cd32a7c56a2161de5ec2c8ec412 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 10 Nov 2025 17:42:24 +0100 Subject: [PATCH 13/17] Shielded-sync for osmosis swaps --- crates/apps_lib/src/cli/api.rs | 1 + crates/apps_lib/src/cli/client.rs | 49 +++++++++++++++++++++++--- crates/node/src/shell/testing/utils.rs | 1 + 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/crates/apps_lib/src/cli/api.rs b/crates/apps_lib/src/cli/api.rs index d9737b5174d..c7822c12d26 100644 --- a/crates/apps_lib/src/cli/api.rs +++ b/crates/apps_lib/src/cli/api.rs @@ -33,6 +33,7 @@ impl CliClient for HttpClient { } } +#[derive(Copy, Clone)] pub struct CliIo; #[async_trait::async_trait(?Send)] diff --git a/crates/apps_lib/src/cli/client.rs b/crates/apps_lib/src/cli/client.rs index bf59bf63f04..e404a5b8c2d 100644 --- a/crates/apps_lib/src/cli/client.rs +++ b/crates/apps_lib/src/cli/client.rs @@ -13,7 +13,7 @@ use crate::cli::cmds::*; use crate::client::{rpc, tx, utils}; impl CliApi { - pub async fn handle_client_command( + pub async fn handle_client_command( client: Option, cmd: cli::NamadaClient, io: IO, @@ -231,9 +231,23 @@ impl CliApi { }); client.wait_until_node_is_synced(&io).await?; + // Pre-extract data for the optional shielded-sync + let extra_sync_vks = chain_ctx + .wallet + .get_viewing_keys() + .into_iter() + .map(|(k, v)| { + DatedViewingKey::new( + v, + chain_ctx.wallet.find_birthday(k).copied(), + ) + }) + .collect::>(); + let shielded = std::mem::take(&mut chain_ctx.shielded); + let args = args.to_sdk(&mut ctx)?; - let namada = ctx.to_sdk(client, io); - let args = args + let namada = ctx.to_sdk(client.clone(), io.clone()); + let mut args = args .into_ibc_transfer( &namada, |_route, min_amount, quote_amount| { @@ -276,7 +290,34 @@ impl CliApi { ) .await?; - tx::submit_ibc_transfer(&namada, args).await?; + if args.source.spending_key().is_some() { + // Sync the shielded context + let mut sync_args = std::mem::take( + &mut args.shielded_sync, + ) + .expect("Missing required shielded-sync arguments"); + sync_args.viewing_keys.extend(extra_sync_vks); + let synced_shielded_ctx = + crate::client::masp::syncing( + ShieldedContext::new(shielded), + client.clone(), + sync_args, + &io, + ) + .await?; + + let namada = namada + .update_shielded_context(synced_shielded_ctx) + .await; + tx::submit_ibc_transfer(&namada, args).await? + } else { + let namada = namada + .update_shielded_context(ShieldedContext::new( + shielded, + )) + .await; + tx::submit_ibc_transfer(&namada, args).await? + } } Sub::TxUpdateAccount(TxUpdateAccount(args)) => { let chain_ctx = ctx.borrow_mut_chain_or_exit(); diff --git a/crates/node/src/shell/testing/utils.rs b/crates/node/src/shell/testing/utils.rs index fcbe77bb1d0..9e786d0fe36 100644 --- a/crates/node/src/shell/testing/utils.rs +++ b/crates/node/src/shell/testing/utils.rs @@ -71,6 +71,7 @@ lazy_static! { AtomicBuffer(std::sync::Arc::new(std::sync::Mutex::new(vec![]))); } +#[derive(Copy, Clone)] pub struct TestingIo; #[async_trait::async_trait(?Send)] From 3c2cc0d4011888c004c622c4c3bb19d454514cdd Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 10 Nov 2025 17:43:02 +0100 Subject: [PATCH 14/17] Adds missing osmosis swap to the command list in e2e setup --- crates/tests/src/e2e/setup.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/tests/src/e2e/setup.rs b/crates/tests/src/e2e/setup.rs index 5b200034036..8bc69e835e9 100644 --- a/crates/tests/src/e2e/setup.rs +++ b/crates/tests/src/e2e/setup.rs @@ -1142,6 +1142,7 @@ where || fst_arg.as_ref() == "ibc-transfer" || fst_arg.as_ref() == "balance" || fst_arg.as_ref() == "estimate-shielding-rewards" + || fst_arg.as_ref() == "osmosis-swap" }) .unwrap_or_default(); From e7bc2b193acb5f90d2b46eb37e15cf0308c25dda Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 11 Nov 2025 16:37:26 +0100 Subject: [PATCH 15/17] Increases timout of shielded osmosis swap in test --- crates/apps_lib/src/cli.rs | 1 - crates/tests/src/e2e/ibc_tests.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index a5afbc61546..e24ba5c310e 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -1912,7 +1912,6 @@ pub mod cmds { } fn def() -> App { - // FIXME: remove the cli shielded-sync command App::new(Self::CMD) .about(wrap!( "Estimate the amount of MASP rewards for the \ diff --git a/crates/tests/src/e2e/ibc_tests.rs b/crates/tests/src/e2e/ibc_tests.rs index 07269af37d9..16c436262ae 100644 --- a/crates/tests/src/e2e/ibc_tests.rs +++ b/crates/tests/src/e2e/ibc_tests.rs @@ -4694,7 +4694,7 @@ fn osmosis_xcs() -> Result<()> { "--gas-limit", "500000", ], - Some(40), + Some(120), )?; // confirm trade From e5bc7a2f68a28ec844b9db2fa0eb65083ac45d90 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Mon, 3 Nov 2025 16:29:40 +0100 Subject: [PATCH 16/17] Changelog #4074 --- .../unreleased/improvements/4074-delete-speculative-context.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/unreleased/improvements/4074-delete-speculative-context.md diff --git a/.changelog/unreleased/improvements/4074-delete-speculative-context.md b/.changelog/unreleased/improvements/4074-delete-speculative-context.md new file mode 100644 index 00000000000..74bb35a403a --- /dev/null +++ b/.changelog/unreleased/improvements/4074-delete-speculative-context.md @@ -0,0 +1,3 @@ +- Removed the shielded speculative context and the cli command for `shielded- + sync`: this call is now embedded directly in the cli commands that need it. + ([\#4074](https://github.com/namada-net/namada/issues/4074)) \ No newline at end of file From d72523dc85bdb5f1c44cf2e660b29483d8027d6e Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Sat, 6 Dec 2025 15:04:32 +0100 Subject: [PATCH 17/17] Implicit shielded-sync as child process --- crates/apps_lib/src/cli.rs | 64 +++++++ crates/apps_lib/src/cli/client.rs | 306 +++++++++--------------------- crates/sdk/src/lib.rs | 25 --- crates/sdk/src/signing.rs | 11 -- 4 files changed, 154 insertions(+), 252 deletions(-) diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index e24ba5c310e..0334ee9a2fc 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -31,6 +31,8 @@ const WALLET_CMD: &str = "wallet"; const RELAYER_CMD: &str = "relayer"; pub mod cmds { + use std::fmt::Display; + use super::args::CliTypes; use super::utils::*; use super::{ @@ -1632,6 +1634,68 @@ pub mod cmds { } } + impl Display for ShieldedSync { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + use crate::cli::args; + + write!(f, "{} ", Self::CMD)?; + + let args::ShieldedSync { + last_query_height, + spending_keys, + viewing_keys, + with_indexer, + wait_for_last_query_height, + max_concurrent_fetches, + block_batch_size, + retry_strategy, + } = &self.0; + + if let Some(lqh) = last_query_height { + write!(f, "--{} {} ", args::BLOCK_HEIGHT_TO_OPT.name, lqh)?; + } + + if !spending_keys.is_empty() { + write!(f, "--{} ", args::DATED_SPENDING_KEYS.name)?; + + for key in spending_keys { + write!(f, "{} ", key.raw)?; + } + } + + if !viewing_keys.is_empty() { + write!(f, "--{} ", args::DATED_VIEWING_KEYS.name)?; + + for key in viewing_keys { + write!(f, "{} ", key.raw)?; + } + } + + if let Some(indexer) = with_indexer { + write!(f, "--{} {} ", args::WITH_INDEXER.name, indexer)?; + } + + if *wait_for_last_query_height { + write!(f, "--{} ", args::WAIT_FOR_LAST_QUERY_HEIGHT.name)?; + } + + write!( + f, + "--{} {} ", + args::MAX_CONCURRENT_FETCHES.name, + max_concurrent_fetches + )?; + write!(f, "--{} {} ", args::BLOCK_BATCH.name, block_batch_size)?; + if let namada_sdk::masp::utils::RetryStrategy::Times(retries) = + retry_strategy + { + write!(f, "--{} {} ", args::RETRIES.name, retries)?; + } + + Ok(()) + } + } + #[derive(Clone, Debug)] pub struct Bond(pub args::Bond); diff --git a/crates/apps_lib/src/cli/client.rs b/crates/apps_lib/src/cli/client.rs index e404a5b8c2d..56a5fbb80e4 100644 --- a/crates/apps_lib/src/cli/client.rs +++ b/crates/apps_lib/src/cli/client.rs @@ -3,9 +3,10 @@ use std::io::Read; use color_eyre::eyre::Result; use namada_sdk::io::{Io, NamadaIo, display_line}; use namada_sdk::masp::ShieldedContext; -use namada_sdk::{Namada, NamadaImpl}; +use namada_sdk::{Namada, NamadaImpl, ShieldedUtils, ShieldedWallet}; use namada_wallet::DatedViewingKey; +use super::args::CliTypes; use crate::cli; use crate::cli::api::{CliApi, CliClient}; use crate::cli::args::CliToSdk; @@ -64,26 +65,8 @@ impl CliApi { let namada = ctx.to_sdk(client, io); tx::submit_transparent_transfer(&namada, args).await?; } - Sub::TxShieldedTransfer(TxShieldedTransfer(args)) => { - let extra_sync_vks = { - let chain_ctx = ctx.borrow_chain_or_exit(); - chain_ctx - .wallet - .get_viewing_keys() - .into_iter() - .map(|(k, v)| { - DatedViewingKey::new( - v, - chain_ctx - .wallet - .find_birthday(k) - .copied(), - ) - }) - .collect::>() - }; + Sub::TxShieldedTransfer(TxShieldedTransfer(mut args)) => { let chain_ctx = ctx.borrow_mut_chain_or_exit(); - let shielded = std::mem::take(&mut chain_ctx.shielded); let ledger_address = chain_ctx.get(&args.tx.ledger_address); let client = client.unwrap_or_else(|| { @@ -91,23 +74,17 @@ impl CliApi { }); client.wait_until_node_is_synced(&io).await?; - let mut args = args.to_sdk(&mut ctx)?; - let mut sync_args = std::mem::take( - &mut args.shielded_sync, - ) - .expect("Missing required shielded-sync arguments"); - sync_args.viewing_keys.extend(extra_sync_vks); - let shielded = crate::client::masp::syncing( - ShieldedContext::new(shielded), - client.clone(), + // Implicit shielded-sync child command + let sync_args = std::mem::take(&mut args.shielded_sync) + .expect("Missing required shielded-sync arguments"); + Self::spawn_child_shielded_sync( + &mut chain_ctx.shielded, sync_args, - &io, ) .await?; + let args = args.to_sdk(&mut ctx)?; let namada = ctx.to_sdk(client, io); - let namada = - namada.update_shielded_context(shielded).await; tx::submit_shielded_transfer(&namada, args).await?; } Sub::TxShieldingTransfer(TxShieldingTransfer(args)) => { @@ -122,26 +99,10 @@ impl CliApi { let namada = ctx.to_sdk(client, io); tx::submit_shielding_transfer(&namada, args).await?; } - Sub::TxUnshieldingTransfer(TxUnshieldingTransfer(args)) => { - let extra_sync_vks = { - let chain_ctx = ctx.borrow_chain_or_exit(); - chain_ctx - .wallet - .get_viewing_keys() - .into_iter() - .map(|(k, v)| { - DatedViewingKey::new( - v, - chain_ctx - .wallet - .find_birthday(k) - .copied(), - ) - }) - .collect::>() - }; + Sub::TxUnshieldingTransfer(TxUnshieldingTransfer( + mut args, + )) => { let chain_ctx = ctx.borrow_mut_chain_or_exit(); - let shielded = std::mem::take(&mut chain_ctx.shielded); let ledger_address = chain_ctx.get(&args.tx.ledger_address); let client = client.unwrap_or_else(|| { @@ -149,29 +110,22 @@ impl CliApi { }); client.wait_until_node_is_synced(&io).await?; - let mut args = args.to_sdk(&mut ctx)?; - let mut sync_args = std::mem::take( - &mut args.shielded_sync, - ) - .expect("Missing required shielded-sync arguments"); - sync_args.viewing_keys.extend(extra_sync_vks); - let shielded = crate::client::masp::syncing( - ShieldedContext::new(shielded), - client.clone(), + // Implicit shielded-sync child command + let sync_args = std::mem::take(&mut args.shielded_sync) + .expect("Missing required shielded-sync arguments"); + Self::spawn_child_shielded_sync( + &mut chain_ctx.shielded, sync_args, - &io, ) .await?; + let args = args.to_sdk(&mut ctx)?; let namada = ctx.to_sdk(client, io); - let namada = - namada.update_shielded_context(shielded).await; tx::submit_unshielding_transfer(&namada, args).await?; } Sub::TxIbcTransfer(args) => { - let TxIbcTransfer(args) = *args; - let chain_ctx = ctx.borrow_mut_chain_or_exit(); - let shielded = std::mem::take(&mut chain_ctx.shielded); + let TxIbcTransfer(mut args) = *args; + let chain_ctx = ctx.borrow_chain_or_exit(); let ledger_address = chain_ctx.get(&args.tx.ledger_address); let client = client.unwrap_or_else(|| { @@ -179,51 +133,26 @@ impl CliApi { }); client.wait_until_node_is_synced(&io).await?; - let mut args = args.to_sdk(&mut ctx)?; - if args.source.spending_key().is_some() { - // Sync the shielded context - let chain_ctx = ctx.borrow_chain_or_exit(); - let extra_sync_vks = chain_ctx - .wallet - .get_viewing_keys() - .into_iter() - .map(|(k, v)| { - DatedViewingKey::new( - v, - chain_ctx - .wallet - .find_birthday(k) - .copied(), - ) - }) - .collect::>(); - let mut sync_args = std::mem::take( + let sdk_args = args.clone().to_sdk(&mut ctx)?; + if sdk_args.source.spending_key().is_some() { + // Implicit shielded-sync child command + let sync_args = std::mem::take( &mut args.shielded_sync, ) .expect("Missing required shielded-sync arguments"); - sync_args.viewing_keys.extend(extra_sync_vks); - let synced_shielded_ctx = - crate::client::masp::syncing( - ShieldedContext::new(shielded), - client.clone(), - sync_args, - &io, - ) - .await?; - - let namada = ctx - .to_sdk(client, io) - .update_shielded_context(synced_shielded_ctx) - .await; - tx::submit_ibc_transfer(&namada, args).await? - } else { - let namada = ctx.to_sdk(client, io); - tx::submit_ibc_transfer(&namada, args).await? + Self::spawn_child_shielded_sync( + &mut ctx.borrow_mut_chain_or_exit().shielded, + sync_args, + ) + .await?; } + + let namada = ctx.to_sdk(client, io); + tx::submit_ibc_transfer(&namada, sdk_args).await? } Sub::TxOsmosisSwap(args) => { - let TxOsmosisSwap(args) = *args; - let chain_ctx = ctx.borrow_mut_chain_or_exit(); + let TxOsmosisSwap(mut args) = *args; + let chain_ctx = ctx.borrow_chain_or_exit(); let ledger_address = chain_ctx.get(&args.transfer.tx.ledger_address); let client = client.unwrap_or_else(|| { @@ -231,23 +160,9 @@ impl CliApi { }); client.wait_until_node_is_synced(&io).await?; - // Pre-extract data for the optional shielded-sync - let extra_sync_vks = chain_ctx - .wallet - .get_viewing_keys() - .into_iter() - .map(|(k, v)| { - DatedViewingKey::new( - v, - chain_ctx.wallet.find_birthday(k).copied(), - ) - }) - .collect::>(); - let shielded = std::mem::take(&mut chain_ctx.shielded); - - let args = args.to_sdk(&mut ctx)?; + let sdk_osmosis_args = args.clone().to_sdk(&mut ctx)?; let namada = ctx.to_sdk(client.clone(), io.clone()); - let mut args = args + let sdk_args = sdk_osmosis_args .into_ibc_transfer( &namada, |_route, min_amount, quote_amount| { @@ -290,34 +205,19 @@ impl CliApi { ) .await?; - if args.source.spending_key().is_some() { - // Sync the shielded context - let mut sync_args = std::mem::take( - &mut args.shielded_sync, + if sdk_args.source.spending_key().is_some() { + // Implicit shielded-sync child command + let sync_args = std::mem::take( + &mut args.transfer.shielded_sync, ) .expect("Missing required shielded-sync arguments"); - sync_args.viewing_keys.extend(extra_sync_vks); - let synced_shielded_ctx = - crate::client::masp::syncing( - ShieldedContext::new(shielded), - client.clone(), - sync_args, - &io, - ) - .await?; - - let namada = namada - .update_shielded_context(synced_shielded_ctx) - .await; - tx::submit_ibc_transfer(&namada, args).await? - } else { - let namada = namada - .update_shielded_context(ShieldedContext::new( - shielded, - )) - .await; - tx::submit_ibc_transfer(&namada, args).await? + Self::spawn_child_shielded_sync( + &mut **namada.shielded_mut().await, + sync_args, + ) + .await?; } + tx::submit_ibc_transfer(&namada, sdk_args).await? } Sub::TxUpdateAccount(TxUpdateAccount(args)) => { let chain_ctx = ctx.borrow_mut_chain_or_exit(); @@ -752,9 +652,8 @@ impl CliApi { let namada = ctx.to_sdk(client, io); rpc::query_block(&namada).await; } - Sub::QueryBalance(QueryBalance(args)) => { + Sub::QueryBalance(QueryBalance(mut args)) => { let chain_ctx = ctx.borrow_mut_chain_or_exit(); - let shielded = std::mem::take(&mut chain_ctx.shielded); let ledger_address = chain_ctx.get(&args.query.ledger_address); let client = client.unwrap_or_else(|| { @@ -762,70 +661,26 @@ impl CliApi { }); client.wait_until_node_is_synced(&io).await?; - let mut args = args.to_sdk(&mut ctx)?; - if args.owner.full_viewing_key().is_some() { - // Sync the shielded context - let chain_ctx = ctx.borrow_chain_or_exit(); - let extra_sync_vks = chain_ctx - .wallet - .get_viewing_keys() - .into_iter() - .map(|(k, v)| { - DatedViewingKey::new( - v, - chain_ctx - .wallet - .find_birthday(k) - .copied(), - ) - }) - .collect::>(); - let mut sync_args = std::mem::take( + let sdk_args = args.clone().to_sdk(&mut ctx)?; + if sdk_args.owner.full_viewing_key().is_some() { + // Implicit shielded-sync child command + let sync_args = std::mem::take( &mut args.shielded_sync, ) .expect("Missing required shielded-sync arguments"); - sync_args.viewing_keys.extend(extra_sync_vks); - let synced_shielded_ctx = - crate::client::masp::syncing( - ShieldedContext::new(shielded), - client.clone(), - sync_args, - &io, - ) - .await?; - - let namada = ctx - .to_sdk(client, io) - .update_shielded_context(synced_shielded_ctx) - .await; - rpc::query_balance(&namada, args).await; - } else { - let namada = ctx.to_sdk(client, io); - rpc::query_balance(&namada, args).await; + Self::spawn_child_shielded_sync( + &mut ctx.borrow_mut_chain_or_exit().shielded, + sync_args, + ) + .await?; } + let namada = ctx.to_sdk(client, io); + rpc::query_balance(&namada, sdk_args).await; } Sub::QueryShieldingRewardsEstimate( - QueryShieldingRewardsEstimate(args), + QueryShieldingRewardsEstimate(mut args), ) => { - let extra_sync_vks = { - let chain_ctx = ctx.borrow_chain_or_exit(); - chain_ctx - .wallet - .get_viewing_keys() - .into_iter() - .map(|(k, v)| { - DatedViewingKey::new( - v, - chain_ctx - .wallet - .find_birthday(k) - .copied(), - ) - }) - .collect::>() - }; let chain_ctx = ctx.borrow_mut_chain_or_exit(); - let shielded = std::mem::take(&mut chain_ctx.shielded); let ledger_address = chain_ctx.get(&args.query.ledger_address); let client = client.unwrap_or_else(|| { @@ -833,23 +688,17 @@ impl CliApi { }); client.wait_until_node_is_synced(&io).await?; - let mut args = args.to_sdk(&mut ctx)?; - let mut sync_args = std::mem::take( - &mut args.shielded_sync, - ) - .expect("Missing required shielded-sync arguments"); - sync_args.viewing_keys.extend(extra_sync_vks); - let shielded = crate::client::masp::syncing( - ShieldedContext::new(shielded), - client.clone(), + // Implicit shielded-sync child command + let sync_args = std::mem::take(&mut args.shielded_sync) + .expect("Missing required shielded-sync arguments"); + Self::spawn_child_shielded_sync( + &mut ctx.borrow_mut_chain_or_exit().shielded, sync_args, - &io, ) .await?; + let args = args.to_sdk(&mut ctx)?; let namada = ctx.to_sdk(client, io); - let namada = - namada.update_shielded_context(shielded).await; rpc::query_rewards_estimate(&namada, args).await; } Sub::QueryBonds(QueryBonds(args)) => { @@ -1197,4 +1046,29 @@ impl CliApi { } Ok(()) } + + // Spawn the implicit shielded-sync command required by some commands into a + // child process to isolate the custom signal handler it installs + async fn spawn_child_shielded_sync( + wallet: &mut ShieldedWallet, + args: crate::cli::args::ShieldedSync, + ) -> Result<()> { + let exec_path = std::env::current_exe()?; + let mut sync_child = tokio::process::Command::new(exec_path) + .arg(ShieldedSync(args).to_string()) + .spawn() + .expect("Failed to spawn shielded-sync child process"); + let sync_res = sync_child.wait().await?; + if sync_res.success() { + // Reload the shielded context from this process if the sync was + // successful + wallet.load().await; + Ok(()) + } else { + Err(eyre::eyre!( + "shielded-sync command failed with: {}", + sync_res + )) + } + } } diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index 68041b7ebdd..1902a527864 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -117,14 +117,6 @@ pub trait Namada: NamadaIo { &self, ) -> RwLockWriteGuard<'_, ShieldedContext>; - /// Update the shielded context with the one provided - async fn update_shielded_context( - self, - context: ShieldedContext, - ) -> impl Namada - where - U: ShieldedUtils + MaybeSend + MaybeSync; - /// Return the native token fn native_token(&self) -> Address; @@ -821,23 +813,6 @@ where self.shielded.write().await } - async fn update_shielded_context( - self, - context: ShieldedContext, - ) -> impl Namada - where - T: ShieldedUtils + MaybeSend + MaybeSync, - { - NamadaImpl:: { - client: self.client, - wallet: self.wallet, - shielded: RwLock::new(context), - io: self.io, - native_token: self.native_token, - prototype: self.prototype, - } - } - fn native_token(&self) -> Address { self.native_token.clone() } diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index 5006c5caeef..f8ca8722fb4 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -2402,7 +2402,6 @@ mod test_signing { use namada_core::token::{Denomination, MaspDigitPos}; use namada_governance::storage::proposal::PGFInternalTarget; use namada_io::client::EncodedResponseQuery; - use namada_token::masp::ShieldedUtils; use namada_tx::{Code, Data}; use namada_wallet::test_utils::TestWalletUtils; use tendermint_rpc::SimpleRequest; @@ -2560,16 +2559,6 @@ mod test_signing { unimplemented!() } - async fn update_shielded_context( - self, - _shielded: ShieldedContext, - ) -> Self - where - U: ShieldedUtils + MaybeSend + MaybeSync, - { - unimplemented!() - } - fn native_token(&self) -> Address { unimplemented!() }