-
Notifications
You must be signed in to change notification settings - Fork 367
CIP-0168? | More BuiltinValue Functions
#1090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
CIP-0168? | More BuiltinValue Functions
#1090
Conversation
|
Tagging people who might be interested in weighing in: @lehins @WhatisRT @zliu41 @effectfully @MicroProofs @colll78 @ana-pantilie @Unisay @Quantumplation @rvcas |
|
@effectfully With respect to your reply here about I agree with you on However, for using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We'll need
mkEmptyValueas a primitive, otherwise in Plutus V4 there will be no way to construct an empty value. - What's the motivation for
negateValue? - It seems useful to generalize
policiesto return[(CurrencySymbol, [(TokenName, Integer)])]? - What's the motivation for
intersection? I think it can be done without a builtin, assuming we add all other builtins proposed, so this will need strong justification.
I had assumed this was covered by
Subtracting values.
I may have gotten ahead of myself on this one. The idea was to have a single builtin support arbitrary higher-level filtering functions like aiken's |
It's actually
This is also only the case for Plutus V1 through V3. From V4, The broader problem with For example, if we have a |
BuiltinValue FunctionsBuiltinValue Functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accepted as CIP candidate at today's CIP meeting based mainly on initial Plutus stakeholder response being queries about the specifics, rather than any claim of infeasibility or disinterest... and because support for this idea was also shown here although not included with the recent CIP-0153 update:
@fallen-icarus please update the link to the current rendered document & change the directory name to CIP-0168 🎉
Co-authored-by: Ryan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems pretty reasonable to me. It would be nice to see the motivation expanded with examples.
| lookupTokens :: BuiltinCurrencySymbol -> BuiltinValue -> BuiltinValue | ||
| ``` | ||
|
|
||
| From these builtin functions, most of Aiken's stdlib `Value` functions can now make use of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be a bit annoying: it would make this proposal much more concrete to have an appendix listing all the interesting functions you want to implement with their implementations using the new primitives. I think they're presumably all quite simple, and it would make the case quite compelling.
Ideally we'd also be able to see how this improves the costing behaviour of these functions.
| -- that the result can make use of the `lookupCoin` builtin added in CIP-0153. It can always be | ||
| -- converted to a `List` for pattern-matching using the `valueData` builtin added in CIP-0153. | ||
| -- See the Rationale section for why `intersection` isn't used instead. | ||
| lookupTokens :: BuiltinCurrencySymbol -> BuiltinValue -> BuiltinValue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the naming a bit confusing since it returns the same type it is given. How about restrictPolicyTo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it is better to make restrictPolicyTo be:
restrictPolicyTo :: [BuiltinCurrencySymbol] -> BuiltinValue -> BuiltinValueThis would support a higher-level lookupTokens as well as aiken's restricted_to (here).
| type BuiltinCurrencySymbol = BuiltinByteString | ||
|
|
||
| -- | Negate all values in a `BuiltinValue`. | ||
| negate :: BuiltinValue -> BuiltinValue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding negate to union gives us a group on Value. The obvious missing thing to me is scalar multiplication (to give us a module). Does anyone need that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't personally have a use for scalar multiplication (right now), but I could see it being useful to others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is negate just scalar multiplication with the scalar being -1? In that case I think instead of adding negate we should just add scale?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@colll78 could this do what you need to do for #1090 (comment)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could, but it would be significantly less efficient than negateValue (roughly 64% more expensive) if we are to assume there is a similar costing difference to multiplyInteger -1 n and subtractInteger 0 n
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two builtins only perform the respective arithmetic operations, scaleValue will have to traverse the given Value, which is going to make the discrepancy less pronounced.
There's a different point however: multiplyInteger was surprisingly tricky to cost, so that's gonna propagate into scaleValue (so much so that it may not even be feasible to implement that with the current costing machinery, not sure), while negateValue would be very straightforward. @kwxm what do you think?
If Kenneth agrees costing scaleValue is hard, then I personally don't mind adding negateValue right away, given your reasoning here, because even with faster caseList and casePair, the derivative negateValue is probably going to be at least an order of magnitude more expensive than a builtin one (not least because negateValue is guaranteed to produce a well-formed Value so we don't need to check any of the invariants (except for quantities being in their range), while a derivative implementation will have to, because we don't provide an unsafe builtin that could create a Value while avoiding the checks).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
multiplyInteger -1 n is 64% more expensive than subtractInteger 0 n? If that's the case I'd be surprised, and it would suggest that either multiplyInteger or subtractInteger is probably not costed properly.
Regarding costing of multiplyInteger in general - since we cap the range of quantities, we should be able to consider it constant time.
Whether it's negateValue or scaleValue, it will be linear whether it's a builtin or not. Certainly the constant factors will differ greatly, but a builtin would be much more useful if it is asymptotically more expensive without it. If we are going to add one more builtin for the intra-era HF, are we absolutely sure that it should be negateValue / scaleValue?
| intersection :: BuiltinValue -> BuiltinValue -> BuiltinValue | ||
|
|
||
| -- | Returns all policy ids found in the value. | ||
| policies :: BuiltinValue -> List [BuiltinCurrencySymbol] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about getting the token names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't seem as useful to me since assets could have the same token name despite having different policy ids.
|
I believe I made a huge mistake not including Without Even if we need time to evaluate the demand and consider alternatives for some of the value operations proposed in this CIP, I believe |
|
@colll78 in Plutus V1 through V3 the builtin Value will be encoded as a Map in Data, so As you can see, there's a suggestion above regarding generalizing |
| intersection :: BuiltinValue -> BuiltinValue -> BuiltinValue | ||
|
|
||
| -- | Returns all policy ids found in the value. | ||
| policies :: BuiltinValue -> List [BuiltinCurrencySymbol] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean
| policies :: BuiltinValue -> List [BuiltinCurrencySymbol] | |
| policies :: BuiltinValue -> List BuiltinCurrencySymbol |
or
| policies :: BuiltinValue -> List [BuiltinCurrencySymbol] | |
| policies :: BuiltinValue -> [BuiltinCurrencySymbol] |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I'm not sure what is the right syntax for Data . I went off of CIP-0153.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be BuiltinList BuiltinCurrencySymbol where type BuiltinCurrencySymbol = BuiltinData
-- | Negate the quantity of every token in a Value
negateValue :: BuiltinData -> BuiltinData
negateValue = BI.mkMap . negateOuter . BI.unsafeDataAsMap
where
negateOuter value =
BI.caseList'
BI.mkNilData
(\pair restCS ->
let newPair =
BI.casePair pair $ \(cs, tkMapData) ->
BI.mkPairData cs (negateTokens $ BI.unsafeDataAsMap tkListData)
in BI.mkCons newPair (negateOuter restCS)
)
value
negateTokens tks =
BI.caseList'
BI.mkNilData
(\tkPair restTk ->
let newTkPair =
BI.casePair tkPair $ \(tkName, amtData) ->
BI.mkPairData tkName (BI.mkI $ 0 - (BI.unsafeDataAsI amtData))
in BI.mkCons newTkPair (negateTokens restTk)
)
tksThe above blows up the budget when applied to even eight or nine values with less than ten unique tokens each. This alone makes it impossible for programmable tokens to go to production. |
|
After the HF, the terms under the |
CIP-0153 added a new
BuiltinValuetype to make working with on-chain values more efficient. However, it added only a few builtin functions for working with this newBuiltinValuetype which limits its real world usability. Adding and maintaining builtin functions is costly, but the importance of validating values in the eUTxO model justifies a wider range of builtin functions for this purpose. With this in mind, this CIP proposes a few extra builtin functions to improve the usability of this newBuiltinValuetype while still trying to minimize the overall maintenance burden.(Rendered Version)