fix: off-by-1 in getBlockHashMembershipWitness archive snapshot

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();

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<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
@@ -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<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