feat(avm): Address derivation gadget by sirasistant · Pull Request #12721 · AztecProtocol/aztec-packages

sirasistant · 2025-03-13T14:50:35Z

Pure part of address derivation, contract updates will come in a followup PR.

fcarreiro

Looking good, will let @jeanmon review the PIL

barretenberg/cpp/src/barretenberg/vm2/simulation/concrete_dbs.cpp

barretenberg/cpp/src/barretenberg/vm2/tracegen/address_derivation_trace.cpp

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

fcarreiro

LG! just some suggestions

fcarreiro · 2025-03-13T16:36:23Z

barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.hpp

        return infinity;
    }

+    static StandardAffinePoint& one()


fcarreiro · 2025-03-13T16:40:06Z

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

+    FF preaddress = poseidon2::hash({ GENERATOR_INDEX__CONTRACT_ADDRESS_V1, public_keys_hash, partial_address });
+
+    EmbeddedCurvePoint g1 = EmbeddedCurvePoint::one();
+    EmbeddedCurvePoint preaddress_public_key = g1 * grumpkin::fr(preaddress);


is grumpkin::fr what we want o do we better want sth from fields.hpp or the flavorsettinggs?

Forgot to change this one to fq

fcarreiro · 2025-03-13T16:41:40Z

barretenberg/cpp/src/barretenberg/vm2/simulation/address_derivation.cpp

+    events.emit({
+        .address = address,
+        .instance = instance,
+        .salted_initialization_hash = salted_initialization_hash,
+        .partial_address = partial_address,
+        .public_keys_hash = public_keys_hash,
+        .preaddress = preaddress,
+        .preaddress_public_key = preaddress_public_key,
+        .address_point = address_point,


do std::move() for anything that is a vector

Nothing is a vector!

AztecAddress address; ContractInstance instance; FF salted_initialization_hash; FF partial_address; FF public_keys_hash; FF preaddress; EmbeddedCurvePoint preaddress_public_key; EmbeddedCurvePoint address_point;

oh my bad, got confused by the vectors in the body

fcarreiro · 2025-03-13T16:42:23Z

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

@@ -0,0 +1,84 @@
+#include "barretenberg/vm2/simulation/address_derivation.hpp"
+
+#include "gmock/gmock.h"


nit: use <>, I know vscode sometimes does this

fcarreiro · 2025-03-13T16:43:58Z

barretenberg/cpp/src/barretenberg/vm2/tracegen/address_derivation_trace.cpp

+        row++;
+    }
+}
+} // namespace bb::avm2::tracegen


nit: newline before

jeanmon · 2025-03-14T10:04:21Z

barretenberg/cpp/pil/vm2/address_derivation.pil

+    sel * (partial_address_domain_separator - constants.GENERATOR_INDEX__PARTIAL_ADDRESS) = 0;
+
+    #[SALTED_INITIALIZATION_HASH_POSEIDON2_0]
+    sel { partial_address_domain_separator, salt, init_hash, salted_init_hash }


IIRC, we do not support a constant nor alias in a lookup tuple?
Because otherwise, we could inline constants.GENERATOR_INDEX__PARTIAL_ADDRESS in the tuple and get rid of
the above relation.
I suggest to write a TODO so that we can simplify once we support constants in lookups.

I'll write a todo, unfortunately we can't, on lookups we need to use real columns ):

sirasistant · 2025-03-14T10:24:46Z

barretenberg/cpp/pil/vm2/address_derivation.pil

+
+    #[SALTED_INITIALIZATION_HASH_POSEIDON2_1]
+    sel { deployer_addr, precomputed.zero, precomputed.zero, salted_init_hash}
+    in poseidon2_hash.end { poseidon2_hash.input_0, poseidon2_hash.input_1, poseidon2_hash.input_2, poseidon2_hash.output };


Poseidon2 was changed to propagate the final output on the whole run. It's however still incomplete, I talked with Ilyas about it because we need to lookup also the iteration index. This hash run in poseidon looks like this:

+----------------------------------+---------+-----------+------------------+-------+-----+-----+ | input_0 | input_1 | input_2 | output | start | end | sel | +----------------------------------+---------+-----------+------------------+-------+-----+-----+ | 0 | 0 | 0 | 0 | 0 | 0 | 0 | | partial_address_domain_separator | salt | init_hash | salted_init_hash | 1 | 0 | 1 | | deployer_addr | 0 | 0 | salted_init_hash | 0 | 1 | 1 | | 0 | 0 | 0 | 0 | 0 | 0 | 0 | +----------------------------------+---------+-----------+------------------+-------+-----+-----+

