Skip to content

feat(standards): add expiration to network account config notes - #3715

Open
onurinanc wants to merge 14 commits into
nextfrom
fix-unconsumed-admin-action-notes
Open

onurinanc wants to merge 14 commits into
nextfrom
fix-unconsumed-admin-action-notes

Conversation

@onurinanc

Copy link
Copy Markdown
Collaborator

Summary

  • Encode an optional expiry block in the unused fourth felt of the NetworkAccountTarget attachment.
  • Add assert_not_expired, which caps the transaction expiration block delta so a stale reference block cannot pass the check.
  • Call assert_not_expired from the config note scripts and add an optional expiry to their builders.
  • Cover delayed consumption of a PauseConfig note held past its expiry block.

Part of #3560.

@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! Left a few suggestions, with the important one being avoiding finding and loading the attachment twice.

#!
#! The attachment is expected to have the following layout:
#! [account_id_suffix, account_id_prefix, exec_hint_tag, 0]
#! [account_id_suffix, account_id_prefix, exec_hint_tag, expiry_block]

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.

nit(consistency): in other places we use expiration_block_num rather than expiry. Consider calling it that. Applies to procedure names, too.

Comment on lines 73 to +82
# bind consumption to the targeted account: reject if the consuming account is not the note's
# target account
exec.network_account_target::active_account_matches_target_account
assert.err=ERR_BLOCKLIST_CONFIG_TARGET_ACCOUNT_MISMATCH
# => [pad(16)]

# reject a note held past the expiry block of its target attachment
exec.network_account_target::assert_not_expired
# => [pad(16)]

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.

While this pattern is really easy to read, we find and unhash the same attachment twice, which adds quite a few unnecessary cycles and hashes.

I think we should either:

  • load the attachment explicitly, then invoke the two procedures on a copy.
  • refactor network_account_target::active_account_matches_target_account into a single network_account_target::assert_target_id_and_expiration that does both checks.

I think the second option would be best, unless we need to check the target match individually from expiration, but I doubt this is or should be the case.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Nice one! I would go with your second option for two reasons:

  • It seems like the target matches are individually checked in agglayer notes
  • moving the assertion into the shared procedure would collapse all note specific error messages into a single one ERR_*_TARGET_ACCOUNT_MISMATCH

dup dup.2 u32lt assert.err=ERR_NETWORK_ACCOUNT_TARGET_EXPIRED
# => [reference_block, expiry_block]

u32wrapping_sub

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.

Since we checked above, sub is sufficient. We don't need wrapping behavior here.

Comment on lines +150 to +153
#! Otherwise the procedure applies two bounds, and both are required for the expiry to hold:
#! - it requires the transaction's reference block to lie strictly before the expiry block, and
#! - it caps the transaction expiration block delta at the number of blocks left until the expiry
#! block, or [`DEFAULT_EXPIRATION_BLOCK_DELTA`] if that is smaller.

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.

Makes sense 👍

Comment on lines +232 to +235
# cap the inclusion window at whichever is smaller: the default delta, or the blocks left
# until the expiration block
push.DEFAULT_EXPIRATION_BLOCK_DELTA u32min
# => [expiration_block_delta]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this is desirable.
I can imagine some config notes' expiration could be longer than DEFAULT_EXPIRATION_BLOCK_DELTA, in which case a user-supplied expiration would just be ignored.

I think that if the caller has taken enough caution to set the expiration != NETWORK_ACCOUNT_TARGET_NO_EXPIRATION, then they will have provided a meaningful value that they care about enforcing, rather than being min-capped at the default

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think the documentation makes this a bit confusing as the user-supplied expiration is not
ignored here. It is enforced by the assert just above:

dup dup.2 u32lt assert.err=ERR_NETWORK_ACCOUNT_TARGET_EXPIRED

The u32min only bounds how long after proving the transaction may still be included into the chain. Also, dropping the u32min would break long expirations as the kernel's update_expiration_block_delta only accepts a delta of at most 65535 blocks and aborts the transaction otherwise, so passing expiration_block_num - reference_block would make any note expiring more than 65535 blocks out permanently unconsumable.

So I think fixing the documentation is sufficient here. Or, am I missing something?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I might be missing something, but I think the numbers don't add up. Walking through some concrete example:

  • DEFAULT_EXPIRATION_BLOCK_DELTA = 20
  • assume the tx is proven against a reference_block = 100

Case 1: tx is expired, can't even be proven, expiration_block_num = 90

throws ERR_NETWORK_ACCOUNT_TARGET_EXPIRED

Case 2: tx can be proven, valid for a few more blocks, expiration_block_num = 105

blocks_until_expiration = 5
expiration_block_delta = min(5, 20) = 5

Case 2a

  • the current block is 102, tx succeeds as expected ✅

Case 2b

  • the current block is 122, tx fails as expected because it's too stale ✅

Case 3: tx is valid for more than the default, expiration_block_num = 125

blocks_until_expiration = 25
expiration_block_delta = min(25, 20) = 20

