From d959139adcf3aa58b7c4da85ebe3a3bf6a9461b0 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Fri, 19 Sep 2025 18:40:42 -0300 Subject: [PATCH] fix: Rejecting txs with duplicate nullifiers --- .../end-to-end/src/e2e_event_logs.test.ts | 8 ++++++-- .../tx_validator/double_spend_validator.test.ts | 9 +++++---- .../tx_validator/double_spend_validator.ts | 2 +- .../public/public_processor/public_processor.ts | 16 +++------------- 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_event_logs.test.ts b/yarn-project/end-to-end/src/e2e_event_logs.test.ts index 8865f8b892d2..d4355092db41 100644 --- a/yarn-project/end-to-end/src/e2e_event_logs.test.ts +++ b/yarn-project/end-to-end/src/e2e_event_logs.test.ts @@ -1,4 +1,4 @@ -import { AztecAddress, type AztecNode, Fr, type Wallet, getDecodedPublicEvents } from '@aztec/aztec.js'; +import { AztecAddress, type AztecNode, Fr, type Logger, type Wallet, getDecodedPublicEvents } from '@aztec/aztec.js'; import { makeTuple } from '@aztec/foundation/array'; import { timesParallel } from '@aztec/foundation/collection'; import type { Tuple } from '@aztec/foundation/serialize'; @@ -20,6 +20,7 @@ describe('Logs', () => { let account1Address: AztecAddress; let account2Address: AztecAddress; + let log: Logger; let teardown: () => Promise; beforeAll(async () => { @@ -28,10 +29,13 @@ describe('Logs', () => { wallet, accounts: [account1Address, account2Address], aztecNode, + logger: log, } = await setup(2)); - await ensureAccountContractsPublished(wallet, [account1Address, account1Address]); + log.warn(`Setup complete, checking account contracts published`); + await ensureAccountContractsPublished(wallet, [account1Address, account2Address]); + log.warn(`Deploying test contract`); testLogContract = await TestLogContract.deploy(wallet).send({ from: account1Address }).deployed(); }); diff --git a/yarn-project/p2p/src/msg_validators/tx_validator/double_spend_validator.test.ts b/yarn-project/p2p/src/msg_validators/tx_validator/double_spend_validator.test.ts index 2085acbab360..a4b03031ea05 100644 --- a/yarn-project/p2p/src/msg_validators/tx_validator/double_spend_validator.test.ts +++ b/yarn-project/p2p/src/msg_validators/tx_validator/double_spend_validator.test.ts @@ -1,3 +1,4 @@ +import { Fr } from '@aztec/foundation/fields'; import { mockTx, mockTxForRollup } from '@aztec/stdlib/testing'; import { type AnyTx, TX_ERROR_DUPLICATE_NULLIFIER_IN_TX, TX_ERROR_EXISTING_NULLIFIER } from '@aztec/stdlib/tx'; @@ -27,8 +28,8 @@ describe('DoubleSpendTxValidator', () => { numberOfNonRevertiblePublicCallRequests: 1, numberOfRevertiblePublicCallRequests: 0, }); - badTx.data.forPublic!.nonRevertibleAccumulatedData.nullifiers[1] = - badTx.data.forPublic!.nonRevertibleAccumulatedData.nullifiers[0]; + const nullifiers = badTx.data.forPublic!.nonRevertibleAccumulatedData.nullifiers; + nullifiers[1] = new Fr(nullifiers[0].toBigInt()); await expectInvalid(badTx, TX_ERROR_DUPLICATE_NULLIFIER_IN_TX); }); @@ -38,8 +39,8 @@ describe('DoubleSpendTxValidator', () => { numberOfRevertiblePublicCallRequests: 1, numberOfRevertibleNullifiers: 1, }); - badTx.data.forPublic!.revertibleAccumulatedData.nullifiers[1] = - badTx.data.forPublic!.revertibleAccumulatedData.nullifiers[0]; + const nullifiers = badTx.data.forPublic!.revertibleAccumulatedData.nullifiers; + nullifiers[1] = new Fr(nullifiers[0].toBigInt()); await expectInvalid(badTx, TX_ERROR_DUPLICATE_NULLIFIER_IN_TX); }); diff --git a/yarn-project/p2p/src/msg_validators/tx_validator/double_spend_validator.ts b/yarn-project/p2p/src/msg_validators/tx_validator/double_spend_validator.ts index 308cbbd0f976..66b680ba4edb 100644 --- a/yarn-project/p2p/src/msg_validators/tx_validator/double_spend_validator.ts +++ b/yarn-project/p2p/src/msg_validators/tx_validator/double_spend_validator.ts @@ -24,7 +24,7 @@ export class DoubleSpendTxValidator implements TxValidator { const nullifiers = tx instanceof Tx ? tx.data.getNonEmptyNullifiers() : tx.txEffect.nullifiers; // Ditch this tx if it has repeated nullifiers - const uniqueNullifiers = new Set(nullifiers); + const uniqueNullifiers = new Set(nullifiers.map(n => n.toBigInt())); if (uniqueNullifiers.size !== nullifiers.length) { this.#log.verbose(`Rejecting tx ${'txHash' in tx ? tx.txHash : tx.hash} for emitting duplicate nullifiers`); return { result: 'invalid', reason: [TX_ERROR_DUPLICATE_NULLIFIER_IN_TX] }; diff --git a/yarn-project/simulator/src/public/public_processor/public_processor.ts b/yarn-project/simulator/src/public/public_processor/public_processor.ts index e919821cb6ee..ffb7efef2529 100644 --- a/yarn-project/simulator/src/public/public_processor/public_processor.ts +++ b/yarn-project/simulator/src/public/public_processor/public_processor.ts @@ -27,7 +27,6 @@ import { StateReference, Tx, TxExecutionPhase, - type TxValidator, makeProcessedTxFromPrivateOnlyTx, makeProcessedTxFromTxWithPublicCalls, } from '@aztec/stdlib/tx'; @@ -384,10 +383,7 @@ export class PublicProcessor implements Traceable { return [processedTx, returnValues ?? []]; } - private async doTreeInsertionsForPrivateOnlyTx( - processedTx: ProcessedTx, - txValidator?: TxValidator, - ): Promise { + private async doTreeInsertionsForPrivateOnlyTx(processedTx: ProcessedTx): Promise { const treeInsertionStart = process.hrtime.bigint(); // Update the state so that the next tx in the loop has the correct .startState @@ -406,14 +402,8 @@ export class PublicProcessor implements Traceable { padArrayEnd(processedTx.txEffect.nullifiers, Fr.ZERO, MAX_NULLIFIERS_PER_TX).map(n => n.toBuffer()), NULLIFIER_SUBTREE_HEIGHT, ); - } catch { - if (txValidator) { - // Ideally the validator has already caught this above, but just in case: - throw new Error(`Transaction ${processedTx.hash} invalid after processing public functions`); - } else { - // We have no validator and assume this call should blindly process txs with duplicates being caught later - this.log.warn(`Detected duplicate nullifier after public processing for: ${processedTx.hash}.`); - } + } catch (cause) { + throw new Error(`Transaction ${processedTx.hash} failed with duplicate nullifiers`, { cause }); } const treeInsertionEnd = process.hrtime.bigint();