Conversation
This was referenced Oct 17, 2024
Contributor
Author
This was referenced Oct 17, 2024
da9a81f to
2d638df
Compare
5404e50 to
6388e81
Compare
Base automatically changed from
ek/feat/new-address-implementation/modify-contract-instance-to-include-public-keys
to
master
October 17, 2024 21:45
6388e81 to
9424c07
Compare
5b8d888 to
dea191e
Compare
5721621 to
58d05a3
Compare
db4767f to
f3fd5d4
Compare
bc6018f to
62b4c08
Compare
e1846a5 to
20c067b
Compare
62b4c08 to
be7b51a
Compare
20c067b to
b561c82
Compare
Base automatically changed from
ek/feat/new-address-scheme-implementation/quick-account-manager-refactor
to
master
October 24, 2024 13:22
9585935 to
9de53fd
Compare
This was referenced Oct 24, 2024
sklppy88
commented
Oct 24, 2024
| const encryptedLogs = encryptedTxLogs.flatMap(encryptedTxLog => encryptedTxLog.unrollLogs()); | ||
|
|
||
| const vsks = await Promise.all(vpks.map(vpk => this.keyStore.getMasterSecretKey(vpk))); | ||
| const vsks = await Promise.all( |
Contributor
Author
There was a problem hiding this comment.
The changes in this file are a bit janky and should be fixed in a follow up PR, have noted it in the description.
sklppy88
commented
Oct 24, 2024
| @@ -75,7 +75,8 @@ mod test { | |||
| #[test] | |||
Contributor
Author
There was a problem hiding this comment.
This file's changes was just due to the formatter not being run on master
nventuro
reviewed
Oct 24, 2024
noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr
Outdated
Show resolved
Hide resolved
noir-projects/noir-protocol-circuits/crates/types/src/address/aztec_address.nr
Outdated
Show resolved
Hide resolved
nventuro
approved these changes
Oct 24, 2024
Contributor
nventuro
left a comment
There was a problem hiding this comment.
Lovely to see this finally be implemented! 🙌
This was referenced Oct 28, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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 innote_processor.ts.Outstanding work:
The interface of
getEventsin 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.nris 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:
