feat!: re-do partial notes #12391
feat!: re-do partial notes #12391
Conversation
| @@ -1,25 +1,320 @@ | |||
| use aztec::{macros::notes::partial_note, oracle::random::random, prelude::AztecAddress}; | |||
| use dep::aztec::{ | |||
There was a problem hiding this comment.
This file is a copy-paste of uint_note.nr except using field instead of u128. Read that one instead.
| let encrypted_log = default_aes128::note::compute_log( | ||
| *context, | ||
| private_log_content, | ||
| storage_slot, | ||
| recipient, | ||
| sender, | ||
| ); | ||
| context.emit_private_log(encrypted_log); |
There was a problem hiding this comment.
is there a way of not-choosing to use default_aes128 encryption strategy here?
can the fn partial have as input an encryption method?
There was a problem hiding this comment.
As of today, encryption is hardcoded in PXE to be AES. We'll soon move it into aztec-nr, at which point it'd be possible to somehow configure which encryption algorithm the contracts wishes to use. But this is just a first step, so it'll take a while to get there. Keep in mind this is the very first attempt to do this this alternative way. We'd similarly want to allow for unconstrained encryption etc.
There was a problem hiding this comment.
this file imports encrypted_logs::log_assembly_strategies::default_aes128::note::encode_and_encrypt_note but could be using other encryption strategy, are both partial notes and full notes processed independently? (could they work with different setups?)
|
|
||
| NFT::at(context.this_address())._store_nft_set_partial_note(partial_note).enqueue(context); | ||
|
|
||
| partial_note | ||
| } |
There was a problem hiding this comment.
Fanstastic how concise ^ this has become.
benesjan
force-pushed
the
nv/redo-partial
branch
2 times, most recently
from
c6fc6cb to
106ce57
Compare
benesjan
force-pushed
the
nv/redo-partial
branch
3 times, most recently
from
2a9675f to
35e25d2
Compare
benesjan
force-pushed
the
nv/redo-partial
branch
from
b63c9a1 to
dfd887c
Compare
benesjan
force-pushed
the
nv/redo-partial
branch
from
dfd887c to
43a43b4
Compare
|
CI should pass once #12552 is merged. |
Back when PXE was a service processing all blocks, we'd remove nullified notes as we saw their nullifiers. This later got changed, and as of #10722 we remove all nullified notes whenever we 'sync' notes. However, note syncing is becoming less and less a part of PXE, and as of #12391 we even deliver notes _outside_ of the PXE-led note syncing process (whenever we complete partial note). This causes problems because we end up adding notes, failing to realize they've been nullified and then returning them via `get_notes` (which is what causes some tests in #12391 to fail). The next time a contract function is run we'll do note syncing again and they'll be then removed, but we did have a full fn call in which they were available. This PR makes it so we always check if newly-added notes have been nullified, and remove them if so. I also added some explanations re. why we're doing things this way, created some follow-up issues (mostly #12550 and #12553), and inlined `produceNoteDaos` to have the whole thing happen in a single place. I think it's now more readable but potentially slightly large - perhaps this will improve as we split `PxeOracleInterface` in multiple files or modules.
| let partial_note = NFTNote::partial( | ||
| to, | ||
| storage.private_nfts.at(to).storage_slot, | ||
| context, | ||
| to, | ||
| context.msg_sender(), | ||
| ); |
There was a problem hiding this comment.
(Not for this PR): It'd be cool if this could be created via the state variable. Like private_nfts.at(to).new_partial(...)
Closes #9375.
This re-implements partial notes leveraging the work from #12122. We now emit a private log with all of the fields plus a 'public log completion tag', which is the tag of the public log that will contain the public fields. These two logs are merged when doing discovery, resulting in the note's packed representation.
All in all this means we need to store much less information in public storage (just one field), and we don't need to deal with publishing public logs with encrypted public content. It also removes a lot of PXE code that dealt with the destructuring of these logs, slicing off the public fields, etc. Once this is merged, moving decryption to NR should be trivial.
I iterated a bit on how the note structs are laid out and thing the current version is a reasonably good starting point. Eventually we'll autogenerate it via macros, but given we may want to introduce further changes I chose to not worry about that for now.
We're currently sort of hackily relying on the fact that note type ids are 7 bits long, and set the 8th bit to indicate the log is that of a partial note. Fixing this requires properly supporting events during log discovery (which would become message discovery), but that's out of scope for this PR.