Merge branch 'grarco/masp-fees' (#3217)

* origin/grarco/masp-fees:
  Changelog #3217
  Assigns issues to TODOs
  Removes `wrapper_changed_keys` from `TxResult`
  Removes fee unshielding e2e test
  Removes fee unshielding integration tests
  Recomputes tx test fixture
  Recomputes wasm for tests modules
  Updates TODO comments
  Reworks sdk fee validation
  Renames `wrapper_fee_check`
  Removes error/functions related to fee unshielding
  Removes `descriptions_limit` protocol param
  Removes redundandt wrapper types
  Removes fee unshielding cli and tx args
  Removes `unshield_section_hash` from `WrapperTx`. Updates the relative functions. Removes event for masp wrapper tx

.changelog/unreleased/improvements/3217-masp-fees.md

@@ -0,0 +1,2 @@
+- Removed fee unshielding from wrapper transactions.
+  ([\#3217](https://github.com/anoma/namada/pull/3217))

crates/apps/src/lib/bench_utils.rs

@@ -611,7 +611,6 @@ impl BenchShell {
             },
             defaults::albert_keypair().ref_to(),
             0.into(),
-            None,
         );
         self.last_block_masp_txs
             .push((masp_tx, self.state.write_log().get_keys()));
@@ -904,7 +903,6 @@ impl Client for BenchShell {
                     .map(|(idx, (tx, changed_keys))| {
                         let tx_result = TxResult::<String> {
                             gas_used: 0.into(),
-                            wrapper_changed_keys: Default::default(),
                             batch_results: BatchResults(
                                 [(
                                     tx.first_commitments().unwrap().get_hash(),

crates/apps/src/lib/cli.rs

@@ -3083,8 +3083,6 @@ pub mod args {
     pub const EXPIRATION_OPT: ArgOpt<DateTimeUtc> = arg_opt("expiration");
     pub const EMAIL: Arg<String> = arg("email");
     pub const EMAIL_OPT: ArgOpt<String> = EMAIL.opt();
-    pub const FEE_UNSHIELD_SPENDING_KEY: ArgOpt<WalletTransferSource> =
-        arg_opt("gas-spending-key");
     pub const FEE_AMOUNT_OPT: ArgOpt<token::DenominatedAmount> =
         arg_opt("gas-price");
     pub const FEE_PAYER_OPT: ArgOpt<WalletPublicKey> = arg_opt("gas-payer");
@@ -6436,9 +6434,6 @@ pub mod args {
                 wallet_alias_force: self.wallet_alias_force,
                 fee_amount: self.fee_amount,
                 fee_token: ctx.get(&self.fee_token).into(),
-                fee_unshield: self
-                    .fee_unshield
-                    .map(|ref fee_unshield| ctx.get_cached(fee_unshield)),
                 gas_limit: self.gas_limit,
                 signing_keys: self
                     .signing_keys
@@ -6518,10 +6513,6 @@ pub mod args {
                  this transaction"
             )))
             .arg(FEE_TOKEN.def().help(wrap!("The token for paying the gas")))
-            .arg(FEE_UNSHIELD_SPENDING_KEY.def().help(wrap!(
-                "The spending key to be used for fee unshielding. If none is \
-                 provided, fee will be paid from the unshielded balance only."
-            )))
             .arg(GAS_LIMIT.def().help(wrap!(
                 "The multiplier of the gas limit resolution defining the \
                  maximum amount of gas needed to run transaction."
@@ -6551,16 +6542,11 @@ pub mod args {
                     ))
                     .conflicts_with_all([EXPIRATION_OPT.name]),
             )
-            .arg(
-                DISPOSABLE_SIGNING_KEY
-                    .def()
-                    .help(wrap!(
-                        "Generates an ephemeral, disposable keypair to sign \
-                         the wrapper transaction. This keypair will be \
-                         immediately discarded after use."
-                    ))
-                    .requires(FEE_UNSHIELD_SPENDING_KEY.name),
-            )
+            .arg(DISPOSABLE_SIGNING_KEY.def().help(wrap!(
+                "Generates an ephemeral, disposable keypair to sign the \
+                 wrapper transaction. This keypair will be immediately \
+                 discarded after use."
+            )))
             .arg(
                 SIGNING_KEYS
                     .def()
@@ -6618,7 +6604,6 @@ pub mod args {
             let fee_amount =
                 FEE_AMOUNT_OPT.parse(matches).map(InputAmount::Unvalidated);
             let fee_token = FEE_TOKEN.parse(matches);
-            let fee_unshield = FEE_UNSHIELD_SPENDING_KEY.parse(matches);
             let _wallet_alias_force = WALLET_ALIAS_FORCE.parse(matches);
             let gas_limit = GAS_LIMIT.parse(matches);
             let wallet_alias_force = WALLET_ALIAS_FORCE.parse(matches);
@@ -6653,7 +6638,6 @@ pub mod args {
                 wallet_alias_force,
                 fee_amount,
                 fee_token,
-                fee_unshield,
                 gas_limit,
                 expiration,
                 disposable_signing_key,

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

@@ -664,18 +664,6 @@ pub async fn query_protocol_parameters(
         fee_unshielding_gas_limit
     );
 
-    let key = param_storage::get_fee_unshielding_descriptions_limit_key();
-    let fee_unshielding_descriptions_limit: u64 =
-        query_storage_value(context.client(), &key)
-            .await
-            .expect("Parameter should be defined.");
-    display_line!(
-        context.io(),
-        "{:4}Fee unshielding descriptions limit: {:?}",
-        "",
-        fee_unshielding_descriptions_limit
-    );
-
     let key = param_storage::get_gas_cost_key();
     let gas_cost_table: BTreeMap<Address, token::Amount> =
         query_storage_value(context.client(), &key)

crates/apps/src/lib/config/genesis.rs

@@ -316,8 +316,6 @@ pub struct Parameters {
     pub max_signatures_per_transaction: u8,
     /// Fee unshielding gas limit
     pub fee_unshielding_gas_limit: u64,
-    /// Fee unshielding descriptions limit
-    pub fee_unshielding_descriptions_limit: u64,
     /// Map of the cost per gas unit for every token allowed for fee payment
     pub minimum_gas_price: BTreeMap<Address, token::Amount>,
 }

crates/apps/src/lib/config/genesis/chain.rs

@@ -304,7 +304,6 @@ impl Finalized {
             epochs_per_year,
             max_signatures_per_transaction,
             fee_unshielding_gas_limit,
-            fee_unshielding_descriptions_limit,
             max_block_gas,
             minimum_gas_price,
             max_tx_bytes,
@@ -350,7 +349,6 @@ impl Finalized {
             max_proposal_bytes,
             max_signatures_per_transaction,
             fee_unshielding_gas_limit,
-            fee_unshielding_descriptions_limit,
             max_block_gas,
             minimum_gas_price: minimum_gas_price
                 .iter()

crates/apps/src/lib/config/genesis/templates.rs

@@ -299,8 +299,6 @@ pub struct ChainParams<T: TemplateValidation> {
     pub max_block_gas: u64,
     /// Fee unshielding gas limit
     pub fee_unshielding_gas_limit: u64,
-    /// Fee unshielding descriptions limit
-    pub fee_unshielding_descriptions_limit: u64,
     /// Map of the cost per gas unit for every token allowed for fee payment
     pub minimum_gas_price: T::GasMinimums,
 }
@@ -324,7 +322,6 @@ impl ChainParams<Unvalidated> {
             max_signatures_per_transaction,
             max_block_gas,
             fee_unshielding_gas_limit,
-            fee_unshielding_descriptions_limit,
             minimum_gas_price,
         } = self;
         let mut min_gas_prices = BTreeMap::default();
@@ -370,7 +367,6 @@ impl ChainParams<Unvalidated> {
             max_signatures_per_transaction,
             max_block_gas,
             fee_unshielding_gas_limit,
-            fee_unshielding_descriptions_limit,
             minimum_gas_price: min_gas_prices,
         })
     }

crates/apps/src/lib/config/genesis/transactions.rs

@@ -85,7 +85,6 @@ fn get_tx_args(use_device: bool) -> TxArgs {
         fee_amount: None,
         wrapper_fee_payer: None,
         fee_token: genesis_fee_token_address(),
-        fee_unshield: None,
         gas_limit: 0.into(),
         expiration: Default::default(),
         disposable_signing_key: false,
@@ -131,7 +130,6 @@ fn get_tx_to_sign(tag: impl AsRef<str>, data: impl BorshSerialize) -> Tx {
         },
         fee_payer,
         0.into(),
-        None,
     );
     tx
 }

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

@@ -9,14 +9,13 @@ use namada::gas::event::GasUsed;
 use namada::governance::pgf::inflation as pgf_inflation;
 use namada::hash::Hash;
 use namada::ledger::events::extend::{
-    ComposeEvent, Height, Info, MaspTxBatchRefs, MaspTxBlockIndex,
-    MaspTxWrapper, TxHash,
+    ComposeEvent, Height, Info, MaspTxBatchRefs, MaspTxBlockIndex, TxHash,
 };
 use namada::ledger::events::EmitEvents;
 use namada::ledger::gas::GasMetering;
 use namada::ledger::ibc;
 use namada::ledger::pos::namada_proof_of_stake;
-use namada::ledger::protocol::{DispatchError, WrapperArgs};
+use namada::ledger::protocol::DispatchError;
 use namada::proof_of_stake;
 use namada::proof_of_stake::storage::{
     find_validator_by_raw_hash, write_last_block_proposer_address,
@@ -214,7 +213,7 @@ where
                 continue;
             }
 
-            let (tx_gas_meter, mut wrapper_args) = match &tx_header.tx_type {
+            let (tx_gas_meter, block_proposer) = match &tx_header.tx_type {
                 TxType::Wrapper(wrapper) => {
                     stats.increment_wrapper_txs();
                     let gas_limit = match Gas::try_from(wrapper.gas_limit) {
@@ -244,14 +243,7 @@ where
                             );
                         }
                     }
-
-                    (
-                        gas_meter,
-                        Some(WrapperArgs {
-                            block_proposer: &native_block_proposer_address,
-                            is_committed_fee_unshield: false,
-                        }),
-                    )
+                    (gas_meter, Some(&native_block_proposer_address))
                 }
                 TxType::Raw => {
                     tracing::error!(
@@ -334,7 +326,7 @@ where
                 &mut self.state,
                 &mut self.vp_wasm_cache,
                 &mut self.tx_wasm_cache,
-                wrapper_args.as_mut(),
+                block_proposer,
             );
             let tx_gas_meter = tx_gas_meter.into_inner();
             let consumed_gas = tx_gas_meter.get_tx_consumed_gas();
@@ -353,7 +345,6 @@ where
                     replay_protection_hashes,
                     consumed_gas,
                     height,
-                    wrapper_args,
                 },
                 TxLogs {
                     tx_event: &mut tx_event,
@@ -545,20 +536,6 @@ where
         tx_data: TxData<'_>,
         mut tx_logs: TxLogs<'_>,
     ) {
-        // Check the commitment of the fee unshielding regardless of the
-        // result, it could be committed even in case of errors
-        if tx_data
-            .wrapper_args
-            .as_ref()
-            .map(|args| args.is_committed_fee_unshield)
-            .unwrap_or_default()
-        {
-            tx_logs.tx_event.extend(MaspTxBlockIndex(
-                TxIndex::must_from_usize(tx_data.tx_index),
-            ));
-            tx_logs.tx_event.extend(MaspTxWrapper);
-        }
-
         match dispatch_result {
             Ok(tx_result) => self.handle_inner_tx_results(
                 response,
@@ -675,9 +652,6 @@ where
             self.commit_batch_hash(tx_data.replay_protection_hashes);
         }
 
-        tx_logs
-            .changed_keys
-            .extend(tx_result.wrapper_changed_keys.iter().cloned());
         tx_logs
             .tx_event
             .extend(GasUsed(tx_result.gas_used))
@@ -781,7 +755,6 @@ struct TxData<'tx> {
     replay_protection_hashes: Option<ReplayProtectionHashes>,
     consumed_gas: Gas,
     height: BlockHeight,
-    wrapper_args: Option<WrapperArgs<'tx>>,
 }
 
 struct TxLogs<'finalize> {
@@ -1091,7 +1064,6 @@ mod test_finalize_block {
                 },
                 keypair.ref_to(),
                 WRAPPER_GAS_LIMIT.into(),
-                None,
             ))));
         wrapper_tx.header.chain_id = shell.chain_id.clone();
         wrapper_tx.set_data(Data::new(
@@ -1133,7 +1105,6 @@ mod test_finalize_block {
                 },
                 sk.ref_to(),
                 WRAPPER_GAS_LIMIT.into(),
-                None,
             ))));
         batch.header.chain_id = shell.chain_id.clone();
         batch.header.atomic = set_atomic;
@@ -2918,7 +2889,6 @@ mod test_finalize_block {
                 },
                 keypair.ref_to(),
                 WRAPPER_GAS_LIMIT.into(),
-                None,
             ))));
         wrapper.header.chain_id = shell.chain_id.clone();
         wrapper.set_code(Code::new(tx_code, None));
@@ -2932,7 +2902,6 @@ mod test_finalize_block {
             },
             keypair_2.ref_to(),
             WRAPPER_GAS_LIMIT.into(),
-            None,
         ))));
         new_wrapper.add_section(Section::Authorization(Authorization::new(
             new_wrapper.sechashes(),
@@ -3027,7 +2996,6 @@ mod test_finalize_block {
                 },
                 keypair.ref_to(),
                 WRAPPER_GAS_LIMIT.into(),
-                None,
             ))));
         wrapper.header.chain_id = shell.chain_id.clone();
         wrapper.set_code(Code::new(tx_code, None));
