Skip to content

feat: add version to AssetId - #3670

Open
PhilippGackstatter wants to merge 4 commits into
nextfrom
pgackst-asset-version
Open

feat: add version to AssetId#3670
PhilippGackstatter wants to merge 4 commits into
nextfrom
pgackst-asset-version

Conversation

@PhilippGackstatter

@PhilippGackstatter PhilippGackstatter commented Aug 21, 2026

Copy link
Copy Markdown
Contributor
  • Removes miden::protocol::account::validate which should not be called by user code. validate_structure stays and should be used instead. See first commit for these changes in isolation.
  • Adds a 4-bit version to the asset ID with version 0 unassigned and consequently invalid, making the empty word a guaranteed invalid asset ID (this is now doubly guaranteed by account ID zero being invalid).

part of #1155

@PhilippGackstatter PhilippGackstatter added the pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority label Aug 21, 2026
@PhilippGackstatter
PhilippGackstatter marked this pull request as ready for review August 21, 2026 11:50

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

Comment on lines +268 to +270
let composition = AssetComposition::try_from(
(metadata_byte & Self::COMPOSITION_MASK) >> Self::COMPOSITION_SHIFT,
)?;

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.

nit: we asserted the reserved section to be zero just above, so there's no need to mask off the bytes beforehand

Comment thread docs/src/asset.md
- `faucet_id_suffix` and `faucet_id_prefix` is the ID of the faucet which issues the asset. The transaction kernel ensures that a given account can only issue assets when the faucet ID matches its own ID.
- `asset_class_suffix` and `asset_class_prefix` is a class that determines if two assets issued by the same faucet are considered to be the same asset. It is set by the asset creator arbitrarily - see [identity](#identity) for more.
- `composition` describes how assets compose. Read on for more details.
- `version` determines how the remainder of the asset is decoded. It is located at a static offset so a parser can read it first and then decode the rest of the asset accordingly. The only valid version is currently `1`. Version `0` is unassigned and invalid, which means an empty word is guaranteed to _not_ be a valid asset ID.

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.

Suggested change
- `version` determines how the remainder of the asset is decoded. It is located at a static offset so a parser can read it first and then decode the rest of the asset accordingly. The only valid version is currently `1`. Version `0` is unassigned and invalid, which means an empty word is guaranteed to _not_ be a valid asset ID.
- `version` determines how the remainder of the asset is decoded. The only valid version is currently `1`. Version `0` is unassigned and invalid, which means an empty word is guaranteed to _not_ be a valid asset ID.

nit: I think this is a little bit wordy for what it's trying to say

@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 a couple of small comments inline.

Comment on lines +72 to +74
/// If we make this public, we may want to instead consider introducing an `AssetIdVersion`
/// struct, similar to [`AccountIdVersion`](crate::account::AccountIdVersion).
pub(in crate::asset) const VERSION_1: u8 = 1;

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'd probably remove this comment. If/when we move to a multi-versioned assets, the AssetId may become an enum.

Comment on lines 42 to 51
pub struct AssetId {
/// The asset class of the asset ID.
asset_class: AssetClass,

/// The ID of the faucet that issued the asset.
faucet_id: AccountId,

/// The composition of the asset.
composition: AssetComposition,
}

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.

Similar comment as in the account version PR - maybe we should add the version field here explicitly? Though, maybe it is fine as is for now too.

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.

3 participants