feat: Add public data read gadget by sirasistant · Pull Request #13138 · AztecProtocol/aztec-packages

sirasistant · 2025-03-28T15:53:49Z

Implements a gadget for public data reads. I'll consider wether to add the columns for write to this gadget or to create a separate gadget for public data writes. Possibly the former.

fcarreiro

Simulation looks good

fcarreiro · 2025-03-28T16:38:03Z

barretenberg/cpp/pil/vm2/public_data_read.pil

+include "./merkle_check.pil";
+include "./ff_gt.pil";
+include "./poseidon2_hash.pil";
+include "./constants_gen.pil";
+include "./precomputed.pil";


I think we don't need ./ ?

fcarreiro · 2025-03-28T16:38:51Z

barretenberg/cpp/src/barretenberg/vm2/constraining/relations/field_gt.test.cpp

 };

-class BasicTest : public TestWithParam<TestParams> {};
+class FieldGreaterThanBasicTest : public TestWithParam<TestParams> {};


I think we were actually calling them SomethingConstrainingTest?

Yes but this class name is appended to FieldGreaterThanConstrainingTest in

INSTANTIATE_TEST_SUITE_P(FieldGreaterThanConstrainingTest, FieldGreaterThanBasicTest, ::testing::ValuesIn(comparison_tests));

results in something like this
[ RUN ] PublicDataTreeReadConstrainingTest/PublicDataReadPositiveTests.Positive/2
I changed the name to make class name not clash with other tests

fcarreiro · 2025-03-28T16:46:28Z

barretenberg/cpp/src/barretenberg/vm2/simulation/public_data_tree_check.test.cpp

+        .low_leaf_hash = low_leaf_hash,
+        .low_leaf_index = low_leaf_index,
+    };
+    EXPECT_THAT(event_emitter.dump_events(), AllOf(SizeIs(1), ElementsAre(expect_event)));


No need to check the size! it's implied in ElementsAre; It does check the size first automatically.

fcarreiro · 2025-03-28T16:47:03Z

barretenberg/cpp/src/barretenberg/vm2/tracegen/public_data_tree_read_trace.cpp

+{
+    using C = Column;
+
+    uint32_t row = 0;


oh wow, no shifts at all ;)

jeanmon · 2025-03-31T11:57:00Z

barretenberg/cpp/pil/vm2/public_data_read.pil

+    pol commit sel;
+    sel * (1 - sel) = 0;
+
+    // Inputs to the gadget


Usage of this gagdet is not clear at first sight.
Is there any output expected?
@sirasistant Could you please add a line of a caller would use this gadget?
Maybe a little description of what it does would be useful.

jeanmon · 2025-03-31T11:57:32Z

barretenberg/cpp/pil/vm2/public_data_read.pil

+namespace public_data_read;
+    pol commit sel;
+    sel * (1 - sel) = 0;
+


@sirasistant We certainly can add a skippable condition.

oh yes pleaaase! I didn't pay attention but basically we shouldn't merge any gadget without skippable

Ugh, my bad, forgot about the skippable condition

jeanmon · 2025-03-31T11:58:03Z

barretenberg/cpp/pil/vm2/public_data_read.pil

+
+    // ========= LOW LEAF MEMBERSHIP =========
+    pol commit low_leaf_hash;
+    // TODO: We need this temporarily while we dont allow for aliases in the lookup tuple


Suggested change

// TODO: We need this temporarily while we dont allow for aliases in the lookup tuple

// TODO: We need this temporarily while we do not allow for aliases in the lookup tuple

jeanmon · 2025-03-31T12:05:46Z

barretenberg/cpp/pil/vm2/public_data_read.pil

+    pol commit slot_low_leaf_slot_diff_inv;
+    pol SLOT_LOW_LEAF_SLOT_DIFF = slot - low_leaf_slot;
+
+    #[EXISTS_FLAG_CHECK]


Add a comment like: SLOT_LOW_LEAF_SLOT_DIFF == 0 <==> LEAF_EXISTS == 0

jeanmon · 2025-03-31T12:06:15Z

barretenberg/cpp/pil/vm2/public_data_read.pil

+    #[EXISTS_FLAG_CHECK]
+    sel * (SLOT_LOW_LEAF_SLOT_DIFF * (LEAF_EXISTS * (1 - slot_low_leaf_slot_diff_inv) + slot_low_leaf_slot_diff_inv) - 1 + LEAF_EXISTS) = 0;
+
+    // If doesn't exist, we need to validate that the slot is greater than the low leaf slot


Suggested change

// If doesn't exist, we need to validate that the slot is greater than the low leaf slot

