Restrict Value quantities to signed 128-bit integer range #7389

Unisay · 2025-10-15T13:27:46Z

Summary

This PR introduces a Quantity newtype to enforce bounds checking for Value amounts, restricting them to the signed 128-bit integer range (-2^127 to 2^127-1).

Changes

Core Changes

Add Quantity newtype in PlutusCore.Value with smart constructor and validators
Update unionValue to return BuiltinResult Value to handle overflow detection
Add validation for currency symbols and token names (max 32 bytes)
Update all Value operations to use Quantity instead of raw Integer
Add ArbitraryBuiltin instance for Quantity with improved value distribution

Test Updates

Update golden test for unionValue signature change (Value -> Value -> Value → Value -> Value -> BuiltinResult Value)
Fix test files to handle BuiltinResult and Quantity types
Migrate property test generators to use arbitraryBuiltin for better test coverage
All existing tests pass with the new implementation

Implementation Details

Quantity uses UnsafeQuantity constructor (not exported)
Smart constructor quantity :: Integer -> Maybe Quantity validates bounds
All entry points validate bounds and return BuiltinResult for operations that can fail
Internal operations use unsafe functions with validation afterward
Consistent terminology: "quantity" instead of "amount" throughout
arbitraryBuiltin provides better distribution of test values than stock Arbitrary, with improved coverage of edge cases near bounds

Closes https://github.com/IntersectMBO/plutus-private/issues/1881

plutus-core/plutus-core/src/PlutusCore/Builtin/Result.hs

plutus-core/plutus-core/src/PlutusCore/Pretty/PrettyConst.hs

Union with overflow detection is not mathematically associative when overflow occurs on different evaluation paths. Updated the test to: 1. Only test associativity when all intermediate operations succeed 2. Discard test cases where overflow occurs, as non-associativity is expected and correct behavior in these cases The test now validates: "IF all union operations succeed, THEN the results are equal regardless of evaluation order." Example of expected non-associativity: - v1 ∪ v2 may underflow (< -2^127), causing left path to fail - v2 ∪ v3 may succeed, allowing right path to succeed - This is correct behavior, not a bug Fixes the CI failure in PR #7389 where the test was incorrectly expecting associativity in all cases.

plutus-core/plutus-core/src/PlutusCore/Value.hs

plutus-core/plutus-core/test/Value/Spec.hs

plutus-core/plutus-core/src/PlutusCore/Value.hs

zliu41 · 2025-10-15T20:30:45Z

