From 6b72c7220d9e214a01af63198511b39466de15dd Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 7 Sep 2023 19:10:21 +0200 Subject: [PATCH 1/3] Shared function to retrieve fee unshielding transaction --- apps/src/lib/node/ledger/shell/mod.rs | 15 ++------- .../lib/node/ledger/shell/prepare_proposal.rs | 15 ++------- .../lib/node/ledger/shell/process_proposal.rs | 16 ++-------- shared/src/ledger/protocol/mod.rs | 31 ++++++++++++------- 4 files changed, 27 insertions(+), 50 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/mod.rs b/apps/src/lib/node/ledger/shell/mod.rs index 46a985b8fc..db71ac98ec 100644 --- a/apps/src/lib/node/ledger/shell/mod.rs +++ b/apps/src/lib/node/ledger/shell/mod.rs @@ -38,7 +38,8 @@ use namada::ledger::pos::namada_proof_of_stake::types::{ ConsensusValidator, ValidatorSetUpdate, }; use namada::ledger::protocol::{ - apply_wasm_tx, get_transfer_hash_from_storage, ShellParams, + apply_wasm_tx, get_fee_unshielding_transaction, + get_transfer_hash_from_storage, ShellParams, }; use namada::ledger::storage::write_log::WriteLog; use namada::ledger::storage::{ @@ -1262,20 +1263,10 @@ where return response; } - let fee_unshield = wrapper - .unshield_section_hash - .and_then(|ref hash| tx.get_section(hash)) - .and_then(|section| { - if let Section::MaspTx(transaction) = section.as_ref() { - Some(transaction.to_owned()) - } else { - None - } - }); // Validate wrapper fees if let Err(e) = self.wrapper_fee_check( &wrapper, - fee_unshield, + get_fee_unshielding_transaction(&tx, &wrapper), &mut TempWlStorage::new(&self.wl_storage.storage), &mut self.vp_wasm_cache.clone(), &mut self.tx_wasm_cache.clone(), diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index e8633940b9..f3a56e7848 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -5,9 +5,10 @@ use namada::core::ledger::gas::TxGasMeter; #[cfg(feature = "abcipp")] use namada::ledger::eth_bridge::{EthBridgeQueries, SendValsetUpd}; use namada::ledger::pos::PosQueries; +use namada::ledger::protocol::get_fee_unshielding_transaction; use namada::ledger::storage::{DBIter, StorageHasher, TempWlStorage, DB}; use namada::proof_of_stake::find_validator_by_raw_hash; -use namada::proto::{Section, Tx}; +use namada::proto::Tx; use namada::types::address::Address; use namada::types::internal::TxInQueue; use namada::types::key::tm_raw_hash_to_string; @@ -249,19 +250,9 @@ where .map_err(|_| ())?; // Check fees - let fee_unshield = - wrapper.unshield_section_hash.and_then(|ref hash| { - tx.get_section(hash).and_then(|section| { - if let Section::MaspTx(transaction) = section.as_ref() { - Some(transaction.to_owned()) - } else { - None - } - }) - }); match self.wrapper_fee_check( &wrapper, - fee_unshield, + get_fee_unshielding_transaction(&tx, &wrapper), temp_wl_storage, vp_wasm_cache, tx_wasm_cache, diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index dca2d4fe4e..0ad847107b 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -6,6 +6,7 @@ use namada::core::hints; use namada::core::ledger::storage::WlStorage; use namada::ledger::eth_bridge::{EthBridgeQueries, SendValsetUpd}; use namada::ledger::pos::PosQueries; +use namada::ledger::protocol::get_fee_unshielding_transaction; use namada::ledger::storage::TempWlStorage; use namada::proof_of_stake::find_validator_by_raw_hash; use namada::types::internal::TxInQueue; @@ -889,22 +890,9 @@ where } // Check that the fee payer has sufficient balance. - let fee_unshield = - wrapper.unshield_section_hash.and_then(|ref hash| { - tx.get_section(hash).and_then(|section| { - if let Section::MaspTx(transaction) = - section.as_ref() - { - Some(transaction.to_owned()) - } else { - None - } - }) - }); - match self.wrapper_fee_check( &wrapper, - fee_unshield, + get_fee_unshielding_transaction(&tx, &wrapper), temp_wl_storage, vp_wasm_cache, tx_wasm_cache, diff --git a/shared/src/ledger/protocol/mod.rs b/shared/src/ledger/protocol/mod.rs index c6ed0814b1..5774c5c9aa 100644 --- a/shared/src/ledger/protocol/mod.rs +++ b/shared/src/ledger/protocol/mod.rs @@ -175,20 +175,9 @@ where apply_protocol_tx(protocol_tx.tx, tx.data(), wl_storage) } TxType::Wrapper(ref wrapper) => { - let masp_transaction = - wrapper.unshield_section_hash.and_then(|ref hash| { - tx.get_section(hash).and_then(|section| { - if let Section::MaspTx(transaction) = section.as_ref() { - Some(transaction.to_owned()) - } else { - None - } - }) - }); - let changed_keys = apply_wrapper_tx( wrapper, - masp_transaction, + get_fee_unshielding_transaction(&tx, wrapper), tx_bytes, ShellParams { tx_gas_meter, @@ -291,6 +280,24 @@ where Ok(changed_keys) } +/// Retrieve the Masp `Transaction` for fee unshielding from the provided +/// transaction, if present +pub fn get_fee_unshielding_transaction( + tx: &Tx, + wrapper: &WrapperTx, +) -> Option { + wrapper + .unshield_section_hash + .and_then(|ref hash| tx.get_section(hash)) + .and_then(|section| { + if let Section::MaspTx(transaction) = section.as_ref() { + Some(transaction.to_owned()) + } else { + None + } + }) +} + /// Charge fee for the provided wrapper transaction. In ABCI returns an error if /// the balance of the block proposer overflows. In ABCI plus returns error if: /// - The unshielding fails From 0ba4d782e33eee73b3de14fad8707fc65ddbf4c7 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 7 Sep 2023 19:24:37 +0200 Subject: [PATCH 2/3] Refactors masp section check in encryption --- core/src/proto/types.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/core/src/proto/types.rs b/core/src/proto/types.rs index 43da551d87..42d7c687b2 100644 --- a/core/src/proto/types.rs +++ b/core/src/proto/types.rs @@ -1500,7 +1500,6 @@ impl Tx { /// signatures over it #[cfg(feature = "ferveo-tpke")] pub fn encrypt(&mut self, pubkey: &EncryptionKey) -> &mut Self { - use crate::types::hash::Hash; let header_hash = self.header_hash(); let mut plaintexts = vec![]; // Iterate backwrds to sidestep the effects of deletion on indexing @@ -1508,7 +1507,7 @@ impl Tx { match &self.sections[i] { Section::Signature(sig) if sig.targets.contains(&header_hash) => {} - Section::MaspTx(_) => { + masp_section @ Section::MaspTx(_) => { // Do NOT encrypt the fee unshielding transaction if let Some(unshield_section_hash) = self .header() @@ -1516,14 +1515,7 @@ impl Tx { .expect("Tried to encrypt a non-wrapper tx") .unshield_section_hash { - if unshield_section_hash - == Hash( - self.sections[i] - .hash(&mut Sha256::new()) - .finalize_reset() - .into(), - ) - { + if unshield_section_hash == masp_section.get_hash() { continue; } } From 78a23f441525f6b6f77e492f69c3f6d171feac86 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Thu, 7 Sep 2023 19:58:25 +0200 Subject: [PATCH 3/3] changelog: add #1877 --- .../unreleased/improvements/1877-refactor-get-fee-unshield.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/1877-refactor-get-fee-unshield.md diff --git a/.changelog/unreleased/improvements/1877-refactor-get-fee-unshield.md b/.changelog/unreleased/improvements/1877-refactor-get-fee-unshield.md new file mode 100644 index 0000000000..8db436c7bd --- /dev/null +++ b/.changelog/unreleased/improvements/1877-refactor-get-fee-unshield.md @@ -0,0 +1,2 @@ +- Refactored retrieval of `Transaction` object for fee unshielding. + ([\#1877](https://github.com/anoma/namada/pull/1877)) \ No newline at end of file