feat: Implement merkle writes in the merkle check gadget by sirasistant · Pull Request #13050 · AztecProtocol/aztec-packages

sirasistant · 2025-03-26T15:19:32Z

Refactor the merkle check gadget to be read on start and propagate only the root
Add functionality in the merkle check gadget to check a write into a merkle tree.

fcarreiro

No concerns on the simulation side.

dbanks12

All looks good to me! Only nit is that we probably should rename MerkleCheck to just Merkle or something similar. I don't feel strongly that it needs to happen in this PR. Also maybe there is an argument that MerkleCheck is still appropriate 🤷

dbanks12 · 2025-03-26T18:08:07Z

barretenberg/cpp/pil/vm2/merkle_check.pil

-    end * (current_index_in_layer * (1 - current_index_in_layer)) = 0;
+    end * (index * (1 - index)) = 0;


I overdid the long names 😅 I like to be verbose sometimes, but I definitely overdid it in a few places here.

I changed it to just index mainly because it's now part of the external interface and externally current_index_in_layer feels odd

dbanks12 · 2025-03-26T18:12:21Z

barretenberg/cpp/src/barretenberg/vm2/simulation/merkle_check.cpp

-    assert(curr_index == 0 || curr_index == 1 && "Merkle check's final node index must be 0 or 1");
-    assert(curr_value == root && "Merkle membership or non-membership check failed");
+    if (curr_index != 0 && curr_index != 1) {
+        throw std::runtime_error("Merkle check's final node index must be 0 or 1");
+    }
+    if (curr_value != root) {
+        throw std::runtime_error("Merkle read check failed");
+    }


fcarreiro · 2025-03-26T19:15:36Z

All looks good to me! Only nit is that we probably should rename MerkleCheck to just Merkle or something similar. I don't feel strongly that it needs to happen in this PR. Also maybe there is an argument that MerkleCheck is still appropriate 🤷

I'd vote for MerkleReadWrite if we changed the name, otherwise MerkleCheck is ok to me. We might wanna bring it up on slack

fcarreiro · 2025-03-26T19:18:21Z

barretenberg/cpp/src/barretenberg/vm2/tracegen/merkle_check_trace.cpp

+        FF write_node = event.new_leaf_value.value_or(FF(0));
+
+        FF root = event.root;
+        FF new_root = write ? event.new_root.value() : FF(0);


why not just value_or(0) as well?

That way it'll throw if you create an incorrect event that has new_leaf but not new_root

Altough it wouldn't throw in the opposite. Maybe i should make a small struct with the write only values and make that optional. Or having a write boolean in the event and the new_root and new_leaf defaulted to zero in the event

IlyasRidhuan · 2025-03-27T05:01:25Z

barretenberg/cpp/src/barretenberg/vm2/simulation/events/merkle_check_event.hpp

    uint64_t leaf_index;
    std::vector<FF> sibling_path;
    FF root;
+    std::optional<FF> new_root;


just a thought - i wonder if it would have been simpler in tracegen if these just defaulted to 0. Since there is no lazy-initialization, we still pay memory cost of having this in the event anyways.

yes, but then extracting the write boolean becomes weird: zero is a valid new leaf, I guess we could say "if new_root is zero, then it's a read only" but that felt a bit ugly

