refactor!: new note emission API

[benesjan]

Refactoring note emission API to use the recently introduced MessageDelivery global var.

- note_emission.emit(encode_and_encrypt_note(&mut context, from));
+ note_emission.emit(&mut context, from, MessageDelivery.CONSTRAINED_ONCHAIN);

- note_emission.emit(encode_and_encrypt_note_unconstrained(&mut context, from));
+ note_emission.emit(&mut context, from, MessageDelivery.UNCONSTRAINED_ONCHAIN);

- note_emission.emit(encode_and_encrypt_note_and_emit_as_offchain_message(&mut context, context.msg_sender());
+ note_emission.emit(&mut context, context.msg_sender(), MessageDelivery.UNCONSTRAINED_OFFCHAIN);

Followup work

Achieving final imports

Would give the highest priority to tackling this issue because the import of MessageDelivery affects contract code and we don't want to force devs to do 2 annoying migrations. Plan on tackling that right after this PR is merged.

Random thoughts that came to my mind while implementing this

Consider renaming OuterNoteEmission

I would consider renaming OuterNoteEmission to MaybeNoteEmission because OuterNoteEmission is not really descriptive (it being outer feels like an implementation detail and not something you want to communicate to the struct consumer). But devs don't really get exposed to this that much so possibly doesn't matter much.

noteEmission.discard(...)

I've also noticed that the discard method on the note emission structs is not ever used in our codebase. I feel like notes might be too important to ever be discarded so would revisit whether having the func makes sense.

Emitting partial notes as offchain message

None of our partial notes implementations support emission via an offchain message.

On using AI

In this PR there was a lot of repetitive changes in the contracts and I realized that if I write nice migration notes first then I can use that as an input to the prompt. This made the AI output flawless.

[benesjan]

• refactor!: new note emission API #16091 👈 (View in Graphite)
• next

This stack of pull requests is managed by Graphite. Learn more about stacking.

[nventuro]

I would consider renaming OuterNoteEmission to MaybeNoteEmission because OuterNoteEmission is not really descriptive (it being outer feels like an implementation detail and not something you want to communicate to the struct consumer). But devs don't really get exposed to this that much so possibly doesn't matter much.

OptionNoteEmission would be ok, yes. It only exists because none of the Option methods are a good fit - we'd want something like and_then but with no chaining.

noteEmission.discard(...)

I've also noticed that the discard method on the note emission structs is not ever used in our codebase. I feel like notes might be too important to ever be discarded so would revisit whether having the func makes sense.

We could add a discard example I guess. It'd come up in transiento reads, e.g.

let note = priv_mut.get_note().discard(); // discard as it's about to get deleted so no point in emitting anything
priv_mut.replace(Note::new(note + 1)).emit(...);

Emitting partial notes as offchain message
None of our partial notes implementations support emission via an offchain message.

Would be good to address this, yes.

[nventuro]

Lovely work!