CIP-0117? | Explicit script return values

[michaelpj]

There is a papercut in how we check whether scripts accept transactions. There are a few ways that people can make mistakes, not notice, and have their scripts fail open. This proposal suggests that we instead be prescriptive about what scripts must return in order to succeed, which would make such mistakes quickly obvious.

This is not a particularly important change, but it would be relatively cheap to do and would fix a minor problem.

---

Rendered Proposal on Fork < edit: changed to latest version in branch

[zliu41]

How about unit (or either unit or true)? Most scripts written in Plutus Tx return unit, so they won't need to change anything.

[effectfully]

Yeah, breaking backwards compatibility in such a horrible implicit way sounds like an even worse footgun than the status quo. I do understand that people will need to explicitly pick a new language version, but we can't be sure that they will adapt their code properly.

[michaelpj]

Most scripts written in Plutus Tx return the Plutus Tx datatype unit, i.e. some scott-encoded or SOP type. What that looks like is up to the compiler, so I don't think we can easily state a good recognition procedure for it here. We could recognize builtin unit, though.

[effectfully]

I don't like it. I don't like the status quo either, though. But less so.

[effectfully]

Having such a corner case would be extremely weird in my opinion and I'm opposed to it.

[michaelpj]

Yeah, I just put it in here for completeness.

[effectfully]

This sounds really annoying because of built-in vs native data types. If you return True in Plutus Tx, will it even evaluate to the built-in True in the end?

[michaelpj]

No, it won't. But you can return (and construct) a BuiltinBool easily enough, or we can provide an adapter.

[effectfully]

Yeah, breaking backwards compatibility in such a horrible implicit way sounds like an even worse footgun than the status quo. I do understand that people will need to explicitly pick a new language version, but we can't be sure that they will adapt their code properly.

[colll78]

Please add unit as a supported acceptance return value for this CIP.

[michaelpj]

I don't think we should have multiple different kinds of accepted return value. unit would be okay, so long as then it has to be unit. The whole point of this CIP is to narrow down what scripts return as much as possible, so its very clear if you're doing it correctly or not. I don't really see the benefit to having a larger space of possibilities.

I will add an alternative for the return type being unit, although I do kind of like the fact that returning bool lets us actually interpret the result of the script as "should this transaction be accepted, y/n?".

[colll78]

I don't think we should have multiple different kinds of accepted return value. unit would be okay, so long as then it has to be unit. The whole point of this CIP is to narrow down what scripts return as much as possible, so its very clear if you're doing it correctly or not. I don't really see the benefit to having a larger space of possibilities.

I will add an alternative for the return type being unit, although I do kind of like the fact that returning bool lets us actually interpret the result of the script as "should this transaction be accepted, y/n?".

That makes sense, I see the value in having a single accepted return value.

I see a case for both unit and bool. Like you suggest, bool makes interpreting the result of the script more direct; however, I would slightly prefer unit be the accepted return value because bool might confuse people into believing that the only possible return values are true / false. unit might be a bit more transparent in signaling that anything other than () should be a script failure.

Either is fine though. I have seen a lot of contract developers especially early on struggle with debugging because they didn't apply enough arguments to their validator, and they couldn't figure out why it was succeeding no matter what they changed so I see a lot of value in this CIP.

[michaelpj]

I updated the PR to suggest using unit. That does seem more parsimonious since we only need a single distinguished value for success.

[michaelpj]

@rphair I think this is ready for review

[rphair]

@michaelpj it's already on Review for meeting # 81 in 1 week's time as agreed in last meeting. I'll plan to give this a number after that meeting (you are invited of course). If no dissent we might keep pushing it forward to Last Check. https://hackmd.io/@cip-editors/81

It seems like the unit question is settled as of #747 (comment) but of course that can also be triple checked with the other devs at the meeting.

Document itself looks good to me & I would approve it after we assign a number: still keeping an eye on the reservations with @effectfully #747 (review). I would rather see harmony about this before assigning Last Check if possible: it would be nice if both if you could be at the meeting & we could hear some debate about it.

[michaelpj]

To be clear, we might not do this, but I think it's helpful to have it merged as Proposed.

[rvcas]

Just seeing this, I think it's a good idea. The unit stuff makes sense, anything else fails including (obviously) explicit error. I think many of the compile-to-uplc languages already have a wrapper that gets injected which uses an ifThenElse to check the Boolean value returned from the inner function. The wrapper redirects true to () and false to error.

[effectfully]

Somehow unit doesn't feel bad to me either, unlike true. I think it's good design, but I believe we should assess the impact before rolling out such a breakage change by studying what kind of validators we already have on the chain. If those routinely return non-unit values, then we still might consider making unit mandatory, but we'd need to go on a campaign explaining to everybody how the new version of Plutus expects things to be done very differently.

[rphair]

thanks @michaelpj - confirmed as candidate at CIP editors' meeting today, though still not certain to be implemented: clearly beneficial but uncertain how much. In any case we agree it should stand as a reference among other valid ideas.

Please update directory name to CIP-0117 and change the link to the rendered proposal in the OP accordingly 🎉

[zliu41]

CIP-69, which makes all Plutus script take a single argument, makes the problem described in this CIP more likely to happen, and thus more desirable to requiring scripts to return builtin unit to succeed.

I don't know how this impacts other languages and compilers. @rvcas @MicroProofs @KtorZ @nielstron @michele-nuzzi @nau any opinions? Do you think this is worth doing? Any concerns?

[nielstron]

OpShin already forces smart contracts to only return builtin unit.

[MicroProofs]

Aiken also enforces unit is returned on a successful execution.

[zliu41]

Ok, this has been implemented in plutus-ledger-api: evaluateScriptCounting and evaluateScriptRestricting now require returning BuiltinUnit for the evaluation to succeed, except if the script is PlutusV1 or PlutusV2.

@rphair I think we can merge this CIP.

[MicroProofs]

Yo that's pretty awesome didn't expect this to make into PlutusV3

[rphair]

thanks @zliu41 - marked Last Check so we should merge this no later than the coming Tuesday's meeting (https://hackmd.io/@cip-editors/90) cc @Ryun1 @Crypto2099

[Ryun1]

Good to go ✅