-
Notifications
You must be signed in to change notification settings - Fork 324
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
CIP-0035 | Redraft to handle Plutus Core language versions #428
CIP-0035 | Redraft to handle Plutus Core language versions #428
Conversation
- Include discussion of Plutus Core language versions and how they change. - Use "ledger language" instead of "language version" or "ledger language version" to avoid confusion. - Remove the reference to the 'Draft' status. - Minor prose improvements.
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.
As far as I can tell this also completes #420. It all looks good to me but @michaelpj could you tag a couple more reviewers who would be willing to go over the text before we merge?
I'll ask a couple of the Plutus team to check it over. |
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've added a few comments, maybe not all of which are directly related to this CIP.
Also, "implementer" or "implementor"? I'm never sure about that myself, but Google and my dictionary seem to favour the former.
All in all I think this does a pretty good job of explaining the complexities of how the different versions of different things all fit together: that's quite confusing so it's very helpful to have it all clearly explained in one place.
| Adding a construct to the language | Minor | Backwards-compatible. | | ||
| Removing a construct from the language | Major | Not backwards-compatible. | | ||
| Changing the behaviour of a construct in the language | Major | Not backwards-compatible. | | ||
| Changing the binary format of the language in a backwards-compatible way | Minor | Safe even if it makes previously non-deserializable scripts deserializable.[^backwards-safe] | |
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.
The links to the notes at the end are maybe a little confusing. Several of them say things like
See "Are backwards-compatible binary format changes really safe?"
but when you click on the link and see that at the end of the document it's not clear what "Are backwards-compatible binary format changes really safe?" refers to. You find relevant notes when you get towards the end of the document, but you don't know that beforehand.
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 blame markdown. It's hard to do reliable section links so I just didn't try. I guess I can try again.
- An argument for the utility of the new builtins. | ||
- If an external implementation is provided: an argument that it is trustworthy. | ||
- Discussion of how any measures and costing functions were determined. |
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.
Currently the costing functions are determined using benchmarks which are included in the plutus
repository. Will we require proposers of new builtins to supply new benchmarks so that others can check the plausibility of the costing 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.
I think I'm deliberately leaving that a little vague because I'm not sure. I think I want them to talk about it but I think we might want more or less depending on the proposal 🤔
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.
Also: I think those benchmarks would be part of the implementation, which is separate from the CIP.
Not being able to make removals or changes to behaviour without a LV is quite painful. | ||
For example, it means that we cannot just fix bugs in the semantics: we must remain bug-for-bug compatible with any given LV. | ||
Not being able to make removals or changes to behaviour without a LL is quite painful. | ||
For example, it means that we cannot just fix bugs in the semantics: we must remain bug-for-bug compatible with any given LL. |
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 do we do if we discover that some builtin contains a bug that can compromise the chain, like a division by zero error in a C library if your builtin gets a particular combination of inputs?
Also, what do we do if there's a change in the behaviour of the (Haskell or C) library function underlying a builtin at some point in the future? Would we have to maintain a version of the library function reproducing the old behaviour in perpetuity and use it for the execution of existing scripts depending on the old behaviour?
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 think that's all out of scope of this CIP (since I would say that's "emergency measures"), but
- No script can ever depend on behaviour that crashes the node. So we can deal with such things. If there's wrong but non-crashing behaviour... we need to preserve that.
- Yes, if there's a change in the behaviour of the underlying function we'll need to maintain a version with the old behaviour. I guess that's another criteria for builtins: the behaviour should be as clearly specified as possible, so it won't change.
A couple of bonus comments. Near the beginning it says
That's kind of self-contradictory. Maybe it could say something like "... expensive to implement using the basic constructs of Plutus Core"? It might also be worth having a sentence saying that the non-builtin part of Plutus Core is something like the untyped lambda calculus. Just below that it says
That's not strictly true (cf |
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.
Looks like @michaelpj last commit addresses last round of feedback but will wait for @KtorZ to approve / merge.
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.
Happy with the update, although this raises an interesting follow-up question @michaelpj:
Why is there a Plutus-core language version if it's always set to 1.0.0 and, according to this new update, is never going to change?
Hmmm, the whole point of this update was to convey that it will change if we change any of the constructs of Plutus Core itself. Which bit made it sound like it won't change? |
| Changing the behaviour of a builtin function or type | LV | This changes the behaviour of existing scripts, so is not backwards compatible. | | ||
| Changing the interface between the ledger and the interpreter | LV | The ledger must provide scripts with exactly the right interface. New interface means new language. | | ||
| Improving model performance | PP | _Must_ strictly follow an improvement in real performance.[^3] | | ||
| Adding a new Plutus Core language version | HF | Backwards-compatible since the new behaviour is guarded by the new LV. | |
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.
Doesn't this depend on what changed in the new LV? Imagine a new LV that removes builtin type Data
(by the way, do we still need Data
once we have sums of products?). This would lead to an unavoidable change in the ledger-script interface, necessitating a new LL.
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.
Sure, but then you're also including a change to the ledger-script interface. If you do multiple changes obviously you need to biggest kind of release.
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.
Fair enough.
Every mention of the Plutus language being removed 😅? There's only mention of the ledger language version now ( |
Now we have the LV for the "program version" and LL for the different Plutus ledger languages. And both can change - there's an explicit type of change for adding a new LV. |
There seems to be something wrong with some of the links in the version that's currently publicly visible. |
…oundation#428) * Redraft to handle Plutus Core language versions - Include discussion of Plutus Core language versions and how they change. - Use "ledger language" instead of "language version" or "ledger language version" to avoid confusion. - Remove the reference to the 'Draft' status. - Minor prose improvements. * Address Kenneth's comments
The actual Plutus Core language version was not discussed previously, as it hadn't really seen much use. However we might want to change Plutus Core itself in the medium-term future, so we should make it clear how to do that.
I also tried to clear up some confusing language - the use of 'language version' for the ledger languages was just too confusing, especially in the presence of another language version! So I've opted to just refer to 'ledger languages', which I think is clearer.
I'm planning to do another PR to do more adjustments to make this fit the new CIP-001 better, but I wanted to do this first as it has some substantive changes.