plutus-core/plutus-core/src/PlutusCore/Value.hs

 {-# INLINE empty #-}

-toList :: Value -> [(K, [(K, Integer)])]
+toList :: Value -> [(K, [(K, Quantity)])]


Given we are using a single letter K for keys, it would be more consistent to use a single letter - Q or V for value.

I find K confusing. I'd rather use Key instead.

plutus-core/plutus-core/src/PlutusCore/Value.hs

zliu41

Looks fine but please get @effectfully's approval before merging

effectfully

LGTM, even though performance can be improved. My comments are mostly nitpicking, feel free to ignore.

I don't like the AI-generated OP, as it's mostly noise. Just the Summary part would be enough, the rest is easily inferable from the code changes.

I didn't review the tests fully, will do that another time.

plutus-core/plutus-core/src/PlutusCore/Parser/Builtin.hs

effectfully · 2025-10-17T00:49:54Z

plutus-core/plutus-core/src/PlutusCore/Value.hs

+  {-# INLINE maxBound #-}
+
+-- | Smart constructor for Quantity that validates bounds.
+quantity :: Integer -> Maybe Quantity


We should perhaps use EvaluationResult instead of Maybe as the former is properly strict.

effectfully · 2025-10-17T00:59:16Z

plutus-core/plutus-core/src/PlutusCore/Value.hs

+  {-# INLINEABLE decode #-}
+
+instance Flat.Flat Quantity where
+  encode (UnsafeQuantity i) = Flat.encode i


I think we should probably create an appropriate ZigZag instance and make use of the fact that we know the maximum width of the data type. It should be a tad faster but I don't know if it actually produces smaller encodings. If it doesn't, then maybe we don't care.

@kwxm do you happen to know?

We normally don't encode negative quantities, what problem would such instance solve?

What you have there is a recursive function for both the encoder and the decoder. But you know the width of Quantity, you don't need any recursion there, you can do what they do for Word64 if you don't want to encode negative quantities (which you do, hence you should sprinkle ZigZag on top).

I.e. negative quantities are irrelevant, you handle them already anyway and you will keep doing so. The point is that this implementation is not efficient (and is likely not compact either).

(and is likely not compact either)

I don't think it'll make any difference in terms of size. Flat encodes natural numbers as a seqence of seven-bit blocks with the final block preceded by a 0 and all of the others preceded by a 1. If you add the zigzag thing on top I think you're going to need eight bits for numbers in [-64..63] and sixteen for numbers in [-128..--65] ∪[64..127].

plutus-core/plutus-core/src/PlutusCore/Value.hs

effectfully · 2025-10-19T14:42:57Z

plutus-core/plutus-core/src/PlutusCore/Pretty/PrettyConst.hs

 instance PrettyBy ConstConfig Value.K where
    prettyBy config = prettyBy config . Value.unK

+instance PrettyBy ConstConfig Value.Quantity where
+    prettyBy config = prettyBy config . Value.unQuantity
+
 instance PrettyBy ConstConfig Value where
    prettyBy config = prettyBy config . Value.toList


Not that it matters much, but these should be newtype-derivable.

It would require data constructors to be in scope (they aren't).

Oh I see. Yeah I guess that makes sense.

plutus-core/plutus-core/test/Value/Spec.hs

plutus-core/plutus-core/src/PlutusCore/Value.hs

zliu41

A few small changes needed

plutus-core/plutus-core/src/PlutusCore/Value.hs

Introduce a Quantity newtype to enforce that all Value amounts are within signed 128-bit integer bounds (-(2^127) to 2^127-1). This replaces the previous unbounded Integer type for quantities. - Add Quantity newtype with UnsafeQuantity constructor - Add smart constructor `quantity` that validates bounds - Implement Bounded instance with 128-bit min/max values - Add helper functions: zeroQuantity, addQuantity - Add serialization instances (CBOR, Flat) with validation - Add Pretty instance for Quantity - Update NestedMap type alias to use Quantity instead of Integer This is a foundational change required for fixing issue #1881 where Value quantities were previously unbounded and could overflow.

Update the Value parser to validate quantity bounds and provide clear error messages when parsing fails. - Update conValue to validate quantities during parsing - Add explicit quantity bound checks with descriptive errors - Add key length validation with descriptive errors - Handle BuiltinResult pattern matching for parser failures - Remove unused TemplateHaskell and FunctionalDependencies extensions Error messages now clearly indicate whether the issue is with key length (32-byte limit) or quantity bounds (128-bit signed integer).

Comprehensively update the test suite to work with the new Quantity type and test bound validation, including using Foldable idioms for cleaner test code. - Replace ValueAmount with V.Quantity in property tests - Use arbitraryBuiltin for generating valid quantities - Add quantity validation tests for min/max bounds - Add comprehensive mixed-quantity validation test - Add test generators for out-of-bounds quantities - Update toPositiveValue to preserve quantity bounds - Simplify prop_unionAssociative with Foldable combinators: - Use `not . null` for succeeded check (BuiltinResult is Foldable) - Use `foldr const error` for extractValue - Use nested if-then-else with do-notation for clarity - Use explicit `discard` for overflow test cases - Update prop_unionCommutative to handle BuiltinResult patterns - Update all test helpers to use Quantity type The new tests ensure that: - Valid quantities within bounds are accepted - Out-of-bounds quantities are properly rejected - Mixed valid/invalid quantities are handled correctly The Foldable approach in union tests reduces code while maintaining the same semantics and properly handling overflow cases.

Update the UnionValue builtin signature to reflect that it now returns BuiltinResult Value instead of Value, enabling proper error handling for overflow cases. - Update unionValueDenotation type signature - Update golden signature test file - Update PlutusTx.Builtins.Internal for new signature This completes the API changes for quantity restriction, ensuring all builtin signatures accurately reflect the new error-handling behavior.

zliu41 · 2025-10-21T14:25:21Z

plutus-core/plutus-core/src/PlutusCore/Value.hs

+-- | Validate all quantities in a nested map are within bounds.
+validateQuantities :: HasCallStack => NestedMap -> BuiltinResult NestedMap
+validateQuantities nestedMap =
+  case find isOutOfBounds allQuantities of


any (any isOutOfBounds . M.elems) (M.elems nestedMap)

GeorgeFlerovsky · 2025-10-22T18:13:01Z

@Unisay @effectfully @zliu41
Should the Value type in PlutusLedgerApi also be changed?
Currently, PlutusLedgerApi.V1.Value.hs (re-exported by V2 and V3) defines Value like this:

newtype Value = Value { getValue :: Map CurrencySymbol (Map TokenName Integer) }

https://github.com/IntersectMBO/plutus/blob/7ebd44f7c83ba4909a66ab3466802ad6b9714fd2/plutus-ledger-api/src/PlutusLedgerApi/V1/Value.hs#L268C71-L268C78

zliu41 · 2025-10-22T18:32:25Z

Yeah we'll do that later, likely after the HF

Unisay self-assigned this Oct 15, 2025

Unisay requested review from ana-pantilie and zliu41 October 15, 2025 13:28

Unisay commented Oct 15, 2025

View reviewed changes

plutus-core/plutus-core/src/PlutusCore/Builtin/Result.hs Show resolved Hide resolved

Unisay commented Oct 15, 2025

View reviewed changes

plutus-core/plutus-core/src/PlutusCore/Pretty/PrettyConst.hs Show resolved Hide resolved

Unisay added the No Changelog Required Add this to skip the Changelog Check label Oct 15, 2025

zliu41 requested review from SeungheonOh, effectfully and kwxm October 15, 2025 19:18

zliu41 reviewed Oct 15, 2025

View reviewed changes

Unisay requested a review from zliu41 October 16, 2025 10:07

Unisay force-pushed the yura/issue-1881-restrict-value-quantity-to-128bit branch from 9959bc9 to 50c29ba Compare October 17, 2025 11:04

zliu41 approved these changes Oct 19, 2025

View reviewed changes

effectfully approved these changes Oct 19, 2025

View reviewed changes

kwxm reviewed Oct 19, 2025

View reviewed changes

plutus-core/plutus-core/src/PlutusCore/Value.hs Show resolved Hide resolved

zliu41 requested changes Oct 20, 2025

View reviewed changes

Unisay added 4 commits October 21, 2025 14:27

Unisay force-pushed the yura/issue-1881-restrict-value-quantity-to-128bit branch from 258f7dc to b98b765 Compare October 21, 2025 12:29

Unisay requested a review from zliu41 October 21, 2025 12:33

zliu41 approved these changes Oct 21, 2025

View reviewed changes

zliu41 merged commit 7ebd44f into master Oct 22, 2025
3 of 10 checks passed

zliu41 deleted the yura/issue-1881-restrict-value-quantity-to-128bit branch October 22, 2025 01:45

Restrict Value quantities to signed 128-bit integer range #7389

Restrict Value quantities to signed 128-bit integer range #7389

Uh oh!

Conversation

Unisay commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Core Changes

Test Updates

Implementation Details

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zliu41 left a comment

Choose a reason for hiding this comment

Uh oh!

effectfully left a comment

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

zliu41 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GeorgeFlerovsky commented Oct 22, 2025

Uh oh!

zliu41 commented Oct 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Unisay commented Oct 15, 2025 •

edited

Loading