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

chore: do not pass redundant txNullifier when computing notes #3943

Merged
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 0 additions & 2 deletions yarn-project/pxe/src/database/deferred_note_dao.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export const randomDeferredNoteDao = ({
contractAddress = AztecAddress.random(),
txHash = randomTxHash(),
storageSlot = Fr.random(),
txNullifier = Fr.random(),
newCommitments = [Fr.random(), Fr.random()],
dataStartIndexForTx = Math.floor(Math.random() * 100),
}: Partial<DeferredNoteDao> = {}) => {
Expand All @@ -19,7 +18,6 @@ export const randomDeferredNoteDao = ({
contractAddress,
storageSlot,
txHash,
txNullifier,
newCommitments,
dataStartIndexForTx,
);
Expand Down
6 changes: 1 addition & 5 deletions yarn-project/pxe/src/database/deferred_note_dao.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ export class DeferredNoteDao {
public contractAddress: AztecAddress,
/** The specific storage location of the note on the contract. */
public storageSlot: Fr,
/** The hash of the tx the note was created in. */
/** The hash of the tx the note was created in. Equal to the first nullifier */
public txHash: TxHash,
/** The first nullifier emitted by the transaction */
public txNullifier: Fr,
/** New commitments in this transaction, one of which belongs to this note */
public newCommitments: Fr[],
/** The next available leaf index for the note hash tree for this transaction */
Expand All @@ -34,7 +32,6 @@ export class DeferredNoteDao {
this.contractAddress.toBuffer(),
this.storageSlot.toBuffer(),
this.txHash.toBuffer(),
this.txNullifier.toBuffer(),
new Vector(this.newCommitments),
this.dataStartIndexForTx,
);
Expand All @@ -47,7 +44,6 @@ export class DeferredNoteDao {
reader.readObject(AztecAddress),
reader.readObject(Fr),
reader.readObject(TxHash),
reader.readObject(Fr),
reader.readVector(Fr),
reader.readNumber(),
);
Expand Down
13 changes: 2 additions & 11 deletions yarn-project/pxe/src/note_processor/note_processor.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ContractNotFoundError } from '@aztec/acir-simulator';
import { MAX_NEW_COMMITMENTS_PER_TX, MAX_NEW_NULLIFIERS_PER_TX, PublicKey } from '@aztec/circuits.js';
import { MAX_NEW_COMMITMENTS_PER_TX, PublicKey } from '@aztec/circuits.js';
import { Grumpkin } from '@aztec/circuits.js/barretenberg';
import { Fr } from '@aztec/foundation/fields';
import { createDebugLogger } from '@aztec/foundation/log';
Expand Down Expand Up @@ -121,10 +121,6 @@ export class NoteProcessor {
indexOfTxInABlock * MAX_NEW_COMMITMENTS_PER_TX,
(indexOfTxInABlock + 1) * MAX_NEW_COMMITMENTS_PER_TX,
);
const newNullifiers = block.newNullifiers.slice(
indexOfTxInABlock * MAX_NEW_NULLIFIERS_PER_TX,
(indexOfTxInABlock + 1) * MAX_NEW_NULLIFIERS_PER_TX,
);
// Note: Each tx generates a `TxL2Logs` object and for this reason we can rely on its index corresponding
// to the index of a tx in a block.
const txFunctionLogs = txLogs[indexOfTxInABlock].functionLogs;
Expand All @@ -136,14 +132,12 @@ export class NoteProcessor {
if (payload) {
// We have successfully decrypted the data.
const txHash = blockContext.getTxHash(indexOfTxInABlock);
const txNullifier = newNullifiers[0];
try {
const noteDao = await produceNoteDao(
this.simulator,
this.publicKey,
payload,
txHash,
txNullifier,
newCommitments,
dataStartIndexForTx,
excludedIndices,
Expand All @@ -160,7 +154,6 @@ export class NoteProcessor {
payload.contractAddress,
payload.storageSlot,
txHash,
txNullifier,
newCommitments,
dataStartIndexForTx,
);
Expand Down Expand Up @@ -254,8 +247,7 @@ export class NoteProcessor {
const excludedIndices: Set<number> = new Set();
const noteDaos: NoteDao[] = [];
for (const deferredNote of deferredNoteDaos) {
const { note, contractAddress, storageSlot, txHash, txNullifier, newCommitments, dataStartIndexForTx } =
deferredNote;
const { note, contractAddress, storageSlot, txHash, newCommitments, dataStartIndexForTx } = deferredNote;
const payload = new L1NotePayload(note, contractAddress, storageSlot);

try {
Expand All @@ -264,7 +256,6 @@ export class NoteProcessor {
this.publicKey,
payload,
txHash,
txNullifier,
newCommitments,
dataStartIndexForTx,
excludedIndices,
Expand Down
14 changes: 7 additions & 7 deletions yarn-project/pxe/src/note_processor/produce_note_dao.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ import { NoteDao } from '../database/note_dao.js';
*
* @param publicKey - The public counterpart to the private key to be used in note decryption.
* @param payload - An instance of l1NotePayload.
* @param txHash - The hash of the transaction that created the note.
* @param txNullifier - The first nullifier emitted by the transaction.
* @param txHash - The hash of the transaction that created the note. Equivalent to the first nullifier of the transaction.
* @param newCommitments - New commitments in this transaction, one of which belongs to this note.
* @param dataStartIndexForTx - The next available leaf index for the note hash tree for this transaction.
* @param excludedIndices - Indices that have been assigned a note in the same tx. Notes in a tx can have the same l1NotePayload, we need to find a different index for each replicate.
Expand All @@ -26,15 +25,14 @@ export async function produceNoteDao(
publicKey: PublicKey,
payload: L1NotePayload,
txHash: TxHash,
txNullifier: Fr,
newCommitments: Fr[],
dataStartIndexForTx: number,
excludedIndices: Set<number>,
): Promise<NoteDao> {
const { commitmentIndex, nonce, innerNoteHash, siloedNullifier } = await findNoteIndexAndNullifier(
simulator,
newCommitments,
txNullifier,
txHash,
payload,
excludedIndices,
);
Expand All @@ -61,7 +59,7 @@ export async function produceNoteDao(
* contract address, and the note associated with the l1NotePayload.
* This method assists in identifying spent commitments in the private state.
* @param commitments - Commitments in the tx. One of them should be the note's commitment.
* @param firstNullifier - First nullifier in the tx.
* @param txHash - First nullifier in the tx.
* @param l1NotePayload - An instance of l1NotePayload.
* @param excludedIndices - Indices that have been assigned a note in the same tx. Notes in a tx can have the same
* l1NotePayload. We need to find a different index for each replicate.
Expand All @@ -71,7 +69,7 @@ export async function produceNoteDao(
async function findNoteIndexAndNullifier(
simulator: AcirSimulator,
commitments: Fr[],
firstNullifier: Fr,
txHash: TxHash,
{ contractAddress, storageSlot, note }: L1NotePayload,
excludedIndices: Set<number>,
) {
Expand All @@ -81,6 +79,8 @@ async function findNoteIndexAndNullifier(
let siloedNoteHash: Fr | undefined;
let uniqueSiloedNoteHash: Fr | undefined;
let innerNullifier: Fr | undefined;
const txNullifier = Fr.fromBuffer(txHash.toBuffer());
Copy link
Contributor

Choose a reason for hiding this comment

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

Would prefer calling this a firstNullifier because we use that name in other places but it's just a nit.


for (; commitmentIndex < commitments.length; ++commitmentIndex) {
if (excludedIndices.has(commitmentIndex)) {
continue;
Expand All @@ -91,7 +91,7 @@ async function findNoteIndexAndNullifier(
break;
}

const expectedNonce = computeCommitmentNonce(firstNullifier, commitmentIndex);
const expectedNonce = computeCommitmentNonce(txNullifier, commitmentIndex);
({ innerNoteHash, siloedNoteHash, uniqueSiloedNoteHash, innerNullifier } =
await simulator.computeNoteHashAndNullifier(contractAddress, expectedNonce, storageSlot, note));
if (commitment.equals(uniqueSiloedNoteHash)) {
Expand Down
58 changes: 51 additions & 7 deletions yarn-project/types/src/l2_block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { L2Tx } from './l2_tx.js';
import { LogType, TxL2Logs } from './logs/index.js';
import { L2BlockL2Logs } from './logs/l2_block_l2_logs.js';
import { PublicDataWrite } from './public_data_write.js';
import { TxHash } from './tx/tx_hash.js';

/**
* The data that makes up the rollup proof, with encoder decoder functions.
Expand Down Expand Up @@ -620,13 +621,7 @@ export class L2Block {
* @returns The tx.
*/
getTx(txIndex: number) {
if (txIndex >= this.numberOfTxs) {
throw new Error(
`Failed to get tx ${txIndex}. Block ${this.header.globalVariables.blockNumber.toBigInt()} only has ${
this.numberOfTxs
} txs.`,
);
}
this.assertIndexInRange(txIndex);

const newCommitments = this.newCommitments
.slice(MAX_NEW_COMMITMENTS_PER_TX * txIndex, MAX_NEW_COMMITMENTS_PER_TX * (txIndex + 1))
Expand Down Expand Up @@ -659,6 +654,19 @@ export class L2Block {
);
}

/**
* A lightweight method to get the tx hash of a tx in the block.
* @param txIndex - the index of the tx in the block
* @returns a hash of the tx, which is the first nullifier in the tx
*/
getTxHash(txIndex: number): TxHash {
this.assertIndexInRange(txIndex);

const firstNullifier = this.newNullifiers[txIndex * MAX_NEW_NULLIFIERS_PER_TX];

return new TxHash(firstNullifier.toBuffer());
}

/**
* Get all the transaction in an L2 block.
* @returns The tx.
Expand Down Expand Up @@ -690,6 +698,16 @@ export class L2Block {
};
}

assertIndexInRange(txIndex: number) {
if (txIndex < 0 || txIndex >= this.numberOfTxs) {
throw new IndexOutOfRangeError({
txIndex,
numberOfTxs: this.numberOfTxs,
blockNumber: this.number,
});
}
}

// /**
// * Inspect for debugging purposes..
// * @param maxBufferSize - The number of bytes to be extracted from buffer.
Expand Down Expand Up @@ -765,3 +783,29 @@ export class L2Block {
return kernelPublicInputsLogsHash;
}
}

/**
* Custom error class for when a requested tx index is out of range.
*/
export class IndexOutOfRangeError extends Error {
constructor({
txIndex,
numberOfTxs,
blockNumber,
}: {
/**
* The requested index of the tx in the block.
*/
txIndex: number;
/**
* The number of txs in the block.
*/
numberOfTxs: number;
/**
* The number of the block.
*/
blockNumber: number;
}) {
super(`IndexOutOfRangeError: Failed to get tx at index ${txIndex}. Block ${blockNumber} has ${numberOfTxs} txs.`);
}
}
2 changes: 1 addition & 1 deletion yarn-project/types/src/l2_block_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class L2BlockContext {
* @returns The tx's hash.
*/
public getTxHash(txIndex: number): TxHash {
return this.txHashes ? this.txHashes[txIndex] : this.block.getTx(txIndex).txHash;
return this.txHashes ? this.txHashes[txIndex] : this.block.getTxHash(txIndex);
}

/**
Expand Down