Merge branch 'tiago/bridge-pool-zero-fees' (#1892)

* origin/tiago/bridge-pool-zero-fees:
  Changelog for #1892
  Refactor validate_changed_keys in the Bridge pool VP
  Validate txs moving 0 value to the Bridge pool

.changelog/unreleased/improvements/1892-bridge-pool-zero-fees.md

@@ -0,0 +1,2 @@
+- Allow Bridge pool transfers to pay zero gas fees
+  ([\#1892](https://github.com/anoma/namada/pull/1892))

shared/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs

@@ -387,6 +387,7 @@ where
                 escrow_account: &BRIDGE_POOL_ADDRESS,
                 expected_debit: expected_gas_debit,
                 expected_credit: expected_gas_credit,
+                transferred_amount: &transfer.gas_fee.amount,
                 _kind: PhantomData,
             },
             token_check: EscrowDelta {
@@ -395,6 +396,7 @@ where
                 escrow_account: token_check_escrow_acc,
                 expected_debit: expected_token_debit,
                 expected_credit: expected_token_credit,
+                transferred_amount: &transfer.transfer.amount,
                 _kind: PhantomData,
             },
         })
@@ -409,21 +411,72 @@ struct EscrowDelta<'a, KIND> {
     escrow_account: &'a Address,
     expected_debit: Amount,
     expected_credit: Amount,
+    transferred_amount: &'a Amount,
     _kind: PhantomData<*const KIND>,
 }
 
 impl<KIND> EscrowDelta<'_, KIND> {
-    fn validate_changed_keys(&self, changed_keys: &BTreeSet<Key>) -> bool {
+    /// Validate an [`EscrowDelta`].
+    ///
+    /// # Conditions for validation
+    ///
+    /// If the transferred amount in the [`EscrowDelta`] is nil,
+    /// then no keys could have been changed. If the transferred
+    /// amount is greater than zero, then the appropriate escrow
+    /// keys must have been written to by some wasm tx.
+    #[inline]
+    fn validate(&self, changed_keys: &BTreeSet<Key>) -> bool {
+        if hints::unlikely(self.transferred_amount_is_nil()) {
+            self.check_escrow_keys_unchanged(changed_keys)
+        } else {
+            self.check_escrow_keys_changed(changed_keys)
+        }
+    }
+
+    /// Check if all required escrow keys in `changed_keys` were modified.
+    #[inline]
+    fn check_escrow_keys_changed(&self, changed_keys: &BTreeSet<Key>) -> bool {
         let EscrowDelta {
             token,
             payer_account,
             escrow_account,
             ..
         } = self;
+
         let owner_key = balance_key(token, payer_account);
         let escrow_key = balance_key(token, escrow_account);
+
         changed_keys.contains(&owner_key) && changed_keys.contains(&escrow_key)
     }
+
+    /// Check if no escrow keys in `changed_keys` were modified.
+    #[inline]
+    fn check_escrow_keys_unchanged(
+        &self,
+        changed_keys: &BTreeSet<Key>,
+    ) -> bool {
+        let EscrowDelta {
+            token,
+            payer_account,
+            escrow_account,
+            ..
+        } = self;
+
+        let owner_key = balance_key(token, payer_account);
+        let escrow_key = balance_key(token, escrow_account);
+
+        !changed_keys.contains(&owner_key)
+            && !changed_keys.contains(&escrow_key)
+    }
+
+    /// Check if the amount transferred to escrow is nil.
+    #[inline]
+    fn transferred_amount_is_nil(&self) -> bool {
+        let EscrowDelta {
+            transferred_amount, ..
+        } = self;
+        transferred_amount.is_zero()
+    }
 }
 
 /// There are two checks we must do when minting wNam.
@@ -437,9 +490,9 @@ struct EscrowCheck<'a> {
 
 impl EscrowCheck<'_> {
     #[inline]
-    fn validate_changed_keys(&self, changed_keys: &BTreeSet<Key>) -> bool {
-        self.gas_check.validate_changed_keys(changed_keys)
-            && self.token_check.validate_changed_keys(changed_keys)
+    fn validate(&self, changed_keys: &BTreeSet<Key>) -> bool {
+        self.gas_check.validate(changed_keys)
+            && self.token_check.validate(changed_keys)
     }
 }
 
