feat: avm inserts nullifiers from private #10129

dbanks12 · 2024-11-22T03:32:45Z

Insert nullifiers from private
Rearrange forking in phase manager just a bit to make sure we always fork before insertions of revertible private side effects
Create inner helper writeSiloedNullifier in state manager
Change NullifierManager/Cache to operate with siloed nullifiers and simplify module a bunch
Prep tests to compare nullifier root computed by ephemeral trees/elsewhere

dbanks12 · 2024-11-22T03:33:00Z

feat: avm inserts nullifiers from private #10129 👈 (View in Graphite)
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

dbanks12 · 2024-11-22T03:38:01Z

yarn-project/simulator/src/avm/journal/journal.ts

    this.trace.merge(forkedState.trace, reverted);
+    this.merkleTrees = forkedState.merkleTrees;
  }


Is this the proper way to "accept the forked ephemeral trees"?

dbanks12 · 2024-11-22T03:39:43Z

yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts


-    // TODO(dbanks12): use siloed nullifier instead of scoped once public kernel stops siloing
-    // and once VM public inputs are meant to contain siloed nullifiers.
-    //const siloedNullifier = siloNullifier(contractAddress, nullifier);
-    const readRequest = new ReadRequest(nullifier, this.sideEffectCounter).scope(contractAddress);
+    // we scope with a dummy address because this read request isn't used
+    const readRequest = new ReadRequest(siloedNullifier, this.sideEffectCounter).scope(AztecAddress.zero());
    if (exists) {


We could probably remove all "read requests" of this sort from the new trace. Would still need to track how many have occurred though to make sure we don't surpass some limit.

Could honestly just enforce the limit based on the length of the hints vectors.

dbanks12 · 2024-11-22T03:41:18Z

yarn-project/simulator/src/public/public_tx_simulator.test.ts

-    expect(output.accumulatedData.nullifiers.slice(0, 4)).toEqual([
+    const siloedNullifiers = [
      new Fr(7777),
      new Fr(8888),
      siloNullifier(contractAddress, new Fr(1)),
      siloNullifier(contractAddress, new Fr(5)),
-    ]);
+    ];
+    expect(output.accumulatedData.nullifiers.slice(0, 4)).toEqual(siloedNullifiers);
+    await checkNullifierRoot(siloedNullifiers, txResult);


We can check the root this way once we have alignment in how insertions will happen in the AVM & base rollup etc

dbanks12 · 2024-11-22T03:46:02Z

yarn-project/simulator/src/public/public_tx_simulator.ts

+
+  public async insertNonRevertiblesFromPrivate(context: PublicTxContext) {
+    const stateManager = context.state.getActiveStateManager();
+    await stateManager.writeSiloedNullifiersFromPrivate(context.nonRevertibleAccumulatedDataFromPrivate.nullifiers);
+  }
+
+  public async insertRevertiblesFromPrivate(context: PublicTxContext) {
+    // Fork the state manager so we can rollback to end of setup if app logic reverts.
+    context.state.fork();
+    const stateManager = context.state.getActiveStateManager();
+    await stateManager.writeSiloedNullifiersFromPrivate(context.revertibleAccumulatedDataFromPrivate.nullifiers);
+  }


Not sure if this belongs here or in the PublicTxContext. Put it here for now.

dbanks12 · 2024-11-22T03:47:26Z

yarn-project/simulator/src/public/transitional_adapters.ts


-  const nullifiersFromPrivate = revertCode.isOK()
-    ? mergeAccumulatedData(
-        avmCircuitPublicInputs.previousNonRevertibleAccumulatedData.nullifiers,
-        avmCircuitPublicInputs.previousRevertibleAccumulatedData.nullifiers,
-      )
-    : avmCircuitPublicInputs.previousNonRevertibleAccumulatedData.nullifiers;
-  avmCircuitPublicInputs.accumulatedData.nullifiers = assertLength(
-    mergeAccumulatedData(nullifiersFromPrivate, avmCircuitPublicInputs.accumulatedData.nullifiers),
-    MAX_NULLIFIERS_PER_TX,
-  );
  const msgsFromPrivate = revertCode.isOK()


Don't need to patch public inputs with nullifiers from private because we are tracing them alongside everything else.

dbanks12 · 2024-11-25T22:28:08Z

yarn-project/simulator/src/avm/avm_tree.ts

@@ -336,93 +349,160 @@ export class AvmEphemeralForest {
    return updates.has(index);
  }

-  private searchForKey(key: Fr, arr: Fr[]): number {


moved outside class since it doesn't use this

dbanks12 · 2024-11-25T22:29:10Z

yarn-project/simulator/src/avm/avm_tree.ts

+  /**
+   * Internal helper to get the leaf or low leaf preimage and its index in the indexed tree given a key (slot or nullifier value).
+   * If the key is not found in the tree, it does an external lookup to the underlying merkle DB.
+   * Indicates whethe the sibling path is absent in the ephemeral tree.
+   * @param treeId - The tree we are looking up in
+   * @param key - The key for which we are look up the leaf or low leaf for.
+   * @param T - The type of the preimage (PublicData or Nullifier)
+   * @returns [
+   *     preimageWitness - The leaf or low leaf info (preimage & leaf index),
+   *     pathAbsentInEphemeralTree - whether its sibling path is absent in the ephemeral tree (useful during insertions)
+   * ]
+   */
+  async _getLeafOrLowLeafInfo<ID extends IndexedTreeId, T extends IndexedTreeLeafPreimage>(
+    treeId: ID,
+    key: Fr,
+  ): Promise<[PreimageWitness<T>, /*pathAbsentInEphemeralTree=*/ boolean]> {


This never mutates the ephemeral tree and instead returns a bool that writeStorage and appendNullifier use to determine whether they should update the tree with the missing path.

yarn-project/simulator/src/avm/avm_tree.ts

Fix hashPreimage in avmtree, add test Co-authored-by: dbanks12 <david@aztecprotocol.com>

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.65.1</summary> ## [0.65.1](aztec-package-v0.65.0...aztec-package-v0.65.1) (2024-11-27) ### Miscellaneous * Delete old serialization methods ([#9951](#9951)) ([10d3f6f](10d3f6f)) </details> <details><summary>barretenberg.js: 0.65.1</summary> ## [0.65.1](barretenberg.js-v0.65.0...barretenberg.js-v0.65.1) (2024-11-27) ### Features * Speed up transaction execution ([#10172](#10172)) ([da265b6](da265b6)) ### Bug Fixes * Add pako as a dependency in bb.js ([#10186](#10186)) ([b773c14](b773c14)) </details> <details><summary>aztec-packages: 0.65.1</summary> ## [0.65.1](aztec-packages-v0.65.0...aztec-packages-v0.65.1) (2024-11-27) ### Features * Add total mana used to header ([#9868](#9868)) ([2478d19](2478d19)) * Assert metrics in network tests ([#10215](#10215)) ([9380c0f](9380c0f)) * Avm inserts nullifiers from private ([#10129](#10129)) ([3fc0c7c](3fc0c7c)) * Burn congestion fee ([#10231](#10231)) ([20a33f2](20a33f2)) * Configure world state block history ([#10216](#10216)) ([01eb392](01eb392)) * Integrate fee into rollup ([#10176](#10176)) ([12744d6](12744d6)) * Speed up transaction execution ([#10172](#10172)) ([da265b6](da265b6)) * Using current gas prices in cli-wallet ([#10105](#10105)) ([15ffeea](15ffeea)) ### Bug Fixes * Add pako as a dependency in bb.js ([#10186](#10186)) ([b773c14](b773c14)) * **avm:** Execution test ordering ([#10226](#10226)) ([49b4a6c](49b4a6c)) * Deploy preview master ([#10227](#10227)) ([321a175](321a175)) * Use current base fee for public fee payment ([#10230](#10230)) ([f081d80](f081d80)) ### Miscellaneous * Add traces and histograms to avm simulator ([#10233](#10233)) ([e83726d](e83726d)), closes [#10146](#10146) * Avm-proving and avm-integration tests do not require simulator to export function with jest mocks ([#10228](#10228)) ([f28fcdb](f28fcdb)) * **avm:** Handle parsing error ([#10203](#10203)) ([3c623fc](3c623fc)), closes [#9770](#9770) * **avm:** Zero initialization in avm public inputs and execution test fixes ([#10238](#10238)) ([0c7c4c9](0c7c4c9)) * Bump timeout for after-hook for data store test again ([#10240](#10240)) ([52047f0](52047f0)) * CIVC VK ([#10223](#10223)) ([089c34c](089c34c)) * Declare global types ([#10206](#10206)) ([7b2e343](7b2e343)) * Delete old serialization methods ([#9951](#9951)) ([10d3f6f](10d3f6f)) * Fix migration notes ([#10252](#10252)) ([05bdcd5](05bdcd5)) * Pull out some sync changes ([#10245](#10245)) ([1bfc15e](1bfc15e)) * Remove docs from sync ([#10241](#10241)) ([eeea0aa](eeea0aa)) * Replace relative paths to noir-protocol-circuits ([e7690ca](e7690ca)) </details> <details><summary>barretenberg: 0.65.1</summary> ## [0.65.1](barretenberg-v0.65.0...barretenberg-v0.65.1) (2024-11-27) ### Features * Add total mana used to header ([#9868](#9868)) ([2478d19](2478d19)) * Configure world state block history ([#10216](#10216)) ([01eb392](01eb392)) * Speed up transaction execution ([#10172](#10172)) ([da265b6](da265b6)) ### Bug Fixes * **avm:** Execution test ordering ([#10226](#10226)) ([49b4a6c](49b4a6c)) ### Miscellaneous * **avm:** Handle parsing error ([#10203](#10203)) ([3c623fc](3c623fc)), closes [#9770](#9770) * **avm:** Zero initialization in avm public inputs and execution test fixes ([#10238](#10238)) ([0c7c4c9](0c7c4c9)) * CIVC VK ([#10223](#10223)) ([089c34c](089c34c)) * Pull out some sync changes ([#10245](#10245)) ([1bfc15e](1bfc15e)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.65.1</summary> ## [0.65.1](AztecProtocol/aztec-packages@aztec-package-v0.65.0...aztec-package-v0.65.1) (2024-11-27) ### Miscellaneous * Delete old serialization methods ([#9951](AztecProtocol/aztec-packages#9951)) ([10d3f6f](AztecProtocol/aztec-packages@10d3f6f)) </details> <details><summary>barretenberg.js: 0.65.1</summary> ## [0.65.1](AztecProtocol/aztec-packages@barretenberg.js-v0.65.0...barretenberg.js-v0.65.1) (2024-11-27) ### Features * Speed up transaction execution ([#10172](AztecProtocol/aztec-packages#10172)) ([da265b6](AztecProtocol/aztec-packages@da265b6)) ### Bug Fixes * Add pako as a dependency in bb.js ([#10186](AztecProtocol/aztec-packages#10186)) ([b773c14](AztecProtocol/aztec-packages@b773c14)) </details> <details><summary>aztec-packages: 0.65.1</summary> ## [0.65.1](AztecProtocol/aztec-packages@aztec-packages-v0.65.0...aztec-packages-v0.65.1) (2024-11-27) ### Features * Add total mana used to header ([#9868](AztecProtocol/aztec-packages#9868)) ([2478d19](AztecProtocol/aztec-packages@2478d19)) * Assert metrics in network tests ([#10215](AztecProtocol/aztec-packages#10215)) ([9380c0f](AztecProtocol/aztec-packages@9380c0f)) * Avm inserts nullifiers from private ([#10129](AztecProtocol/aztec-packages#10129)) ([3fc0c7c](AztecProtocol/aztec-packages@3fc0c7c)) * Burn congestion fee ([#10231](AztecProtocol/aztec-packages#10231)) ([20a33f2](AztecProtocol/aztec-packages@20a33f2)) * Configure world state block history ([#10216](AztecProtocol/aztec-packages#10216)) ([01eb392](AztecProtocol/aztec-packages@01eb392)) * Integrate fee into rollup ([#10176](AztecProtocol/aztec-packages#10176)) ([12744d6](AztecProtocol/aztec-packages@12744d6)) * Speed up transaction execution ([#10172](AztecProtocol/aztec-packages#10172)) ([da265b6](AztecProtocol/aztec-packages@da265b6)) * Using current gas prices in cli-wallet ([#10105](AztecProtocol/aztec-packages#10105)) ([15ffeea](AztecProtocol/aztec-packages@15ffeea)) ### Bug Fixes * Add pako as a dependency in bb.js ([#10186](AztecProtocol/aztec-packages#10186)) ([b773c14](AztecProtocol/aztec-packages@b773c14)) * **avm:** Execution test ordering ([#10226](AztecProtocol/aztec-packages#10226)) ([49b4a6c](AztecProtocol/aztec-packages@49b4a6c)) * Deploy preview master ([#10227](AztecProtocol/aztec-packages#10227)) ([321a175](AztecProtocol/aztec-packages@321a175)) * Use current base fee for public fee payment ([#10230](AztecProtocol/aztec-packages#10230)) ([f081d80](AztecProtocol/aztec-packages@f081d80)) ### Miscellaneous * Add traces and histograms to avm simulator ([#10233](AztecProtocol/aztec-packages#10233)) ([e83726d](AztecProtocol/aztec-packages@e83726d)), closes [#10146](AztecProtocol/aztec-packages#10146) * Avm-proving and avm-integration tests do not require simulator to export function with jest mocks ([#10228](AztecProtocol/aztec-packages#10228)) ([f28fcdb](AztecProtocol/aztec-packages@f28fcdb)) * **avm:** Handle parsing error ([#10203](AztecProtocol/aztec-packages#10203)) ([3c623fc](AztecProtocol/aztec-packages@3c623fc)), closes [#9770](AztecProtocol/aztec-packages#9770) * **avm:** Zero initialization in avm public inputs and execution test fixes ([#10238](AztecProtocol/aztec-packages#10238)) ([0c7c4c9](AztecProtocol/aztec-packages@0c7c4c9)) * Bump timeout for after-hook for data store test again ([#10240](AztecProtocol/aztec-packages#10240)) ([52047f0](AztecProtocol/aztec-packages@52047f0)) * CIVC VK ([#10223](AztecProtocol/aztec-packages#10223)) ([089c34c](AztecProtocol/aztec-packages@089c34c)) * Declare global types ([#10206](AztecProtocol/aztec-packages#10206)) ([7b2e343](AztecProtocol/aztec-packages@7b2e343)) * Delete old serialization methods ([#9951](AztecProtocol/aztec-packages#9951)) ([10d3f6f](AztecProtocol/aztec-packages@10d3f6f)) * Fix migration notes ([#10252](AztecProtocol/aztec-packages#10252)) ([05bdcd5](AztecProtocol/aztec-packages@05bdcd5)) * Pull out some sync changes ([#10245](AztecProtocol/aztec-packages#10245)) ([1bfc15e](AztecProtocol/aztec-packages@1bfc15e)) * Remove docs from sync ([#10241](AztecProtocol/aztec-packages#10241)) ([eeea0aa](AztecProtocol/aztec-packages@eeea0aa)) * Replace relative paths to noir-protocol-circuits ([e7690ca](AztecProtocol/aztec-packages@e7690ca)) </details> <details><summary>barretenberg: 0.65.1</summary> ## [0.65.1](AztecProtocol/aztec-packages@barretenberg-v0.65.0...barretenberg-v0.65.1) (2024-11-27) ### Features * Add total mana used to header ([#9868](AztecProtocol/aztec-packages#9868)) ([2478d19](AztecProtocol/aztec-packages@2478d19)) * Configure world state block history ([#10216](AztecProtocol/aztec-packages#10216)) ([01eb392](AztecProtocol/aztec-packages@01eb392)) * Speed up transaction execution ([#10172](AztecProtocol/aztec-packages#10172)) ([da265b6](AztecProtocol/aztec-packages@da265b6)) ### Bug Fixes * **avm:** Execution test ordering ([#10226](AztecProtocol/aztec-packages#10226)) ([49b4a6c](AztecProtocol/aztec-packages@49b4a6c)) ### Miscellaneous * **avm:** Handle parsing error ([#10203](AztecProtocol/aztec-packages#10203)) ([3c623fc](AztecProtocol/aztec-packages@3c623fc)), closes [#9770](AztecProtocol/aztec-packages#9770) * **avm:** Zero initialization in avm public inputs and execution test fixes ([#10238](AztecProtocol/aztec-packages#10238)) ([0c7c4c9](AztecProtocol/aztec-packages@0c7c4c9)) * CIVC VK ([#10223](AztecProtocol/aztec-packages#10223)) ([089c34c](AztecProtocol/aztec-packages@089c34c)) * Pull out some sync changes ([#10245](AztecProtocol/aztec-packages#10245)) ([1bfc15e](AztecProtocol/aztec-packages@1bfc15e)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

dbanks12 requested a review from fcarreiro as a code owner November 22, 2024 03:32

dbanks12 force-pushed the db/insert-private-nullifiers-avm branch from 9be355e to b914be5 Compare November 22, 2024 03:33

dbanks12 requested review from IlyasRidhuan and removed request for fcarreiro November 22, 2024 03:34

dbanks12 force-pushed the db/insert-private-nullifiers-avm branch from b914be5 to e659364 Compare November 22, 2024 03:37

dbanks12 commented Nov 22, 2024

View reviewed changes

dbanks12 force-pushed the db/insert-private-nullifiers-avm branch from e659364 to a3724d3 Compare November 22, 2024 03:46

dbanks12 commented Nov 22, 2024

View reviewed changes

dbanks12 force-pushed the db/insert-private-nullifiers-avm branch 2 times, most recently from 8b5ea85 to 64f296e Compare November 25, 2024 22:26

dbanks12 commented Nov 25, 2024

View reviewed changes

dbanks12 force-pushed the db/insert-private-nullifiers-avm branch 3 times, most recently from 8971154 to dc7ed33 Compare November 26, 2024 02:59

feat: avm inserts nullifiers from private

a8185c9

dbanks12 force-pushed the db/insert-private-nullifiers-avm branch from dc7ed33 to a8185c9 Compare November 26, 2024 17:53

IlyasRidhuan reviewed Nov 26, 2024

View reviewed changes

yarn-project/simulator/src/avm/avm_tree.ts Outdated Show resolved Hide resolved

IlyasRidhuan reviewed Nov 26, 2024

View reviewed changes

yarn-project/simulator/src/avm/avm_tree.ts Show resolved Hide resolved

IlyasRidhuan reviewed Nov 26, 2024

View reviewed changes

yarn-project/simulator/src/avm/avm_tree.ts Show resolved Hide resolved

IlyasRidhuan and others added 2 commits November 27, 2024 09:14

test: Ephemeral tree hashPreimage (#10232)

f04e540

Fix hashPreimage in avmtree, add test Co-authored-by: dbanks12 <david@aztecprotocol.com>

fmt

6e328f4

dbanks12 enabled auto-merge (squash) November 27, 2024 15:09

dbanks12 requested a review from IlyasRidhuan November 27, 2024 15:09

IlyasRidhuan approved these changes Nov 27, 2024

View reviewed changes

Merge branch 'master' into db/insert-private-nullifiers-avm

5ef4eb1

dbanks12 merged commit 3fc0c7c into master Nov 27, 2024
68 checks passed

dbanks12 deleted the db/insert-private-nullifiers-avm branch November 27, 2024 16:06

AztecBot mentioned this pull request Nov 27, 2024

chore(master): Release 0.65.1 #10219

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: avm inserts nullifiers from private #10129

feat: avm inserts nullifiers from private #10129

dbanks12 commented Nov 22, 2024

dbanks12 commented Nov 22, 2024

dbanks12 Nov 22, 2024

dbanks12 Nov 22, 2024

dbanks12 Nov 22, 2024

dbanks12 Nov 22, 2024

dbanks12 Nov 22, 2024

dbanks12 Nov 22, 2024

dbanks12 Nov 25, 2024

dbanks12 Nov 25, 2024

feat: avm inserts nullifiers from private #10129

feat: avm inserts nullifiers from private #10129

Conversation

dbanks12 commented Nov 22, 2024

dbanks12 commented Nov 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment