CIP-???? | More Descriptive Script Purposes #1072
Conversation
|
On second thought, we could remove |
|
ping @lehins @WhatisRT (Ledger) Please tag anyone else who might be interested |
There was a problem hiding this comment.
For the most part this affects DApp developers and is non-issue for Ledger.
There was only one real concern that I have, which has to do with indices for minting, withdrawing and voting. If we are to change the semantics of ordering and preserve the order instead of what we do today, then it would be a bigger change for ledger and it would affect DApp developers quite a bit, because they would no longer get them (mints, withdrawals and votes) in a sorted order.
I don't really know exactly how this CIP would affect DApp developers, so I can't comment on that. I might be wrong, but from what I know deserialization of plutus context is a big part of execution cost. So, if I am right, then we should consider this kind of changes carefully. Although, there might have been new developments on this front, so there is a chance that this is no longer a problem. I'll let @zliu41 comment on that.
| -- | 0-based index of the input being spent in `txInfoInputs` | ||
| Haskell.Integer |
There was a problem hiding this comment.
This index cannot be added until CIP-0128 - Preserving Order of Transaction Inputs is implemented. I think it is important to point out this prerequisite in this CIP.
| -- | 0-based index of the given `CurrencySymbol` in `txInfoMint` | ||
| Haskell.Integer |
There was a problem hiding this comment.
This index can't be added, because the order in which multi-assets are listed in the serialized transaction will not necessarily match the order in which they will appear in plutus context. We would need a similar CIP to CIP-128 for minting before we can go this route.
There was a problem hiding this comment.
the order in which multi-assets are listed in the serialized transaction will not necessarily match the order in which they will appear in plutus context
I'm not sure why this is a problem? This is just for indexing into txInfoMint - why does the order matter?
There was a problem hiding this comment.
Because this index can be different from the index that was actually supplied in the redeemer in the serialized transaction. And that is dangerous in my opinion, since many people do not know that.
There was a problem hiding this comment.
I don't think this is an unacceptable concern. The smart contract itself will still only succeed if whatever token indexed in the context matches what the contract expects, this would only impact offchain tooling which is for some reason relying on these indices for analytic purposes or similar non-value transfer related functionality.
I agree it would be better if they matched, but if ledger has other priorities to work on, or if this change to the ledger would take significant amount of time then I don't see any issues with introducing this now and then when you free up change the ledger in a subsequent hf to assure they indices provided in the serialized tx match the ones in the context.
There was a problem hiding this comment.
I don't think this is an unacceptable concern
It took me a moment to unwrap the double negation 😄
Yes, as far as the script execution is concerned it should be fine, unless the index in the redeemer pointer is also passed in the argument to the script, which wouldn't guarantee to match the index in the script info and the script purpose.
@colll78 and @zliu41 I'll then trust your judgement on this one and won't worry about.
We'll treat this as a separate concern that is only relevant to the Ledger
| -- | 0-based index of the given vote in `txInfoVotes` | ||
| Haskell.Integer |
There was a problem hiding this comment.
Same issue as with minting. Order of votes is Haskell specific and the only way we would be able to report this index is if we preserve the order in which votes where submitted on the wire.
Note, that I am not opposing this, I am just pointing out an issue that would need to be resolved if we want to supply indices for maps.
| -- | 0-based index of the withdrawal being verified in `txInfoWdrl` | ||
| Haskell.Integer |
| ``` | ||
|
|
||
| ```hs | ||
| data ScriptPurpose |
There was a problem hiding this comment.
I have the same comments for indices in the script purpose as I made for ScriptInfo
|
|
||
| This allows to optimize for the very common operation of deriving the hash of the script itself onchain, greatly reducing onchain complexity and cost of execution. | ||
|
|
||
| The change also propses to add the resovled input in the case of the very common spending purpose, where the majority of the relevant informations is present. |
There was a problem hiding this comment.
We've discussed this exact change in the latest Ledger Working Group in the context of protected addresses CIP. Seems there is large demand for it.
| data ScriptInfo | ||
| = MintingScript | ||
| -- hash of the script being executed (same as polciy) | ||
| V2.CurrencySymbol |
There was a problem hiding this comment.
Why not ScriptHash for consistency sake?
It is always possible to go from ScriptHash to CurrencySymbol
| V2.CurrencySymbol | |
| V2.ScriptHash |
| V2.ScriptHash | ||
| -- | 0-based index of the input being spent in `txInfoInputs` | ||
| Haskell.Integer | ||
| V3.TxOutRef |
There was a problem hiding this comment.
We could also add the resolved TxOut here as well. I think it would be useful for a script to know other outputs that are being spent in this transaction
| V3.TxOutRef | |
| V3.TxOut | |
| V3.TxOutRef |
With this suggestion, however, we are loosing any distinction between ScriptInfo and ScriptPurpose, so it would probably make sense then to remove ScritpInfo in favor of ScritpPurpose, like it was before.
There was a problem hiding this comment.
Adding TxOut would be great if we can make sure that in the ledger code that generates the script context we don't do any redundant work (right now, where the same data exists in the script context in multiple places, we do redundant work to get and toPlutusType it each time).
Otherwise, since the proposed SpendingScript includes the index of that input, and Plutus is introducing BuiltinArray which will allow O(1) access of elements by index, this isn't a huge concern, because getting this TxOut will have a trivial cost. (Right now getting the resolved input from the information provided in SpendingScript / SpendingPurpose is extremely costly, it requires O(n) decoding of BuiltinData (each tx input), O(n) head operations (getting first field of the decoded input, which is TxOutRef), O(n) equality comparisons for identifying the correct TxOutRef.
With BuiltinArray all of this can be done by invoking single O(1) builtin.
There was a problem hiding this comment.
It might not be a huge concern, but even if you are indexing into an array with O(1), I presume you would need to decode every element in the array before you could index it. So, if all your script cares about is its own TxOut, then I suspect it would be less expensive to just decode that TxOut from script purpose, rather than decode the full array and lookup that TxOut, right?
There was a problem hiding this comment.
No you don't need to decode any elements to index into it. You just need to call BI.unsafeDataAsArray, this is O(1), to get the array, which would be of type: BuiltinArray BuiltinData, and then you do BI.indexArray idx txInputsArray which is also O(1), and then you have BuiltinData which is ownInput, which you can extract whatever fields you want from.
(this assumes we have BuiltinArrays and a hardfork has occurred which changed the representation of the script context to have the inputs / outputs as builtinarray instead of builtinlist, and afaik the next HF isn't expected for a while)
There was a problem hiding this comment.
Thanks again @michele-nuzzi & have tagged Triage for the next CIP meeting: https://hackmd.io/@cip-editors/117
| @@ -0,0 +1,156 @@ | |||
| --- | |||
| CIP: ? | |||
There was a problem hiding this comment.
| CIP: ? | |
| CIP: "?" |
We've found out by trial & error that quoting this character prevents the YAML error on GitHub preview mapping keys are not allowed in this context at line 1 column 6 but haven't gotten this into the documentation & templates yet.
| --- | ||
|
|
||
| ## Abstract | ||
| <!-- A short (\~200 word) description of the proposed solution and the technical issue being addressed. --> |
There was a problem hiding this comment.
| <!-- A short (\~200 word) description of the proposed solution and the technical issue being addressed. --> | |
| <!-- A short (\~200 word) description of the proposed solution and the technical issue being addressed. --> |
Please remove template comment scaffolding here & all the way down.
| ## Specification | ||
| <!-- The technical specification should describe the proposed improvement in sufficient technical detail. In particular, it should provide enough information that an implementation can be performed solely on the basis of the design in the CIP. This is necessary to facilitate multiple, interoperable implementations. This must include how the CIP should be versioned, if not covered under an optional Versioning main heading. If a proposal defines structure of on-chain data it must include a CDDL schema in its specification.--> | ||
|
|
||
| this CIP proposes to modify the definition of `ScriptInfo` and `ScriptPurpose` in [`plutus-ledger-api`](https://github.com/IntersectMBO/plutus/blob/618480658ec1750179c2832c6b619bc2d872b225/plutus-ledger-api/src/PlutusLedgerApi/V3/Contexts.hs#L428-L461) as follows |
There was a problem hiding this comment.
| this CIP proposes to modify the definition of `ScriptInfo` and `ScriptPurpose` in [`plutus-ledger-api`](https://github.com/IntersectMBO/plutus/blob/618480658ec1750179c2832c6b619bc2d872b225/plutus-ledger-api/src/PlutusLedgerApi/V3/Contexts.hs#L428-L461) as follows | |
| This CIP proposes to modify the definition of `ScriptInfo` and `ScriptPurpose` in [`plutus-ledger-api`](https://github.com/IntersectMBO/plutus/blob/618480658ec1750179c2832c6b619bc2d872b225/plutus-ledger-api/src/PlutusLedgerApi/V3/Contexts.hs#L428-L461) as follows: |
(small grammar errors at beginning & end)
| ### Acceptance Criteria | ||
| <!-- Describes what are the acceptance criteria whereby a proposal becomes 'Active' --> | ||
|
|
||
| The change is included in an hardfork |
There was a problem hiding this comment.
| The change is included in an hardfork | |
| The change is included in a hardfork. |
This could use some elaboration but will wait until the Ledger responses are all in & Plutus reps have reviewed with respect to when this might be included (cc @zliu41).
There was a problem hiding this comment.
This requires a new era and new ledger language version, not just a HF
rphair
added
Category: Plutus
rphair
changed the title
the same issue is already present in the current input ordering. since we only care about the plutus ordering, would it be possible to derive the index from the tx info plutus data? any tx builder that supports redeemer indexing already does it this way. |
| -- | 0-based index of the given `CurrencySymbol` in `txInfoMint` | ||
| Haskell.Integer |
There was a problem hiding this comment.
the order in which multi-assets are listed in the serialized transaction will not necessarily match the order in which they will appear in plutus context
I'm not sure why this is a problem? This is just for indexing into txInfoMint - why does the order matter?
|
|
||
| This modification would be reflected in the underlying data representation as always having the script hash of the given script as the first field of the purpose, and the index relevant to the purpose as second element. | ||
|
|
||
| This allows to optimize for the very common operation of deriving the hash of the script itself onchain, greatly reducing onchain complexity and cost of execution. |
There was a problem hiding this comment.
The hash should be factored out of ScriptInfo and ScriptPurpose. Then you don't need a special optimization to get the hash.
| ## Motivation: why is this CIP necessary? | ||
| <!-- A clear explanation that introduces the reason for a proposal, its use cases and stakeholders. If the CIP changes an established design then it must outline design issues that motivate a rework. For complex proposals, authors must write a Cardano Problem Statement (CPS) as defined in CIP-9999 and link to it as the `Motivation`. --> | ||
|
|
||
| Common operations such as deriving the hash of the script being executed, or finding the relevant data for validation could be optimized if the trusted data provided to the script was more complete. |
There was a problem hiding this comment.
For completeness it's good to briefly explain why obtaining the hash of the script being executed is a common and useful operation.
| ### Acceptance Criteria | ||
| <!-- Describes what are the acceptance criteria whereby a proposal becomes 'Active' --> | ||
|
|
||
| The change is included in an hardfork |
There was a problem hiding this comment.
This requires a new era and new ledger language version, not just a HF
| SpendingScript
-- hash of the script being executed
V2.ScriptHash
-- | 0-based index of the input being spent in `txInfoInputs`
Haskell.Integer
-- removed optional datum, since its presence (or not) is determined by the resolved input
-- (Haskell.Maybe V2.Datum)
-- resolved input being spent
V3.TxOut
V3.TxOutRefWhat is the point of having an index and the |
|
The way this CIP is written makes it hard to understand for me:
|
the point is integrity. keeping the same data types in the same fields allows field extraction without matching first. regardless, the index could turn useful while filtering or checking relations with other elements of the list |
michele-nuzzi
changed the title
If you mean deserialization within plutus execution (ie. Extracting fields out of BuiltinData) then this is not significantly expensive, and better yet, it's becoming leagues more efficient with the introduction of casing on BuiltinData and other improvements to Plutus. The reason historically this was a huge burden on ex-units was the use of full SOP / Scott conversion of types. If you operate directly on BuiltinData and only convent to SOP when you need, and only with the specific fields you need, then you won't experience these bottlenecks. PlutusLedgerApi now provides AsData variants for everything, including the ScriptContext. If you use the AsData variant, these additional fields will have no impact on execution budget unless you choose to use them, and even if you do the impact would be quite small and even smaller once we get these improvements (ie builtin array, casing on BuiltinData, and so forth). |
I agree with @zliu41 100% here, the script hash should be moved out of the script purpose directly into a field in the ScriptContext. So we would have: txInfo, redeemer, scriptPurpose and ownScriptHash as the four fields of the ScriptContext. |
|
yes makes sense to move out the script hash |
|
@colll78 That is really good to hear. Cause I do remember this being a problem in the past. |
|
I really like the proposal to add the script's own hash into the context. It has come up many times that something is difficult to do because the script doesn't know it's own hash, and there's no reason it needs to be like that. Otherwise I don't have much to add other than I agree with the proposed changes as to how the data is passed exactly. |
rphair
changed the title
|
@michele-nuzzi @fallen-icarus @zliu41 @WhatisRT @lehins - we had some ambitious proposals on the |
|
@michele-nuzzi at the CIP meeting today we decided to leave this at This has the potential to be a solid CIP if you can engage with the diverse feedback so far, so nobody wanted to mark it |
rphair
added
State: Waiting for Author
8 participants
This CIP proposes better script purposes to optimize for common operations for each purpose.
In particoular, it should standardize how to derive the hash of the script for all puropses, and should simplify the offchain by passing the index of ther relevant data in the
TxInfopassed as context.(Rendered proposal in branch)