🤖 I have created a new Aztec Packages release --- ## [0.82.3](v0.82.2...v0.82.3) (2025-03-27) ### Features * `msgpack` encoding for `Program` and `WitnessStack` ([#12841](#12841)) ([1e58eb1](1e58eb1)) * 64 bit log type id, 64 bit log metadata ([#12956](#12956)) ([20d734a](20d734a)) * AVM parsing tag validation ([#12936](#12936)) ([56b1f0d](56b1f0d)) * **avm:** add calldata & returndata to context ([#13008](#13008)) ([f03b2e5](f03b2e5)) * **avm:** merkle db hints (part 1) ([#12922](#12922)) ([34ec9e8](34ec9e8)) * **avm:** merkle hints (part 2) ([#13077](#13077)) ([fbbc6c7](fbbc6c7)) * **avm:** vm2 initial context ([#12972](#12972)) ([e2b1361](e2b1361)) * benchmark avm simulator ([#12985](#12985)) ([00fae1b](00fae1b)) * client flows benchmarks ([#13007](#13007)) ([9bf7568](9bf7568)) * gas benchmark for "normal usage" ([#13073](#13073)) ([4eb1156](4eb1156)) * Implement merkle writes in the merkle check gadget ([#13050](#13050)) ([c94fe50](c94fe50)) * LogEncryption trait ([#12942](#12942)) ([0b7e564](0b7e564)) * Node snapshot sync ([#12927](#12927)) ([afde851](afde851)), closes [#12926](#12926) * **p2p:** capture all gossipsub metrics ([#12930](#12930)) ([cc940cb](cc940cb)) * Prover node snapshot sync ([#13097](#13097)) ([1e77efb](1e77efb)) * staking asset handler ([#12968](#12968)) ([af48184](af48184)), closes [#12932](#12932) * stream crs data to disk ([#12996](#12996)) ([d016e4d](d016e4d)), closes [#12948](#12948) * track failed tests. add flake. ([f4936d7](f4936d7)) * Track test history. ([#13037](#13037)) ([036bb32](036bb32)) * track total tx fee ([#12601](#12601)) ([9612a4e](9612a4e)) * Validators sentinel ([#12818](#12818)) ([770695c](770695c)) ### Bug Fixes * added #[derive(Eq)] to EcdsaPublicKeyNote ([#12966](#12966)) ([0c21c74](0c21c74)) * Allow use of local blob sink client ([#13025](#13025)) ([ba8d654](ba8d654)) * **avm:** semicolons are hard ([#12999](#12999)) ([8871c83](8871c83)) * bootstrap network and sponsored fpc devnet ([#13044](#13044)) ([8a47d8b](8a47d8b)) * Bump tsc target ([#13052](#13052)) ([985e83b](985e83b)) * cycle_group fuzzer ([#12921](#12921)) ([69f426e](69f426e)) * **docs:** Fix import errors in aztec.js tutorial ([#12969](#12969)) ([856208a](856208a)) * **docs:** Load token artifact from the compiled source in the sample dapp tutorial ([#12802](#12802)) ([0838084](0838084)), closes [#12810](#12810) * **docs:** Update sponsored fpc docs to use 82.2 syntax ([#13054](#13054)) ([e5d425b](e5d425b)) * **e2e:** p2p ([#13002](#13002)) ([1ece539](1ece539)) * extend e2e 2 pxes timeout. strip color codes for error_regex. ([73820e4](73820e4)) * flake ([6cc9e81](6cc9e81)) * fuzzer on staking asset handler constructor test ([#13101](#13101)) ([d936285](d936285)) * invalid getCommittee function ([#13072](#13072)) ([327341f](327341f)) * mac publish should use clang 18 like x-compiler, and use it ([#12983](#12983)) ([7b83c45](7b83c45)) * make circuit parsing deterministic ([#11772](#11772)) ([76ef873](76ef873)) * parse away trailing slash from consensus host ([#12577](#12577)) ([6701806](6701806)) * prerelease versions should be pushed to install.aztec.network ([#13086](#13086)) ([c4e6039](c4e6039)) * smoke ([#13060](#13060)) ([7756b15](7756b15)) * some flake additions ([58638f1](58638f1)) * sponsored fpc arg parsed correctly ([#12976](#12976)) ([#12977](#12977)) ([a85f530](a85f530)) * starting the sandbox with no pxe should still deploy initial test accounts ([#13047](#13047)) ([d92d895](d92d895)) * Syntax error when running tests via jest after tsc build ([#13051](#13051)) ([f972db9](f972db9)) * Use the correct image in aztec start ([#13058](#13058)) ([06285cd](06285cd)) * yolo fix ([91e2f4b](91e2f4b)) * yolo fix nightly ([b3b3259](b3b3259)) * yolo fix obvious thing to track fails. ([2fee630](2fee630)) * yolo flakes ([e3b030a](e3b030a)) * yolo set -x ([bfd3205](bfd3205)) * yolo we suspect the halt is making tests fail that would have passed ([04e3fa2](04e3fa2)) ### Miscellaneous * `getIndexedTaggingSecretAsSender` oracle cleanup ([#13015](#13015)) ([8e71e55](8e71e55)) * Add a script to generate cpp files for AVM2 ([#13091](#13091)) ([7bb43a9](7bb43a9)) * add default native proving for cli wallet ([#12855](#12855)) ([c0f773c](c0f773c)) * add default native proving for cli wallet retry ([#13028](#13028)) ([b2f4785](b2f4785)) * Alpha testnet into master ([#13033](#13033)) ([d98fdbd](d98fdbd)) * AVM TS - move tag validation outside of instruction constructors ([#13038](#13038)) ([45548ab](45548ab)), closes [#12934](#12934) * **avm:** final codegen nuking ([#13089](#13089)) ([9c82f3f](9c82f3f)) * **avm:** remove codegen (all but flavor) ([#13079](#13079)) ([e1f2bdd](e1f2bdd)) * **bb:** minor acir buf C++ improvements ([#13042](#13042)) ([1ebd044](1ebd044)) * boxes dep cleanup ([#12979](#12979)) ([6540b7c](6540b7c)) * **ci:** less catch all e2e_p2p flakes ([#12737](#12737)) ([2134634](2134634)) * comprehensive cleanup of translator flavor and use inheritance properly in flavors ([#13041](#13041)) ([dc5f78f](dc5f78f)) * compress storage footprint ([#12871](#12871)) ([58c110f](58c110f)) * display warning when installing bb versions < 0.82.0 ([#13027](#13027)) ([7247fe7](7247fe7)) * **docs:** Update docs on fees and various other updates ([#12929](#12929)) ([1dec907](1dec907)) * dump dmesg/net/cpu/mem usage at end of ci run ([#12967](#12967)) ([8877792](8877792)) * fix governance util issue ([#13043](#13043)) ([d768d26](d768d26)) * redundant if in affine from projective constructor ([#13045](#13045)) ([3a7ba2d](3a7ba2d)) * remove addition of dummy ops in mock circuit producer ([#13003](#13003)) ([a64d1dc](a64d1dc)) * remove dummy ops in decider pk ([#13049](#13049)) ([da6d021](da6d021)) * replace relative paths to noir-protocol-circuits ([e1b88f6](e1b88f6)) * replace relative paths to noir-protocol-circuits ([849b4b0](849b4b0)) * replace relative paths to noir-protocol-circuits ([18a02d6](18a02d6)) * Revert "chore: add default native proving for cli wallet ([#12855](#12855))" ([#13013](#13013)) ([98e2576](98e2576)) * Speed up and deflake sentinel test ([#13078](#13078)) ([27f1eca](27f1eca)) * **testnet:** making consensus host mandatory input ([#12716](#12716)) ([d47c74a](d47c74a)) * towards no more mock op_queues ([#12984](#12984)) ([fefffa7](fefffa7)) * update bb version for noir 1.0.0-beta.0+ ([#13026](#13026)) ([dd68074](dd68074)) * update CODEOWNERS to reflect new sync method ([#12998](#12998)) ([a3d1915](a3d1915)) ### Documentation * Add fees to cli reference ([#12884](#12884)) ([4a0fd58](4a0fd58)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

