feat: do not discard logs on revert since the kernel has pruned rever…

…tible logs. (#7076)

Tx objects need to look at the kernel outputs to remove logs that
reverted.
Add test of private fee payment method with revert.

fix #4712

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

@@ -52,6 +52,10 @@ export class EncryptedL2NoteLog {
     return sha256Trunc(preimage);
   }
 
+  public getSiloedHash(): Buffer {
+    return this.hash();
+  }
+
   /**
    * Crates a random log.
    * @returns A random log.

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

@@ -37,9 +37,8 @@ export abstract class FunctionL2Logs<TLog extends UnencryptedL2Log | EncryptedL2
    * @returns Total length of serialized data.
    */
   public getSerializedLength(): number {
-    // Adding 4 to each log's length to account for the size stored in the serialized buffer and then one more time
     // adding 4 for the resulting buffer length.
-    return this.logs.reduce((acc, log) => acc + log.length + 4, 0) + 4;
+    return this.getKernelLength() + 4;
   }
 
   /**

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

@@ -1,4 +1,6 @@
 import {
+  Fr,
+  type LogHash,
   MAX_ENCRYPTED_LOGS_PER_TX,
   MAX_NOTE_ENCRYPTED_LOGS_PER_TX,
   MAX_UNENCRYPTED_LOGS_PER_TX,
@@ -22,6 +24,8 @@ import { type UnencryptedL2Log } from './unencrypted_l2_log.js';
  * Data container of logs emitted in 1 tx.
  */
 export abstract class TxL2Logs<TLog extends UnencryptedL2Log | EncryptedL2NoteLog | EncryptedL2Log> {
+  abstract hash(): Buffer;
+
   constructor(
     /** * An array containing logs emitted in individual function invocations in this tx. */
     public readonly functionLogs: FunctionL2Logs<TLog>[],
@@ -94,6 +98,28 @@ export abstract class TxL2Logs<TLog extends UnencryptedL2Log | EncryptedL2NoteLo
   public equals(other: TxL2Logs<TLog>): boolean {
     return isEqual(this, other);
   }
+
+  /**
+   * Filter the logs from functions from this TxL2Logs that
+   * appear in the provided logHashes
+   * @param logHashes hashes we want to keep
+   * @param output our aggregation
+   * @returns our aggregation
+   */
+  public filter(logHashes: LogHash[], output: TxL2Logs<TLog>): TxL2Logs<TLog> {
+    for (const fnLogs of this.functionLogs) {
+      let include = false;
+      for (const log of fnLogs.logs) {
+        if (logHashes.findIndex(lh => lh.value.equals(Fr.fromBuffer(log.getSiloedHash()))) !== -1) {
+          include = true;
+        }
+      }
+      if (include) {
+        output.addFunctionLogs([fnLogs]);
+      }
+    }
+    return output;
+  }
 }
 
 export class UnencryptedTxL2Logs extends TxL2Logs<UnencryptedL2Log> {
@@ -156,17 +182,18 @@ export class UnencryptedTxL2Logs extends TxL2Logs<UnencryptedL2Log> {
    * Note: This is a TS implementation of `computeKernelUnencryptedLogsHash` function in Decoder.sol. See that function documentation
    *       for more details.
    */
-  public hash(): Buffer {
-    if (this.unrollLogs().length == 0) {
+  public override hash(): Buffer {
+    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)]);
     }
 
@@ -234,17 +261,18 @@ export class EncryptedNoteTxL2Logs extends TxL2Logs<EncryptedL2NoteLog> {
    * Note: This is a TS implementation of `computeKernelNoteEncryptedLogsHash` function in Decoder.sol. See that function documentation
    *       for more details.
    */
-  public hash(): Buffer {
-    if (this.unrollLogs().length == 0) {
+  public override hash(): Buffer {
+    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)]);
     }
 
@@ -312,17 +340,18 @@ export class EncryptedTxL2Logs extends TxL2Logs<EncryptedL2Log> {
    * Note: This is a TS implementation of `computeKernelEncryptedLogsHash` function in Decoder.sol. See that function documentation
    *       for more details.
    */
-  public hash(): Buffer {
-    if (this.unrollLogs().length == 0) {
+  public override hash(): Buffer {
+    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/mocks.ts

@@ -107,24 +107,50 @@ export const mockTx = (
 
     if (hasLogs) {
       let i = 1; // 0 used in first nullifier
-      encryptedLogs.functionLogs.forEach((log, j) => {
-        // ts complains if we dont check .forPublic here, even though it is defined ^
-        if (data.forPublic) {
-          data.forPublic.end.encryptedLogsHashes[j] = new LogHash(
-            Fr.fromBuffer(log.hash()),
-            i++,
-            new Fr(log.toBuffer().length),
-          );
-        }
+      let nonRevertibleIndex = 0;
+      let revertibleIndex = 0;
+      let functionCount = 0;
+      encryptedLogs.functionLogs.forEach(functionLog => {
+        functionLog.logs.forEach(log => {
+          // ts complains if we dont check .forPublic here, even though it is defined ^
+          if (data.forPublic) {
+            const hash = new LogHash(
+              Fr.fromBuffer(log.getSiloedHash()),
+              i++,
+              // +4 for encoding the length of the buffer
+              new Fr(log.length + 4),
+            );
+            // make the first log non-revertible
+            if (functionCount === 0) {
+              data.forPublic.endNonRevertibleData.encryptedLogsHashes[nonRevertibleIndex++] = hash;
+            } else {
+              data.forPublic.end.encryptedLogsHashes[revertibleIndex++] = hash;
+            }
+          }
+        });
+        functionCount++;
       });
-      unencryptedLogs.functionLogs.forEach((log, j) => {
-        if (data.forPublic) {
-          data.forPublic.end.unencryptedLogsHashes[j] = new LogHash(
-            Fr.fromBuffer(log.hash()),
-            i++,
-            new Fr(log.toBuffer().length),
-          );
-        }
+      nonRevertibleIndex = 0;
+      revertibleIndex = 0;
+      functionCount = 0;
+      unencryptedLogs.functionLogs.forEach(functionLog => {
+        functionLog.logs.forEach(log => {
+          if (data.forPublic) {
+            const hash = new LogHash(
+              Fr.fromBuffer(log.getSiloedHash()),
+              i++,
+              // +4 for encoding the length of the buffer
+              new Fr(log.length + 4),
+            );
+            // make the first log non-revertible
+            if (functionCount === 0) {
+              data.forPublic.endNonRevertibleData.unencryptedLogsHashes[nonRevertibleIndex++] = hash;
+            } else {
+              data.forPublic.end.unencryptedLogsHashes[revertibleIndex++] = hash;
+            }
+          }
+        });
+        functionCount++;
       });
     }
   } else {

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

@@ -160,9 +160,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/circuit-types/src/tx/tx.ts

@@ -3,6 +3,7 @@ import {
   PrivateKernelTailCircuitPublicInputs,
   Proof,
   PublicCallRequest,
+  type PublicKernelCircuitPublicInputs,
 } from '@aztec/circuits.js';
 import { arraySerializedSizeOfNonEmpty } from '@aztec/foundation/collection';
 import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize';
@@ -29,15 +30,15 @@ export class Tx {
     /**
      * Encrypted note logs generated by the tx.
      */
-    public readonly noteEncryptedLogs: EncryptedNoteTxL2Logs,
+    public noteEncryptedLogs: EncryptedNoteTxL2Logs,
     /**
      * Encrypted logs generated by the tx.
      */
-    public readonly encryptedLogs: EncryptedTxL2Logs,
+    public encryptedLogs: EncryptedTxL2Logs,
     /**
      * Unencrypted logs generated by the tx.
      */
-    public readonly unencryptedLogs: UnencryptedTxL2Logs,
+    public unencryptedLogs: UnencryptedTxL2Logs,
     /**
      * Enqueued public functions from the private circuit to be run by the sequencer.
      * Preimages of the public call stack entries from the private kernel circuit output.
@@ -249,6 +250,37 @@ export class Tx {
       publicTeardownFunctionCall,
     );
   }
+
+  /**
+   * Filters out logs from functions that are not present in the provided kernel output.
+   *
+   * The purpose of this is to remove logs that got dropped due to a revert,
+   * in which case, we only have the kernel's hashes to go on, as opposed to
+   * this grouping by function maintained in this class.
+   *
+   * The logic therefore is to drop all FunctionLogs if any constituent hash
+   * does not appear in the provided hashes: it is impossible for part of a
+   * function to revert.
+   *
+   * @param logHashes the individual log hashes we want to keep
+   * @param out the output to put passing logs in, to keep this function abstract
+   */
+  public filterRevertedLogs(kernelOutput: PublicKernelCircuitPublicInputs) {
+    this.encryptedLogs = this.encryptedLogs.filter(
+      kernelOutput.endNonRevertibleData.encryptedLogsHashes,
+      EncryptedTxL2Logs.empty(),
+    );
+
+    this.unencryptedLogs = this.unencryptedLogs.filter(
+      kernelOutput.endNonRevertibleData.unencryptedLogsHashes,
+      UnencryptedTxL2Logs.empty(),
+    );
+
+    this.noteEncryptedLogs = this.noteEncryptedLogs.filter(
+      kernelOutput.endNonRevertibleData.noteEncryptedLogsHashes,
+      EncryptedNoteTxL2Logs.empty(),
+    );
+  }
 }
 
 /** Utility type for an entity that has a hash property for a txhash */