Dealing with accidental "always true" minting policies

[Quantumplation]

I recently discovered a fun quirk of UPLC. If you have the following Aiken code:

validator {
  fn mint(_datum: Void, redeemer: XYZ, ctx: ScriptContext) {
    // ...
  }
}

This effectively creates an "always succeed" minting policy.

The reason for this is because the node will apply the minting redeemer to the _datum parameter, and the script context to the redeemer parameter, and the result will be a lambda waiting for the script context parameter. Since the node executed this UPLC and didn't receive an error, it considers the transaction to have passed.

In particular, the type checking code injected for the parameters doesn't run until the body of the method, and so the fact that the redeemer wasn't Void or the script context wasn't an XYZ never got a chance to fail the transaction.

This is super non-intuitive and indeed dangerous; A newish Aiken developer unaware of this quirk, who wants to include the datum parameter because it looks more uniform with other scripts (or out of habit, etc.), will run their end to end tests (even going so far as to submit it to the chain and see that it succeeds!) with no problems, and introduce a devastating vulnerability into their protocol.

I figured I'd start a discussion to brainstorm ways we can catch / surface this for the user. Some ideas I came up with:

• campaign for the ledger team to treat lambda as a failure; this is probably a dead end
• interleave the type-checking with the applications; so, applying each argument does the type checking immediately, rather than waiting for each parameter to resolve
• go back to having special functions names (spend, mint, etc.) that understand the expected parameters.

I'm not a UPLC expert, so there may be something preventing 2, but that would be my strong preference.

At the very least, perhaps some heuristic based warnings: if the function name has mint, but takes 3 arguments, surface a warning, etc.

[KtorZ]

Seems related to: #814 ? Nevermind. This is different. Here we're talking about defining a mint validator as a 3-arg function.

[colll78]

This is an unfortunate issue resulting from how we don't have explicit accept values for Plutus scripts.

This will be solved by the following CIP:
cardano-foundation/CIPs#747

[Quantumplation]

asterisk, if accepted. Note mpj's last comment:

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

[Quantumplation]

To elaborate a bit on bullet 2, right now the generated code is something along the lines of:

(datum) => {
  (redeemer) => {
    (scriptContext) => {
      // type check datum
      // type check redeemer
      // script body
    }
  }
}

And the reason this doesn't fail, is because we apply two parameters, and are left with a lambda, type checking code un-run.

If we instead did:

(datum) => {
  // type check datum
  (redeemer) => {
    // type check redeemer
    (scriptContext) => {
      // script body
    }
  }
}

Then we would fail as soon as the node tried to apply the script context as the redeemer parameter (or the redeemer as the datum, if it was something other than Data).

Admittedly, this doesn't catch you if you write either of the following:

fn mint(_data: Data, _redeemer: Data, ctx: ScriptContext)
fn mint(_data: Data, _redeemer: ScriptContext, ctx: ScriptContext)

but it seems at least better than the status quo...

[KtorZ]

@MicroProofs this suggestion makes sense to me. I think it might conflict a bit with the conversation we had the other day and how people using the withdrawal trick wanted their validation deferred (or simply avoided) in their main spending script.

[MicroProofs]

It wouldn't be an issue at all to add this. I originally did the three args then run type checking and validation to keep in line with how PlutusTx accepts params funny enough. I have no issue switching to doing each arg then doing type checking. But keep in mind if the redeemer is typed as Data or unused, then type checking the datum has no effect on solving on solving this issue. A clever transaction could easily exploit a redeemer with type Data. So this seems like a more half solution rather than a full one.

Edit: Nvm I see you mentioned it in the last point.

[KtorZ]

Yes, doing incremental validation isn't perfect but it seems like a low-enough hanging fruit that gets us in a better status quo anyway.

Now, if one still use Data as a type, it doesn't really help indeed.

[rvcas]

I don't really think we should do anything to be honest. This is more of a quirk with how things work in the node. It would be better to push for the CIP @colll78 mentioned imo.

[Mercurial]

here is an example tx that proves this on preview testnet: https://preview.cardanoscan.io/transaction/be078384dcce92068c1d7fc0311ed2ae5b6e68c21f5039632274d223b749cbab

and the validator

https://github.com/coinecta/staking-contracts/blob/04e1646a636d20436ff6e064ebd78567895d2bcb/validators/stake_nft_mint.ak#L33

offchain code

[KtorZ]

We might want to enforce that we have either:

• Always 2 validator functions specified.
• If only one, it has to have 2 arguments only and be a mint/withdraw/publish validator.

So said differently, you can't ever specify a spend validator alone. If you do, you must also provide an equivalent mint validator. Might it be just fail should you want to disallow minting altogether. This is something we could do by default; effectively making all validators hybrid ones.

[MicroProofs]

Hmm then it requires people to know about wrapping only the Spend redeemer. While it's an option, I think simply improving the status quo might be a better step for now??

[KtorZ]

Improving the status quo by interleaving the checks is a must at the moment (net benefit without drawback).

I am just thinking out loud about what more we could do. And you're right that it'd require a wrapped redeemer always. Which in itself has the nice property of becoming a consistent behaviour, which is always good.

[MicroProofs]

Status quo improved on main as of #819