sirasistant added 7 commits March 25, 2025 10:01

refactor merkle check

9eef70b

implement writes

e34ba1b

wip

a17c61f

moar tests

dc2096f

rename

6f36949

refactor

4cff78e

Merge branch 'master' into arv/merkle_write_gadget

33d152b

sirasistant marked this pull request as ready for review March 26, 2025 15:47

sirasistant requested review from IlyasRidhuan, Maddiaa0, fcarreiro and jeanmon as code owners March 26, 2025 15:47

sirasistant changed the title ~~feat: Implement merkle writes in the merkle check gadget (WIP)~~ feat: Implement merkle writes in the merkle check gadget Mar 26, 2025

improve test

c08ad03

sirasistant requested a review from dbanks12 March 26, 2025 15:51

sirasistant added 6 commits March 26, 2025 15:58

fix

2113ffa

expect with message

2db96b8

refine tests

6010c4a

remove extraneous change

84e0ab5

refactor

df067e9

improve test

30155b7

fcarreiro reviewed Mar 26, 2025

View reviewed changes

dbanks12 approved these changes Mar 26, 2025

View reviewed changes

fcarreiro reviewed Mar 26, 2025

View reviewed changes

IlyasRidhuan reviewed Mar 27, 2025

View reviewed changes

sirasistant added 2 commits March 27, 2025 09:37

small refactor

d303ce2

Merge branch 'master' into arv/merkle_write_gadget

2445f07

sirasistant merged commit c94fe50 into master Mar 27, 2025
7 checks passed

sirasistant deleted the arv/merkle_write_gadget branch March 27, 2025 13:33

AztecBot mentioned this pull request Mar 27, 2025

chore(master): release 0.82.3 #12980

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implement merkle writes in the merkle check gadget#13050

feat: Implement merkle writes in the merkle check gadget#13050
sirasistant merged 16 commits intomasterfrom
arv/merkle_write_gadget

sirasistant commented Mar 26, 2025 •

edited

Loading

Uh oh!

fcarreiro left a comment

Uh oh!

dbanks12 left a comment

Uh oh!

dbanks12 Mar 26, 2025

Uh oh!

sirasistant Mar 27, 2025

Uh oh!

dbanks12 Mar 26, 2025

Uh oh!

fcarreiro commented Mar 26, 2025

Uh oh!

fcarreiro Mar 26, 2025

Uh oh!

sirasistant Mar 27, 2025

Uh oh!

sirasistant Mar 27, 2025

Uh oh!

IlyasRidhuan Mar 27, 2025

Uh oh!

sirasistant Mar 27, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		end * (current_index_in_layer * (1 - current_index_in_layer)) = 0;
		end * (index * (1 - index)) = 0;

Conversation

sirasistant commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fcarreiro left a comment

Choose a reason for hiding this comment

Uh oh!

dbanks12 left a comment

Choose a reason for hiding this comment

Uh oh!

dbanks12 Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

sirasistant Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

dbanks12 Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

fcarreiro commented Mar 26, 2025

Uh oh!

fcarreiro Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

sirasistant Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

sirasistant Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

IlyasRidhuan Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

sirasistant Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sirasistant commented Mar 26, 2025 •

edited

Loading

sirasistant Mar 27, 2025 •

edited

Loading