From 09df28b197b7b99010f72185abe1c23077717059 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 25 Jun 2024 14:27:31 +0200 Subject: [PATCH 1/3] Early sapling balance check in masp vp --- crates/namada/src/ledger/native_vp/masp.rs | 64 +++++++++++----------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index 51e86f018b..c87666dbb4 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -390,7 +390,7 @@ where } // The Sapling value balance adds to the transparent tx pool - let mut transparent_tx_pool = shielded_tx.sapling_value_balance(); + let transparent_tx_pool = shielded_tx.sapling_value_balance(); // Check the validity of the keys and get the transfer data let mut changed_balances = @@ -436,7 +436,7 @@ where validate_transparent_bundle( &shielded_tx, &mut changed_balances, - &mut transparent_tx_pool, + transparent_tx_pool, masp_epoch, conversion_state, &mut signers, @@ -523,31 +523,6 @@ where } } - // Ensure that the shielded transaction exactly balances - match transparent_tx_pool.partial_cmp(&I128Sum::zero()) { - None | Some(Ordering::Less) => { - let error = native_vp::Error::new_const( - "Transparent transaction value pool must be nonnegative. \ - Violation may be caused by transaction being constructed \ - in previous epoch. Maybe try again.", - ) - .into(); - tracing::debug!("{error}"); - // Section 3.4: The remaining value in the transparent - // transaction value pool MUST be nonnegative. - return Err(error); - } - Some(Ordering::Greater) => { - let error = native_vp::Error::new_const( - "Transaction fees cannot be paid inside MASP transaction.", - ) - .into(); - tracing::debug!("{error}"); - return Err(error); - } - _ => {} - } - // Verify the proofs verify_shielded_tx(&shielded_tx, |gas| self.ctx.charge_gas(gas)) .map_err(Error::NativeVpError) @@ -724,11 +699,12 @@ fn validate_transparent_output( // Update the transaction value pool and also ensure that the Transaction is // consistent with the balance changes. I.e. the transparent inputs are not more // than the initial balances and that the transparent outputs are not more than -// the final balances. +// the final balances. Also ensure that the sapling value balance is exactly 0. fn validate_transparent_bundle( shielded_tx: &Transaction, changed_balances: &mut ChangedBalances, - transparent_tx_pool: &mut I128Sum, + // Take ownership to prevent further usage after this call + mut transparent_tx_pool: I128Sum, epoch: MaspEpoch, conversion_state: &ConversionState, signers: &mut BTreeSet, @@ -738,7 +714,7 @@ fn validate_transparent_bundle( validate_transparent_input( vin, changed_balances, - transparent_tx_pool, + &mut transparent_tx_pool, epoch, conversion_state, signers, @@ -749,13 +725,37 @@ fn validate_transparent_bundle( validate_transparent_output( out, changed_balances, - transparent_tx_pool, + &mut transparent_tx_pool, epoch, conversion_state, )?; } } - Ok(()) + + // Ensure that the shielded transaction exactly balances + match transparent_tx_pool.partial_cmp(&I128Sum::zero()) { + None | Some(Ordering::Less) => { + let error = native_vp::Error::new_const( + "Transparent transaction value pool must be nonnegative. \ + Violation may be caused by transaction being constructed in \ + previous epoch. Maybe try again.", + ) + .into(); + tracing::debug!("{error}"); + // The remaining value in the transparent transaction value pool + // MUST be nonnegative. + Err(error) + } + Some(Ordering::Greater) => { + let error = native_vp::Error::new_const( + "Transaction fees cannot be left on the MASP balance.", + ) + .into(); + tracing::debug!("{error}"); + Err(error) + } + _ => Ok(()), + } } // Apply the given Sapling value balance component to the accumulator From dd907d551a4c6d3ad8fd4b71b8058a50b55f4236 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 25 Jun 2024 14:35:02 +0200 Subject: [PATCH 2/3] Changelog #2721 --- .../unreleased/improvements/2721-early-sapling-balance-check.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/2721-early-sapling-balance-check.md diff --git a/.changelog/unreleased/improvements/2721-early-sapling-balance-check.md b/.changelog/unreleased/improvements/2721-early-sapling-balance-check.md new file mode 100644 index 0000000000..2bde17c756 --- /dev/null +++ b/.changelog/unreleased/improvements/2721-early-sapling-balance-check.md @@ -0,0 +1,2 @@ +- Moved up the check on the sapling value balance in the masp vp. + ([\#2721](https://github.com/anoma/namada/issues/2721)) \ No newline at end of file From a3bb632a1f40700abae9027a80cc9fcd88725f70 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Tue, 25 Jun 2024 15:53:18 +0200 Subject: [PATCH 3/3] Extracts the sapling value balance directly in `validate_transparent_bundle` --- crates/namada/src/ledger/native_vp/masp.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index c87666dbb4..5f0d15abef 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -389,9 +389,6 @@ where return Err(error); } - // The Sapling value balance adds to the transparent tx pool - let transparent_tx_pool = shielded_tx.sapling_value_balance(); - // Check the validity of the keys and get the transfer data let mut changed_balances = self.validate_state_and_get_transfer_data(keys_changed)?; @@ -436,7 +433,6 @@ where validate_transparent_bundle( &shielded_tx, &mut changed_balances, - transparent_tx_pool, masp_epoch, conversion_state, &mut signers, @@ -703,12 +699,13 @@ fn validate_transparent_output( fn validate_transparent_bundle( shielded_tx: &Transaction, changed_balances: &mut ChangedBalances, - // Take ownership to prevent further usage after this call - mut transparent_tx_pool: I128Sum, epoch: MaspEpoch, conversion_state: &ConversionState, signers: &mut BTreeSet, ) -> Result<()> { + // The Sapling value balance adds to the transparent tx pool + let mut transparent_tx_pool = shielded_tx.sapling_value_balance(); + if let Some(transp_bundle) = shielded_tx.transparent_bundle() { for vin in transp_bundle.vin.iter() { validate_transparent_input(