No extra redeemers

[polinavino]

Add a check to the UTXOW rule that forbids redeemers that have pointers that do not point to script purposes required for validation of the Tx's contracts

[lehins]

A more efficient way of doing the same thing

Suggested change

	let extraRdmrs :: [RdmrPtr]
	extraRdmrs =
	Map.keys $
	Map.filterWithKey
	(\el _ -> not $ elem (SJust el) [rdptr @era txbody sp | (sp, _) <- sphs])
	(unRedeemers $ txrdmrs . wits $ tx)
	let rdptrs = Set.fromList [el | (sp, _) <- sphs, SJust el <- [rdptr @era txbody sp]]
	extraRdmrs :: [RdmrPtr]
	extraRdmrs = Map.keys $ Map.withoutKeys (unRedeemers $ txrdmrs $ wits tx) rdptrs

[JaredCorduan]

We should also include a test, similar to the ones here:

https://github.com/input-output-hk/cardano-ledger-specs/blob/51561fcb13170e3849873eef82e634c1041ee1d6/cardano-ledger-test/src/Test/Cardano/Ledger/Examples/TwoPhaseValidation.hs#L1653-L1661

[polinavino]

@JaredCorduan do you happen to know why "Chain and Ledger traces cover the relevant cases" is failing now? I have no idea what to do about that...

[JaredCorduan]

@JaredCorduan do you happen to know why "Chain and Ledger traces cover the relevant cases" is failing now? I have no idea what to do about that...

yea, the alonzo traces are still flakey, it's very frustrating. no tests are really failing, it's that we do not get a high enough percentage of good traces:

*** Gave up! Passed only 199 tests; 37 discarded tests:

Once the PR is ready to merge, I can always re-run the tests if this happens again.

[polinavino]

I added a test with an extra redeemer - so it could be ready, unless more tests are needed

[JaredCorduan]

I added a test with an extra redeemer - so it could be ready, unless more tests are needed

fantastic, thank you! let me spin the wheel! 🎰

[JaredCorduan]

looks good to me, thank you @polinavino !

[WhatisRT]

I'm a bit confused about what this is doing, this looks like a weakening in the condition in the formal spec to me. checkScriptData verifies that every necessary script is present, and that all 2-phase scripts have redeemers. This means that all required redeemers are present. The next line then was just a size check previously, which already implies that the redeemers that are present are exactly the redeemers required.

Now, it's a check that all redeemers are required by scriptsNeeded, but this also includes 1-phase scripts. So now, in addition to the 2-phase redeemers it can also contain redeemers for 1-phase scripts. So this change now allows a larger set of redeemers than before, which is not at all what the intent of this PR seems to be. This smells like a mismatch between spec and code for me

[WhatisRT]

And from reading the Haskell diff, it seems like this size check was just not implemented before?

[polinavino]

yea, youre right! fixing this now (adding the constraint that SP has to be for a non-native script). There was no check at all before I put the size check, and the reason I changed it from the size-check to the current one is that we want to be able to get a better error report (a list of the extra redeemers)