redeemers check mistake

[polinavino]

as @WhatisRT points out, the current check for extra redeemers allowed redeemers for non-native scripts : (
this fixes that

[JaredCorduan]

good catch!

[JaredCorduan]

The unit test for "missing 2-phase script witness" is now also returning an addition predicate failure, namely:

ExtraRedeemers [RdmrPtr Mint 1,RdmrPtr Cert 1,RdmrPtr Rewrd 0]

Something seems wrong...

unexpected test failure

[polinavino]

those were redeemers for phase-1 scripts, there is only 1 phase-2 script (used for 1 purpose, so needs 1 redeemer, and the transaction has a total of 4)

[polinavino]

The example failure is fixed.. but now I do not understand why this is happening.. is it something to do with recent changes?:

@nc6 it looks like the same error as in the latest merged PR build?

`[6 of 9] Compiling Test.Cardano.Ledger.Alonzo.Tools ( test/Test/Cardano/Ledger/Alonzo/Tools.hs, dist/build/cardano-ledger-alonzo-test/cardano-ledger-alonzo-test-tmp/Test/Cardano/Ledger/Alonzo/Tools.o, dist/build/cardano-ledger-alonzo-test/cardano-ledger-alonzo-test-tmp/Test/Cardano/Ledger/Alonzo/Tools.dyn_o )

test/Test/Cardano/Ledger/Alonzo/Tools.hs:22:1: error:
Could not find module ‘Cardano.Ledger.Tx’
Perhaps you meant
Cardano.Ledger.Era (from cardano-ledger-core-0.1.0.0)
Cardano.Ledger.Val (from cardano-ledger-core-0.1.0.0)
Cardano.Ledger.Coin (from cardano-ledger-core-0.1.0.0)
Use -v (or :set -v in ghci) to see a list of the files searched for.
|
22 | import qualified Cardano.Ledger.Tx as Core`

[nc6]

Ah, maybe suggests an out-of-order merge issue. I'll fix separately.

[WhatisRT]

I think we can cut down on the complexity now, if we really don't want the size check. checkScriptData can simply check if all the required scripts are present (and no redeemer checks, so the name should probably be changed) and we can simply do all the checks on the redeemers in this set comparison. I'll push a change to the spec that does this to this PR

[WhatisRT]

I made the changes to the formal spec we just discussed

[WhatisRT]

These changes also made it obvious that we've been checking that all the scripts are present twice, so I've removed that duplicate as well. UTXOW looks a lot simpler now :)

[polinavino]

Thank you!!

[JaredCorduan]

LGTM!

[JaredCorduan]

So this predicate failure is happening because the lack of script witness means that we do not know that the script hash is a plutus script, and hence these look superfluous?

[polinavino]

yes, there is no plutus/phase-2 script validating those three things

approved too soon...

[WhatisRT]

I think what's in here is technically equivalent to what's in the spec but it's not obvious. If someone else agrees, we could merge it and fix it later (probably as part of our current ongoing audit)

[JaredCorduan]

LGTM

[WhatisRT]

I think we can merge this now, I'd probably like Polina to have another look at this but we probably don't want to wait till she's back