Skip to content

feat(protocol): bound note storage loading in the OwnerConfig and FaucetPolicyConfig scripts - #3563

Merged
bobbinth merged 8 commits into
nextfrom
feat-active-note-bounded-storage
Aug 21, 2026
Merged

feat(protocol): bound note storage loading in the OwnerConfig and FaucetPolicyConfig scripts#3563
bobbinth merged 8 commits into
nextfrom
feat-active-note-bounded-storage

Conversation

@onurinanc

Copy link
Copy Markdown
Collaborator

Closes: #3562

@bobbinth bobbinth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Thank you! I left some comments inline - but they should be pretty easy to address (and one of them probably shouldn't be addressed at all).

Comment thread crates/miden-protocol/asm/protocol/src/active_note.masm Outdated
#! - num_storage_items is greater than max_num_storage_items.
#!
#! Invocation: exec
pub proc get_bounded_storage(dest_ptr: MemoryAddress, max_num_storage_items: u16) -> u16

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably not a good idea, but I wonder if we should just change the get_storage procedure to work like this. The biggest issue is that that would be a pretty bad breaking change (not easy to detect downstream) - but if we were designing the protocol from scratch, that's how I'd probably make get_storage work.

Comment thread crates/miden-protocol/asm/protocol/src/active_note.masm Outdated
Comment thread docs/src/protocol_library.md

# write the note storage to memory starting at STORAGE_PTR
push.STORAGE_PTR exec.active_note::get_storage
push.MAX_NUM_ITEMS push.STORAGE_PTR exec.active_note::get_bounded_storage

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we propagate this everywhere (maybe in a different PR)? Basically, I'm not sure there should be much usage of get_storage any more.

@PhilippGackstatter PhilippGackstatter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

Comment on lines +325 to +330
# push a placeholder note_index (ignored when is_active_note = 1) and the active note flag
push.0.1
# => [is_active_note = 1, note_index = 0, dest_ptr, max_num_storage_items]

exec.input_note_internal::get_storage_info_raw
# => [NOTE_STORAGE_COMMITMENT, num_storage_items, dest_ptr, max_num_storage_items]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is the same as exec.get_storage_info right? I'd replace that.

Comment thread crates/miden-standards/asm/standards/notes/fee_sponsorship.masm
@bobbinth
bobbinth enabled auto-merge August 21, 2026 04:11
@bobbinth
bobbinth added this pull request to the merge queue Aug 21, 2026
Merged via the queue into next with commit 27cdf30 Aug 21, 2026
20 checks passed
@bobbinth
bobbinth deleted the feat-active-note-bounded-storage branch August 21, 2026 04:23
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.

Management note scripts load and hash full note storage before validating num_storage_items, enabling avoidable resource exhaustion

3 participants