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

Re-do partial note delivery using pending work db #10724

Open
Tracked by #10652
nventuro opened this issue Dec 13, 2024 · 4 comments
Open
Tracked by #10652

Re-do partial note delivery using pending work db #10724

nventuro opened this issue Dec 13, 2024 · 4 comments

Comments

@nventuro
Copy link
Contributor

nventuro commented Dec 13, 2024

Once contracts have access to a PXE DB(#10730), we can migrate to the new (final) partial notes flow. The concept is as follows:

  • Logs are encrypted and emitted from private as soon as partial notes are created. This prevents the tagging index from being left hanging in case the note is not completed soon, since the log will be delivered immediately. This log will of course not be complete.
  • Partial notes will have a different note type id that will mark them as such. Perhaps we could use 127 bits for the id, and leave the msb for a complete/partial boolean flag.
  • Whenever we find a partial note after decrypting a log, we store what we've found in PXE as 'pending work'.
  • As part of the note syncing process, we read the pending work database and try to find any logs that correspond to the public component by searching for their tag. This tag might simply be the hash of the partial note content.
  • If the public log is found, we merge it with the private part and compute the full note content. We'd then require the tx note hashes in order to do nonce discovery (Consider bundling tx effects in getLogsByTag #9789)
@benesjan
Copy link
Contributor

benesjan commented Dec 19, 2024

With this PR note type id is now only 7 bits long but it is serialized to 2 bytes in the to_be_bytes function. This means there is a space for the 1 bit partial note flag.

Look for TODO(#10724) to spot where in the codebase this is.

@nventuro
Copy link
Contributor Author

Note that to_be_bytes will go away once we move to field-based logs.

@benesjan
Copy link
Contributor

As discussed in DMs will leave the encoding as it is now so the comment above no longer applies.

@nventuro
Copy link
Contributor Author

nventuro commented Jan 8, 2025

#10867 introduced an end to end test for the pxe database that we should remove once we implement this, as the note discovery tests will indirectly be testing the db already.

nventuro added a commit that referenced this issue Jan 16, 2025
Closes #9576

This PR offloads the later stages of note log processing from PXE into
aztec-nr. The plan is to take over at the decrypted log payload stage,
and later on expand scope to also include decryption and (eventually)
tagging indices management.

Update: this now works, with some caveats. I'll leave some comments here
re. current status in case anyone wants to take a look, and for me to
resume work once I'm back.

Contracts are now expected to have a `process_logs` function that will
be called during note syncing. This function is not yet being
autogenerated, but I did manually add it to SchnorrAccount and
TokenContract for local testing - it should be fairly easy to
autogenerate it.

PXE still performs tagging index synchronization, fetches all relevant
logs, decrypts them, and merges the public and private components when
they are partial note logs. This will continue to be this way for a
while: we'll tackle these problems in #10723 and #10724. However, past
this point we delegate work to the contract.[^1] The contract performs
nonce discovery and computes note hash and nullifier, and then calls a
new PXE oracle called `deliverNote`. PXE validates that the note hash
exists in the tree, and adds the note to its database. **Edit:** I now
updated this section to remove all of the old relevant TS code. We no
longer do nonce discovery in PXE.

With this first step, PXE no longer needs to know about note type ids,
which become exclusively an aztec-nr concept. It will continue to know
about storage slots, but only because we index by them. More importantly
however, this makes us quite ready to continue building on top of this
work in order to fully move the other parts of the stack (notably
decryption and partial notes) into the contract as well.

[^1]: As of right now we're still doing all of the work in PXE,
including payload destructuring and nonce discovery, but we discard the
results and re-compute them in the contract. Changing this involves
deleting a bunch of files and re-structuring some dataflows, and I
haven't gotten round to it yet. We should do this in this PR.

---------

Co-authored-by: Jan Beneš <[email protected]>
nventuro added a commit that referenced this issue Jan 17, 2025
This extends the features introduced in
#10867 so that we
can also delete entries and copy multiple entries from one place to
another. With these two new primitives I also built a Noir `DBArray`,
which is a dynamic array backed by this database supporting pushes and
deletion of arbitrary indexes (which we'll need for
#10724).

I took the liberty of renaming `store` and `load` to `dbStore` and
`dbLoad` in TS, since I found the original names to be too generic. I
kept those in Noir since we call them as `pxe_db::store` etc. anyway,
and don't really expose them much either. I also renamed `key` to
`slot`, since the actual kv store _does_ have a key, but it's not what
we now call the slot (the key is `${contract}:${slot}`).

I found it slightly annoying that I had to modify so many TS files to
get this working, perhaps a symptom of over-abstraction?

---------

Co-authored-by: Gregorio Juliana <[email protected]>
AztecBot pushed a commit to AztecProtocol/aztec-nr that referenced this issue Jan 18, 2025
This extends the features introduced in
AztecProtocol/aztec-packages#10867 so that we
can also delete entries and copy multiple entries from one place to
another. With these two new primitives I also built a Noir `DBArray`,
which is a dynamic array backed by this database supporting pushes and
deletion of arbitrary indexes (which we'll need for
AztecProtocol/aztec-packages#10724).

I took the liberty of renaming `store` and `load` to `dbStore` and
`dbLoad` in TS, since I found the original names to be too generic. I
kept those in Noir since we call them as `pxe_db::store` etc. anyway,
and don't really expose them much either. I also renamed `key` to
`slot`, since the actual kv store _does_ have a key, but it's not what
we now call the slot (the key is `${contract}:${slot}`).

I found it slightly annoying that I had to modify so many TS files to
get this working, perhaps a symptom of over-abstraction?

---------

Co-authored-by: Gregorio Juliana <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants