Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the aztec-nr Address derivation #8966

Closed
Tracked by #8953
nventuro opened this issue Oct 2, 2024 · 1 comment · Fixed by #9272
Closed
Tracked by #8953

Change the aztec-nr Address derivation #8966

nventuro opened this issue Oct 2, 2024 · 1 comment · Fixed by #9272
Assignees
Labels
C-aztec.nr Component: Aztec smart contract framework team-fairies Nico's team

Comments

@nventuro
Copy link
Contributor

nventuro commented Oct 2, 2024

The AztecAddress is currently computed as poseidon2(pub_keys_hash, partial_address). This needs to change to:

let h: Field = poseidon2(pub_keys_hash, partial_address); // h is also called 'pre_address'
let mut maybe_address: Point = h * G + Ivpk_m;
if maybe_address.y > Y_HALF_RANGE { maybe_address = -maybe_address }
let address: Field = maybe_address.x; // wrap in AztecAddress

Note that this means we can no longer derive an address from just the pub keys hash and the partial address, we now also need the Ivpk_m (which part of the preimage of the pub keys hash, and we should know it in all flows in which we also know the pub keys hash).

@github-project-automation github-project-automation bot moved this to Todo in A3 Oct 2, 2024
@nventuro nventuro added C-aztec.nr Component: Aztec smart contract framework team-fairies Nico's team labels Oct 2, 2024
@nventuro
Copy link
Contributor Author

nventuro commented Oct 10, 2024

Contract instances also have to prove the derivation of the public keys hash to show that they Ivpk_m they're using in the final address derivation is the same as the one that's behind said hash. This prevents malleability issues as otherwise Ivpk_m is a free parameter unlinked to the keys hash (i.e. uncomitted).

sklppy88 added a commit that referenced this issue Oct 25, 2024
… address secret (#9272)

Resolves #9326, #8966, #8969

This PR finally implements the new address scheme in encryption and
decryption. We encrypt with the address's point, and decrypt with
preaddress + ivsk (addressSecret).

Some nomenclature before we start:
The old address (h in Mike's presentation; hash(partialAddress,
publicKeysHash)) -> preAddress
The secret corresponding to the address point -> addressSecret

The flow generally works by taking a secret, and deriving a valid point
from it.

We then store the x-coordinate of this point as the address. We do this
even though we know that this x-coordinate has two valid y-coordinates
(a positive and negative one), but we do not store any information about
the sign in the address.

Even still, we can support secrets that get computed into a positive and
a negative y coordinate.

To do this, whenever we recompute the y-coordinate to recover the point
from the x-coordinate, we make sure to encrypt to the positive point
only. i.e. if we solve for y with x, and we get a negative coordinate,
we subtract it from the Field modulus to get a positive one.

But if you think "hey, we can't do that, our secret corresponds to a
negative y-coordinate", you would be right. In order to address this, we
as the owner of the secret, can recompute our full point as we know all
of the information that can derive this point. Thus we know what sign
our "true" y-coordinate is. In this case, if our y-coordinate is
negative, all we need to do is negate our secret (Field modulus minus
secret) to derive the secret for point containing the negated negative
(and now positive) y-coordinate.

You can see that this above process is being done, with the the
encryption taking place in `payload.nr`, and that the decryption taking
place in `note_processor.ts`.

Outstanding work:
The interface of `getEvents` in pxe_service should be investigated. With
these changes it works... but it's unnecessarily disgusting I think.

The interface of the encryption api in `payload.nr` is extremely jank,
but this pr is getting pretty big, so it will be handled imminently in
#9390.

Look through the rest of the tests, think about replacing arbitrary
addresses with "valid" ones.

Remove any excess code relating to needing the ivpk in both ts and nr

Docs and migration notes. As this change is pretty big I think it would
be good to go through this also later / with someone on the devrel team
to make sure the docs are comprehensively updated.

More of this stack doesn't show up on the graphite comment here:
<img width="1415" alt="image"
src="https://github.com/user-attachments/assets/4570a071-29ad-4148-9e7b-60c2fc978324">
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Oct 25, 2024
@iAmMichaelConnor iAmMichaelConnor removed this from A3 Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-aztec.nr Component: Aztec smart contract framework team-fairies Nico's team
Projects
None yet
2 participants