fix: create monitor increment note via the account context#2344
Open
SantiagoPittella wants to merge 1 commit into
Open
fix: create monitor increment note via the account context#2344SantiagoPittella wants to merge 1 commit into
SantiagoPittella wants to merge 1 commit into
Conversation
Collaborator
Author
|
@Mirko-von-Leipzig we may want to consider releasing this as alpha.2 or something since the monitor doesn't work without this |
igamigo
approved these changes
Jul 15, 2026
igamigo
left a comment
Collaborator
There was a problem hiding this comment.
LGTM! A couple of notes:
- Since we are creating a component already for increasing the counter, that same component could also create the note. This has a couple of small benefits: "atomicity" of storage increase with note creation (right now you could create a transaction that creates the note but does nto increase the counter or viceversa) and I think it may make the code just a tiny bit easier to follow since the transaction script would effectively be one call, and you don't need to explicitly add the basic wallet or note creator component. Obviously these are minor so feel free to disregard, but it might make it a bit more correct/idiomatic
- Not sure what the plans are for #1827 but it might have caught this. With everything getting much more stable soon, addressing it might be diminishing returns but could still be interesting to tackle
| let account = AccountBuilder::new(init_seed) | ||
| .account_type(AccountType::Public) | ||
| .with_auth_component(auth_component) | ||
| .with_component(BasicWallet) |
Collaborator
There was a problem hiding this comment.
nit: I think we could replace this with a NoteCreator component (no real benefit though AFAICT, just reduces the scope of the account)
Mirko-von-Leipzig
approved these changes
Jul 15, 2026
Mirko-von-Leipzig
left a comment
Collaborator
There was a problem hiding this comment.
Thanks! LGTM modulo the comments by @igamigo
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
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.
Summary
The network monitor's counter flow was broken on next: every increment transaction failed during execution with:
The monitor's increment transaction script was written to mirror miden-standards 0.15's
build_send_notes_script, which created output notes by executingoutput_note::createdirectly from the transaction-script context. After migrating to 0.16, the kernel makes note creation account-context only.I changed it to create the note through the standard
note_creator::create_noteaccount procedure via call (switching into the account context)Changelog