Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: do not discard logs on revert since the kernel has pruned revertible logs. #7076

Merged
merged 2 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions yarn-project/circuit-types/src/logs/encrypted_l2_note_log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ export class EncryptedL2NoteLog {
return sha256Trunc(preimage);
}

public getSiloedHash(): Buffer {
return this.hash();
}

/**
* Crates a random log.
* @returns A random log.
Expand Down
3 changes: 1 addition & 2 deletions yarn-project/circuit-types/src/logs/function_l2_logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
53 changes: 41 additions & 12 deletions yarn-project/circuit-types/src/logs/tx_l2_logs.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import {
Fr,
type LogHash,
MAX_ENCRYPTED_LOGS_PER_TX,
MAX_NOTE_ENCRYPTED_LOGS_PER_TX,
MAX_UNENCRYPTED_LOGS_PER_TX,
Expand All @@ -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>[],
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending how many logs a tx hash, it might be worth adding a break here? Or creating a set out of the siloed hashes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, but there are currently only 8

}
}
if (include) {
output.addFunctionLogs([fnLogs]);
}
}
return output;
}
}

export class UnencryptedTxL2Logs extends TxL2Logs<UnencryptedL2Log> {
Expand Down Expand Up @@ -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)]);
}

Expand Down Expand Up @@ -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)]);
}

Expand Down Expand Up @@ -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)]);
}

Expand Down
60 changes: 43 additions & 17 deletions yarn-project/circuit-types/src/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions yarn-project/circuit-types/src/tx/processed_tx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
38 changes: 35 additions & 3 deletions yarn-project/circuit-types/src/tx/tx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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.
Expand Down Expand Up @@ -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 */
Expand Down
Loading
Loading