@@ -3050,7 +3018,6 @@ mod test_finalize_block {
             },
             keypair_2.ref_to(),
             WRAPPER_GAS_LIMIT.into(),
-            None,
         ))));
         new_wrapper.add_section(Section::Authorization(Authorization::new(
             vec![new_wrapper.raw_header_hash()],
@@ -3138,7 +3105,6 @@ mod test_finalize_block {
                     },
                     keypair.ref_to(),
                     0.into(),
-                    None,
                 ))));
             wrapper_tx.header.chain_id = shell.chain_id.clone();
             wrapper_tx.set_data(Data::new(
@@ -3169,7 +3135,6 @@ mod test_finalize_block {
                 },
                 keypair.ref_to(),
                 WRAPPER_GAS_LIMIT.into(),
-                None,
             ))));
         unsigned_wrapper.header.chain_id = shell.chain_id.clone();
 
@@ -3368,7 +3333,6 @@ mod test_finalize_block {
                 },
                 keypair.ref_to(),
                 0.into(),
-                None,
             ))));
         wrapper.header.chain_id = shell.chain_id.clone();
         wrapper.set_code(Code::new("wasm_code".as_bytes().to_owned(), None));
@@ -3439,7 +3403,6 @@ mod test_finalize_block {
                 },
                 keypair.ref_to(),
                 WRAPPER_GAS_LIMIT.into(),
-                None,
             ))));
         wrapper.header.chain_id = shell.chain_id.clone();
         wrapper.set_code(Code::new(TestWasms::TxFail.read_bytes(), None));
@@ -3537,7 +3500,6 @@ mod test_finalize_block {
                 },
                 keypair.ref_to(),
                 WRAPPER_GAS_LIMIT.into(),
-                None,
             ))));
         wrapper.header.chain_id = shell.chain_id.clone();
         wrapper.set_code(Code::new("wasm_code".as_bytes().to_owned(), None));
@@ -3631,7 +3593,6 @@ mod test_finalize_block {
                 },
                 crate::wallet::defaults::albert_keypair().ref_to(),
                 5_000_000.into(),
-                None,
             ))));
         wrapper.header.chain_id = shell.chain_id.clone();
         wrapper.set_code(Code::new(tx_code, None));