// If it doesn't exist, we need to validate that the slot is greater than the low leaf slot

jeanmon · 2025-03-31T12:06:26Z

barretenberg/cpp/pil/vm2/public_data_read.pil

+
+    // If doesn't exist, we need to validate that the slot is greater than the low leaf slot
+
+    // TODO: We need this temporarily while we dont allow for aliases in the lookup tuple


Suggested change

// TODO: We need this temporarily while we dont allow for aliases in the lookup tuple

// TODO: We need this temporarily while we do not allow for aliases in the lookup tuple

jeanmon · 2025-03-31T12:37:31Z

barretenberg/cpp/pil/vm2/public_data_read.pil

+    next_slot_is_nonzero { low_leaf_next_slot, slot, one }
+    in ff_gt.sel_gt { ff_gt.a, ff_gt.b, ff_gt.result };
+
+    // ========= VALUE VALIDATION =========


For readability, I would prefer to have this relation just after the definition of LEAF_EXISTS.

This one?

#[VALUE_IS_CORRECT] low_leaf_value * LEAF_EXISTS - value = 0;

jeanmon

Good job!

🤖 I have created a new Aztec Packages release --- ## [0.83.0](v0.82.3...v0.83.0) (2025-04-03) ### ⚠ BREAKING CHANGES * operation mouthwash ([#13171](#13171)) * change version and protocol version to rollupVersion ([#13145](#13145)) * processing events in Aztec.nr ([#12957](#12957)) ### Features * accept multiple consensus client endpoints ([#13022](#13022)) ([1d0185d](1d0185d)) * add ecdsa non ssh account to cli wallet ([#13085](#13085)) ([c5f9984](c5f9984)) * Add parent log link to top of CI logs. ([#13170](#13170)) ([a851087](a851087)) * Add public data read gadget ([#13138](#13138)) ([6bb76db](6bb76db)) * **avm:** merkle db hints (part 3) ([#13199](#13199)) ([7f96676](7f96676)) * **Barretenberg:** static analyzer's routine ([#13207](#13207)) ([84890c2](84890c2)) * derived pending notes capsules slot ([#13102](#13102)) ([6307ba0](6307ba0)) * get mana limit from rollup by default. ([#13029](#13029)) ([a406c54](a406c54)) * Increase CIVC depth with no rollup cost ([#13106](#13106)) ([3a555ef](3a555ef)) * making SyncDataProvider throw before sync ([#13151](#13151)) ([9840241](9840241)) * more benchmarks ([#13103](#13103)) ([7a2c9b7](7a2c9b7)) * **noir:** Allow missing optional fields in msgpack ([#13141](#13141)) ([493dede](493dede)) * processing events in Aztec.nr ([#12957](#12957)) ([88c0e04](88c0e04)) * Prover id defaults to publisher address ([#13206](#13206)) ([f459c3e](f459c3e)) * purge of log decryption in TS ([#12992](#12992)) ([800ab8d](800ab8d)) * register private-only contracts in cli-wallet and misc improvements ([#13245](#13245)) ([ad0530f](ad0530f)) * remove unary trick in decomposition and constraints polishing ([#13080](#13080)) ([0e60255](0e60255)) * split public inputs from proof ([#12816](#12816)) ([ca7151e](ca7151e)) * use magic address for fee juice portal in msgs ([#13241](#13241)) ([6fbc378](6fbc378)) * util for computing proposer/forwarder address ([#13169](#13169)) ([87809b2](87809b2)) * Zw/goblin avm tests ([#12904](#12904)) ([baea4b3](baea4b3)) ### Bug Fixes * add check for rollup version in tx validator ([#13197](#13197)) ([c8220c9](c8220c9)), closes [#13192](#13192) * add flake ([13bdfa7](13bdfa7)) * add flakes ([547d50a](547d50a)) * **avm:** alu interface ([#13115](#13115)) ([101ff78](101ff78)) * await transaction to make proposal ([#13132](#13132)) ([180db9e](180db9e)) * bb mac publish on tag push ([#13174](#13174)) ([8d46c8c](8d46c8c)) * bb workdir permission issue ([#13095](#13095)) ([3685a80](3685a80)) * **bb:** dont publish .zst ([#13173](#13173)) ([4dba4e9](4dba4e9)) * boolean config helper for cli args works now ([#13110](#13110)) ([a93ce6e](a93ce6e)) * **docs:** Register FPC docs, contract deployment, events ([#13222](#13222)) ([49aabfb](49aabfb)) * don't log sepolia acc mnemonic on github action ([#13178](#13178)) ([809b44d](809b44d)) * eq instead of !== ([#13161](#13161)) ([921e347](921e347)) * fetch the correct vk in getSolidityVerifier ([#13157](#13157)) ([71b6719](71b6719)) * force anvil/blob networking to ipv4 on localhost. attempt to fix port flakes ([#13099](#13099)) ([970dae5](970dae5)) * fuzzing build issues ([#13114](#13114)) ([bacae3d](bacae3d)) * Handle proven chain events referring to unseen blocks ([#13144](#13144)) ([229515f](229515f)), closes [#13142](#13142) * handling multiple identical logs in a tx ([#13184](#13184)) ([c100499](c100499)) * incorrect blocknumber in syncTaggedLogs ([e58ea95](e58ea95)) * indexeddb multimap dupes ([#13254](#13254)) ([1e470f0](1e470f0)) * load two more points ([#13119](#13119)) ([2a2904a](2a2904a)) * Masternet to run with the nightly tag ([#13239](#13239)) ([f16b70e](f16b70e)) * newline weirdness in package.json ([#13111](#13111)) ([244ea99](244ea99)) * nightly deploy quotes ([#13253](#13253)) ([775eecf](775eecf)) * prover config read from L1 ([#13237](#13237)) ([13426fe](13426fe)) * Race condition while unwinding blocks ([#13148](#13148)) ([1c2291a](1c2291a)) * read and pass rollup version ([#13232](#13232)) ([4ee1e4a](4ee1e4a)) * recursive sumcheck bugs ([#12885](#12885)) ([a784802](a784802)) * reenable test cache ([4dfca94](4dfca94)) * remove [skip ci] edge condition ([#13228](#13228)) ([4e699ea](4e699ea)) * separator in pending partial notes capsule array slot ([#13153](#13153)) ([0d6ec63](0d6ec63)) * sysdig oops ([5cf6ad1](5cf6ad1)) * tagging bug ([#13061](#13061)) ([280e1a6](280e1a6)) * test tracking failures in merge queue ([#13235](#13235)) ([650fdc1](650fdc1)) * title and external check need to run in merge queue ([e326ed6](e326ed6)) * Transpile cmov ([#13194](#13194)) ([6b94555](6b94555)) * trying to fix EADDRINUSE ([#13176](#13176)) ([260a057](260a057)) * Uninstall gossipsub event handler on service stop ([#13190](#13190)) ([081f30d](081f30d)) * use version from registry for rollup instead of config ([#12938](#12938)) ([7dac390](7dac390)) * validate private double spends in txs with public funcs ([#13088](#13088)) ([4555871](4555871)) * yolo flake and alert fix ([f8de52b](f8de52b)) ### Miscellaneous * add container id to netlog ([9ecdf15](9ecdf15)) * Add e2e test to bootstrap test_cmds ([#13146](#13146)) ([09e4722](09e4722)) * add rollup version as universal cli option ([#13205](#13205)) ([#13213](#13213)) ([568d9e9](568d9e9)) * Add ultra versions of fuzzers in stdlib ([#13139](#13139)) ([aea210b](aea210b)) * align TS and C++ indexed leaf types ([#13185](#13185)) ([d2574cc](d2574cc)) * Assign bb test flake ([#13127](#13127)) ([69fdb04](69fdb04)) * **avm:** remove check_interaction from tests ([#13136](#13136)) ([7d875a6](7d875a6)) * change version and protocol version to rollupVersion ([#13145](#13145)) ([24d7f8b](24d7f8b)) * Cleanup config vars for alpha-testnet ([#13204](#13204)) ([153a606](153a606)) * convenient way to run app ivc from bb ([#13158](#13158)) ([cb1a857](cb1a857)) * Cron snapshot upload in spartan ([#13108](#13108)) ([7c520a8](7c520a8)) * Do not close store before stopping p2p client in tests ([#13223](#13223)) ([63a843e](63a843e)) * docker flake diagnostics. ([48da272](48da272)) * **docs:** alpha-testnet versioning ([#13016](#13016)) ([9ca5d3c](9ca5d3c)) * **docs:** Update CLI faucet command ([#13104](#13104)) ([6eb71de](6eb71de)) * **docs:** Update versions-updating.md ([#13090](#13090)) ([0310d4e](0310d4e)) * Enable debug logging for annoying unit tests ([#13191](#13191)) ([5556524](5556524)) * enable sentinel in sentinel test ([#13219](#13219)) ([f2cfe3f](f2cfe3f)) * Exclude nightly versions from stable fn ([#13196](#13196)) ([a832e90](a832e90)) * Fix flake in e2e fees failures ([#13229](#13229)) ([5defe47](5defe47)) * fix kind logs ([#13023](#13023)) ([c6c2727](c6c2727)), closes [#13053](#13053) * fuzzing build in ci ([#13105](#13105)) ([1c08d38](1c08d38)) * Improve callstacks for public dispatch fns ([#13120](#13120)) ([f67375d](f67375d)) * log out the slash factory when a new rollup is deployed ([#13131](#13131)) ([4643a31](4643a31)) * Merge alpha back to master ([#13128](#13128)) ([504c338](504c338)) * metric attributes ([#13126](#13126)) ([f87d5e3](f87d5e3)), closes [#13063](#13063) * minor tagging API improvement ([#13092](#13092)) ([f5bcece](f5bcece)) * more benchmarking ([#13211](#13211)) ([4b0a4ad](4b0a4ad)) * move unbound impl generics to their functions ([#13147](#13147)) ([62f497f](62f497f)) * operation mouthwash ([#13171](#13171)) ([384f4e5](384f4e5)) * remove catch-all branch from opcode match in transpiler ([#13210](#13210)) ([2bc9ca2](2bc9ca2)) * remove templating by flavor in merge protocol ([#13098](#13098)) ([cf5e217](cf5e217)) * rename journal dir and file to state manager & mv to up to public/ ([#13159](#13159)) ([f13be09](f13be09)) * replace relative paths to noir-protocol-circuits ([f18336d](f18336d)) * replace relative paths to noir-protocol-circuits ([d620c12](d620c12)) * replace relative paths to noir-protocol-circuits ([be3e74e](be3e74e)) * replace relative paths to noir-protocol-circuits ([5bfac63](5bfac63)) * replace relative paths to noir-protocol-circuits ([f41ab2c](f41ab2c)) * replace relative paths to noir-protocol-circuits ([806e560](806e560)) * streamlined log processing ([#13107](#13107)) ([b184865](b184865)) * use testnet optimized trace ([#13135](#13135)) ([5a4f2ac](5a4f2ac)) * uses a new larger redis cache. flake file tweaks. ([#13167](#13167)) ([0951fb6](0951fb6)) ### Documentation * Add intermediate migration note versions ([#13183](#13183)) ([f5de7b8](f5de7b8)) --- 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 11 commits March 27, 2025 14:40

