refactor!: fixing illogical code locations#16891
Merged
Merged
Conversation
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
benesjan
changed the title
AztecBot
force-pushed
the
09-09-refactor_moving_emit_note_func
branch
from
8570bd3 to
e986ade
Compare
AztecBot
requested review from
IlyasRidhuan,
fcarreiro and
jeanmon
as code owners
benesjan
changed the base branch from
09-09-refactor_moving_emit_note_func
to
graphite-base/16891
benesjan
force-pushed
the
09-09-refactor_fixing_illogical_code_locations
branch
from
47cdd19 to
f6c9f9f
Compare
benesjan
commented
Sep 9, 2025
Comment on lines
+99
to
+101
| - `CONSTRAINED_ONCHAIN`: For on-chain delivery with cryptographic guarantees that recipients can discover and decrypt messages. Uses constrained encryption but is slower to prove. Best for critical messages that contracts need to verify. | ||
| - `UNCONSTRAINED_ONCHAIN`: For on-chain delivery without encryption constraints. Faster proving but trusts the sender. Good when the sender is incentivized to perform encryption correctly (e.g. they are buying something and will only get it if the recipient sees the note). No guarantees that recipients will be able to find or decrypt messages. | ||
| - `UNCONSTRAINED_OFFCHAIN`: For off-chain delivery (e.g. cloud storage) without constraints. Lowest cost since no on-chain storage needed. Requires custom infrastructure for delivery. No guarantees that messages will be delivered or that recipients will ever find them. |
Contributor
Author
There was a problem hiding this comment.
Copied this from a section above in the file that described the changes related to notes as the description was better.
nventuro
approved these changes
Sep 10, 2025
nventuro
left a comment
nventuro
left a comment
Contributor
There was a problem hiding this comment.
Thanks for keeping things tidy.
| /// over unconstrained encryption and is slower to prove (requiring more constraints). | ||
| /// | ||
| /// There are three available delivery modes described below. | ||
| // TODO(#16771): This is now used by notes as well. Move it. |
Contributor
There was a problem hiding this comment.
Doesn't this PR address this issue?
Contributor
Author
There was a problem hiding this comment.
Forgot to remove it. Thanks for noticing it
Contributor
Author
There was a problem hiding this comment.
Will remove in that other PR of mine as I'm AFK now and don't want to then wait 3 hours for it to merge.
ludamad
pushed a commit
that referenced
this pull request
Dec 16, 2025
ludamad
pushed a commit
that referenced
this pull request
Dec 16, 2025
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
2 participants
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.
With my recent PR, location of the event and note emission related code became illogical. In this PR i reshuffle it around.
I perceive it as quite important to have this merged relatively quickly as that will allow devs to deal with 1 migration instead of 2.