fix: checking funded amount is enough (#7648)

noir-projects/noir-contracts/contracts/token_with_refunds_contract/src/main.nr

@@ -457,7 +457,6 @@ contract TokenWithRefunds {
         let (fee_payer_point, user_point) = TokenNote::generate_refund_points(
             fee_payer_npk_m_hash,
             user_npk_m_hash,
-            funded_amount,
             user_randomness,
             fee_payer_randomness
         );
@@ -473,20 +472,25 @@ contract TokenWithRefunds {
         // function has access to the final transaction fee, which is needed to compute the actual refund amount.
         context.set_public_teardown_function(
             context.this_address(),
-            FunctionSelector::from_signature("complete_refund((Field,Field,bool),(Field,Field,bool))"),
+            FunctionSelector::from_signature("complete_refund((Field,Field,bool),(Field,Field,bool),Field)"),
             [
-            slotted_fee_payer_point.x, slotted_fee_payer_point.y, slotted_fee_payer_point.is_infinite as Field, slotted_user_point.x, slotted_user_point.y, slotted_user_point.is_infinite as Field
+            slotted_fee_payer_point.x, slotted_fee_payer_point.y, slotted_fee_payer_point.is_infinite as Field, slotted_user_point.x, slotted_user_point.y, slotted_user_point.is_infinite as Field, funded_amount
         ]
         );
     }
 
     #[aztec(public)]
     #[aztec(internal)]
-    fn complete_refund(fee_payer_point: Point, user_point: Point) {
-        // 1. We get the final note hiding points by calling a `complete_refund` function on the note.
-        // We use 1:1 exchange rate between fee juice and token. So using `tx_fee` is enough
-        let tx_fee = context.transaction_fee();
-        let (fee_payer_note_hash, user_note_hash) = TokenNote::complete_refund(fee_payer_point, user_point, tx_fee);
+    fn complete_refund(fee_payer_point: Point, user_point: Point, funded_amount: Field) {
+        // 1. We get the final note hashes by calling a `complete_refund` function on the note.
+        // We use 1:1 exchange rate between fee juice and token so just passing transaction fee and funded amount
+        // to `complete_refund(...)` function is enough.
+        let (fee_payer_note_hash, user_note_hash) = TokenNote::complete_refund(
+            fee_payer_point,
+            user_point,
+            funded_amount,
+            context.transaction_fee()
+        );
 
         // 2. At last we emit the note hashes.
         context.push_note_hash(fee_payer_note_hash);

noir-projects/noir-contracts/contracts/token_with_refunds_contract/src/test/basic.nr

@@ -41,8 +41,8 @@ unconstrained fn setup_refund_success() {
 
     // When the refund was set up, we would've spent the note worth mint_amount, and inserted a note worth
     //`mint_amount - funded_amount`. When completing the refund, we would've constructed a hash corresponding to a note
-    // worth  `funded_amount - transaction_fee`. We "know" the transaction fee was 1 (it is hardcoded in TXE oracle)
-    // but we need to notify TXE of the note (preimage).
+    // worth  `funded_amount - transaction_fee`. We "know" the transaction fee was 1 (it is hardcoded in
+    // `executePublicFunction` TXE oracle) but we need to notify TXE of the note (preimage).
     env.store_note_in_cache(
         &mut TokenNote {
         amount: U128::from_integer(funded_amount - 1),
@@ -68,3 +68,36 @@ unconstrained fn setup_refund_success() {
     utils::check_private_balance(token_contract_address, fee_payer, 1)
 }
 
+// TODO(#7694): Ideally we would check the error message here but it's currently not possible because TXE does not
+// support checking messages of errors thrown in a public teardown function. Once this is supported, check the message
+// here and delete the e2e test checking it.
+// #[test(should_fail_with = "tx fee is higher than funded amount")]
+#[test(should_fail)]
+unconstrained fn setup_refund_insufficient_funded_amount() {
+    let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(true);
+
+    // Renaming owner and recipient to match naming in TokenWithRefunds
+    let user = owner;
+    let fee_payer = recipient;
+
+    // We set funded amount to 0 to make the transaction fee higher than the funded amount
+    let funded_amount = 0;
+    let user_randomness = 42;
+    let fee_payer_randomness = 123;
+    let mut context = env.private();
+
+    let setup_refund_from_call_interface = TokenWithRefunds::at(token_contract_address).setup_refund(
+        fee_payer,
+        user,
+        funded_amount,
+        user_randomness,
+        fee_payer_randomness
+    );
+
+    authwit_cheatcodes::add_private_authwit_from_call_interface(user, fee_payer, setup_refund_from_call_interface);
+
+    env.impersonate(fee_payer);
+
+    // The following should fail with "tx fee is higher than funded amount" because funded amount is 0
+    env.call_private_void(setup_refund_from_call_interface);
+}

noir-projects/noir-contracts/contracts/token_with_refunds_contract/src/types/token_note.nr

@@ -18,14 +18,14 @@ trait PrivatelyRefundable {
     fn generate_refund_points(
         fee_payer_npk_m_hash: Field,
         user_npk_m_hash: Field,
-        funded_amount: Field,
         user_randomness: Field,
         fee_payer_randomness: Field
     ) -> (Point, Point);
 
     fn complete_refund(
         incomplete_fee_payer_point: Point,
         incomplete_user_point: Point,
+        funded_amount: Field,
         transaction_fee: Field
     ) -> (Field, Field);
 }
@@ -157,25 +157,26 @@ impl OwnedNote for TokenNote {
  *
  * However we can still perform addition/subtraction on points! That is why we generate those two points, which are:
  * incomplete_fee_payer_point := G_npk * fee_payer_npk + G_rnd * fee_payer_randomness
- * incomplete_user_point := G_npk * user_npk + G_amt * funded_amount + G_rnd * user_randomness
- *
- * where `funded_amount` is the total amount in tokens that the sponsored user initially supplied, from which
- * the transaction fee will be subtracted.
+ * incomplete_user_point := G_npk * user_npk + G_rnd * user_randomness
  *
  * So we pass those points into the teardown function (here) and compute a third point corresponding to the transaction
  * fee as just:
  *
  * fee_point := G_amt * transaction_fee
+ * refund_point := G_amt * (funded_amount - transaction_fee)
  *
- * Then we arrive at the final points via addition/subtraction of that transaction fee point:
+ * where `funded_amount` is the total amount in tokens that the sponsored user initially supplied and the transaction
+ * fee is the final transaction fee whose value is made available in the public teardown function.
+ *
+ * Then we arrive at the final points via addition of the fee and refund points:
  *
  * fee_payer_point := incomplete_fee_payer_point + fee_point =
  *                  = (G_npk * fee_payer_npk + G_rnd * fee_payer_randomness) + G_amt * transaction_fee =
  *                  = G_amt * transaction_fee + G_npk * fee_payer_npk + G_rnd * fee_payer_randomness
  *
- * user_point := incomplete_user_point - fee_point =
- *             = (G_amt * funded_amount + G_npk * user_npk + G_rnd + user_randomness) - G_amt * transaction_fee =
- *             = G_amt * (funded_amount - transaction_fee) + G_npk * user_npk + G_rnd + user_randomness
+ * user_point := incomplete_user_point + refund_point =
+ *             = (G_npk * user_npk + G_rnd + user_randomness) + G_amt * (funded_amount - transaction_fee) =
+ *             = G_amt * (funded_amount - transaction_fee) + G_npk * user_npk + G_rnd * user_randomness
  * 
  * The point above matches the note_hiding_point of (and therefore *is*) notes like:
  * {
@@ -189,17 +190,16 @@ impl OwnedNote for TokenNote {
  *      1) randomness_influence = incomplete_fee_payer_point - G_npk * fee_payer_npk =
  *                              = (G_npk * fee_payer_npk + G_rnd * randomness) - G_npk * fee_payer_npk =
  *                              = G_rnd * randomness
- *      2) user_fingerprint = incomplete_user_point - G_amt * funded_amount - randomness_influence =
- *                          = (G_npk * user_npk + G_amt * funded_amount + G_rnd * randomness) - G_amt * funded_amount
- *                            - G_rnd * randomness =
+ *      2) user_fingerprint = incomplete_user_point - randomness_influence =
+ *                          = (G_npk * user_npk + G_rnd * randomness) - G_rnd * randomness =
  *                          = G_npk * user_npk
  *      3) Then the second time the user would use this fee paying contract we would recover the same fingerprint and
  *         link that the 2 transactions were made by the same user. Given that it's expected that only a limited set
  *         of fee paying contracts will be used and they will be known, searching for fingerprints by trying different
  *         fee payer npk values of these known contracts is a feasible attack.
  */
 impl PrivatelyRefundable for TokenNote {
-    fn generate_refund_points(fee_payer_npk_m_hash: Field, user_npk_m_hash: Field, funded_amount: Field, user_randomness: Field, fee_payer_randomness: Field) -> (Point, Point) {
+    fn generate_refund_points(fee_payer_npk_m_hash: Field, user_npk_m_hash: Field, user_randomness: Field, fee_payer_randomness: Field) -> (Point, Point) {
         // 1. To be able to multiply generators with randomness and npk_m_hash using barretneberg's (BB) blackbox
         // function we first need to convert the fields to high and low limbs.
         // We use the unsafe version because the multi_scalar_mul will constrain the scalars.
@@ -215,44 +215,44 @@ impl PrivatelyRefundable for TokenNote {
 
         // 3. We do the necessary conversion for values relevant for the sponsored user point.
         // We use the unsafe version because the multi_scalar_mul will constrain the scalars.
-        let funded_amount_scalar = from_field_unsafe(funded_amount);
         let user_npk_m_hash_scalar = from_field_unsafe(user_npk_m_hash);
         let user_randomness_scalar = from_field_unsafe(user_randomness);
 
-        // 4. We compute `G_amt * funded_amount + G_npk * user_npk_m_hash + G_rnd * randomness`.
+        // 4. We compute `G_npk * user_npk_m_hash + G_rnd * randomness`.
         let incomplete_user_point = multi_scalar_mul(
-            [G_amt, G_npk, G_rnd],
-            [funded_amount_scalar, user_npk_m_hash_scalar, user_randomness_scalar]
+            [G_npk, G_rnd],
+            [user_npk_m_hash_scalar, user_randomness_scalar]
         );
 
         // 5. At last we return the points.
         (incomplete_fee_payer_point, incomplete_user_point)
     }
 
-    fn complete_refund(incomplete_fee_payer_point: Point, incomplete_user_point: Point, transaction_fee: Field) -> (Field, Field) {
-        // 1. We convert the transaction fee to high and low limbs to be able to use BB API.
+    fn complete_refund(incomplete_fee_payer_point: Point, incomplete_user_point: Point, funded_amount: Field, transaction_fee: Field) -> (Field, Field) {
+        // 1. We check that user funded the fee payer contract with at least the transaction fee.
+        assert(!funded_amount.lt(transaction_fee), "tx fee is higher than funded amount"); // funded_amout >= transaction_fee
+
+        // 2. We convert the transaction fee and refund amount to high and low limbs to be able to use BB API.
         // We use the unsafe version because the multi_scalar_mul will constrain the scalars.
         let transaction_fee_scalar = from_field_unsafe(transaction_fee);
+        let refund_scalar = from_field_unsafe(funded_amount - transaction_fee);
 
-        // 2. We compute the fee point as `G_amt * transaction_fee`
-        let fee_point = multi_scalar_mul(
-            [G_amt],
-            [transaction_fee_scalar]
-        );
+        // 3. We compute the fee point as `G_amt * transaction_fee`
+        let fee_point = multi_scalar_mul([G_amt], [transaction_fee_scalar]);
 
-        // 3. Now we leverage homomorphism to privately add the fee to fee payer point and subtract it from
-        // the sponsored user point in public.
-        let fee_payer_point = incomplete_fee_payer_point + fee_point;
-        let user_point = incomplete_user_point - fee_point;
+        // 4. We compute the refund point as `G_amt * refund`
+        let refund_point = multi_scalar_mul([G_amt], [refund_scalar]);
 
-        assert_eq(user_point.is_infinite, false);
+        // 5. Now we leverage homomorphism to privately add the fee to fee payer point and we add refund to the user point.
+        let fee_payer_point = incomplete_fee_payer_point + fee_point;
+        let user_point = incomplete_user_point + refund_point;
 
-        // 4. We no longer need to do any elliptic curve operations with the points so we collapse them to the final
+        // 6. We no longer need to do any elliptic curve operations with the points so we collapse them to the final
         // note hashes.
         let fee_payer_note_hash = fee_payer_point.x;
         let user_note_hash = user_point.x;
 
-        // 5. Finally we return the hashes.
+        // 7. Finally we return the hashes.
         (fee_payer_note_hash, user_note_hash)
     }
 }

yarn-project/end-to-end/src/e2e_fees/private_refunds.test.ts

@@ -129,6 +129,31 @@ describe('e2e_fees/private_refunds', () => {
       [initialAliceBalance - tx.transactionFee!, initialBobBalance + tx.transactionFee!],
     );
   });
+
+  // TODO(#7694): Remove this test once the lacking feature in TXE is implemented.
+  it('insufficient funded amount is correctly handled', async () => {
+    // 1. We generate randomness for Alice and derive randomness for Bob.
+    const aliceRandomness = Fr.random(); // Called user_randomness in contracts
+    const bobRandomness = poseidon2Hash([aliceRandomness]); // Called fee_payer_randomness in contracts
+
+    // 2. We call arbitrary `private_get_name(...)` function to check that the fee refund flow works.
+    await expect(
+      tokenWithRefunds.methods.private_get_name().prove({
+        fee: {
+          gasSettings: t.gasSettings,
+          paymentMethod: new PrivateRefundPaymentMethod(
+            tokenWithRefunds.address,
+            privateFPC.address,
+            aliceWallet,
+            aliceRandomness,
+            bobRandomness,
+            t.bobWallet.getAddress(), // Bob is the recipient of the fee notes.
+            true, // We set max fee/funded amount to zero to trigger the error.
+          ),
+        },
+      }),
+    ).rejects.toThrow('tx fee is higher than funded amount');
+  });
 });
 
 class PrivateRefundPaymentMethod implements FeePaymentMethod {
@@ -163,6 +188,12 @@ class PrivateRefundPaymentMethod implements FeePaymentMethod {
      * Address that the FPC sends notes it receives to.
      */
     private feeRecipient: AztecAddress,
+
+    /**
+     * If true, the max fee will be set to 0.
+     * TODO(#7694): Remove this param once the lacking feature in TXE is implemented.
+     */
+    private setMaxFeeToZero = false,
   ) {}
 
   /**
@@ -183,8 +214,9 @@ class PrivateRefundPaymentMethod implements FeePaymentMethod {
    * @returns The function call to pay the fee.
    */
   async getFunctionCalls(gasSettings: GasSettings): Promise<FunctionCall[]> {
-    // we assume 1:1 exchange rate between fee juice and token. But in reality you would need to convert feeLimit (maxFee) to be in token denomination
-    const maxFee = gasSettings.getFeeLimit();
+    // We assume 1:1 exchange rate between fee juice and token. But in reality you would need to convert feeLimit
+    // (maxFee) to be in token denomination.
+    const maxFee = this.setMaxFeeToZero ? Fr.ZERO : gasSettings.getFeeLimit();
 
     await this.wallet.createAuthWit({
       caller: this.paymentContract,

yarn-project/txe/package.json

@@ -18,7 +18,7 @@
     "formatting": "run -T prettier --check ./src && run -T eslint ./src",
     "formatting:fix": "run -T eslint --fix ./src && run -T prettier -w ./src",
     "test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules ../node_modules/.bin/jest --passWithNoTests",
-    "dev": "DEBUG='aztec:*' LOG_LEVEL=debug && node ./dest/bin/index.js",
+    "dev": "DEBUG='aztec:*' LOG_LEVEL=debug node ./dest/bin/index.js",
     "start": "node ./dest/bin/index.js"
   },
   "inherits": [

yarn-project/txe/src/oracle/txe_oracle.ts

@@ -772,8 +772,12 @@ export class TXE implements TypedOracle {
 
     const executionResult = await this.executePublicFunction(targetContractAddress, args, callContext);
 
+    if (executionResult.reverted) {
+      throw new Error(`Execution reverted with reason: ${executionResult.revertReason}`);
+    }
+
     // Apply side effects
-    this.sideEffectsCounter = executionResult.endSideEffectCounter.toNumber();
+    this.sideEffectsCounter += executionResult.endSideEffectCounter.toNumber();
     this.setContractAddress(currentContractAddress);
     this.setMsgSender(currentMessageSender);
     this.setFunctionSelector(currentFunctionSelector);
@@ -808,6 +812,10 @@ export class TXE implements TypedOracle {
 
     const executionResult = await this.executePublicFunction(targetContractAddress, args, callContext);
 
+    if (executionResult.reverted) {
+      throw new Error(`Execution reverted with reason: ${executionResult.revertReason}`);
+    }
+
     // Apply side effects
     this.sideEffectsCounter += executionResult.endSideEffectCounter.toNumber();
     this.setContractAddress(currentContractAddress);