Skip to content

feat: implement UpgradeManager and UpgradeNote - #3299

Merged
PhilippGackstatter merged 7 commits into
nextfrom
pgackst-upgrade-standard
Jul 16, 2026
Merged

feat: implement UpgradeManager and UpgradeNote#3299
PhilippGackstatter merged 7 commits into
nextfrom
pgackst-upgrade-standard

Conversation

@PhilippGackstatter

@PhilippGackstatter PhilippGackstatter commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Implements UpgradeManager account component gated by Authority and a corresponding UpgradeNote that triggers the upgrade. For now this is purely the infrastructure without any real functionality.

Went with the extensions module, but alternatives could be capabilities. I tried to come up with a name that could also be the home of the generic NoteCreator. I would like to avoid utils since it is so generic, though the options I named are also pretty generic.

Does not yet add the UpgradeManager to any of the convenience account builders.

Do we need the ability to upgrade code or storage on the agglayer bridge? If so, the bridge does not have Authority and so it wouldn't be able to use UpgradeManager or UpgradeNote and would need a custom version to use something like assert_sender_is_bridge_admin for upgrade authorization. cc @mmagician

part of #2183

@PhilippGackstatter PhilippGackstatter added the pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority label Jul 14, 2026
@PhilippGackstatter
PhilippGackstatter marked this pull request as ready for review July 14, 2026 13:53

@zeapoz zeapoz left a comment

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.

Looks good to me in general!

Comment thread crates/miden-testing/tests/auth/network_account.rs Outdated
Comment thread crates/miden-standards/asm/standards/notes/upgrade.masm Outdated

@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 mostly reviewed the non-test code and left a couple of small comments inline. The main one is about the location of the component. I don't love it being in access or extensions. capabilities is better - though, in a way, every account component adds a "capability" to the account. I actually don't mind utils for this (and for NoteCreator) but I'm fine with capabilities too.

Another thing I realized is that we either need to add the UPGRADE note to allowlists of all network accounts. Or we need to enable the ability to update this list of notes. The latter is covered by #3240 and I've assigned @Fumuran to it.

Comment thread crates/miden-standards/asm/components/miden-project.toml Outdated
Comment thread crates/miden-standards/asm/standards/utils/mod.masm Outdated
Comment on lines +208 to +211
pub struct UpgradeNoteStorage {
code_upgrade_commitment: Word,
storage_upgrade_commitment: Word,
}

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 fine for now, but when we actually get to the point when we want to make it work, we'll somehow need to include the underlying code and storage data that matches these commitments. Maybe this could be done with attachments (i.e., one attachment for code update and another one for storage update).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, I missed that we can't easily provide advice for network notes, so this doesn't work the way it is currently setup. If we have #3240, it may be best to not implement the UpgradeNote now and instead leave that for later. I think it would basically need to carry the new account code and account storage in its note storage and then compute the commitments and insert into advice map upon consumption. But given #3240, we can fortunately deal with this later.

@mmagician mmagician left a comment

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.

In terms of module naming, I'd probably place this in its own standards/upgrade module, rather than trying to make it also fit the NoteCreator

Otherwise LGTM, though the PR description mentions an UPGRADE note standard which is missing here (a version of it is inlined in tests, do we want to make this a proper note standard now or defer to later?)

Comment thread crates/miden-standards/asm/standards/utils/account_upgrade.masm Outdated
Comment thread crates/miden-standards/asm/standards/utils/account_upgrade.masm Outdated
Comment thread crates/miden-standards/asm/standards/utils/account_upgrade.masm Outdated
@PhilippGackstatter

Copy link
Copy Markdown
Contributor Author

Per @mmagician's suggestion, I changed it to miden::standards::account_upgrade.

@PhilippGackstatter
PhilippGackstatter added this pull request to the merge queue Jul 16, 2026
Merged via the queue into next with commit 845b536 Jul 16, 2026
19 checks passed
@PhilippGackstatter
PhilippGackstatter deleted the pgackst-upgrade-standard branch July 16, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants