CIP-0153 | Update and clarify Value primitives

[zliu41]

• Removed deleteCoin, since it can be expressed via insertCoin.
• Removed unionValues since the benefits are unclear, especially since we can now process lists more efficiently in UPLC (via casing on lists). If the need for it arises in the future we can open a separate CIP for it.
• Revised the semantics for valueContains, making its behavior more sensible.
• Explained how valueData and unValueData works - the behavior will be different in Plutus V1-V3, vs. V4 onwards.

[zliu41]

cc: @colll78 @kwxm @ana-pantilie @effectfully @Unisay @basetunnel @fallen-icarus @michaelpj @SeungheonOh

[zliu41]

On a second thought I think it's better to just give valueContains a very simple definition: valueContains a b == True if and only if: for each (currency, token, quantity) in b, lookupCoin currency token a >= quantity. So I updated the text accordingly.

This is good because it makes valueContains a partial order, and unionValue an order-preserving operation.

Note that this means valueContains [] [("c", "t", -1)], which is not the case in the original CIP, since the original CIP requires every token in the second value to exist in the first value. But [] is really just a normalized form of [("c", "t", 0)], so it doesn't really make sense to treat them differently. @colll78

[ana-pantilie]

On a second thought I think it's better to just give valueContains a very simple definition: valueContains a b == True if and only if: for each (currency, token, quantity) in b, lookupCoin currency token a >= quantity. So I updated the text accordingly.
...
Note that this means valueContains [] [("c", "t", -1)] ...

@zliu41 Your example generalizes to valueContains a [(c, t, v)] == True for any v < 0 and (c, t) not in a. For example, valueContains [("c1", "t1", 2)] [("c2", "t1", -7)] == True, or even (valueContains [("c1", "t1", -100)] [("c2", "t2", -3)] == True) && (valueContains [("c2", "t2", -3)] [("c1", "t1", -100)] == True), basically if both a and b contain only negative values and don't have any keys in common they will always contain each other. This breaks antisymmetry and valueContains no longer defines a partial order!

[ana-pantilie]

In the ledger spec, if we are to translate the definitions there into the language we use here, value ordering is defined as: a <= b iff for all t and c where (t, c) is the pair which uniquely identifies an asset, lookup t c a <= lookup t c b holds.
Similarly, valueContains a b would be lookup t c b <= lookup t c a, which doesn't suffer from the issues described above: (valueContains [("c1", "t1", -100)] [("c2", "t2", -3)] == False) && (valueContains [("c2", "t2", -3)] [("c1", "t1", -100)] == False) so it doesn't break antisymmetry anymore, but it does conflate "does not contain" with "incomparable", which is fine unless you want to distinguish between the two. We also have valueContains [] [("c", "t", -1)] == True, similar to @zliu41's relation above.