update_expiration_block_delta(20) -> admits at most 20 block past

Case 3a:

  • current block is 102. Tx is fresh enough ✅

Case 3b:

  • current block is 122. Tx is "fresh enough", i.e. within the caller-supplied expiration_block_num of 25, but because we updated update_expiration_block_delta with a value of 20, the transaction fails - unexpected

I might have made some mistakes here, I still get confused sometimes about the expiration, so please feel free to point any gaps in my understanding.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think Case 3b is expected. The reference block is chosen by whoever builds the transaction, so the note is still consumable at block 122 there, the user can just choose a more recent block such as 120 than the reference block 100, so the transaction will be included as expected.

So, I think Case 3b shows that a proof tied to reference block 100 is rejected 22 blocks later. That is the freshness bound of “default expiration delta”, which applied in other places as default as well rather than the note's expiration being ignored. So, I think this resolves what #3560 expect as to resolve about the stale proofs.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the user can just choose a more recent block such as 120

Yes and no. It does require that the transaction proof is re-generated.
If the user submits a tx for proving with the remote prover, but there is a queue - there is a chance that this transaction is already outdated by the time it is proven.

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.

I haven't looked at the code yet - so, just commenting on general concepts:

We discussed chain-wide expiration (i.e., all transactions have expiration timeframes) a long time ago, but eventually decided against this. There were various pros and cons, but IIRC, the main reason why we decided not to do this was to support "send-only" accounts that never need to sync with the chain but can still still generate transactions (something conceptually similar to a pre-paid card from which you can spend w/o internet connection).

My understanding though, is that this PR is about something different. It is not so much about transaction expiration but about note expiration. That is, a note gets committed onchain, but after some time becomes unexecutable. I think conceptually this may be useful, but I'd fist like to see how this integrates with the "note sequencing" setup we discussed in another issue (don't remember the issue number off the top of my head). Basically, I wonder if that design may influence what we do here and whether explicit note expiration is needed at all.

For example, we could decide that we want to use the 2 "free" elements of the attachment for "domain" and "sequence" number - or something similar.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The issue you mention, for reference: #3551


While both the domain+sequence feature and the note expiration work towards that linked issue, they are independent concepts, and actually complementary in my view.
The ordering is a useful feature in its own right, and matters when, well - we need to impose the ordering on the consumption of notes. It says nothing about their expiration.

Expiration is also useful independently, for cases when we want to impose a limit on how long a particular note should stay consumable - and this is useful for both the ntx builder to filter notes that they should try for consumption, but also for the note senders who may want to restrict consumability past a certain date.

So I think we want this expiration feature

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.

Agreed that note expiration is an independent and useful concept - but it is something I haven't thought through before, and in the context of this PR, there are a couple of open questions:

  • Should note expiration be a broader concept? For example, should we have a separate attachment that enforces note expiration. This attachment could then be used not just for network notes and would uniformly signal that some note becomes un-executable after some block.
  • If we do decide to keep the note expiration info in the network note attachment, how would this interplay with the note ordering (at least in broad strokes)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Should note expiration be a broader concept? For example, should we have a separate attachment that enforces note expiration. This attachment could then be used not just for network notes and would uniformly signal that some note becomes un-executable after some block.

I think note expiration makes sense as a broader concept, and a separate attachment that enforces it sounds right to me.

The main reason is that expiration is not specific to config notes, but today it cannot be expressed without one: the only way to carry an expiration is to carry a NetworkAccountTarget,
which requires the target account to be public. So, a P2ID or MINT note that just wants to stop
being consumable after some block cannot say so without taking on a public network target it does not need. A separate NoteExpiration attachment also gives the node and the ntx builder a single scheme to look up for every note, instead of knowing that the expiration is hidden inside the network target attachment.

If we do decide to keep the note expiration info in the network note attachment, how would this interplay with the note ordering (at least in broad strokes)?

I think the interaction is the same in either case. Ordering needs at least a domain and a sequence number, plus a sub-key if we want per-role granularity, which does not fit the single free felt of the attachment's current one-word layout, the one this PR uses for the expiration, so it would need either a second word or its own scheme anyway. And since sequencing is stateful (a per-domain counter in account storage, written on every consumption) while the network target is just routing information, I think its own scheme is the better home either way. So where the expiration lives does not constrain ordering.

Since we would go with a separate attachment, I think the expiration would live in its own attachment. So, they wouldn't share the same layout.

I believe ordering and expiration are complementary rather than one making the other redundant. They check two different things, and they don't rely on the same data:

  • Expiration is a property of a single note, checked against the block height, and is stateless
  • Sequencing is more like a relation between notes, checked against a per-domain
    counter in account storage, so it is stateful and writes on every consumption.

@PhilippGackstatter PhilippGackstatter Sep 10, 2026

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.

I'm not sure if a separate attachment is necessary, or at least, note expiration hasn't come up outside of network transactions, as far as I'm aware.

As for the concrete mechanism, to take an example: An admin issues a "grant role to user X" note and a subsequent "revoke role from user X" note. These should apply in order grant, revoke.

