From 35ad0ab94c2c1fc26f916fcff6d5a38fa9ebc9e9 Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 17 Mar 2026 07:00:49 +0000 Subject: [PATCH] fix: off-by-1 in getBlockHashMembershipWitness archive snapshot The Noir circuit checks archive membership against `anchor_block_header.last_archive.root`, which is the archive tree root BEFORE the anchor block was added (state after block N-1). The node was returning a sibling path from the archive at block N, causing the proof to fail. Fix by resolving the reference block number and fetching world state at block N-1 instead. Also adds comprehensive tests for get_block_header_at covering past blocks, anchor block, future block rejection, and block number mismatch. Closes https://github.com/AztecProtocol/aztec-packages/issues/21635 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../aztec-nr/aztec/src/oracle/block_header.nr | 102 +++++++++++++++++- .../aztec-node/src/aztec-node/server.ts | 25 ++++- 2 files changed, 123 insertions(+), 4 deletions(-) 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/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 172286005cbb..92dfc102a387 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -1046,7 +1046,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 @@ -1655,6 +1659,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