Add `ScaleValue` primitive #7398

SeungheonOh · 2025-11-03T20:26:45Z

adds

scaleValue :: Integer -> Value -> Value

where all of the quantities of the value (second argument) get multiplied by the given constant factor(first argument)

SeungheonOh · 2025-11-03T20:27:35Z

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

+    --   alg n inner = n + Map.size (Map.filter (< zeroQuantity) inner)
+
+    -- TODO: make sure (size - neg) is correct
+    BuiltinSuccess $ Value outer' sizes size (size - neg)


please check here!

Need to make sure if I can just size - neg here or if I need to count all negative quantities again.

I think this is correct, but it's a good idea to add a prop_scaleValueBookkeeping test.

Okay, added bookkeeping property as well. It seems to be fine.

I think that's right.

plutus-core/testlib/PlutusCore/Generators/QuickCheck/Builtin.hs

zliu41 · 2025-11-04T01:20:50Z

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

+prop_negateIsInverse v =
+  let
+    inverseUnion = do
+      vInv <- V.scaleValue (-1) v


This will occasionally fail, right? You may want to use suchThat to ensure that v doesn't contain minBound :: Quantity.

nice catch. Thanks

This will occasionally fail, right? You may want to use suchThat to ensure that v doesn't contain minBound :: Quantity.

Can we avoid problems like this by making minBound equal to -(2^127-1) (ie, -maxBound) instead of -2^127?

Weird things might also happen if the scalar is close to the upper or lower bound for Quantity. I think that ArbitraryBuiltin for Integer will generate values like that, or you could do it yourself.

Can we avoid problems like this by making minBound equal to -(2^127-1) (ie, -maxBound) instead of -2^127?

I think that's more problematic (i.e., weird) in a different way.

zliu41 · 2025-11-04T01:26:31Z

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

+    --   alg n inner = n + Map.size (Map.filter (< zeroQuantity) inner)
+
+    -- TODO: make sure (size - neg) is correct
+    BuiltinSuccess $ Value outer' sizes size (size - neg)


I think this is correct, but it's a good idea to add a prop_scaleValueBookkeeping test.

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

kwxm

This looks good. There was a discussion about whether we should have a negateValue function instead/as well because it might be more efficient than scaling by -1. I suspect that it wouldn't be much different, but we could do a quick implementation and check benchmark times against this to make sure. If negateValue surprises us by turning out to be noticeably faster then I suppose we could have a special case when the scalar is -1 here that traverses the value and just negates everything. That'd need some special treament in the costing to reflect the improved efficiency for that case though, so it might be a bit of a pain.

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

kwxm · 2025-11-04T12:42:36Z

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

+    --   alg n inner = n + Map.size (Map.filter (< zeroQuantity) inner)
+
+    -- TODO: make sure (size - neg) is correct
+    BuiltinSuccess $ Value outer' sizes size (size - neg)


I think that's right.

kwxm · 2025-11-04T13:00:12Z

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

+prop_negateIsInverse v =
+  let
+    inverseUnion = do
+      vInv <- V.scaleValue (-1) v


Weird things might also happen if the scalar is close to the upper or lower bound for Quantity. I think that ArbitraryBuiltin for Integer will generate values like that, or you could do it yourself.

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

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

zliu41

LGTM!

zliu41 · 2025-11-04T23:27:15Z

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

+prop_negateIsInverse v =
+  let
+    inverseUnion = do
+      vInv <- V.scaleValue (-1) v


Can we avoid problems like this by making minBound equal to -(2^127-1) (ie, -maxBound) instead of -2^127?

I think that's more problematic (i.e., weird) in a different way.

plutus-core/testlib/PlutusCore/Generators/QuickCheck/Builtin.hs

SeungheonOh · 2025-11-05T03:31:06Z

For some reason, goldens changed (a577ce9). I'm trying to figure out why.

Update:
I think somehow number of CSE passes changes. I'm not sure how having extra builtin changes CSE pass count. Any idea?

zliu41 · 2025-11-05T03:40:33Z

I don't think it's the number of CSE passes - unless of course you have clear evidence of that. I think it's likely that the additional definitions in plutus-tx-plugin caused certain things to be ordered slightly differently or certain optimizations to work slightly differently (IIRC the initial PIR contains all those definitions, and the redundant ones are then optimized away). This has happened before and is not entirely surprising. I wouldn't spend too much time on it.

SeungheonOh · 2025-11-05T03:42:31Z

Okay. I saw some cse term not being pulled out and thought it was something to do with CSE. But, in that case, I'll ignore and merge when CI passes.

effectfully

Sorry for the late review. LGTM.

SeungheonOh requested a review from zliu41 November 3, 2025 20:26

SeungheonOh commented Nov 3, 2025

View reviewed changes

plutus-core/testlib/PlutusCore/Generators/QuickCheck/Builtin.hs Show resolved Hide resolved

Add ScaleValue primitive

ea0f767

SeungheonOh force-pushed the sho/scaleValue branch from 4730720 to ea0f767 Compare November 3, 2025 20:31

zliu41 requested review from Unisay, ana-pantilie, basetunnel, effectfully and kwxm November 4, 2025 00:05

zliu41 reviewed Nov 4, 2025

View reviewed changes

SeungheonOh added 2 commits November 4, 2025 00:41

Add more properties for ScaleValue

8f342d9

cleanup

8e735c2

SeungheonOh requested a review from zliu41 November 4, 2025 06:43

fix builtin scaleValue

1a1603c

SeungheonOh force-pushed the sho/scaleValue branch from 299d38b to 1a1603c Compare November 4, 2025 10:05

kwxm approved these changes Nov 4, 2025

View reviewed changes

SeungheonOh self-assigned this Nov 4, 2025

SeungheonOh added the Builtins label Nov 4, 2025

More property, typo

bf97e4d

zliu41 approved these changes Nov 4, 2025

View reviewed changes

SeungheonOh added 2 commits November 4, 2025 19:43

Fix build

50248e1

update golden?

a577ce9

rerun CI

d5040c9

SeungheonOh force-pushed the sho/scaleValue branch from 2970a9c to d5040c9 Compare November 5, 2025 05:45

zliu41 merged commit 996e08b into master Nov 5, 2025
4 of 8 checks passed

zliu41 deleted the sho/scaleValue branch November 5, 2025 15:24

effectfully reviewed Nov 5, 2025

View reviewed changes

Add ScaleValue primitive #7398

Add ScaleValue primitive #7398

Uh oh!

Conversation

SeungheonOh commented Nov 3, 2025

Uh oh!

SeungheonOh Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kwxm Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

kwxm left a comment

Choose a reason for hiding this comment

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!

Uh oh!

zliu41 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SeungheonOh commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zliu41 commented Nov 5, 2025

Uh oh!

SeungheonOh commented Nov 5, 2025

Uh oh!

Uh oh!

effectfully left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Add `ScaleValue` primitive #7398

Add `ScaleValue` primitive #7398

SeungheonOh Nov 3, 2025 •

edited

Loading

kwxm Nov 4, 2025 •

edited

Loading

SeungheonOh commented Nov 5, 2025 •

edited

Loading