@@ -541,7 +594,7 @@ where
         let wnam_address = read_native_erc20_address(&self.ctx.pre())?;
         let escrow_checks =
             self.determine_escrow_checks(&wnam_address, &transfer)?;
-        if !escrow_checks.validate_changed_keys(keys_changed) {
+        if !escrow_checks.validate(keys_changed) {
             tracing::debug!(
                 ?transfer,
                 "Missing storage modifications in the Bridge pool"
@@ -1845,4 +1898,94 @@ mod test_bridge_pool_vp {
             Expect::True,
         );
     }
+
+    /// Test that the Bridge pool native VP validates transfers that
+    /// do not contain gas fees and no associated changed keys.
+    #[test]
+    fn test_no_gas_fees_with_no_changed_keys() {
+        let nam_addr = nam();
+        let delta = EscrowDelta {
+            token: Cow::Borrowed(&nam_addr),
+            payer_account: &bertha_address(),
+            escrow_account: &BRIDGE_ADDRESS,
+            expected_debit: Amount::zero(),
+            expected_credit: Amount::zero(),
+            // NOTE: testing 0 amount
+            transferred_amount: &Amount::zero(),
+            // NOTE: testing gas fees
+            _kind: PhantomData::<*const GasCheck>,
+        };
+        // NOTE: testing no changed keys
+        let empty_keys = BTreeSet::new();
+
+        assert!(delta.validate(&empty_keys));
+    }
+
+    /// Test that the Bridge pool native VP rejects transfers that
+    /// do not contain gas fees and has associated changed keys.
+    #[test]
+    fn test_no_gas_fees_with_changed_keys() {
+        let nam_addr = nam();
+        let delta = EscrowDelta {
+            token: Cow::Borrowed(&nam_addr),
+            payer_account: &bertha_address(),
+            escrow_account: &BRIDGE_ADDRESS,
+            expected_debit: Amount::zero(),
+            expected_credit: Amount::zero(),
+            // NOTE: testing 0 amount
+            transferred_amount: &Amount::zero(),
+            // NOTE: testing gas fees
+            _kind: PhantomData::<*const GasCheck>,
+        };
+        let owner_key = balance_key(&nam_addr, &bertha_address());
+        // NOTE: testing changed keys
+        let some_changed_keys = BTreeSet::from([owner_key]);
+
+        assert!(!delta.validate(&some_changed_keys));
+    }
+
+    /// Test that the Bridge pool native VP validates transfers
+    /// moving no value and with no associated changed keys.
+    #[test]
+    fn test_no_amount_with_no_changed_keys() {
+        let nam_addr = nam();
+        let delta = EscrowDelta {
+            token: Cow::Borrowed(&nam_addr),
+            payer_account: &bertha_address(),
+            escrow_account: &BRIDGE_ADDRESS,
+            expected_debit: Amount::zero(),
+            expected_credit: Amount::zero(),
+            // NOTE: testing 0 amount
+            transferred_amount: &Amount::zero(),
+            // NOTE: testing token transfers
+            _kind: PhantomData::<*const TokenCheck>,
+        };
+        // NOTE: testing no changed keys
+        let empty_keys = BTreeSet::new();
+
+        assert!(delta.validate(&empty_keys));
+    }
+
+    /// Test that the Bridge pool native VP rejects transfers
+    /// moving no value and with associated changed keys.
+    #[test]
+    fn test_no_amount_with_changed_keys() {
+        let nam_addr = nam();
+        let delta = EscrowDelta {
+            token: Cow::Borrowed(&nam_addr),
+            payer_account: &bertha_address(),
+            escrow_account: &BRIDGE_ADDRESS,
+            expected_debit: Amount::zero(),
+            expected_credit: Amount::zero(),
+            // NOTE: testing 0 amount
+            transferred_amount: &Amount::zero(),
+            // NOTE: testing token transfers
+            _kind: PhantomData::<*const TokenCheck>,
+        };
+        let owner_key = balance_key(&nam_addr, &bertha_address());
+        // NOTE: testing changed keys
+        let some_changed_keys = BTreeSet::from([owner_key]);
+
+        assert!(!delta.validate(&some_changed_keys));
+    }
 }