From 5cf3c6da006163f0d26f39291f974440a9fbed39 Mon Sep 17 00:00:00 2001
From: dbanks12 <david@aztecprotocol.com>
Date: Tue, 10 Dec 2024 03:26:35 +0000
Subject: [PATCH 1/4] debugging value mismatch

---
 .../end-to-end/src/e2e_avm_simulator.test.ts  |  9 +++++++--
 .../simulator/src/avm/journal/journal.ts      |  3 +--
 .../src/avm/journal/public_storage.ts         | 19 +++++++++++++++++++
 .../public/enqueued_call_side_effect_trace.ts |  6 +++---
 4 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
index fe71b683719..476ff73bf60 100644
--- a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
+++ b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
@@ -1,4 +1,4 @@
-import { type AccountWallet, AztecAddress, BatchCall, Fr, TxStatus } from '@aztec/aztec.js';
+import { type AccountWallet, AztecAddress, BatchCall, Fr, TxStatus, sleep } from '@aztec/aztec.js';
 import { AvmInitializerTestContract, AvmTestContract } from '@aztec/noir-contracts.js';
 
 import { jest } from '@jest/globals';
@@ -30,6 +30,9 @@ describe('e2e_avm_simulator', () => {
       avmContract = await AvmTestContract.deploy(wallet).send().deployed();
       secondAvmContract = await AvmTestContract.deploy(wallet).send().deployed();
     });
