Skip to content
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

More Alonzo unit tests & clean-up #2272

Merged
merged 8 commits into from
May 10, 2021

Conversation

JaredCorduan
Copy link
Contributor

This PR might be easiest to review commit by commit.

📏 New examples / unit tests

  • A transaction with scripts (both 1-phase and 2-phase) in every spot that is allowed
  • Invalid transactions with expected predicate failures

🧹 Cleanup

  • re-enable the AlonzoPredFail round trip serialization test
  • The Alonzo UTXOW rule was sneakily calling the UTXO rule twice. In alonzo, all the predicates that are in common with the previous eras are checked via the shelleyStyleWitness, which calls the UTXO rule. But the Alonzo UTXOW was also calling UTXO directly. This was only noticeable in the predicate failures, since they were repeated twice (the the state transformation was happen twice, the first results of which were discarded).
  • The evaluation of 1-phase scripts were being checked twice in the Alonzo era. Once directly with the Phase1ScriptWitnessNotValidating failure, and once in the shelleyStyleWitness function.
  • The check for all needed 1-phase script hashes in the witness set was also being done twice in Alonzo. Once directly with the MissingNeededScriptHash failure, and once in the shelleyStyleWitness function.
  • Rename the predicate failure ShouldNeverHappenScriptInputsNotFound to CollectErrors. This error should never happen in isolation, that would indicate a logic error in the rules. It does, however, occur alongside other errors. Though (hopefully) redundant, it is a nice sanity check.
  • We hit the field puns & pattern synonyms GHC bug, so I removed the pun to get rid of the warning.

🪲 Bug

  • The collectTwoPhaseScriptInputs function needs to filter out script hashes for every one that is known to not be a 1-phase script, ie those in the script witnesses which are 1-phase. The formal spec does this in a slightly sneaky way right now by applying a function language to the script hashes, which is not defined on 1-phase script hashes.

📚 Verbosity

The current examples are very verbose and much of what is visible is noise. My next goal is to find a way to make them much more concise, in a way that highlights what is interesting about each example.

Jared Corduan added 7 commits May 10, 2021 14:37
The function collectTwoPhaseScriptInputs should not be looking for
plutus data for the 1-phase scripts.
The function shelleyStyleWitness calls the UTXO rule in every era.
Moreover, the Alonzo UTXOW rule calls both shelleyStyleWitness and the
UTXO rule directly (ignoring the state change in the first call).
This results in wasted computation and duplicate predicate failures.

We now remove the direct call of the UTXO rule in the alonzo UTOXW and
call shelleyStyleWitness at the end.
The function shelleyStyleWitness checks the 1-phase scripts, so the
error Phase1ScriptWitnessNotValidating in the Alonzo UTXOW rule is
redundant.
This error should never happen in isolation, that would indicate a logic
error in the rules.  It does, however, occur alongside other errors.
I have renamed it to CollectErrors.
Because of https://gitlab.haskell.org/ghc/ghc/-/issues/14630, we get a
name shadowing warning when we use named field puns together with
pattern synonyms.
It is checked with MissingScriptWitnessesUTXOW in the function
shelleyStyleWitness.
Copy link
Contributor

@polinavino polinavino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great catches on the double-doing things! and super helpful examples, thanks!

We add an example transaction that has scripts everywhere, including
both 1-phase and 2-phase scripts.

We add examples of invalid transactions.
@JaredCorduan JaredCorduan force-pushed the jc/utxow-examples-with-timelocks branch from 86722e2 to 0989b33 Compare May 10, 2021 20:49
@JaredCorduan JaredCorduan merged commit c37546d into master May 10, 2021
@iohk-bors iohk-bors bot deleted the jc/utxow-examples-with-timelocks branch May 10, 2021 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants