feat: introduce AssetAmount wrapper type#2658
Open
giwaov wants to merge 1 commit into0xMiden:nextfrom
Open
Conversation
8393a32 to
7afa99c
Compare
Introduces a new AssetAmount newtype that wraps u64 and enforces the MAX = 2^63 - 2^31 invariant at construction time, closing the gap where invalid amounts could be created via direct u64 manipulation. Key changes: - Add AssetAmount with validated new(), From<u8/u16/u32>, TryFrom<u64> - Update FungibleAsset::amount() to return AssetAmount instead of u64 - Keep FungibleAsset::new() accepting u64 for ergonomic construction - Retain MAX_AMOUNT constant for backward compatibility - Update all callers across miden-protocol, miden-tx, and miden-testing Closes 0xMiden#2532
7afa99c to
5fed54c
Compare
Author
|
Hi @bobbinth just checking in. All CI checks are passing. Happy to address any feedback. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces a new
AssetAmountnewtype wrapper overu64that enforces theMAX = 2^63 - 2^31invariant at construction time, as described in #2532.Motivation
Currently,
FungibleAssetstores its amount as a rawu64and validates the range only during construction. This means any internal code that manipulates the amount directly could accidentally create invalid values. By introducingAssetAmountas a validated wrapper type, the type system enforces correctness at all construction points.Changes
New type:
AssetAmount(crates/miden-protocol/src/asset/asset_amount.rs)AssetAmount(u64)withMAXandZEROconstantsnew(u64) -> Result<Self, AssetError>inner() -> u64accessor for when the raw value is needednew_unchecked(u64)(crate-visible) for cases where validity is already guaranteed (e.g., subtraction of two valid amounts)From<u8>,From<u16>,From<u32>(infallible, always in range)TryFrom<u64>andFrom<AssetAmount> for u64Display,Serializable,DeserializableDebug,Default,Copy,Clone,PartialEq,Eq,PartialOrd,Ord,HashIntegration into
FungibleAssetamount: u64->amount: AssetAmountamount(): now returnsAssetAmountinstead ofu64new(): still acceptsu64for ergonomic construction, validates internally viaAssetAmount::new()MAX_AMOUNT: retained aspub const MAX_AMOUNT: u64for backward compatibility, delegates toAssetAmount::MAX.inner()add()/sub(): operate on inner values, construct validatedAssetAmountfor resultsUpdated callers
All call sites across 3 crates updated to use
.inner()where a rawu64is needed:delta/vault.rs,vault/mod.rs,kernel/mod.rsexec_host.rs,mod.rstest_account.rs,test_account_delta.rs,test_epilogue.rs,test_fee.rs,bridge_in.rs,multisig.rs,multisig_psm.rs,hybrid_multisig.rs,faucet.rs,fee.rs,p2id.rsDesign Decisions
AssetAmountper @bobbinth's suggestion in Introduce wrapper type for asset amounts #2532 (concise, since it's only used for fungible assets)FungibleAsset::new()signature unchanged: Keepsu64parameter to avoid breaking all callers validates internallyMAX_AMOUNTretained: Backward-compatibleu64constant for existing comparisonsnew_uncheckedispub(crate): Needed internally (e.g., subtraction results are guaranteed valid) but not exposed publiclyCloses #2532