Skip to content
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
22 changes: 22 additions & 0 deletions yarn-project/.claude/skills/unit-test-implementation/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,28 @@ beforeEach(() => {
});
```

### NEVER Pass Complex Objects as mock() Props

`jest-mock-extended`'s `mock<T>(props)` deep-processes any objects passed as initial properties. When those objects contain class instances with internal state (like `Fr`, `EthAddress`, `AztecAddress`, `GasFees`, `Buffer`, etc.), this causes **O(2^n) exponential slowdown** across tests — each test doubles the time of the previous one.

```typescript
// ❌ NEVER: Passing complex domain objects as mock props
// This causes exponential test slowdown (1s → 2s → 4s → 8s → ...)
const constants = { chainId: new Fr(1), coinbase: EthAddress.random(), gasFees: GasFees.empty() };
beforeEach(() => {
builder = mock<CheckpointBuilder>({ checkpointNumber, constants });
});

// ✅ GOOD: Create mock without props, then set properties directly
beforeEach(() => {
builder = mock<CheckpointBuilder>();
Object.defineProperty(builder, 'checkpointNumber', { value: checkpointNumber });
Object.defineProperty(builder, 'constants', { value: constants });
});
```

Simple primitives (strings, numbers, booleans) and arrow functions are safe to pass as props. The issue is specifically with class instances that have complex prototypes.

### When to Use Real Instances vs Mocks

**Mock external dependencies** that are:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { isL1ToL2MessageReady } from '@aztec/aztec.js/messaging';
import { waitForTx } from '@aztec/aztec.js/node';
import { RollupContract } from '@aztec/ethereum/contracts';
import type { Operator } from '@aztec/ethereum/deploy-aztec-l1-contracts';
import { waitUntilL1Timestamp } from '@aztec/ethereum/l1-tx-utils';
import { asyncMap } from '@aztec/foundation/async-map';
import { CheckpointNumber, SlotNumber } from '@aztec/foundation/branded-types';
import { times, timesAsync } from '@aztec/foundation/collection';
Expand All @@ -17,6 +18,8 @@ import { retryUntil } from '@aztec/foundation/retry';
import { bufferToHex } from '@aztec/foundation/string';
import { executeTimeout } from '@aztec/foundation/timer';
import { TestContract } from '@aztec/noir-test-contracts.js/Test';
import { getSlotAtTimestamp, getTimestampForSlot } from '@aztec/stdlib/epoch-helpers';
import { GasFees } from '@aztec/stdlib/gas';
import { TxStatus } from '@aztec/stdlib/tx';

import { jest } from '@jest/globals';
Expand Down Expand Up @@ -67,6 +70,7 @@ describe('e2e_epochs/epochs_mbps', () => {
maxTxsPerBlock?: number;
buildCheckpointIfEmpty?: boolean;
deployCrossChainContract?: boolean;
skipPushProposedBlocksToArchiver?: boolean;
}) {
const { syncChainTip = 'checkpointed', deployCrossChainContract = false, ...setupOpts } = opts;

Expand Down Expand Up @@ -493,4 +497,91 @@ describe('e2e_epochs/epochs_mbps', () => {
const multiBlockCheckpoint = await assertMultipleBlocksPerSlot(2, logger);
await waitForProvenCheckpoint(multiBlockCheckpoint);
});

it('deploys a contract and calls it in separate blocks within a slot', async () => {
await setupTest({
syncChainTip: 'checkpointed',
minTxsPerBlock: 1,
maxTxsPerBlock: 1,
});

// Prepare deploy tx for a new TestContract. Get the instance address so we can construct the call tx.
const highPriority = new GasFees(100, 100);
const lowPriority = new GasFees(1, 1);

const deployMethod = TestContract.deploy(wallet);
const deployInstance = await deployMethod.getInstance();
logger.warn(`Will deploy TestContract at ${deployInstance.address}`);

// Register the contract on the PXE so we can prove the call interaction against it.
await wallet.registerContract(deployInstance, TestContract.artifact);
const deployedContract = TestContract.at(deployInstance.address, wallet);

// Pre-prove both txs before starting sequencers. This ensures both arrive in the pool
// at the same time, so the sequencer can sort by priority fee for correct ordering.
logger.warn(`Pre-proving deploy tx (high priority) and call tx (low priority)`);
const deployTx = await proveInteraction(wallet, deployMethod, {
from,
fee: { gasSettings: { maxPriorityFeesPerGas: highPriority } },
});
const callTx = await proveInteraction(wallet, deployedContract.methods.emit_nullifier_public(new Fr(42)), {
from,
fee: { gasSettings: { maxPriorityFeesPerGas: lowPriority } },
});
logger.warn(`Pre-proved both txs`);

// Start the sequencers
await Promise.all(nodes.map(n => n.getSequencer()!.start()));
logger.warn(`Started all sequencers`);

// Wait until one L1 slot before the start of the next L2 slot.
// This ensures both txs land in the pending pool right before the proposer starts building.
// REFACTOR: This should go into a shared "waitUntilNextSlotStartsBuilding" utility
const currentL1Block = await test.l1Client.getBlock({ blockTag: 'latest' });
const currentTimestamp = currentL1Block.timestamp;
const currentSlot = getSlotAtTimestamp(currentTimestamp, test.constants);
const nextSlot = SlotNumber(currentSlot + 1);
const nextSlotTimestamp = getTimestampForSlot(nextSlot, test.constants);
const targetTimestamp = nextSlotTimestamp - BigInt(test.L1_BLOCK_TIME_IN_S);
logger.warn(`Waiting until L1 timestamp ${targetTimestamp} (one L1 slot before L2 slot ${nextSlot})`, {
currentTimestamp,
currentSlot,
nextSlot,
nextSlotTimestamp,
targetTimestamp,
});
await waitUntilL1Timestamp(test.l1Client, targetTimestamp, undefined, test.L2_SLOT_DURATION_IN_S * 3);

// Send both pre-proved txs simultaneously, waiting for them to be checkpointed.
const timeout = test.L2_SLOT_DURATION_IN_S * 5;
logger.warn(`Sending both txs and waiting for checkpointed receipts`);
const [deployReceipt, callReceipt] = await executeTimeout(
() => Promise.all([deployTx.send({ wait: { timeout } }), callTx.send({ wait: { timeout } })]),
timeout * 1000,
);
logger.warn(`Both txs checkpointed`, {
deployBlock: deployReceipt.blockNumber,
callBlock: callReceipt.blockNumber,
});

// Both txs should succeed (send throws on revert). Deploy should be in an earlier block.
expect(deployReceipt.blockNumber).toBeLessThan(callReceipt.blockNumber!);

// Verify both blocks belong to the same checkpoint.
const deployCheckpointedBlock = await retryUntil(
async () => (await context.aztecNode.getCheckpointedBlocks(deployReceipt.blockNumber!, 1))[0],
'deploy checkpointed block',
timeout,
);
const callCheckpointedBlock = await retryUntil(
async () => (await context.aztecNode.getCheckpointedBlocks(callReceipt.blockNumber!, 1))[0],
'call checkpointed block',
timeout,
);
expect(deployCheckpointedBlock.checkpointNumber).toBe(callCheckpointedBlock.checkpointNumber);
logger.warn(`Both blocks in checkpoint ${deployCheckpointedBlock.checkpointNumber}`);

// Wait for the checkpoint to be proven.
await waitForProvenCheckpoint(deployCheckpointedBlock.checkpointNumber);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,9 @@ export class PublicProcessor implements Traceable {
totalBlockGas = totalBlockGas.add(processedTx.gasUsed.totalGas);
totalSizeInBytes += txSize;
totalBlobFields += txBlobFields;

// Commit the tx-level contracts checkpoint on success
this.contractsDB.commitCheckpoint();
} catch (err: any) {
if (err?.name === 'PublicProcessorTimeoutError') {
this.log.warn(`Stopping tx processing due to timeout.`);
Expand Down Expand Up @@ -354,7 +357,6 @@ export class PublicProcessor implements Traceable {
} finally {
// Base case is we always commit the checkpoint. Using the ForkCheckpoint means this has no effect if the tx was previously reverted
await checkpoint.commit();
this.contractsDB.commitCheckpointOkIfNone();
}
}

Expand Down
65 changes: 59 additions & 6 deletions yarn-project/validator-client/src/checkpoint_builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Fr } from '@aztec/foundation/curves/bn254';
import { EthAddress } from '@aztec/foundation/eth-address';
import { TestDateProvider } from '@aztec/foundation/timer';
import type { LightweightCheckpointBuilder } from '@aztec/prover-client/light';
import type { PublicProcessor } from '@aztec/simulator/server';
import type { PublicContractsDB, PublicProcessor } from '@aztec/simulator/server';
import { AztecAddress } from '@aztec/stdlib/aztec-address';
import { L2Block } from '@aztec/stdlib/block';
import type { ContractDataSource } from '@aztec/stdlib/contract';
Expand All @@ -22,17 +22,22 @@ import {
type PublicProcessorLimits,
type PublicProcessorValidator,
} from '@aztec/stdlib/interfaces/server';
import { TxHash } from '@aztec/stdlib/tx';
import type { CheckpointGlobalVariables, GlobalVariables, ProcessedTx, Tx } from '@aztec/stdlib/tx';
import {
type CheckpointGlobalVariables,
type GlobalVariables,
type ProcessedTx,
type Tx,
TxHash,
} from '@aztec/stdlib/tx';
import type { TelemetryClient } from '@aztec/telemetry-client';

import { describe, expect, it } from '@jest/globals';
import { describe, expect, it, jest } from '@jest/globals';
import { type MockProxy, mock } from 'jest-mock-extended';

import { CheckpointBuilder } from './checkpoint_builder.js';

describe('CheckpointBuilder', () => {
let checkpointBuilder: CheckpointBuilder;
let checkpointBuilder: TestCheckpointBuilder;
let lightweightCheckpointBuilder: MockProxy<LightweightCheckpointBuilder>;
let fork: MockProxy<MerkleTreeWriteOperations>;
let config: FullNodeBlockBuilderConfig;
Expand All @@ -57,6 +62,8 @@ describe('CheckpointBuilder', () => {
};

class TestCheckpointBuilder extends CheckpointBuilder {
declare public contractsDB: PublicContractsDB;

public override makeBlockBuilderDeps(_globalVariables: GlobalVariables, _fork: MerkleTreeWriteOperations) {
return Promise.resolve({ processor, validator });
}
Expand Down Expand Up @@ -101,7 +108,9 @@ describe('CheckpointBuilder', () => {
}

beforeEach(() => {
lightweightCheckpointBuilder = mock<LightweightCheckpointBuilder>({ checkpointNumber, constants });
lightweightCheckpointBuilder = mock<LightweightCheckpointBuilder>();
Object.defineProperty(lightweightCheckpointBuilder, 'checkpointNumber', { value: checkpointNumber });
Object.defineProperty(lightweightCheckpointBuilder, 'constants', { value: constants });
lightweightCheckpointBuilder.getBlocks.mockReturnValue([]);

fork = mock<MerkleTreeWriteOperations>();
Expand All @@ -117,6 +126,50 @@ describe('CheckpointBuilder', () => {
setupBuilder();
});

describe('contractsDB checkpointing', () => {
let createCheckpointSpy: jest.SpiedFunction<() => void>;
let commitCheckpointSpy: jest.SpiedFunction<() => void>;
let revertCheckpointSpy: jest.SpiedFunction<() => void>;

beforeEach(() => {
const db = checkpointBuilder.contractsDB;
createCheckpointSpy = jest.spyOn(db, 'createCheckpoint');
commitCheckpointSpy = jest.spyOn(db, 'commitCheckpoint');
revertCheckpointSpy = jest.spyOn(db, 'revertCheckpoint');

lightweightCheckpointBuilder.getBlockCount.mockReturnValue(0);
});

async function mockSuccessfulBlock() {
const block = await L2Block.random(blockNumber);
lightweightCheckpointBuilder.addBlock.mockResolvedValue({ block, timings: {} });
processor.process.mockResolvedValue([[{ hash: TxHash.random() } as ProcessedTx], [], [], [], []]);
return block;
}

it('uses the same contractsDB across multiple block builds', async () => {
await mockSuccessfulBlock();
await checkpointBuilder.buildBlock([], blockNumber, 1000n);

await mockSuccessfulBlock();
await checkpointBuilder.buildBlock([], BlockNumber(blockNumber + 1), 1001n);

expect(createCheckpointSpy).toHaveBeenCalledTimes(2);
expect(commitCheckpointSpy).toHaveBeenCalledTimes(2);
expect(revertCheckpointSpy).not.toHaveBeenCalled();
});

it('calls revertCheckpoint when public processor fails', async () => {
processor.process.mockRejectedValue(new Error('processor failure'));

await expect(checkpointBuilder.buildBlock([], blockNumber, 1000n)).rejects.toThrow('processor failure');

expect(createCheckpointSpy).toHaveBeenCalledTimes(1);
expect(commitCheckpointSpy).not.toHaveBeenCalled();
expect(revertCheckpointSpy).toHaveBeenCalledTimes(1);
});
});

describe('buildBlock', () => {
it('builds a block successfully when transactions are processed', async () => {
lightweightCheckpointBuilder.getBlockCount.mockReturnValue(0);
Expand Down
13 changes: 12 additions & 1 deletion yarn-project/validator-client/src/checkpoint_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ export type { BuildBlockInCheckpointResult } from '@aztec/stdlib/interfaces/serv
export class CheckpointBuilder implements ICheckpointBlockBuilder {
private log: Logger;

/** Persistent contracts DB shared across all blocks in this checkpoint. */
protected contractsDB: PublicContractsDB;

constructor(
private checkpointBuilder: LightweightCheckpointBuilder,
private fork: MerkleTreeWriteOperations,
Expand All @@ -60,6 +63,7 @@ export class CheckpointBuilder implements ICheckpointBlockBuilder {
...bindings,
instanceId: `checkpoint-${checkpointBuilder.checkpointNumber}`,
});
this.contractsDB = new PublicContractsDB(this.contractDataSource, this.log.getBindings());
}

getConstantData(): CheckpointGlobalVariables {
Expand Down Expand Up @@ -104,6 +108,8 @@ export class CheckpointBuilder implements ICheckpointBlockBuilder {
...this.capLimitsByCheckpointBudgets(opts),
};

// Create a block-level checkpoint on the contracts DB so we can roll back on failure
this.contractsDB.createCheckpoint();
// We execute all merkle tree operations on a world state fork checkpoint
// This enables us to discard all modifications in the event that we fail to successfully process sufficient transactions
const forkCheckpoint = await ForkCheckpoint.new(this.fork);
Expand All @@ -112,6 +118,7 @@ export class CheckpointBuilder implements ICheckpointBlockBuilder {
const [publicProcessorDuration, [processedTxs, failedTxs, usedTxs]] = await elapsed(() =>
processor.process(pendingTxs, cappedOpts, validator),
);

// Throw before updating state if we don't have enough valid txs
const minValidTxs = opts.minValidTxs ?? 0;
if (processedTxs.length < minValidTxs) {
Expand All @@ -126,6 +133,8 @@ export class CheckpointBuilder implements ICheckpointBlockBuilder {
expectedEndState: opts.expectedEndState,
});

this.contractsDB.commitCheckpoint();

this.log.debug('Built block within checkpoint', {
header: block.header.toInspect(),
processedTxs: processedTxs.map(tx => tx.hash.toString()),
Expand All @@ -140,6 +149,8 @@ export class CheckpointBuilder implements ICheckpointBlockBuilder {
usedTxs,
};
} catch (err) {
// Revert all changes to contracts db
this.contractsDB.revertCheckpoint();
// If we reached the point of committing the checkpoint, this does nothing
// Otherwise it reverts any changes made to the fork for this failed block
await forkCheckpoint.revert();
Expand Down Expand Up @@ -233,7 +244,7 @@ export class CheckpointBuilder implements ICheckpointBlockBuilder {
...(await getDefaultAllowedSetupFunctions()),
...(this.config.txPublicSetupAllowListExtend ?? []),
];
const contractsDB = new PublicContractsDB(this.contractDataSource, this.log.getBindings());
const contractsDB = this.contractsDB;
const guardedFork = new GuardedMerkleTreeOperations(fork);

const collectDebugLogs = this.debugLogStore.isEnabled;
Expand Down
Loading