Skip to content

Conversation

@nventuro
Copy link
Contributor

@nventuro nventuro commented Sep 12, 2025

In discussing how to improve developer experience by getting rid of the context object as much as possible, I realized some of it is quite low hanging fruit. We can take a similar approach to the storage object and have the aztec-nr intermediate structs keep references to the context, resulting in the end-user invoking methods that already reference the context instead of having to manually provide it. The diff speaks for itself.

Closes https://linear.app/aztec-labs/issue/F-109/remove-context-when-delivering-messages.

@nventuro nventuro changed the title feat!: remove context from NoteEmission feat!: remove context from NoteEmission methods Sep 12, 2025
Copy link
Contributor

@benesjan benesjan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extremely nice change 🤩

Nico very smort

@nventuro
Copy link
Contributor Author

It turns out returning references (or structs holding references) from an if expression is not allowed. Now that NoteEmission contains such a reference, we run into the following:

error: Cannot return a reference type from an if or match expression
    ┌─ src/state_vars/private_mutable.nr:358:9
    │  
358 │ ╭         if !is_initialized {
359 │ │             self.initialize(init_note)
360 │ │         } else {
361 │ │             self.replace(transform_fn)
362 │ │         }

To address that, I created an internal auxiliary NoteEmissionNoContext type that can be created by calling strip_context, and have the context then re-attached via attach_context. The above snippet now reads

if !is_initialized {
    self.initialize(init_note).strip_context()
} else {
    self.replace(transform_fn).strip_context()
}
    .attach_context(self.context)

This is fine to do because the context in self is the same one as in the emissions in both branches (because it is a singleton).

@nventuro
Copy link
Contributor Author

I updated OuterNoteEmission to avoid the problematic mut ref on a return expression. The result is a bit nasty, but I want to get this out now - once we do a pass over NoteEmission and that API we'll also address Outer, its naming and how it's created, fixing this.

In discussing how to improve developer experience by getting rid of the `context` object as much as possible, I realized some of it is quite low hanging fruit. We can take a similar approach to the `storage` object and have the aztec-nr intermediate structs keep references to the context, resulting in the end-user invoking methods that already reference the context instead of having to manually provide it. The diff speaks for itself.

Closes https://linear.app/aztec-labs/issue/F-109/remove-context-when-delivering-messages.

Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
@AztecBot AztecBot force-pushed the nv/purge-ctx-emission branch from 4ad88c6 to 93e81cc Compare October 27, 2025 21:18
@AztecBot AztecBot enabled auto-merge October 27, 2025 21:18
@AztecBot AztecBot added this pull request to the merge queue Oct 27, 2025
@AztecBot
Copy link
Collaborator

Flakey Tests

🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry.

\033[38;2;188;109;208mFLAKED\033[0m (\033[38;2;250;217;121m8;;http://ci.aztec-labs.com/e26f98c6edcaf46a�e26f98c6edcaf46a8;;�\033[0m): BOX=vanilla BROWSER=* run_compose_test vanilla-all-browsers box boxes (457s) (code: 1) (\033[38;2;188;109;208mAztec Bot\033[0m: feat!: remove context from NoteEmission methods (#17024))

Merged via the queue into next with commit 750a8bd Oct 27, 2025
14 checks passed
@AztecBot AztecBot deleted the nv/purge-ctx-emission branch October 27, 2025 22:02
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

Successfully merging this pull request may close these issues.

4 participants