diff --git a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp index 01888967c45b..8093d2884049 100644 --- a/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp +++ b/barretenberg/cpp/src/barretenberg/crypto/merkle_tree/node_store/cached_content_addressed_tree_store.hpp @@ -867,6 +867,10 @@ void ContentAddressedCachedTreeStore::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, @@ -874,10 +878,6 @@ void ContentAddressedCachedTreeStore::advance_finalized_block(con 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) { diff --git a/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr b/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr index d1f7b81243e2..b213b73e4a7b 100644 --- a/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr +++ b/noir-projects/aztec-nr/aztec/src/oracle/block_header.nr @@ -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(); diff --git a/spartan/environments/staging-public.env b/spartan/environments/staging-public.env index fd3c5b52df47..fa894ca74041 100644 --- a/spartan/environments/staging-public.env +++ b/spartan/environments/staging-public.env @@ -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 diff --git a/spartan/scripts/deploy_network.sh b/spartan/scripts/deploy_network.sh index 9a5300e83796..2973265afd68 100755 --- a/spartan/scripts/deploy_network.sh +++ b/spartan/scripts/deploy_network.sh @@ -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:-} diff --git a/yarn-project/.claude/skills/unit-test-implementation/SKILL.md b/yarn-project/.claude/skills/unit-test-implementation/SKILL.md index 0bc59961fb87..da1c1eab66f7 100644 --- a/yarn-project/.claude/skills/unit-test-implementation/SKILL.md +++ b/yarn-project/.claude/skills/unit-test-implementation/SKILL.md @@ -23,6 +23,28 @@ beforeEach(() => { }); ``` +### NEVER Pass Complex Objects as mock() Props + +`jest-mock-extended`'s `mock(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({ checkpointNumber, constants }); +}); + +// ✅ GOOD: Create mock without props, then set properties directly +beforeEach(() => { + builder = mock(); + 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: diff --git a/yarn-project/archiver/src/modules/data_store_updater.ts b/yarn-project/archiver/src/modules/data_store_updater.ts index 6bf2975b3c3f..40875ecec4e6 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.ts @@ -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; } diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index cb373bc7d042..30de30bf7966 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -1038,7 +1038,11 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable { referenceBlock: BlockParameter, blockHash: BlockHash, ): Promise | 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, [blockHash]); return pathAndIndex === undefined ? undefined @@ -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 { + 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 diff --git a/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts b/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts index ae84e24204d7..aa837388211c 100644 --- a/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts +++ b/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts @@ -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); } /** diff --git a/yarn-project/validator-client/src/block_proposal_handler.ts b/yarn-project/validator-client/src/block_proposal_handler.ts index 43c890bdafa8..1582c74b334c 100644 --- a/yarn-project/validator-client/src/block_proposal_handler.ts +++ b/yarn-project/validator-client/src/block_proposal_handler.ts @@ -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'; diff --git a/yarn-project/validator-client/src/checkpoint_builder.test.ts b/yarn-project/validator-client/src/checkpoint_builder.test.ts index 903615bfa1d8..609b42426cff 100644 --- a/yarn-project/validator-client/src/checkpoint_builder.test.ts +++ b/yarn-project/validator-client/src/checkpoint_builder.test.ts @@ -101,7 +101,9 @@ describe('CheckpointBuilder', () => { } beforeEach(() => { - lightweightCheckpointBuilder = mock({ checkpointNumber, constants }); + lightweightCheckpointBuilder = mock(); + Object.defineProperty(lightweightCheckpointBuilder, 'checkpointNumber', { value: checkpointNumber }); + Object.defineProperty(lightweightCheckpointBuilder, 'constants', { value: constants }); lightweightCheckpointBuilder.getBlocks.mockReturnValue([]); fork = mock(); diff --git a/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts b/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts index 511579f3d2e4..37d55fce1eb4 100644 --- a/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts +++ b/yarn-project/world-state/src/synchronizer/server_world_state_synchronizer.ts @@ -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; @@ -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); diff --git a/yarn-project/world-state/src/test/integration.test.ts b/yarn-project/world-state/src/test/integration.test.ts index 4f75871da279..fd1c096460dc 100644 --- a/yarn-project/world-state/src/test/integration.test.ts +++ b/yarn-project/world-state/src/test/integration.test.ts @@ -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); + }); }); });