+    afterEach(async () => {
+      await sleep(1000);
+    });
 
     describe('Assertions', () => {
       describe('Not nested', () => {
@@ -63,7 +66,9 @@ describe('e2e_avm_simulator', () => {
 
     describe('From private', () => {
       it('Should enqueue a public function correctly', async () => {
-        await avmContract.methods.enqueue_public_from_private().simulate();
+        const request = await avmContract.methods.enqueue_public_from_private().create();
+        const simulation = await wallet.simulateTx(request, true);
+        expect(simulation.publicOutput!.revertReason).toBeUndefined();
       });
     });
 
diff --git a/yarn-project/simulator/src/avm/journal/journal.ts b/yarn-project/simulator/src/avm/journal/journal.ts
index c769fe11be4..ee85a8ed766 100644
--- a/yarn-project/simulator/src/avm/journal/journal.ts
+++ b/yarn-project/simulator/src/avm/journal/journal.ts
@@ -170,8 +170,7 @@ export class AvmPersistableStateManager {
       const lowLeafPath = lowLeafInfo.siblingPath;
 
       const newLeafPreimage = result.element as PublicDataTreeLeafPreimage;
-      let insertionPath;
-
+      let insertionPath: Fr[] | undefined;
       if (!result.update) {
         insertionPath = result.insertionPath;
       }
diff --git a/yarn-project/simulator/src/avm/journal/public_storage.ts b/yarn-project/simulator/src/avm/journal/public_storage.ts
index da2565e8d2f..04a768d6dc9 100644
--- a/yarn-project/simulator/src/avm/journal/public_storage.ts
+++ b/yarn-project/simulator/src/avm/journal/public_storage.ts
@@ -30,6 +30,8 @@ export class PublicStorage {
    * Create a new public storage manager forked from this one
    */
   public fork() {
+    console.log(`Forking public storage`);
+    this.cache.dumpCache();
     return new PublicStorage(this.hostPublicStorage, this);
   }
 
@@ -49,12 +51,16 @@ export class PublicStorage {
    * @returns value: the latest value written according to this cache or the parent's. undefined on cache miss.
    */
   public readHereOrParent(contractAddress: AztecAddress, slot: Fr): Fr | undefined {
+    console.log(`Reading here or in parent`);
+    this.cache.dumpCache();
     // First try check this storage cache
     let value = this.cache.read(contractAddress, slot);
+    console.log(`Reading here ${value}`);
     // Then try parent's storage cache
     if (!value && this.parent) {
       // Note: this will recurse to grandparent/etc until a cache-hit is encountered.
       value = this.parent.readHereOrParent(contractAddress, slot);
+      console.log(`Reading in parent ${value}`);
     }
     return value;
   }
@@ -79,6 +85,7 @@ export class PublicStorage {
       // This functions returns Fr.ZERO if it has never been written to before
       // we explicity coalesce to Fr.ZERO in case we have some implementations that cause this to return undefined
       value = (await this.hostPublicStorage.storageRead(contractAddress, slot)) ?? Fr.ZERO;
+      console.log(`Fell back on host storage: ${value}`);
       // TODO(dbanks12): if value retrieved from host storage, we can cache it here
       // any future reads to the same slot can read from cache instead of more expensive
       // DB access
@@ -177,6 +184,9 @@ class PublicStorageCache {
     // Iterate over all incoming contracts with staged writes.
     for (const [incomingAddress, incomingCacheAtContract] of incomingStorageCache.cachePerContract) {
       const thisCacheAtContract = this.cachePerContract.get(incomingAddress);
+      for (const [slot, value] of incomingCacheAtContract) {
+        console.log(`acceptAndMerge: slot: ${slot}, value: ${value}`);
+      }
       if (!thisCacheAtContract) {
         // The contract has no storage writes staged here
         // so just accept the incoming cache as-is for this contract.
@@ -190,4 +200,13 @@ class PublicStorageCache {
       }
     }
   }
+
+  public dumpCache() {
+    for (const [addr, cache] of this.cachePerContract) {
+      console.log(`dumping cache for contract @ ${addr}`);
+      for (const [slot, value] of cache) {
+        console.log(`\tslot: ${slot}, value: ${value}`);
+      }
+    }
+  }
 }
diff --git a/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts b/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts
index 9905d454fcf..d84f0ce541a 100644
--- a/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts
+++ b/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts
@@ -51,7 +51,7 @@ import { Fr } from '@aztec/foundation/fields';
 import { jsonStringify } from '@aztec/foundation/json-rpc';
 import { createLogger } from '@aztec/foundation/log';
 
-import { assert } from 'console';
+import { strict as assert } from 'assert';
 
 import { type AvmContractCallResult, type AvmFinalizedCallResult } from '../avm/avm_contract_call_result.js';
 import { type AvmExecutionEnvironment } from '../avm/avm_execution_environment.js';
@@ -206,7 +206,7 @@ export class PublicEnqueuedCallSideEffectTrace implements PublicSideEffectTraceI
   ) {
     if (!leafIndex.equals(Fr.zero())) {
       // if we have real merkle hint content, make sure the value matches the the provided preimage
-      assert(leafPreimage.value.equals(value), 'Value mismatch when tracing in public data write');
+      assert(leafPreimage.value.equals(value), `Value mismatch when tracing in public data read (value: ${value}, value in leaf preimage: ${leafPreimage.value})`);
     }
 
     this.avmCircuitHints.publicDataReads.items.push(new AvmPublicDataReadTreeHint(leafPreimage, leafIndex, path));
@@ -226,7 +226,7 @@ export class PublicEnqueuedCallSideEffectTrace implements PublicSideEffectTraceI
   ) {
     if (!lowLeafIndex.equals(Fr.zero())) {
       // if we have real merkle hint content, make sure the value matches the the provided preimage
-      assert(newLeafPreimage.value.equals(value), 'Value mismatch when tracing in public data read');
+      assert(newLeafPreimage.value.equals(value), `Value mismatch when tracing in public data write (value: ${value}, value in leaf preimage: ${newLeafPreimage.value})`);
     }
     if (
       this.publicDataWrites.length + this.previousSideEffectArrayLengths.publicDataWrites >=

From 6602fa763928266387281eb009e6232e3dc7a07a Mon Sep 17 00:00:00 2001
From: dbanks12 <david@aztecprotocol.com>
Date: Tue, 10 Dec 2024 04:15:08 +0000
Subject: [PATCH 2/4] cleanup, fix

---
 .../end-to-end/src/e2e_avm_simulator.test.ts  |  5 +---
 yarn-project/ethereum/package.json            |  2 +-
 .../simulator/src/avm/journal/journal.ts      | 24 ++++++++++++++-----
 .../src/avm/journal/public_storage.ts         | 19 ---------------
 .../public/enqueued_call_side_effect_trace.ts | 17 ++++---------
 5 files changed, 25 insertions(+), 42 deletions(-)

diff --git a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
index 476ff73bf60..8ec9c96f24d 100644
--- a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
+++ b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
@@ -1,4 +1,4 @@
-import { type AccountWallet, AztecAddress, BatchCall, Fr, TxStatus, sleep } from '@aztec/aztec.js';
+import { type AccountWallet, AztecAddress, BatchCall, Fr, TxStatus } from '@aztec/aztec.js';
 import { AvmInitializerTestContract, AvmTestContract } from '@aztec/noir-contracts.js';
 
 import { jest } from '@jest/globals';
@@ -30,9 +30,6 @@ describe('e2e_avm_simulator', () => {
       avmContract = await AvmTestContract.deploy(wallet).send().deployed();
       secondAvmContract = await AvmTestContract.deploy(wallet).send().deployed();
     });
-    afterEach(async () => {
-      await sleep(1000);
-    });
 
     describe('Assertions', () => {
       describe('Not nested', () => {
diff --git a/yarn-project/ethereum/package.json b/yarn-project/ethereum/package.json
index 0e34d8dcbf1..3300f21bba8 100644
--- a/yarn-project/ethereum/package.json
+++ b/yarn-project/ethereum/package.json
@@ -91,4 +91,4 @@
   "engines": {
     "node": ">=18"
   }
-}
\ No newline at end of file
+}
diff --git a/yarn-project/simulator/src/avm/journal/journal.ts b/yarn-project/simulator/src/avm/journal/journal.ts
index ee85a8ed766..96ea1a1401f 100644
--- a/yarn-project/simulator/src/avm/journal/journal.ts
+++ b/yarn-project/simulator/src/avm/journal/journal.ts
@@ -156,9 +156,11 @@ export class AvmPersistableStateManager {
    */
   public async writeStorage(contractAddress: AztecAddress, slot: Fr, value: Fr): Promise<void> {
     this.log.debug(`Storage write (address=${contractAddress}, slot=${slot}): value=${value}`);
+    const leafSlot = computePublicDataTreeLeafSlot(contractAddress, slot);
+    this.log.debug(`leafSlot=${leafSlot}`);
     // Cache storage writes for later reference/reads
     this.publicStorage.write(contractAddress, slot, value);
-    const leafSlot = computePublicDataTreeLeafSlot(contractAddress, slot);
+
     if (this.doMerkleOperations) {
       const result = await this.merkleTrees.writePublicStorage(leafSlot, value);
       assert(result !== undefined, 'Public data tree insertion error. You might want to disable doMerkleOperations.');
@@ -175,6 +177,13 @@ export class AvmPersistableStateManager {
         insertionPath = result.insertionPath;
       }
 
+      if (lowLeafIndex !== 0n) {
+        assert(
+          newLeafPreimage.value.equals(value),
+          `Value mismatch when performing public data write (got value: ${value}, value in ephemeral tree: ${newLeafPreimage.value})`,
+        );
+      }
+
       this.trace.tracePublicStorageWrite(
         contractAddress,
         slot,
@@ -200,8 +209,8 @@ export class AvmPersistableStateManager {
   public async readStorage(contractAddress: AztecAddress, slot: Fr): Promise<Fr> {
     const { value, cached } = await this.publicStorage.read(contractAddress, slot);
     this.log.debug(`Storage read  (address=${contractAddress}, slot=${slot}): value=${value}, cached=${cached}`);
-
     const leafSlot = computePublicDataTreeLeafSlot(contractAddress, slot);
+    this.log.debug(`leafSlot=${leafSlot}`);
 
     if (this.doMerkleOperations) {
       // Get leaf if present, low leaf if absent
@@ -221,7 +230,13 @@ export class AvmPersistableStateManager {
       );
       this.log.debug(`leafPreimage.slot: ${leafPreimage.slot}, leafPreimage.value: ${leafPreimage.value}`);
 
-      if (!alreadyPresent) {
+      if (alreadyPresent) {
+        assert(
+          leafPreimage.value.equals(value),
+          `Value mismatch when performing public data read (got value: ${value}, value in ephemeral tree: ${leafPreimage.value})`,
+        );
+      } else {
+        this.log.debug(`Slot has never been written before!`);
         // Sanity check that the leaf slot is skipped by low leaf when it doesn't exist
         assert(
           leafPreimage.slot.toBigInt() < leafSlot.toBigInt() &&
@@ -229,9 +244,6 @@ export class AvmPersistableStateManager {
           'Public data tree low leaf should skip the target leaf slot when the target leaf does not exist or is the max value.',
         );
       }
-      this.log.debug(
-        `Tracing storage leaf preimage slot=${slot}, leafSlot=${leafSlot}, value=${value}, nextKey=${leafPreimage.nextSlot}, nextIndex=${leafPreimage.nextIndex}`,
-      );
       // On non-existence, AVM circuit will need to recognize that leafPreimage.slot != leafSlot,
       // prove that this is a low leaf that skips leafSlot, and then prove membership of the leaf.
       this.trace.tracePublicStorageRead(contractAddress, slot, value, leafPreimage, new Fr(leafIndex), leafPath);
diff --git a/yarn-project/simulator/src/avm/journal/public_storage.ts b/yarn-project/simulator/src/avm/journal/public_storage.ts
index 04a768d6dc9..da2565e8d2f 100644
--- a/yarn-project/simulator/src/avm/journal/public_storage.ts
+++ b/yarn-project/simulator/src/avm/journal/public_storage.ts
@@ -30,8 +30,6 @@ export class PublicStorage {
    * Create a new public storage manager forked from this one
    */
   public fork() {
-    console.log(`Forking public storage`);
-    this.cache.dumpCache();
     return new PublicStorage(this.hostPublicStorage, this);
   }
 
@@ -51,16 +49,12 @@ export class PublicStorage {
    * @returns value: the latest value written according to this cache or the parent's. undefined on cache miss.
    */
   public readHereOrParent(contractAddress: AztecAddress, slot: Fr): Fr | undefined {
-    console.log(`Reading here or in parent`);
-    this.cache.dumpCache();
     // First try check this storage cache
     let value = this.cache.read(contractAddress, slot);
-    console.log(`Reading here ${value}`);
     // Then try parent's storage cache
     if (!value && this.parent) {
       // Note: this will recurse to grandparent/etc until a cache-hit is encountered.
       value = this.parent.readHereOrParent(contractAddress, slot);
-      console.log(`Reading in parent ${value}`);
     }
     return value;
   }
@@ -85,7 +79,6 @@ export class PublicStorage {
       // This functions returns Fr.ZERO if it has never been written to before
       // we explicity coalesce to Fr.ZERO in case we have some implementations that cause this to return undefined
       value = (await this.hostPublicStorage.storageRead(contractAddress, slot)) ?? Fr.ZERO;
-      console.log(`Fell back on host storage: ${value}`);
       // TODO(dbanks12): if value retrieved from host storage, we can cache it here
       // any future reads to the same slot can read from cache instead of more expensive
       // DB access
@@ -184,9 +177,6 @@ class PublicStorageCache {
     // Iterate over all incoming contracts with staged writes.
     for (const [incomingAddress, incomingCacheAtContract] of incomingStorageCache.cachePerContract) {
       const thisCacheAtContract = this.cachePerContract.get(incomingAddress);
-      for (const [slot, value] of incomingCacheAtContract) {
-        console.log(`acceptAndMerge: slot: ${slot}, value: ${value}`);
-      }
       if (!thisCacheAtContract) {
         // The contract has no storage writes staged here
         // so just accept the incoming cache as-is for this contract.
@@ -200,13 +190,4 @@ class PublicStorageCache {
       }
     }
   }
-
-  public dumpCache() {
-    for (const [addr, cache] of this.cachePerContract) {
-      console.log(`dumping cache for contract @ ${addr}`);
-      for (const [slot, value] of cache) {
-        console.log(`\tslot: ${slot}, value: ${value}`);
-      }
-    }
-  }
 }
diff --git a/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts b/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts
index d84f0ce541a..9b30dd016f3 100644
--- a/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts
+++ b/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts
@@ -197,20 +197,17 @@ export class PublicEnqueuedCallSideEffectTrace implements PublicSideEffectTraceI
   }
 
   public tracePublicStorageRead(
-    _contractAddress: AztecAddress,
+    contractAddress: AztecAddress,
     slot: Fr,
     value: Fr,
     leafPreimage: PublicDataTreeLeafPreimage = PublicDataTreeLeafPreimage.empty(),
     leafIndex: Fr = Fr.zero(),
     path: Fr[] = emptyPublicDataPath(),
   ) {
-    if (!leafIndex.equals(Fr.zero())) {
-      // if we have real merkle hint content, make sure the value matches the the provided preimage
-      assert(leafPreimage.value.equals(value), `Value mismatch when tracing in public data read (value: ${value}, value in leaf preimage: ${leafPreimage.value})`);
-    }
-
     this.avmCircuitHints.publicDataReads.items.push(new AvmPublicDataReadTreeHint(leafPreimage, leafIndex, path));
-    this.log.debug(`SLOAD cnt: ${this.sideEffectCounter} val: ${value} slot: ${slot}`);
+    this.log.debug(
+      `Tracing storage read (address=${contractAddress}, slot=${slot}): value=${value} (counter=${this.sideEffectCounter})`,
+    );
     this.incrementSideEffectCounter();
   }
 
@@ -224,10 +221,6 @@ export class PublicEnqueuedCallSideEffectTrace implements PublicSideEffectTraceI
     newLeafPreimage: PublicDataTreeLeafPreimage = PublicDataTreeLeafPreimage.empty(),
     insertionPath: Fr[] = emptyPublicDataPath(),
   ) {
-    if (!lowLeafIndex.equals(Fr.zero())) {
-      // if we have real merkle hint content, make sure the value matches the the provided preimage
-      assert(newLeafPreimage.value.equals(value), `Value mismatch when tracing in public data write (value: ${value}, value in leaf preimage: ${newLeafPreimage.value})`);
-    }
     if (
       this.publicDataWrites.length + this.previousSideEffectArrayLengths.publicDataWrites >=
       MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX
@@ -248,7 +241,7 @@ export class PublicEnqueuedCallSideEffectTrace implements PublicSideEffectTraceI
     );
 
     this.log.debug(
-      `Traced public data write (address=${contractAddress}, slot=${slot}, leafSlot=${leafSlot}): value=${value} (counter=${this.sideEffectCounter})`,
+      `Traced public data write (address=${contractAddress}, slot=${slot}): value=${value} (counter=${this.sideEffectCounter})`,
     );
     this.incrementSideEffectCounter();
   }

From 908abf6117534a80daa71b1d0b2258c3226b9776 Mon Sep 17 00:00:00 2001
From: dbanks12 <david@aztecprotocol.com>
Date: Tue, 10 Dec 2024 04:23:41 +0000
Subject: [PATCH 3/4] fmt

---
 yarn-project/ethereum/package.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/yarn-project/ethereum/package.json b/yarn-project/ethereum/package.json
index 3300f21bba8..0e34d8dcbf1 100644
--- a/yarn-project/ethereum/package.json
+++ b/yarn-project/ethereum/package.json
@@ -91,4 +91,4 @@
   "engines": {
     "node": ">=18"
   }
-}
+}
\ No newline at end of file

From 59e715c5c8ab84a9573fcca6140418feeaf4b8f2 Mon Sep 17 00:00:00 2001
From: dbanks12 <david@aztecprotocol.com>
Date: Tue, 10 Dec 2024 14:58:35 +0000
Subject: [PATCH 4/4] fix

---
 yarn-project/simulator/src/avm/journal/journal.ts | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/yarn-project/simulator/src/avm/journal/journal.ts b/yarn-project/simulator/src/avm/journal/journal.ts
index 96ea1a1401f..ac21e62894b 100644
--- a/yarn-project/simulator/src/avm/journal/journal.ts
+++ b/yarn-project/simulator/src/avm/journal/journal.ts
@@ -175,9 +175,6 @@ export class AvmPersistableStateManager {
       let insertionPath: Fr[] | undefined;
       if (!result.update) {
         insertionPath = result.insertionPath;
-      }
-
-      if (lowLeafIndex !== 0n) {
         assert(
           newLeafPreimage.value.equals(value),
           `Value mismatch when performing public data write (got value: ${value}, value in ephemeral tree: ${newLeafPreimage.value})`,
@@ -225,10 +222,10 @@ export class AvmPersistableStateManager {
       const leafPath = await this.merkleTrees.getSiblingPath(MerkleTreeId.PUBLIC_DATA_TREE, leafIndex);
       const leafPreimage = preimage as PublicDataTreeLeafPreimage;
 
+      this.log.debug(`leafPreimage.slot: ${leafPreimage.slot}, leafPreimage.value: ${leafPreimage.value}`);
       this.log.debug(
         `leafPreimage.nextSlot: ${leafPreimage.nextSlot}, leafPreimage.nextIndex: ${Number(leafPreimage.nextIndex)}`,
       );
-      this.log.debug(`leafPreimage.slot: ${leafPreimage.slot}, leafPreimage.value: ${leafPreimage.value}`);
 
       if (alreadyPresent) {
         assert(