Risks:

  • Reverse application, "revoke then grant": Assuming revocation is a no-op if the role isn't assigned to the user, the notes could be consumed in reverse order, leaving the user with the role.
  • If the note ordering is enforced, and the grant note is never applied for some reason (e.g. it always fails), then the revocation (and potentially all other subsequent notes) cannot be applied either. This may not be a problem in this case, but could be in others (e.g. when the second note doesn't strictly revert the action of the first one).

The first risk is what a proper ordering mechanism resolves, but it introduces the second one.

For the second risk, we may need an override built into the ordering so that a failed sequence of notes doesn't kill liveness of the entire account.

I can think of two potential approaches for ordering:

  • "Config domain" approach. As @onurinanc mentioned: "Ordering needs at least a domain and a sequence number" proposed above: This requires additional per-domain storage in the account. An override could set sequence = 0 and thereby not have any restriction of sequence - 1 needs to be applied before the current note, and so allow bypassing the ordering in case of failures mentioned above.
  • Nullifier-based ordering: Config note B encodes that "it can only be consumed if nullifier A is in the nullifier tree or the note with the specified preceding nullifier is being consumed in the same transaction before the current note". The nullifier can be omitted (set to empty word) to override.

I think the second approach has two nice properties:

  • It does not require extra account storage. A chain of notes simply builds on top of each other by specifying the nullifiers.
  • It is not domain-specific: The NTX builder can easily figure out the sequence of notes by checking the nullifiers. This may also be possible with the "config domain" approach, but it would be more complicated.

The main downside is that we don't have a nice tx kernel API for checking the state of a nullifier, but I assume it should be fairly easy to add.

One other scenario may be that I change my mind and no longer want the two notes to be applied at all.

  • In the nullifier approach I have to wait for the notes to expire.
  • In the sequence approach, there could be an additional override mechanism that bumps the sequence number to a higher value, preventing the older notes from being applied. I'm not sure if this is a very common scenario, though.

My feeling is that expiration provides a good enough mechanism for this.

There are also downsides to the nullifier approach:

  • I believe there is no standard node API to get a nullifier witness (required for checking a nullifier within a tx). There is an internal one for block building, but it is not exposed.
  • Exposing the nullifier tree now might make it more difficult to implement nullifier tree rotation later, but that would require a bit more investigation.

Comment thread crates/miden-standards/asm/standards/attachments/network_account_target.masm Outdated
Comment thread crates/miden-standards/asm/standards/attachments/network_account_target.masm Outdated

@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.

Overall, I think this direction looks good to me.

I'm also fine with a more generalized note expiration standard, mainly because it would allow the node to prune all notes that use it after they have expired, which seems useful.

On the other hand, for private notes this would add more data to the chain than it would save. It's not clear that it would be used all that often for public, non-network notes. All notes consumed in local transactions require a conscious agreement from a user, so such notes are expired whenever a user decides against consumption.

Network notes are consumed automatically and so expiration is definitely needed for them. So, it seems to me, the most scoped version of this feature is to have it just for network notes (as part of NetworkAccountTarget) and that's what this PR already does.

The other main question seems to be around the min logic for the expiration delta (left an inline comment related to this).

Comment on lines +161 to +162
#! The attachment is loaded with [`load_attachment`]. Callers that also need the expiration check
#! should load it once and run both checks on a copy of it.

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.

Suggested change
#! The attachment is loaded with [`load_attachment`]. Callers that also need the expiration check
#! should load it once and run both checks on a copy of it.

nit: I don't think this needs explanation.

@@ -78,29 +108,28 @@ end
#! Invocation: exec
pub proc new

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.

Since network_account_target::new creates a non-expiring attachment, the agglayer notes that call it (mint and burn note) are non-expiring. I think it makes sense that minting and burning bridged-in/out funds don't expire, but just flagging this, so this is a conscious decision cc @mmagician.

Comment on lines +206 to +209
/// Returns the last block at which the note may take effect, or `None` if it never expires.
pub fn expiration_block_num(&self) -> Option<BlockNumber> {
self.expiration_block_num
}

@PhilippGackstatter PhilippGackstatter Sep 13, 2026

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.

If we keep the expiration delta min logic in the MASM code, this value is potentially misleading. It encodes the configured expiration block number, but since we compute the actual expiration block number as ref_block + min(expiration_block_num - ref_block, DEFAULT_EXPIRATION_BLOCK_DELTA), the actual expiration block might be lower than what is returned here. (Related to the examples in #3715).

So if we keep this min logic, we should document this and probably also provide something like compute_expiration_block(&self, ref_block: BlockNumber) -> BlockNumber so the node can call this method to easily compute the actual expiration block of a transaction involving this note in combination with some ref block. The note itself expires at that block at the latest, so we could also alternatively treat it as an upper bound. But in any case documenting this in a few sentences seems useful.

Overall, I agree with @mmagician that the min logic isn't necessary, and would conveniently not require any of the changes I just mentioned.

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