Conversation
benesjan
left a comment
benesjan
left a comment
There was a problem hiding this comment.
Lovely. Just needs fixing some comments.
|
I adjusted some of the naming: we now correctly call same-tx notes 'pending' instead of 'transient', and clarified that the separation is not revertible vs non-revertible, but rather same phase or previous phase. I also removed the note hash counter from the struct. This value was not being used anywhere, but was dangerous to have around since it was underconstrained. We only ever check if it is zero or non-zero to determine if the note is pending or not, but we never do validate its content (nor can we, since the read request does not include the counter, and we cannot inspect the new note hashes array). Therefore the only correct thing to do is to get rid of it. |
benesjan
left a comment
benesjan
left a comment
There was a problem hiding this comment.
Looks good. Would just expand a bit in those 2 comments
| /// requests) nor note hash (since it would be meaningless to tie this to a note that has been read). | ||
| /// | ||
| /// Second, it always compute the nullifier for a **settled** note, i.e. a note that has been created in a previous | ||
| /// transaction, which therefore has a nonce. This is typically fine, since this function will mostly be used in |
There was a problem hiding this comment.
| /// transaction, which therefore has a nonce. This is typically fine, since this function will mostly be used in | |
| /// transaction, which therefore has a nonce (nonces are injected in the kernel tail circuit after pending note squashing). This is typically fine, since this function will mostly be used in |
I think readers would generally have no clue why this implies that it has a nonce.
There was a problem hiding this comment.
After some reflection I think we just simplify this fn so that it also takes a note hash. I'll do that once both our PRs are in.
|
Hey, we had unresolved arguments about this, from yesterday |
Follow up from #12240. This aligns both compute nullifier functions. With the new metadata bits calling this is quite simple, and it's very clear that we're only using it for note discovery. Some macros were simplified a bit as a result as well.
3 participants
Follow up to #11942. This improves the readability and management of code that reads and destroys notes by introducing the
NoteMetadatastruct, which handles the different combinations of note hash counter and nonce internally. This is a stepping stone towards #8589.