Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -867,17 +867,17 @@ void ContentAddressedCachedTreeStore<LeafValueType>::advance_finalized_block(con
ReadTransactionPtr readTx = create_read_transaction();
get_meta(uncommittedMeta);
get_meta(committedMeta, *readTx, false);
// do nothing if the block is already finalized
if (committedMeta.finalizedBlockHeight >= blockNumber) {
return;
}
if (!dataStore_->read_block_data(blockNumber, blockPayload, *readTx)) {
throw std::runtime_error(format("Unable to advance finalized block: ",
blockNumber,
". Failed to read block data. Tree name: ",
forkConstantData_.name_));
}
}
// do nothing if the block is already finalized
if (committedMeta.finalizedBlockHeight >= blockNumber) {
return;
}

// can currently only finalize up to the unfinalized block height
if (committedMeta.finalizedBlockHeight > committedMeta.unfinalizedBlockHeight) {
Expand Down
102 changes: 99 additions & 3 deletions noir-projects/aztec-nr/aztec/src/oracle/block_header.nr
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,107 @@ fn constrain_get_block_header_at_internal(
}

mod test {
use crate::protocol::traits::Hash;
use crate::test::helpers::test_environment::TestEnvironment;
use super::{constrain_get_block_header_at_internal, get_block_header_at_internal};
use super::{constrain_get_block_header_at_internal, get_block_header_at, get_block_header_at_internal};

#[test(should_fail_with = "Proving membership of a block in archive failed")]
unconstrained fn fetching_header_with_mismatched_block_number_should_fail() {
#[test]
unconstrained fn fetching_earliest_block_header_succeeds() {
let env = TestEnvironment::new();

env.mine_block();
env.mine_block();
env.mine_block();
env.mine_block();

env.private_context(|context| {
let anchor_block_header = context.anchor_block_header;

let header = get_block_header_at_internal(1);
constrain_get_block_header_at_internal(header, 1, anchor_block_header);

assert_eq(header.block_number(), 1);
});
}

#[test]
unconstrained fn fetching_past_block_header_succeeds() {
let env = TestEnvironment::new();

env.mine_block();
env.mine_block();
env.mine_block();
env.mine_block();

env.private_context(|context| {
let anchor_block_header = context.anchor_block_header;
let target_block_number = anchor_block_header.block_number() - 2;

let header = get_block_header_at_internal(target_block_number);
constrain_get_block_header_at_internal(header, target_block_number, anchor_block_header);

assert_eq(header.block_number(), target_block_number);
});
}

#[test]
unconstrained fn fetching_header_immediately_before_anchor_succeeds() {
let env = TestEnvironment::new();

env.mine_block();
env.mine_block();
env.mine_block();
env.mine_block();

// Block N-1 is the boundary case: last_archive covers exactly up to block N-1.
env.private_context(|context| {
let anchor_block_header = context.anchor_block_header;
let target_block_number = anchor_block_header.block_number() - 1;

let header = get_block_header_at_internal(target_block_number);
constrain_get_block_header_at_internal(header, target_block_number, anchor_block_header);

assert_eq(header.block_number(), target_block_number);
});
}

#[test]
unconstrained fn fetching_anchor_block_header_works() {
let env = TestEnvironment::new();

env.mine_block();
env.mine_block();
env.mine_block();
env.mine_block();

env.private_context(|context| {
let anchor_block_number = context.anchor_block_header.block_number();

let header = get_block_header_at(anchor_block_number, *context);

assert_eq(header.block_number(), anchor_block_number);
assert_eq(header.hash(), context.anchor_block_header.hash());
});
}

#[test(should_fail_with = "Last archive block number is smaller than the block number")]
unconstrained fn fetching_future_block_header_fails() {
let env = TestEnvironment::new();

env.mine_block();
env.mine_block();
env.mine_block();
env.mine_block();

env.private_context(|context| {
let anchor_block_number = context.anchor_block_header.block_number();

let _header = get_block_header_at(anchor_block_number + 1, *context);
});
}

#[test(should_fail_with = "Block number provided is not the same as the block number from the header hint")]
unconstrained fn fetching_header_with_mismatched_block_number_fails() {
let env = TestEnvironment::new();

env.mine_block();
Expand Down
1 change: 1 addition & 0 deletions spartan/environments/staging-public.env
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ SEQ_MAX_TX_PER_CHECKPOINT=7 # 0.1 TPS
# Build checkpoint even if block is empty.
SEQ_BUILD_CHECKPOINT_IF_EMPTY=true
SEQ_BLOCK_DURATION_MS=6000
SEQ_L1_PUBLISHING_TIME_ALLOWANCE_IN_SLOT=36

CREATE_ROLLUP_CONTRACTS=false
P2P_TX_POOL_DELETE_TXS_AFTER_REORG=true
Expand Down
4 changes: 0 additions & 4 deletions spartan/scripts/deploy_network.sh
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,7 @@ PROVER_FAILED_PROOF_STORE=${PROVER_FAILED_PROOF_STORE:-}
SEQ_MIN_TX_PER_BLOCK=${SEQ_MIN_TX_PER_BLOCK:-1}
SEQ_MAX_TX_PER_BLOCK=${SEQ_MAX_TX_PER_BLOCK:-null}
SEQ_MAX_TX_PER_CHECKPOINT=${SEQ_MAX_TX_PER_CHECKPOINT:-8}
<<<<<<< HEAD
SEQ_PER_BLOCK_ALLOCATION_MULTIPLIER=${SEQ_PER_BLOCK_ALLOCATION_MULTIPLIER:-2}
=======
SEQ_PER_BLOCK_ALLOCATION_MULTIPLIER=${SEQ_PER_BLOCK_ALLOCATION_MULTIPLIER:-}
>>>>>>> origin/v4
SEQ_BLOCK_DURATION_MS=${SEQ_BLOCK_DURATION_MS:-}
SEQ_L1_PUBLISHING_TIME_ALLOWANCE_IN_SLOT=${SEQ_L1_PUBLISHING_TIME_ALLOWANCE_IN_SLOT:-}
SEQ_BUILD_CHECKPOINT_IF_EMPTY=${SEQ_BUILD_CHECKPOINT_IF_EMPTY:-}
Expand Down
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
2 changes: 1 addition & 1 deletion yarn-project/archiver/src/modules/data_store_updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ export class ArchiverDataStoreUpdater {
if (validFnCount > 0) {
this.log.verbose(`Storing ${validFnCount} functions for contract class ${contractClassId.toString()}`);
}
return await this.store.addFunctions(contractClassId, validPrivateFns, validUtilityFns);
await this.store.addFunctions(contractClassId, validPrivateFns, validUtilityFns);
}
return true;
}
Expand Down
25 changes: 24 additions & 1 deletion yarn-project/aztec-node/src/aztec-node/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,11 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable {
referenceBlock: BlockParameter,
blockHash: BlockHash,
): Promise<MembershipWitness<typeof ARCHIVE_HEIGHT> | undefined> {
const committedDb = await this.getWorldState(referenceBlock);
// The Noir circuit checks the archive membership proof against `anchor_block_header.last_archive.root`,
// which is the archive tree root BEFORE the anchor block was added (i.e. the state after block N-1).
// So we need the world state at block N-1, not block N, to produce a sibling path matching that root.
const referenceBlockNumber = await this.resolveBlockNumber(referenceBlock);
const committedDb = await this.getWorldState(BlockNumber(referenceBlockNumber - 1));
const [pathAndIndex] = await committedDb.findSiblingPaths<MerkleTreeId.ARCHIVE>(MerkleTreeId.ARCHIVE, [blockHash]);
return pathAndIndex === undefined
? undefined
Expand Down Expand Up @@ -1660,6 +1664,25 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable {
return snapshot;
}

/** Resolves a block parameter to a block number. */
protected async resolveBlockNumber(block: BlockParameter): Promise<BlockNumber> {
if (block === 'latest') {
return BlockNumber(await this.blockSource.getBlockNumber());
}
if (BlockHash.isBlockHash(block)) {
const initialBlockHash = await this.#getInitialHeaderHash();
if (block.equals(initialBlockHash)) {
return BlockNumber.ZERO;
}
const header = await this.blockSource.getBlockHeaderByHash(block);
if (!header) {
throw new Error(`Block hash ${block.toString()} not found.`);
}
return header.getBlockNumber();
}
return block as BlockNumber;
}

/**
* Ensure we fully sync the world state
* @returns A promise that fulfils once the world state is synced
Expand Down
43 changes: 23 additions & 20 deletions yarn-project/stdlib/src/p2p/checkpoint_proposal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,29 +178,32 @@ export class CheckpointProposal extends Gossipable {
blockNumber: lastBlockInfo?.blockHeader?.globalVariables.blockNumber ?? BlockNumber(0),
dutyType: DutyType.CHECKPOINT_PROPOSAL,
};
const checkpointSignature = await payloadSigner(checkpointHash, checkpointContext);

if (!lastBlockInfo) {
return new CheckpointProposal(checkpointHeader, archiveRoot, feeAssetPriceModifier, checkpointSignature);
if (lastBlockInfo) {
// Sign block proposal before signing checkpoint proposal to ensure HA protection
const lastBlockProposal = await BlockProposal.createProposalFromSigner(
lastBlockInfo.blockHeader,
lastBlockInfo.indexWithinCheckpoint,
checkpointHeader.inHash,
archiveRoot,
lastBlockInfo.txHashes,
lastBlockInfo.txs,
payloadSigner,
);

const checkpointSignature = await payloadSigner(checkpointHash, checkpointContext);

return new CheckpointProposal(checkpointHeader, archiveRoot, feeAssetPriceModifier, checkpointSignature, {
blockHeader: lastBlockInfo.blockHeader,
indexWithinCheckpoint: lastBlockInfo.indexWithinCheckpoint,
txHashes: lastBlockInfo.txHashes,
signature: lastBlockProposal.signature,
signedTxs: lastBlockProposal.signedTxs,
});
}

const lastBlockProposal = await BlockProposal.createProposalFromSigner(
lastBlockInfo.blockHeader,
lastBlockInfo.indexWithinCheckpoint,
checkpointHeader.inHash,
archiveRoot,
lastBlockInfo.txHashes,
lastBlockInfo.txs,
payloadSigner,
);

return new CheckpointProposal(checkpointHeader, archiveRoot, feeAssetPriceModifier, checkpointSignature, {
blockHeader: lastBlockInfo.blockHeader,
indexWithinCheckpoint: lastBlockInfo.indexWithinCheckpoint,
txHashes: lastBlockInfo.txHashes,
signature: lastBlockProposal.signature,
signedTxs: lastBlockProposal.signedTxs,
});
const checkpointSignature = await payloadSigner(checkpointHash, checkpointContext);
return new CheckpointProposal(checkpointHeader, archiveRoot, feeAssetPriceModifier, checkpointSignature);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/validator-client/src/block_proposal_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,9 @@ export class BlockProposalHandler {
}

private getReexecuteFailureReason(err: any): BlockProposalValidationFailureReason {
if (err instanceof ReExInitialStateMismatchError) {
if (err instanceof TransactionsNotAvailableError) {
return 'txs_not_available';
} else if (err instanceof ReExInitialStateMismatchError) {
return 'initial_state_mismatch';
} else if (err instanceof ReExStateMismatchError) {
return 'state_mismatch';
Expand Down
4 changes: 3 additions & 1 deletion yarn-project/validator-client/src/checkpoint_builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,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 Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,18 @@ export class ServerWorldStateSynchronizer

private async handleChainFinalized(blockNumber: BlockNumber) {
this.log.verbose(`Finalized chain is now at block ${blockNumber}`);
// If the finalized block number is older than the oldest available block in world state,
// skip entirely. The finalized block number can jump backwards (e.g. when the finalization
// heuristic changes) and try to read block data that has already been pruned. When this
// happens, there is nothing useful to do — the native world state is already finalized
// past this point and pruning has already happened.
const currentSummary = await this.merkleTreeDb.getStatusSummary();
if (blockNumber < currentSummary.oldestHistoricalBlock || blockNumber < 1) {
this.log.trace(
`Finalized block ${blockNumber} is older than the oldest available block ${currentSummary.oldestHistoricalBlock}. Skipping.`,
);
return;
}
const summary = await this.merkleTreeDb.setFinalized(blockNumber);
if (this.historyToKeep === undefined) {
return;
Expand Down Expand Up @@ -421,6 +433,12 @@ export class ServerWorldStateSynchronizer
}
// Find the block at the start of the checkpoint and remove blocks up to this one
const newHistoricBlock = historicCheckpoint.checkpoint.blocks[0];
if (newHistoricBlock.number <= currentSummary.oldestHistoricalBlock) {
this.log.debug(
`Historic block ${newHistoricBlock.number} is not newer than oldest available ${currentSummary.oldestHistoricalBlock}. Skipping prune.`,
);
return;
}
this.log.verbose(`Pruning historic blocks to ${newHistoricBlock.number}`);
const status = await this.merkleTreeDb.removeHistoricalBlocks(BlockNumber(newHistoricBlock.number));
this.log.debug(`World state summary `, status.summary);
Expand Down
38 changes: 38 additions & 0 deletions yarn-project/world-state/src/test/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,44 @@ describe('world-state integration', () => {
await awaitSync(5, 4);
await expectSynchedToBlock(5, 4);
});

it('does not throw when finalized block jumps backwards past pruned blocks', async () => {
// Create 20 blocks and sync them all
await archiver.createBlocks(MAX_CHECKPOINT_COUNT);
await synchronizer.start();
await awaitSync(MAX_CHECKPOINT_COUNT);
await expectSynchedToBlock(MAX_CHECKPOINT_COUNT);

// Manually finalize to block 15 and prune historical blocks up to block 10
// to simulate world-state having pruned old data.
await db.setFinalized(BlockNumber(15));
await db.removeHistoricalBlocks(BlockNumber(10));

const summary = await db.getStatusSummary();
log.info(
`After manual finalize+prune: oldest=${summary.oldestHistoricalBlock}, finalized=${summary.finalizedBlockNumber}`,
);
expect(summary.oldestHistoricalBlock).toBe(10);
expect(summary.finalizedBlockNumber).toBe(15);

// Now simulate the scenario from PR #21597: finalized block jumps backwards
// to a block M that is older than oldestHistoricalBlock.
// This should NOT throw — the clamping logic should handle it.
const backwardsFinalized = BlockNumber(5);
log.info(
`Sending chain-finalized for block ${backwardsFinalized} (below oldest ${summary.oldestHistoricalBlock})`,
);
await expect(
synchronizer.handleBlockStreamEvent({
type: 'chain-finalized',
block: { number: backwardsFinalized, hash: '' },
}),
).resolves.not.toThrow();

// Finalized block should remain at 15 (unchanged by the backwards event)
const afterSummary = await db.getStatusSummary();
expect(afterSummary.finalizedBlockNumber).toBe(15);
});
});
});

Expand Down
Loading