Merge branch 'grarco/invalidate-masp-notes-in-client' (#2534)

* origin/grarco/invalidate-masp-notes-in-client:
  Refactors `check_balance_too_low_err` to accept either a balance key or a balance
  Changelog #2534
  Refactors the `ShieldedUtils` trait
  Fixes e2e test
  Updates masp proofs
  Invalidate spent keys when generating masp transactions
  Returns error if the provided gas-spending-key is not needed
  Improves tests of fee unshielding
  [chore]: Fix integration masp test fixtures
  I don't know
  [chore] Rebase on main
  [fix] Some e2e test fixes
  Moved fetch calls completely from other calls. Updated cli. Fixed tests
  Added changelog entry.
  Changed the note scanning algorithm to not require additional context.

.changelog/unreleased/SDK/2534-invalidate-masp-notes-in-client.md

@@ -0,0 +1,6 @@
+- Reworked the sdk to support the new speculative state of the
+  `ShieldedContext`:\n-`ShieldedContext` now has an extra field to determin its
+  state\n-When calling `gen_shielded_transfer` the context now invalidates the
+  spent notes (if any)\n-The fee unshielding `Transaction` is now built before
+  the actual transaction\n-`find_viewing_key` only requires a shared reference
+  now ([\#2534](https://github.com/anoma/namada/pull/2534))

.changelog/unreleased/improvements/2534-invalidate-masp-notes-in-client.md

@@ -0,0 +1,3 @@
+- The client, when generating a shielded transfer, invalidates the
+  masp notes that have been spent without the need to sync with a node.
+  ([\#2534](https://github.com/anoma/namada/pull/2534))

crates/apps/src/lib/bench_utils.rs

@@ -76,7 +76,7 @@ use namada::types::token::{Amount, DenominatedAmount, Transfer};
 use namada::vm::wasm::run;
 use namada::{proof_of_stake, tendermint};
 use namada_sdk::masp::{
-    self, ShieldedContext, ShieldedTransfer, ShieldedUtils,
+    self, ContextSyncStatus, ShieldedContext, ShieldedTransfer, ShieldedUtils,
 };
 pub use namada_sdk::tx::{
     TX_BECOME_VALIDATOR_WASM, TX_BOND_WASM, TX_BRIDGE_POOL_WASM,
@@ -117,6 +117,8 @@ 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";
 
 /// For `tracing_subscriber`, which fails if called more than once in the same
 /// process
@@ -654,10 +656,19 @@ impl ShieldedUtils for BenchShieldedUtils {
     async fn load<U: ShieldedUtils>(
         &self,
         ctx: &mut ShieldedContext<U>,
+        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 = File::open(
-            self.context_dir.0.path().to_path_buf().join(FILE_NAME),
+            self.context_dir.0.path().to_path_buf().join(file_name),
         )?;
         let mut bytes = Vec::new();
         ctx_file.read_to_end(&mut bytes)?;
@@ -674,8 +685,14 @@ impl ShieldedUtils for BenchShieldedUtils {
         &self,
         ctx: &ShieldedContext<U>,
     ) -> std::io::Result<()> {
+        let (tmp_file_name, file_name) = match ctx.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
@@ -696,8 +713,20 @@ 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 = ctx.sync_status {
+            let _ = std::fs::remove_file(
+                self.context_dir
+                    .0
+                    .path()
+                    .to_path_buf()
+                    .join(SPECULATIVE_FILE_NAME),
+            );
+        }
         Ok(())
     }
 }

crates/apps/src/lib/cli/client.rs

@@ -322,7 +322,7 @@ impl CliApi {
                         });
                         client.wait_until_node_is_synced(&io).await?;
                         let args = args.to_sdk(&mut ctx);
-                        let mut chain_ctx = ctx.take_chain_or_exit();
+                        let chain_ctx = ctx.take_chain_or_exit();
                         let vks = chain_ctx
                             .wallet
                             .get_viewing_keys()
@@ -338,7 +338,6 @@ impl CliApi {
                             .into_iter()
                             .map(|sk| sk.into())
                             .collect::<Vec<_>>();
-                        let _ = chain_ctx.shielded.load().await;
                         crate::client::masp::syncing(
                             chain_ctx.shielded,
                             &client,

crates/apps/src/lib/client/tx.rs

@@ -23,6 +23,7 @@ use namada::types::dec::Dec;
 use namada::types::io::Io;
 use namada::types::key::{self, *};
 use namada_sdk::rpc::{InnerTxResult, TxBroadcastData, TxResponse};
+use namada_sdk::signing::validate_fee_and_gen_unshield;
 use namada_sdk::wallet::alias::validator_consensus_key;
 use namada_sdk::wallet::{Wallet, WalletIo};
 use namada_sdk::{display_line, edisplay_line, error, signing, tx, Namada};
@@ -418,13 +419,20 @@ pub async fn submit_change_consensus_key(
 
     let signing_data =
         init_validator_signing_data(namada, &tx_args, vec![new_key]).await?;
+    let (fee_amount, _, unshield) = validate_fee_and_gen_unshield(
+        namada,
+        &tx_args,
+        &signing_data.fee_payer,
+    )
+    .await?;
 
     tx::prepare_tx(
-        namada,
+        namada.client(),
         &tx_args,
         &mut tx,
+        unshield,
+        fee_amount,
         signing_data.fee_payer.clone(),
-        None,
     )
     .await?;
 
@@ -750,13 +758,20 @@ pub async fn submit_become_validator(
 
     let signing_data =
         init_validator_signing_data(namada, &tx_args, all_pks).await?;
+    let (fee_amount, _, unshield) = validate_fee_and_gen_unshield(
+        namada,
+        &tx_args,
+        &signing_data.fee_payer,
+    )
+    .await?;
 
     tx::prepare_tx(
-        namada,
+        namada.client(),
         &tx_args,
         &mut tx,
+        unshield,
+        fee_amount,
         signing_data.fee_payer.clone(),
-        None,
     )
     .await?;
 

crates/apps/src/lib/node/ledger/shell/testing/node.rs

@@ -438,7 +438,26 @@ impl MockNode {
             votes,
         };
 
-        locked.finalize_block(req).expect("Test failed");
+        let resp = locked.finalize_block(req).expect("Test failed");
+        let mut error_codes = resp
+            .events
+            .into_iter()
+            .map(|e| {
+                let code = ResultCode::from_u32(
+                    e.attributes
+                        .get("code")
+                        .map(|e| u32::from_str(e).unwrap())
+                        .unwrap_or_default(),
+                )
+                .unwrap();
+                if code == ResultCode::Ok {
+                    NodeResults::Ok
+                } else {
+                    NodeResults::Failed(code)
+                }
+            })
+            .collect::<Vec<_>>();
+        self.results.lock().unwrap().append(&mut error_codes);
         locked.commit();
 
         // Cache the block
@@ -501,7 +520,7 @@ impl MockNode {
 
     /// Send a tx through Process Proposal and Finalize Block
     /// and register the results.
-    fn submit_txs(&self, txs: Vec<Vec<u8>>) {
+    pub fn submit_txs(&self, txs: Vec<Vec<u8>>) {
         // The block space allocator disallows encrypted txs in certain blocks.
         // Advance to block height that allows txs.
         self.advance_to_allowed_block();

crates/sdk/src/eth_bridge/bridge_pool.rs

@@ -40,7 +40,7 @@ use crate::queries::{
     TransferToEthereumStatus, RPC,
 };
 use crate::rpc::{query_storage_value, query_wasm_code_hash, validate_amount};
-use crate::signing::aux_signing_data;
+use crate::signing::{aux_signing_data, validate_fee_and_gen_unshield};
 use crate::tx::prepare_tx;
 use crate::{
     args, display, display_line, edisplay_line, MaybeSync, Namada,
@@ -87,6 +87,12 @@ pub async fn build_bridge_pool_tx(
             Some(sender_),
         ),
     )?;
+    let (fee_amount, _, unshield) = validate_fee_and_gen_unshield(
+        context,
+        &tx_args,
+        &signing_data.fee_payer,
+    )
+    .await?;
 
     let chain_id = tx_args
         .chain_id
@@ -104,11 +110,12 @@ pub async fn build_bridge_pool_tx(
     .add_data(transfer);
 
     prepare_tx(
-        context,
+        context.client(),
         &tx_args,
         &mut tx,
+        unshield,
+        fee_amount,
         signing_data.fee_payer.clone(),
-        None,
     )
     .await?;