refactor(standards): rename the config note selector to the note variant by onurinanc · Pull Request #3838 · 0xMiden/protocol

onurinanc · 2026-09-09T12:59:47Z

Summary

Rename the config note SELECTOR_* constants and selector() helpers to VARIANT_* and variant(), and the ERR_*_UNKNOWN_SELECTOR error constants to ERR_*_UNKNOWN_VARIANT.
Document the note variant convention in note::config, including the three notes that do not keep the variant in the first storage item.
Correct the NetworkAccountConfig script and type docs, which placed the variant in the first storage item instead of the item after the root.

Closes #3834.

zeapoz

Looks great to me!

zeapoz · 2026-09-09T13:53:29Z

 - [BREAKING] Updated the Miden VM and crypto crate family to v0.30.0 and `midenc-hir-type` to v0.12.0. `LocalTransactionProver::new` now takes `miden_prover::Prover`, `CoreLibrary` exposes one merged package, and `TransactionVerifier::verify` now returns `VerificationOutcome` so callers can handle outstanding precompile work ([#3782](https://github.com/0xMiden/protocol/pull/3782)).
 - [BREAKING] Removed the `BlockProof` placeholder in favor of `ExecutionProof` on `ProvenBlock`, matching `ProvenTransaction` and `ProvenBatch`, and `LocalBlockProver::prove` now takes an `ExecutedBlock` ([#3703](https://github.com/0xMiden/protocol/pull/3703)).
 - Added the `miden::protocol::tx::before_block_witness_load` kernel event, emitted before a block other than the reference block is read from the partial blockchain ([#3699](https://github.com/0xMiden/protocol/pull/3699)).
+- [BREAKING] Renamed the config note dispatch selector to the note variant, which renames the `VARIANT_*` and `ERR_*_UNKNOWN_VARIANT` constants and changes the config note script roots ([#3838](https://github.com/0xMiden/protocol/pull/3838)).


Suggested change

- [BREAKING] Renamed the config note dispatch selector to the note variant, which renames the `VARIANT_*` and `ERR_*_UNKNOWN_VARIANT` constants and changes the config note script roots ([#3838](https://github.com/0xMiden/protocol/pull/3838)).

- [BREAKING] Renamed the config note dispatch selector to the note variant; the config note script roots change ([#3838](https://github.com/0xMiden/protocol/pull/3838)).

nit: the constants are more like implementation details that we don't really need to include imo

…mechanism

PhilippGackstatter

LGTM! Left a few optional suggestions.

PhilippGackstatter · 2026-09-15T10:46:52Z

 # =================================================================================================

-const ERR_NETWORK_ACCOUNT_CONFIG_UNKNOWN_SELECTOR = "network account config note selector does not match a known action"
+const ERR_NETWORK_ACCOUNT_CONFIG_UNKNOWN_VARIANT = "network account config note variant does not match a known action"


nit: I think the situation is now that we use both "variant" and "action" synonymously, so two terms for the same thing and ideally it would be just one. But I'm not sure how to fix it since just using "variant" isn't always clear. Feel free to ignore.

Agreed, it seems similar, but I would keep this for now as the "variant" is more like the discriminator in the storage for an "action" note.

PhilippGackstatter · 2026-09-15T11:50:20Z

+//! - Mirrors the variant in a Rust enum over the actions, with the `VARIANT_*` constants the note
+//!   script dispatches on kept in sync with the constants in its `.masm` file.


Suggested change

//! - Mirrors the variant in a Rust enum over the actions, with the `VARIANT_*` constants the note

//! script dispatches on kept in sync with the constants in its `.masm` file.

nit: Not meaningful information I think.

PhilippGackstatter · 2026-09-15T11:51:13Z

+//! Two notes predate the first-item rule and keep the variant in the item following a word-sized
+//! payload ([`FaucetPolicyConfig`] and [`NetworkAccountConfig`]), so that payload stays
+//! word-aligned and loadable with a single `mem_loadw_le`; [`FaucetMetadataConfig`] instead keeps
+//! the variant first and pads it out to a full word. Moving them onto the first item is tracked
+//! separately, since it changes their storage layout.


nit: I think this belongs in a GH issue, not in a code comment. Consider removing.

mmagician

Overall LGTM ✅ but I'd slightly adapt the docs

mmagician · 2026-09-16T06:54:36Z


 #! NetworkAccountConfig script: mutates a network account's script allowlists and allowed fee policy
-#! roots, dispatched on a selector stored in the first note storage item.
+#! roots, dispatched on the note variant stored in the note storage item after the root.


nit: I actually think the position in storage is irrelevant for the procedure docs altogether

For this PR, we should leave it as is, but I wonder if more generally we want the variant item to always be the first item in the storage. If we do decide to do this, this can be done in a follow-up PR.

I wonder if more generally we want the variant item to always be the first item in the storage

Why would we want that? I don't see any added benefit.

In fact, it makes some things more complicated: e.g. for some notes, there is also a root passed in. And if the layout is [variant, ROOT], then we cannot use an aligned mem_loadw for the root anymore, we'd need to read it in element-by-element.

I think this could simplify parsing downstream (e.g., in the wallet, explorer etc.) - but I agree there are trade-offs. So, maybe just create an issue for now and then we can discuss pros/cons. In any case, not a priority for v0.17 release trunk.

mmagician · 2026-09-16T06:57:46Z

+//! variant value in its storage. The variant is never inferred from the number of storage items
+//! the note carries, nor from the procedures the consuming account happens to expose: the first
+//! overloads one number to both select the path and size the payload, and the second makes the
+//! path depend on the account the note is consumed against rather than on what the note was


let's avoid describing the non-behavior and focus on how the variant is conveyed

mmagician · 2026-09-16T06:58:30Z

+//!
+//! A note adopting the convention:
+//!
+//! - Carries the variant as a single felt in its first storage item, so reading it requires neither


is this true? some of the notes have the variant after the root

and as mentioned in other comments, I don't think we should put emphasis on the position in storage. So I'd drop that line entirely

As mentioned in another comment, I think having the variant always be the first storage element would be a good property to have - but if it is not already the case, I'd do it separately.

bobbinth

Looks good! Thank you! I left a couple of comments inline - but these are for potential future changes.

bobbinth · 2026-09-16T08:52:39Z


 #! NetworkAccountConfig script: mutates a network account's script allowlists and allowed fee policy
-#! roots, dispatched on a selector stored in the first note storage item.
+#! roots, dispatched on the note variant stored in the note storage item after the root.


For this PR, we should leave it as is, but I wonder if more generally we want the variant item to always be the first item in the storage. If we do decide to do this, this can be done in a follow-up PR.

bobbinth · 2026-09-16T08:55:36Z

+//!
+//! A note adopting the convention:
+//!
+//! - Carries the variant as a single felt in its first storage item, so reading it requires neither


As mentioned in another comment, I think having the variant always be the first storage element would be a good property to have - but if it is not already the case, I'd do it separately.

onurinanc added 4 commits September 9, 2026 14:47

refactor(standards): rename the config note selector to the note variant

b74dcd6

docs(standards): document the note variant convention for standard notes

56ff326

chore: add a changelog entry for the note variant rename

fffd9eb

changelog

1da811c

zeapoz approved these changes Sep 9, 2026

View reviewed changes

onurinanc requested review from bobbinth and mmagician September 10, 2026 08:08

onurinanc added 2 commits September 15, 2026 10:52

Merge remote-tracking branch 'origin' into fix-inconsistent-dispatch-…

77de503

…mechanism

changelog

60c9ed9

PhilippGackstatter approved these changes Sep 15, 2026

View reviewed changes

remove redundant comments

ee1c4bc

mmagician approved these changes Sep 16, 2026

View reviewed changes

bobbinth and others added 2 commits September 16, 2026 01:38

Merge branch 'next' into fix-inconsistent-dispatch-mechanism

8da666e

docs(standards): drop storage positions from the note variant docs

826957b

bobbinth approved these changes Sep 16, 2026

View reviewed changes

bobbinth added this pull request to the merge queue Sep 16, 2026

Merged via the queue into next with commit d5f69d9 Sep 16, 2026
19 checks passed

bobbinth deleted the fix-inconsistent-dispatch-mechanism branch September 16, 2026 09:32

	- [BREAKING] Renamed the config note dispatch selector to the note variant, which renames the `VARIANT_` and `ERR__UNKNOWN_VARIANT` constants and changes the config note script roots ([#3838](https://github.com/0xMiden/protocol/pull/3838)).
	- [BREAKING] Renamed the config note dispatch selector to the note variant; the config note script roots change ([#3838](https://github.com/0xMiden/protocol/pull/3838)).

		//! - Mirrors the variant in a Rust enum over the actions, with the `VARIANT_*` constants the note
		//! script dispatches on kept in sync with the constants in its `.masm` file.

Conversation

onurinanc commented Sep 9, 2026

Summary

Uh oh!

zeapoz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmagician left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants