fix(sync): Sync latest globals within merkle tree ops (#1612)

## Overview
Ci has been failing due to a race condition where world state operations
were not being synchronised.
The offending value was that the world latestGlobalVariablesHash was not
stored with the rest of the merkle trees, and such could not use the
same serial queue to sync their update operations. This lead to the
trees being out of sync with the globals value on some occasions. (I
think)

This pr may be slightly contraversial as it stores non merkle tree data
within the merkle_trees abstraction. However think the naming of the
merkle_trees abstraction is the limiting factor. The file just stores
and synchronises the world state, it just so happens that until now this
state was all trees. (although this could be me bending reality to suit
my current needs)


Adds a Committable type which allows us to add rollback functionality to
anything

yarn-project/aztec-node/src/aztec-node/server.ts

@@ -317,16 +317,26 @@ export class AztecNodeService implements AztecNode {
    * @returns The current committed roots for the data trees.
    */
   public async getTreeRoots(): Promise<Record<MerkleTreeId, Fr>> {
-    const getTreeRoot = async (id: MerkleTreeId) =>
-      Fr.fromBuffer((await this.merkleTreeDB.getTreeInfo(id, false)).root);
+    const committedDb = this.worldStateSynchroniser.getCommitted();
+    const getTreeRoot = async (id: MerkleTreeId) => Fr.fromBuffer((await committedDb.getTreeInfo(id)).root);
+
+    const [privateDataTree, nullifierTree, contractTree, l1ToL2MessagesTree, blocksTree, publicDataTree] =
+      await Promise.all([
+        getTreeRoot(MerkleTreeId.PRIVATE_DATA_TREE),
+        getTreeRoot(MerkleTreeId.NULLIFIER_TREE),
+        getTreeRoot(MerkleTreeId.CONTRACT_TREE),
+        getTreeRoot(MerkleTreeId.L1_TO_L2_MESSAGES_TREE),
+        getTreeRoot(MerkleTreeId.BLOCKS_TREE),
+        getTreeRoot(MerkleTreeId.PUBLIC_DATA_TREE),
+      ]);
 
     return {
-      [MerkleTreeId.CONTRACT_TREE]: await getTreeRoot(MerkleTreeId.CONTRACT_TREE),
-      [MerkleTreeId.PRIVATE_DATA_TREE]: await getTreeRoot(MerkleTreeId.PRIVATE_DATA_TREE),
-      [MerkleTreeId.NULLIFIER_TREE]: await getTreeRoot(MerkleTreeId.NULLIFIER_TREE),
-      [MerkleTreeId.PUBLIC_DATA_TREE]: await getTreeRoot(MerkleTreeId.PUBLIC_DATA_TREE),
-      [MerkleTreeId.L1_TO_L2_MESSAGES_TREE]: await getTreeRoot(MerkleTreeId.L1_TO_L2_MESSAGES_TREE),
-      [MerkleTreeId.BLOCKS_TREE]: await getTreeRoot(MerkleTreeId.BLOCKS_TREE),
+      [MerkleTreeId.CONTRACT_TREE]: contractTree,
+      [MerkleTreeId.PRIVATE_DATA_TREE]: privateDataTree,
+      [MerkleTreeId.NULLIFIER_TREE]: nullifierTree,
+      [MerkleTreeId.PUBLIC_DATA_TREE]: publicDataTree,
+      [MerkleTreeId.L1_TO_L2_MESSAGES_TREE]: l1ToL2MessagesTree,
+      [MerkleTreeId.BLOCKS_TREE]: blocksTree,
     };
   }
 
@@ -335,19 +345,17 @@ export class AztecNodeService implements AztecNode {
    * @returns The current committed block data.
    */
   public async getHistoricBlockData(): Promise<HistoricBlockData> {
-    const getTreeRoot = async (id: MerkleTreeId) =>
-      Fr.fromBuffer((await this.merkleTreeDB.getTreeInfo(id, false)).root);
-
-    const globalsHash = this.worldStateSynchroniser.latestGlobalVariablesHash;
+    const committedDb = this.worldStateSynchroniser.getCommitted();
+    const [roots, globalsHash] = await Promise.all([this.getTreeRoots(), committedDb.getLatestGlobalVariablesHash()]);
 
     return new HistoricBlockData(
-      await getTreeRoot(MerkleTreeId.PRIVATE_DATA_TREE),
-      await getTreeRoot(MerkleTreeId.NULLIFIER_TREE),
-      await getTreeRoot(MerkleTreeId.CONTRACT_TREE),
-      await getTreeRoot(MerkleTreeId.L1_TO_L2_MESSAGES_TREE),
-      await getTreeRoot(MerkleTreeId.BLOCKS_TREE),
+      roots[MerkleTreeId.PRIVATE_DATA_TREE],
+      roots[MerkleTreeId.NULLIFIER_TREE],
+      roots[MerkleTreeId.CONTRACT_TREE],
+      roots[MerkleTreeId.L1_TO_L2_MESSAGES_TREE],
+      roots[MerkleTreeId.BLOCKS_TREE],
       Fr.ZERO,
-      await getTreeRoot(MerkleTreeId.PUBLIC_DATA_TREE),
+      roots[MerkleTreeId.PUBLIC_DATA_TREE],
       globalsHash,
     );
   }

yarn-project/aztec-rpc/src/synchroniser/synchroniser.ts

@@ -69,7 +69,7 @@ export class Synchroniser {
   protected async initialSync() {
     const [blockNumber, historicBlockData] = await Promise.all([
       this.node.getBlockHeight(),
-      Promise.resolve(this.node.getHistoricBlockData()),
+      this.node.getHistoricBlockData(),
     ]);
     this.initialSyncBlockHeight = blockNumber;
     this.synchedToBlock = this.initialSyncBlockHeight;

yarn-project/end-to-end/src/e2e_multi_transfer.test.ts

@@ -2,8 +2,7 @@ import { AztecNodeService } from '@aztec/aztec-node';
 import { AztecRPCServer } from '@aztec/aztec-rpc';
 import { AztecAddress, Contract, Fr, Wallet } from '@aztec/aztec.js';
 import { DebugLogger } from '@aztec/foundation/log';
-import { PrivateTokenAirdropContract } from '@aztec/noir-contracts/types';
-import { MultiTransferContract } from '@aztec/noir-contracts/types';
+import { MultiTransferContract, PrivateTokenAirdropContract } from '@aztec/noir-contracts/types';
 import { AztecRPC, CompleteAddress } from '@aztec/types';
 
 import { expectsNumOfEncryptedLogsInTheLastBlockToBe, setup } from './fixtures/utils.js';

yarn-project/foundation/package.json

@@ -30,7 +30,8 @@
     "./wasm": "./dest/wasm/index.js",
     "./worker": "./dest/worker/index.js",
     "./bigint-buffer": "./dest/bigint-buffer/index.js",
-    "./types": "./dest/types/index.js"
+    "./types": "./dest/types/index.js",
+    "./committable": "./dest/committable/index.js"
   },
   "scripts": {
     "prepare": "node ../yarn-project-base/scripts/update_build_manifest.mjs package.json",

yarn-project/foundation/src/committable/committable.test.ts

@@ -0,0 +1,32 @@
+import { Committable } from './committable.js';
+
+describe('committable', () => {
+  it('include uncommitted should work as expected', () => {
+    const committableNumber: Committable<number> = new Committable(0);
+
+    // Check the initial value is 0
+    expect(committableNumber.get(true)).toBe(0);
+
+    // Update the value to 1
+    committableNumber.set(1);
+    committableNumber.commit();
+
+    // Check the value is 1
+    expect(committableNumber.get()).toBe(1);
+
+    // Test include uncommitted
+    committableNumber.set(2);
+    expect(committableNumber.get(true)).toBe(2);
+    expect(committableNumber.get(false)).toBe(1);
+
+    committableNumber.commit();
+    expect(committableNumber.get()).toBe(2);
+
+    // Test rollback
+    committableNumber.set(3);
+    expect(committableNumber.get(true)).toBe(3);
+    expect(committableNumber.get(false)).toBe(2);
+    committableNumber.rollback();
+    expect(committableNumber.get()).toBe(2);
+  });
+});

yarn-project/foundation/src/committable/committable.ts

@@ -0,0 +1,46 @@
+/**
+ * A class that allows for a value to be committed or rolled back.
+ */
+export class Committable<T> {
+  private currentValue: T;
+  private nextValue: T | undefined = undefined;
+
+  constructor(initialValue: T) {
+    this.currentValue = initialValue;
+  }
+
+  /**
+   * Commits the uncommitted value.
+   */
+  public commit() {
+    if (this.nextValue === undefined) {
+      return;
+    }
+    this.currentValue = this.nextValue;
+    this.nextValue = undefined;
+  }
+
+  /**
+   * Rolls back the uncommitted value.
+   */
+  public rollback() {
+    this.nextValue === undefined;
+  }
+
+  /**
+   * Gets the current value.
+   * @param includeUncommitted - Whether to include the uncommitted value.
+   * @returns The current value if includeUncommitted is false, otherwise the uncommitted value.
+   */
+  public get(includeUncommitted: boolean = false): T {
+    return includeUncommitted && this.nextValue ? this.nextValue : this.currentValue;
+  }
+
+  /**
+   * Sets the next value to be committed to.
+   * @param value - The new value to be set.
+   */
+  public set(value: T) {
+    this.nextValue = value;
+  }
+}

yarn-project/foundation/src/committable/index.ts

@@ -0,0 +1 @@
+export * from './committable.js';

yarn-project/sequencer-client/src/block_builder/solo_block_builder.ts

@@ -31,7 +31,7 @@ import { Fr } from '@aztec/foundation/fields';
 import { createDebugLogger } from '@aztec/foundation/log';
 import { assertLength } from '@aztec/foundation/serialize';
 import { ContractData, L2Block, L2BlockL2Logs, MerkleTreeId, PublicDataWrite, TxL2Logs } from '@aztec/types';
-import { MerkleTreeOperations } from '@aztec/world-state';
+import { MerkleTreeOperations, computeGlobalVariablesHash } from '@aztec/world-state';
 
 import chunk from 'lodash.chunk';
 import flatMap from 'lodash.flatmap';
@@ -295,7 +295,9 @@ export class SoloBlockBuilder implements BlockBuilder {
     // Update the root trees with the latest data and contract tree roots,
     // and validate them against the output of the root circuit simulation
     this.debug(`Updating and validating root trees`);
-    await this.db.updateHistoricBlocksTree(left[0].constants.globalVariables);
+    const globalVariablesHash = await computeGlobalVariablesHash(left[0].constants.globalVariables);
+    await this.db.updateLatestGlobalVariablesHash(globalVariablesHash);
+    await this.db.updateHistoricBlocksTree(globalVariablesHash);
 
     await this.validateRootOutput(rootOutput);
 

yarn-project/sequencer-client/src/sequencer/utils.ts

@@ -11,7 +11,7 @@ export async function getHistoricBlockData(
 ) {
   const wasm = await CircuitsWasm.get();
   const prevGlobalsHash = computeGlobalsHash(wasm, prevBlockGlobalVariables);
-  const roots = db.getTreeRoots();
+  const roots = await db.getTreeRoots();
 
   return new HistoricBlockData(
     Fr.fromBuffer(roots.privateDataTreeRoot),

yarn-project/world-state/src/merkle-tree/merkle_tree_operations_facade.ts

@@ -1,4 +1,4 @@
-import { GlobalVariables } from '@aztec/circuits.js';
+import { Fr } from '@aztec/foundation/fields';
 import { LowLeafWitnessData } from '@aztec/merkle-tree';
 import { L2Block, MerkleTreeId, SiblingPath } from '@aztec/types';
 
@@ -24,7 +24,7 @@ export class MerkleTreeOperationsFacade implements MerkleTreeOperations {
    * Get the current roots of the commitment trees.
    * @returns The current roots of the trees.
    */
-  getTreeRoots(): CurrentTreeRoots {
+  getTreeRoots(): Promise<CurrentTreeRoots> {
     return this.trees.getTreeRoots(this.includeUncommitted);
   }
 
@@ -116,11 +116,26 @@ export class MerkleTreeOperationsFacade implements MerkleTreeOperations {
   /**
    * Inserts into the roots trees (CONTRACT_TREE_ROOTS_TREE, PRIVATE_DATA_TREE_ROOTS_TREE)
    * the current roots of the corresponding trees (CONTRACT_TREE, PRIVATE_DATA_TREE).
-   * @param globalVariables - The current global variables to include in the block hash.
+   * @param globalVariablesHash - The hash of the current global variables to include in the block hash.
    * @returns Empty promise.
    */
-  public updateHistoricBlocksTree(globalVariables: GlobalVariables): Promise<void> {
-    return this.trees.updateHistoricBlocksTree(globalVariables, this.includeUncommitted);
+  public updateHistoricBlocksTree(globalVariablesHash: Fr): Promise<void> {
+    return this.trees.updateHistoricBlocksTree(globalVariablesHash, this.includeUncommitted);
+  }
+
+  /**
+   * Updates the latest global variables hash
+   * @param globalVariablesHash - The latest global variables hash
+   */
+  public updateLatestGlobalVariablesHash(globalVariablesHash: Fr): Promise<void> {
+    return this.trees.updateLatestGlobalVariablesHash(globalVariablesHash, this.includeUncommitted);
+  }
+
+  /**
+   * Gets the global variables hash from the previous block
+   */
+  public getLatestGlobalVariablesHash(): Promise<Fr> {
+    return this.trees.getLatestGlobalVariablesHash(this.includeUncommitted);
   }
 
   /**

yarn-project/world-state/src/synchroniser/server_world_state_synchroniser.ts

@@ -1,8 +1,7 @@
-import { Fr } from '@aztec/foundation/fields';
 import { createDebugLogger } from '@aztec/foundation/log';
 import { L2Block, L2BlockDownloader, L2BlockSource } from '@aztec/types';
 
-import { MerkleTreeDb, MerkleTreeOperations, computeGlobalVariablesHash } from '../index.js';
+import { MerkleTreeDb, MerkleTreeOperations } from '../index.js';
 import { MerkleTreeOperationsFacade } from '../merkle-tree/merkle_tree_operations_facade.js';
 import { getConfigEnvVars } from './config.js';
 import { WorldStateRunningState, WorldStateStatus, WorldStateSynchroniser } from './world_state_synchroniser.js';
@@ -23,9 +22,6 @@ export class ServerWorldStateSynchroniser implements WorldStateSynchroniser {
   private runningPromise: Promise<void> = Promise.resolve();
   private currentState: WorldStateRunningState = WorldStateRunningState.IDLE;
 
-  /** The latest Global Variables hash for the HEAD of the chain. */
-  public latestGlobalVariablesHash: Fr = Fr.ZERO;
-
   constructor(
     private merkleTreeDb: MerkleTreeDb,
     private l2BlockSource: L2BlockSource,
@@ -57,7 +53,6 @@ export class ServerWorldStateSynchroniser implements WorldStateSynchroniser {
 
     // get the current latest block number
     this.latestBlockNumberAtStart = await this.l2BlockSource.getBlockHeight();
-    this.latestGlobalVariablesHash = await computeGlobalVariablesHash();
 
     const blockToDownloadFrom = this.currentL2BlockNum + 1;
 
@@ -121,8 +116,6 @@ export class ServerWorldStateSynchroniser implements WorldStateSynchroniser {
   private async handleL2Block(l2Block: L2Block) {
     await this.merkleTreeDb.handleL2Block(l2Block);
     this.currentL2BlockNum = l2Block.number;
-    this.latestGlobalVariablesHash = await computeGlobalVariablesHash(l2Block.globalVariables);
-    this.log(`Synced global variables with hash ${this.latestGlobalVariablesHash.toString()}`);
     if (
       this.currentState === WorldStateRunningState.SYNCHING &&
       this.currentL2BlockNum >= this.latestBlockNumberAtStart

yarn-project/world-state/src/synchroniser/world_state_synchroniser.ts

@@ -1,5 +1,3 @@
-import { Fr } from '@aztec/circuits.js';
-
 import { MerkleTreeOperations } from '../index.js';
 
 /**
@@ -58,9 +56,4 @@ export interface WorldStateSynchroniser {
    * @returns An instance of MerkleTreeOperations that will not include uncommitted data.
    */
   getCommitted(): MerkleTreeOperations;
-
-  /**
-   * The latest Global Variables hash for the HEAD of the chain.
-   */
-  latestGlobalVariablesHash: Fr;
 }

yarn-project/world-state/src/world-state-db/index.ts

@@ -1,4 +1,5 @@
-import { GlobalVariables, MAX_NEW_NULLIFIERS_PER_TX } from '@aztec/circuits.js';
+import { MAX_NEW_NULLIFIERS_PER_TX } from '@aztec/circuits.js';
+import { Fr } from '@aztec/foundation/fields';
 import { createDebugLogger } from '@aztec/foundation/log';
 import { LeafData, LowLeafWitnessData } from '@aztec/merkle-tree';
 import { L2Block, MerkleTreeId, SiblingPath } from '@aztec/types';
@@ -115,7 +116,7 @@ export interface MerkleTreeOperations {
   /**
    * Gets the current roots of the commitment trees.
    */
-  getTreeRoots(): CurrentTreeRoots;
+  getTreeRoots(): Promise<CurrentTreeRoots>;
 
   /**
    * Gets sibling path for a leaf.
@@ -175,9 +176,20 @@ export interface MerkleTreeOperations {
   /**
    * Inserts the new block hash into the new block hashes tree.
    * This includes all of the current roots of all of the data trees and the current blocks global vars.
-   * @param globalVariables - The global variables to insert into the block hash.
+   * @param globalVariablesHash - The global variables hash to insert into the block hash.
    */
-  updateHistoricBlocksTree(globalVariables: GlobalVariables): Promise<void>;
+  updateHistoricBlocksTree(globalVariablesHash: Fr): Promise<void>;
+
+  /**
+   * Updates the latest global variables hash
+   * @param globalVariablesHash - The latest global variables hash
+   */
+  updateLatestGlobalVariablesHash(globalVariablesHash: Fr): Promise<void>;
+
+  /**
+   * Gets the global variables hash from the previous block
+   */
+  getLatestGlobalVariablesHash(): Promise<Fr>;
 
   /**
    * Batch insert multiple leaves into the tree.