@colll78 @zliu41 is there anything wrong with the ledger spec's definition of value comparison? Unless we want a different semantics, I would proceed with this definition as it's canonical (unless for some reason we don't regard the ledger spec as canonical, which means we have bigger problems!).

[fallen-icarus]

I alluded to this question in the main discussion, but from a dev's perspective, is valueContains using >= really a good idea? IMHO using >= defies intuition and will likely cause devs to accidentally create security vulnerabilities due to misusing it.

Consider a dApp using tokens for authorization. For certain actions, these tokens need to be minted. These auth tokens may share the same policy id but not the same token name. So at runtime, the script builds up a Value of auth tokens that need to be minted. Then, my intuition would lead me to use valueContains txMintValue authMintValue but this will return True even if the user is maliciously minting more auth tokens than they should. I can't also do valueContains authMintValue txMintValue because that prevents dApp composability - other dApps may be minting/burning tokens in the tx.

IMHO either valueContains needs to use == or we need another builtin to extract out all tokens of a specific policy id to then exact match on. But even in the latter case, valueContains still seems too dangerous to use so the former is actually preferred. In what real world use case is >= the preferred behavior? If the user needs to deposit a minimum quantity of a specific asset, there is already lookupCoin.

[rphair]

One thing please for reviewers and Plutus integrators on all node teams to consider posting about... this is tagged Triage for the CIP meeting in 8 days; as for all new proposals & significant updates we would then confirm it (tagging Confirmed) if & only if there appears to be evidence of feasibility and support in the Plutus community.

I think any of the reviewers @zliu41 tagged in #1088 (comment) would be able to support this qualification & of course all are always welcome to join the CIP meetings themselves (next = https://hackmd.io/@cip-editors/120).

[zliu41]

@colll78 @zliu41 is there anything wrong with the ledger spec's definition of value comparison? Unless we want a different semantics, I would proceed with this definition as it's canonical (unless for some reason we don't regard the ledger spec as canonical, which means we have bigger problems!).

@ana-pantilie It is more expensive to implement. It will take O(n1)log n2 + O(n2)log n1, instead of just O(n2)log n1. If we don't really care about negative amounts, like @colll78 told me, then I doubt it's worth doing that.

IMHO either valueContains needs to use == or we need another builtin to extract out all tokens of a specific policy id to then exact match on.

@colll78 what do you think about using == for valueContains?

[colll78]

@colll78 @zliu41 is there anything wrong with the ledger spec's definition of value comparison? Unless we want a different semantics, I would proceed with this definition as it's canonical (unless for some reason we don't regard the ledger spec as canonical, which means we have bigger problems!).

@ana-pantilie It is more expensive to implement. It will take O(n1)log n2 + O(n2)log n1, instead of just O(n2)log n1. If we don't really care about negative amounts, like @colll78 told me, then I doubt it's worth doing that.

IMHO either valueContains needs to use == or we need another builtin to extract out all tokens of a specific policy id to then exact match on.

@colll78 what do you think about using == for valueContains?

Using == is not an option. Such a definition would completely kill the vast majority of use-cases for this builtin. If there is sufficient demand to perform partial subset equality on values then that should be added as a separate builtin.

[zliu41]

@colll78 I see. So there doesn't seem to be a sensible semantics on negative values for valueContains. So my current preference is simply let it fail if either Value contains negative amounts.

Also, we should enforce a 32-byte length limit on currency symbols and token names, for insertCoin, unValueData, and when decoding Values from CBOR and Flat. This makes costing a lot easier, since the cost model won't need to consider map keys lengths. We can be more precise for currency symbols, which are either 0 or 28 bytes, but there's no need to.

Any objections on either point?

[fallen-icarus]

So my current preference is simply let it fail if either Value contains negative amounts.

This may be an amateur question, but doesn't failing on negative make valueContains useless for checking tx mint/burn values? I need the exact match mostly for working with minting/burning, so if we get a separate builtin for partial subset equality, then UTxO values can use the valueContains which fails for negative values (they don't make sense anyway) and uses <=.

[rphair]

FYI @zliu41 @colll78 now that CIP content isn't going into a site builder that had problems with backticks in the YAML titles (Docusaurus on the Developer Portal), if you wish to update the CIP title also to quote MaryEraValue it would be acceptable.

[fallen-icarus]

Why not add a separate lookupTokens builtin?

-- | Returns a `BuiltinValue` containing only tokens with the specified policy id.
lookupTokens :: BuiltinValue -> BuiltinCurrencySymbol -> BuiltinValue

[zliu41]

but doesn't failing on negative make valueContains useless for checking tx mint/burn values?

I thought so too, but @colll78 mentioned that this need is uncommon. Also, given we are doing >= on positive values, there doesn't seem to be a single sensible way to define it on negative values: we could do >= or == or <=, but none is a clear winner, especially if we want to make valueContains a partial order, and union a monotone operation.

So a separate builtin makes more sense to me.

Why not add a separate lookupTokens builtin?

That could be useful but the proper path forward is to write a new CIP.

[fallen-icarus]

I thought so too, but @colll78 mentioned that this need is uncommon.

I mean no disrespect, but this is heavily subjective. For CIP-89 based dApps, it is extremely important for securing the beacon tokens. I recognize it is a new paradigm that hasn't really taken hold yet, but we shouldn't base decisions solely off the current DeFi paradigm which most people agree is far from ideal.

Why not add a separate lookupTokens builtin?

That could be useful but the proper path forward is to write a new CIP.

@rphair Is this correct? IMHO adding lookupTokens is minor and already in the spirit of this current PR which is removing 2 other builtins from the original CIP. Having to create a new CIP just to add this new builtin seems excessive... Smart contract languages like aiken already support this function and a dedicated builtin would be more efficient.

[rphair]

@fallen-icarus #1088 (comment): @rphair Is this correct? IMHO adding lookupTokens is minor and already in the spirit of this current PR ...

Though I agree with you about the spirit of the change, I suspect that @zliu41 could be right... mainly since one doesn't extend the scope of a CIP by removing something: although one might be doing this by making additions to it. (You already know that Plutus is not my field so I'm not qualified to debate the relative significance of these changes.)

CIP-0035 is pretty strict about adding elements without verifying what performance impact they would have: the kind of review that would be done for a new CIP (benchmarks, etc) but likely harder to evaluate for a CIP-based feature that people are already using.

I couldn't imagine what such performance implications there might be, but maybe @kwxm could help with this... and also @effectfully to evaluate whether the semantic addition could be made as easily as you suggest. If there is consensus from these sources then maybe @zliu41 would agree that we could relax the usual strict suggestion of a new CIP.

[zliu41]

it is extremely important for securing the beacon tokens

Out of curiosity, what semantics do such use cases require on negative amounts, Is it ==? If so it should be a separate builtin - we can't have the same builtin that does >= on positive amounts and == on negative amounts. Also this can be done without a builtin, so there's also the question of whether it is worth adding a builtin for that.

[fallen-icarus]

I consider myself a junior programmer so there could be better ways of doing it, but this is what I do:

1. Whenever beacons are minted in a tx, I have to check all UTxO outputs to prevent them from going to the wrong addresses and being misconfigured. For that, I use the tokens aiken function like this line. If there are no beacons, I just move to the next UTxO. If there are beacons, I check everything else about the UTxO, including whether it has the right beacons.

2. For some actions, certain beacons must be burned. And since actions can be composed, the observer script must tally up the required beacons to burn like here. The tallies are negative amounts. Then, I get the beacon mint/burns and directly use == against my tallies like here. I'm using custom functions a lot right now (like this) due to how inefficient it is to work with the current Value.

we can't have the same builtin that does >= on positive amounts and == on negative amounts.

I don't need that. If we had lookupTokens, I could just use it and then manually use == on the result. I could even convert it to a list for pattern matching like I do here.

Also this can be done without a builtin, so there's also the question of whether it is worth adding a builtin for that.

Literally everything with Value can be done without builtins. We are trying to add builtins because those approaches have terrible performance. Wouldn't flatten also be significantly more performant with a dedicated builtin? The motivation for the original CIP was to make Value more performant to work with. How is leaving out any of the current API satisfying that motivation? @colll78 If the API can be built up from builtins, great. But if not, we are missing an important builtin. I apologize for not voicing these concerns earlier; I didn't spend enough time reviewing the original CIP. But IMHO it is not too late.

[colll78]

Again, I am not opposed to adding a new builtin for this purpose at all.

My point is that valueContains as defined by >= is a critical component of this CIP, and the lack of this builtin is a huge blocker for the programmable tokens CIP.

So we cannot afford to modify valueContains to accommodate this specific use-case.

We can introduce a new CIP with the lookupTokens builtin you propose, and this I would support and see value in.

[colll78]

@colll78 I see. So there doesn't seem to be a sensible semantics on negative values for valueContains. So my current preference is simply let it fail if either Value contains negative amounts.

Also, we should enforce a 32-byte length limit on currency symbols and token names, for insertCoin, unValueData, and when decoding Values from CBOR and Flat. This makes costing a lot easier, since the cost model won't need to consider map keys lengths. We can be more precise for currency symbols, which are either 0 or 28 bytes, but there's no need to.

Any objections on either point?

Agreed with 100% of the above.

[fallen-icarus]

@zliu41 Are you okay with adding lookupTokens and possibly flattenValue? The former is more important to me. For lookupTokens, it might be better to return something closer to [(TokenName, Integer)] than BuiltinValue but I'm open to ideas.

If I have to create a separate CIP, I will. But that seems excessive and just fragments documentation for this new BuiltinValue.

[zliu41]

Are you okay with adding lookupTokens and possibly flattenValue?

Both can be done without builtins (for Plutus V1, V2 and V3). They will be much more useful for Plutus V4 onwards, so I agree we should have something along those lines. But I really suggest opening a new CIP because we need discussion and consensus on:

• Adding and maintaining a new builtin is costly. Is every new builtin really worth it?
• Is there an alternative builtin, or alternative behavior of a builtin that may work better?
• Are there any other issues, especially security related?

Also, we've committed to implementing CIP-0153 for the upcoming intra-era hard fork, so increasing the scope this close to the hard fork doesn't seem like a good idea.

[zliu41]

@rphair How should I join the CIP meeting? I don't see any link in https://hackmd.io/@cip-editors/120

[fallen-icarus]

@zliu41 It is usually hosted on discord in the Cardano Improvement Proposals server here.

[rphair]

@zliu41 @fallen-icarus we keep the CIP Discord invitation link here — https://github.com/cardano-foundation/CIPs/wiki#key-links--resources (in 1 place only so it doesn't attract so much spam membership) — along with all the information about the meetings.

[rphair]

Apologies @zliu41 @fallen-icarus @colll78 that we couldn't help settle this question at the CIP meeting today due to the flash-mob review of Leios (#1078 (comment)). Also, progressing this to Confirmed would not have been possible with only 1 editor present at today's meeting.

Therefore I'm keeping this in Triage to ensure we keep visibility on the question of "adding lookupTokens and possibly flattenValue" — and whether a new CIP should be required — for the next meeting, if not answered by then. We'll try to ensure this question is settled by that next meeting based on the evidence offered in the meantime.

[effectfully]

At this point it's pretty clear we cannot choose single reasonable non-failing semantics for negative integers w.r.t. valueContains. So yeah, let's just make it fail. I'd even call it valueContainsPositive to make it very clear that handling of negative and positive integers differs.

lookupTokens sounds very similar to multiIndexArray, so that's an argument in favor of adding it.

I mostly agree with Ziyang that it deserves its own CIP (although see below for a potential quick solution that doesn't require another CIP). The builtin is simple, sure, but maybe we want to generalize it so that it can handle not only a currency lookup but also a token lookup. At which point maybe what we actually want is to add Map as a built-in type with its own multiIndexMap.

Or maybe we should generalize the builtin in a different direction and add intersectionValue based on the intersection function over Maps as a builtin. Which I think would be a very reasonable thing to do given that we already have unionValue based on union.

If we can agree that intersectionValue sounds like a useful generalization of lookupTokens, then I'm ok with extending the scope of this CIP. It's not a major extension anyway, given how similar this builtin would be to the existing unionValue.

I wouldn't add flattenValue to this CIP, because in my opinion valueData is close enough. Sure it gives you a list of lists rather than a flattened list of 3-tuples, but we don't have 3-tuples as a built-in type anyway, so a nested list is the best we can do and unwrapping from the Map constructor is a single constant-cost builtin call. Hence in my opinion that one definitely deserves its own CIP if we want to consider adding it.

And yeah, I get that intersectionValue introduces a bunch of constant overhead over lookupTokens and processing the result of a valueData call introduces a bunch of constant overhead per currency symbol -- and all of that adds up, but we can't be adding specialized builtins for everything. Not only is it implementation and maintanence burden, it also has the eternal cost of raising the size of serialized scripts on the blockchain. The more builtins we have, the more expensive storage-wise it becomes to add new ones. Not to mention making the attack surface larger.

So if a slightly less efficient but more general builtin can be added, then I think it's a good trade-off. Obviously, exceptions exist. But I'd say that the best place to argue about a builtin being an exception is a dedicated CIP.

[zliu41]

I updated the text based on the above consensus: (1) valueContains should fail upon any negative amount; (2) limit the map keys to 32 bytes.

This PR is really mainly intended to settle the question on valueContains. For proposals of new builtins, even if a new CIP isn't needed, I'd suggest discussing them in a separate issue or PR - especially since comments on GH are linear, making it tricky to discuss multiple topics in the same PR.

[fallen-icarus]

Okay, I'll draft another CIP to discuss the other builtins. I spoke to some other DeFi devs and they are asking for other functions too. Perhaps the title of this PR should be altered since the scope is way more specific than "Value primitives"? Apologies for the tangent.

[rphair]

I think this is ready to merge as an update now that the request to extend the scope has been settled with #1088 (comment) - no worries @fallen-icarus and personally I think your "tangent" was very appropriate considering the CIP-0153 implementation right around the corner as per #1088 (comment).

Since merging this would provide clarity about what's being implemented at that hard fork, I don't want this to get stuck in the queue so I'm personally approving & marking Last Check so we should have 2 weeks to note any objections to the change or any other other adjustments requested. Though this seemed doomed to another 2 weeks of Triage I think @effectfully's #1088 (comment) lack of objection to the CIP update (as submitted here) is confirmation enough for now.

[Ryun1]

looks like discussions have reached conclusion
appreciate the update on this @zliu41 🙏

[zliu41]

@Ryun1 I have no more updates; I think this is ready to merge - can we merge it? I'd like to make another update to this CIP on top of this, so merging this PR first would be helpful. cc @rphair

[colll78]

I believe I made a huge mistake not including negateValue in the original CIP. negateValue is required for a huge number of use-cases of unionValue (to subtract one value from another). Can anyone evaluate the scope of this builtin? Is it possible we could introduce it? It is undoubtably the single most critical value builtin I neglected.

[rphair]

@colll78 could this be covered by this new CIP PR under review? (cc @fallen-icarus)

• CIP-0168? | More BuiltinValue Functions #1090

... created from this discussion point:

• CIP-0153 | Update and clarify Value primitives #1088 (comment)

I would expect that evaluation of the new builtin would occur in an updated #1090 — or a further new CIP, if that were recommended (I'm not, but @zliu41 might) — rather than here. It would seem best to me to include this in #1090 if you & @fallen-icarus can agree upon that. Maybe @effectfully could also advise about questions of scope & help decide which.

[colll78]

settle the question on valueContains. For proposals of new builtins, even if a new CIP isn't needed, I'd suggest discussing them in a separate issue or PR - especially since comments on GH are linear, making it tricky to discuss multiple topics in the same PR.

It could be covered under that CIP, but that CIP proposes the introduction of multiple other builtins in addition to negateValue which I believe hurts its chances of making it into the next release. It would be great if it could be broken down into multiple CIPs that propose them individually so that even if they all don't make it into the next release, individual builtins from it might.

[zliu41]

@colll78 in Plutus V1 through V3 the builtin Value will be encoded as a Map in Data, so negateValue (and indeed anything else) can be done without a builtin. So even if it doesn't make it into the intra-era HF, this shouldn't be a deal breaker for most applications, should it? We can certainly add something like negateValue for V4.

[zliu41]

Let's continue the discussion in #1090