In order for it to be safe I need to assert that start is on on the first one and end is on in the second one. However, a malicious prover could still insert more rounds between the two rows, that's why poseidon will need to be improved to include a round_index column, so I can lookup with 0 on start and 1 on end so I can be safe that a malicious prover hasn't inserted more rounds

jeanmon

pil file looks good to me. I just skimmed over the rest of the PR.

jeanmon · 2025-03-14T10:37:11Z

barretenberg/cpp/pil/vm2/address_derivation.pil

+    sel * (g1_x - 1) = 0;
+
+    pol commit g1_y;
+    sel * (g1_y - 17631683881184975370165255887551781615748388533673675138860) = 0;


We could define a constant for this monster literal and add a comment about what it is.

jeanmon · 2025-03-14T10:46:17Z

barretenberg/cpp/src/barretenberg/vm2/generated/flavor.cpp

 {
    for (auto [poly, full_poly] : zip_view(get_all(), full_polynomials.get_all())) {
-        // After the initial sumcheck round, the new size is CEIL(size/2).
-        size_t desired_size = full_poly.end_index() / 2 + full_poly.end_index() % 2;


If you rebase on latest master and rerun bb_pilcom/bootstrap.sh, this should disappear.

jeanmon · 2025-03-14T10:48:33Z

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

+    check_interaction<lookup_public_preaddress_poseidon2>(trace);
+    check_interaction<lookup_public_preaddress_scalar_mul>(trace);
+    check_interaction<lookup_address_ecadd>(trace);
+}


There are no negative tests but not sure what is meaningful to test as it consists mainly in lookups and therefore we would test that lookups are working as expected.

yes the only relations are trivial constants and lookups

* master: fix: filter for empty attestations when pulling from block (#12740) feat: one-way noir sync (#12592) feat(avm): Address derivation gadget (#12721) chore(ci): add workflow dispatch to masternet (#12739) feat: add default accounts (#12734) feat: gas reports and snapshots (#12724) fix(avm): fix vm1 fake proof size (#12733) feat(bb): extend_edges optimization for zero values past end_index (#12703) fix: remove hard coding of constructor for account manager (#12678) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg fix: verify_honk_proof inputs generation in bootstrap (#12457) fix: Patches to cycle_group and cycle_group fuzzer (#12385)

sirasistant added 2 commits March 11, 2025 17:01

address derivation pil

3aa9a5c

fixes and tests

ed880cb

sirasistant requested review from IlyasRidhuan, Maddiaa0, fcarreiro and jeanmon as code owners March 13, 2025 14:50

sirasistant removed request for IlyasRidhuan, Maddiaa0, fcarreiro and jeanmon March 13, 2025 14:51

fcarreiro reviewed Mar 13, 2025

View reviewed changes

sirasistant added 4 commits March 13, 2025 15:51

improve testing

df46e1a

embedded curve one

238cb52

remove address from instance struct

ea30a85

refactor

8d48474

sirasistant requested review from fcarreiro and jeanmon March 13, 2025 16:33

fcarreiro reviewed Mar 13, 2025

View reviewed changes

sirasistant added 4 commits March 13, 2025 16:56

apply suggestions from code review

062b4cd

use Fq

9cd6810

fix bulk test

f3d2d6f

Merge branch 'master' into arv/address_derivation_gadget

215fae5

jeanmon reviewed Mar 14, 2025

View reviewed changes

added todos

d8aae85

sirasistant commented Mar 14, 2025

View reviewed changes

jeanmon reviewed Mar 14, 2025

View reviewed changes

apply suggestions from code review

74d33a6

fcarreiro approved these changes Mar 14, 2025

View reviewed changes

sirasistant enabled auto-merge (squash) March 14, 2025 11:54

sirasistant merged commit 268a614 into master Mar 14, 2025
6 checks passed

sirasistant deleted the arv/address_derivation_gadget branch March 14, 2025 12:04

AztecBot mentioned this pull request Mar 14, 2025

chore(master): release 0.81.0 #12689

Merged

		@@ -0,0 +1,84 @@
		#include "barretenberg/vm2/simulation/address_derivation.hpp"

		#include "gmock/gmock.h"

Conversation

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

Uh oh!

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!

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 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sirasistant commented Mar 13, 2025 •

edited

Loading