implementation

6157356

interactions tests

55a8ddd

relation names

169acb1

tests

4484452

Merge branch 'master' into arv/public_data_read_gadget

301f3fc

fix

85a891f

more fix

951fa2a

more fix

6c6d0de

test

b9dca21

fmt

765a697

style

b78da93

sirasistant marked this pull request as ready for review March 28, 2025 16:08

sirasistant requested review from IlyasRidhuan, Maddiaa0, fcarreiro and jeanmon as code owners March 28, 2025 16:08

sirasistant removed the request for review from Maddiaa0 March 28, 2025 16:09

fcarreiro reviewed Mar 28, 2025

View reviewed changes

address pr comments

a2ab083

jeanmon reviewed Mar 31, 2025

View reviewed changes

sirasistant added 4 commits March 31, 2025 12:14

address PR comments

f1802f6

add explainer

659e77f

skippable codegen

f2a5bf5

Merge branch 'master' into arv/public_data_read_gadget

4798808

sirasistant requested a review from jeanmon March 31, 2025 12:29

jeanmon reviewed Mar 31, 2025

View reviewed changes

docs

46cff64

sirasistant requested a review from jeanmon March 31, 2025 15:39

sirasistant enabled auto-merge (squash) March 31, 2025 15:39

typo

24bce68

jeanmon approved these changes Apr 1, 2025

View reviewed changes

sirasistant merged commit 6bb76db into master Apr 1, 2025
7 checks passed

sirasistant deleted the arv/public_data_read_gadget branch April 1, 2025 07:58

AztecBot mentioned this pull request Apr 1, 2025

chore(master): release 0.83.0 #13112

Merged

	// TODO: We need this temporarily while we dont allow for aliases in the lookup tuple
	// TODO: We need this temporarily while we do not allow for aliases in the lookup tuple

	// If doesn't exist, we need to validate that the slot is greater than the low leaf slot
	// If it doesn't exist, we need to validate that the slot is greater than the low leaf slot

Conversation

sirasistant commented Mar 28, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sirasistant Mar 28, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeanmon Mar 31, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeanmon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sirasistant commented Mar 28, 2025 •

edited

Loading

sirasistant Mar 28, 2025 •

edited

Loading

jeanmon Mar 31, 2025 •

edited

Loading