do not discard logs on revert since the kernel has pruned revertible …

…logs.

add test of private fee payment method with revert.

yarn-project/circuit-types/src/logs/tx_l2_logs.ts

@@ -157,16 +157,17 @@ export class UnencryptedTxL2Logs extends TxL2Logs<UnencryptedL2Log> {
    *       for more details.
    */
   public hash(): Buffer {
-    if (this.unrollLogs().length == 0) {
+    const unrolledLogs = this.unrollLogs();
+    if (unrolledLogs.length == 0) {
       return Buffer.alloc(32);
     }
 
     let flattenedLogs = Buffer.alloc(0);
-    for (const logsFromSingleFunctionCall of this.unrollLogs()) {
+    for (const logsFromSingleFunctionCall of unrolledLogs) {
       flattenedLogs = Buffer.concat([flattenedLogs, logsFromSingleFunctionCall.getSiloedHash()]);
     }
     // pad the end of logs with 0s
-    for (let i = 0; i < MAX_UNENCRYPTED_LOGS_PER_TX - this.unrollLogs().length; i++) {
+    for (let i = 0; i < MAX_UNENCRYPTED_LOGS_PER_TX - unrolledLogs.length; i++) {
       flattenedLogs = Buffer.concat([flattenedLogs, Buffer.alloc(32)]);
     }
 
@@ -235,16 +236,17 @@ export class EncryptedNoteTxL2Logs extends TxL2Logs<EncryptedL2NoteLog> {
    *       for more details.
    */
   public hash(): Buffer {
-    if (this.unrollLogs().length == 0) {
+    const unrolledLogs = this.unrollLogs();
+    if (unrolledLogs.length == 0) {
       return Buffer.alloc(32);
     }
 
     let flattenedLogs = Buffer.alloc(0);
-    for (const logsFromSingleFunctionCall of this.unrollLogs()) {
+    for (const logsFromSingleFunctionCall of unrolledLogs) {
       flattenedLogs = Buffer.concat([flattenedLogs, logsFromSingleFunctionCall.hash()]);
     }
     // pad the end of logs with 0s
-    for (let i = 0; i < MAX_NOTE_ENCRYPTED_LOGS_PER_TX - this.unrollLogs().length; i++) {
+    for (let i = 0; i < MAX_NOTE_ENCRYPTED_LOGS_PER_TX - unrolledLogs.length; i++) {
       flattenedLogs = Buffer.concat([flattenedLogs, Buffer.alloc(32)]);
     }
 
@@ -313,16 +315,17 @@ export class EncryptedTxL2Logs extends TxL2Logs<EncryptedL2Log> {
    *       for more details.
    */
   public hash(): Buffer {
-    if (this.unrollLogs().length == 0) {
+    const unrolledLogs = this.unrollLogs();
+    if (unrolledLogs.length == 0) {
       return Buffer.alloc(32);
     }
 
     let flattenedLogs = Buffer.alloc(0);
-    for (const logsFromSingleFunctionCall of this.unrollLogs()) {
+    for (const logsFromSingleFunctionCall of unrolledLogs) {
       flattenedLogs = Buffer.concat([flattenedLogs, logsFromSingleFunctionCall.getSiloedHash()]);
     }
     // pad the end of logs with 0s
-    for (let i = 0; i < MAX_ENCRYPTED_LOGS_PER_TX - this.unrollLogs().length; i++) {
+    for (let i = 0; i < MAX_ENCRYPTED_LOGS_PER_TX - unrolledLogs.length; i++) {
       flattenedLogs = Buffer.concat([flattenedLogs, Buffer.alloc(32)]);
     }
 

yarn-project/circuit-types/src/tx/processed_tx.ts

@@ -159,9 +159,9 @@ export function makeProcessedTx(
     data: kernelOutput,
     proof,
     // TODO(4712): deal with non-revertible logs here
-    noteEncryptedLogs: revertReason ? EncryptedNoteTxL2Logs.empty() : tx.noteEncryptedLogs,
-    encryptedLogs: revertReason ? EncryptedTxL2Logs.empty() : tx.encryptedLogs,
-    unencryptedLogs: revertReason ? UnencryptedTxL2Logs.empty() : tx.unencryptedLogs,
+    noteEncryptedLogs: tx.noteEncryptedLogs,
+    encryptedLogs: tx.encryptedLogs,
+    unencryptedLogs: tx.unencryptedLogs,
     isEmpty: false,
     revertReason,
     publicProvingRequests,

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

@@ -4,9 +4,11 @@ import {
   Fr,
   type FunctionCall,
   FunctionSelector,
+  PrivateFeePaymentMethod,
   PublicFeePaymentMethod,
   TxStatus,
   computeAuthWitMessageHash,
+  computeSecretHash,
 } from '@aztec/aztec.js';
 import { Gas, GasSettings } from '@aztec/circuits.js';
 import { FunctionType } from '@aztec/foundation/abi';
@@ -120,6 +122,103 @@ describe('e2e_fees failures', () => {
     // Can't do presently because all logs are "revertible" so we lose notes that get broadcasted during unshielding.
   });
 
+  it('reverts transactions but still pays fees using PrivateFeePaymentMethod', async () => {
+    const OutrageousPublicAmountAliceDoesNotHave = BigInt(1e8);
+    const PrivateMintedAlicePrivateBananas = BigInt(1e15);
+
+    const [initialAlicePrivateBananas, initialFPCPrivateBananas] = await t.bananaPrivateBalances(
+      aliceAddress,
+      bananaFPC.address,
+    );
+    const [initialAlicePublicBananas, initialFPCPublicBananas] = await t.bananaPublicBalances(
+      aliceAddress,
+      bananaFPC.address,
+    );
+    const [initialAliceGas, initialFPCGas, initialSequencerGas] = await t.gasBalances(
+      aliceAddress,
+      bananaFPC.address,
+      sequencerAddress,
+    );
+
+    await t.mintPrivateBananas(PrivateMintedAlicePrivateBananas, aliceAddress);
+
+    // if we simulate locally, it throws an error
+    await expect(
+      bananaCoin.methods
+        // still use a public transfer so as to fail in the public app logic phase
+        .transfer_public(aliceAddress, sequencerAddress, OutrageousPublicAmountAliceDoesNotHave, 0)
+        .send({
+          fee: {
+            gasSettings,
+            paymentMethod: new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet),
+          },
+        })
+        .wait(),
+    ).rejects.toThrow(/attempt to subtract with underflow 'hi == high'/);
+
+    // we did not pay the fee, because we did not submit the TX
+    await expectMapping(
+      t.bananaPrivateBalances,
+      [aliceAddress, bananaFPC.address],
+      [initialAlicePrivateBananas + PrivateMintedAlicePrivateBananas, initialFPCPrivateBananas],
+    );
+    await expectMapping(
+      t.bananaPublicBalances,
+      [aliceAddress, bananaFPC.address],
+      [initialAlicePublicBananas, initialFPCPublicBananas],
+    );
+    await expectMapping(t.gasBalances, [aliceAddress, bananaFPC.address], [initialAliceGas, initialFPCGas]);
+
+    // if we skip simulation, it includes the failed TX
+    const rebateSecret = Fr.random();
+    const currentSequencerL1Gas = await t.getCoinbaseBalance();
+    const txReceipt = await bananaCoin.methods
+      .transfer_public(aliceAddress, sequencerAddress, OutrageousPublicAmountAliceDoesNotHave, 0)
+      .send({
+        skipPublicSimulation: true,
+        fee: {
+          gasSettings,
+          paymentMethod: new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet, rebateSecret),
+        },
+      })
+      .wait({ dontThrowOnRevert: true });
+
+    expect(txReceipt.status).toBe(TxStatus.APP_LOGIC_REVERTED);
+    const feeAmount = txReceipt.transactionFee!;
+    const newSequencerL1Gas = await t.getCoinbaseBalance();
+    expect(newSequencerL1Gas).toEqual(currentSequencerL1Gas + feeAmount);
+
+    // and thus we paid the fee
+    await expectMapping(
+      t.bananaPrivateBalances,
+      [aliceAddress, bananaFPC.address],
+      [
+        // alice paid the maximum amount in private bananas
+        initialAlicePrivateBananas + PrivateMintedAlicePrivateBananas - gasSettings.getFeeLimit().toBigInt(),
+        initialFPCPrivateBananas,
+      ],
+    );
+    await expectMapping(
+      t.bananaPublicBalances,
+      [aliceAddress, bananaFPC.address],
+      [initialAlicePublicBananas, initialFPCPublicBananas + feeAmount],
+    );
+    await expectMapping(t.gasBalances, [aliceAddress, bananaFPC.address], [initialAliceGas, initialFPCGas - feeAmount]);
+
+    // Alice can redeem her shield to get the rebate
+    const refund = gasSettings.getFeeLimit().toBigInt() - feeAmount;
+    expect(refund).toBeGreaterThan(0n);
+    const secretHashForRebate = computeSecretHash(rebateSecret);
+    await t.addPendingShieldNoteToPXE(t.aliceWallet, refund, secretHashForRebate, txReceipt.txHash);
+    await bananaCoin.methods.redeem_shield(aliceAddress, refund, rebateSecret).send().wait();
+
+    await expectMapping(
+      t.bananaPrivateBalances,
+      [aliceAddress, bananaFPC.address],
+      [initialAlicePrivateBananas + PrivateMintedAlicePrivateBananas - feeAmount, initialFPCPrivateBananas],
+    );
+  });
+
   it('fails transaction that error in setup', async () => {
     const OutrageousPublicAmountAliceDoesNotHave = BigInt(100e12);