-
Notifications
You must be signed in to change notification settings - Fork 88
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
Add missing explicit error expectation for transaction mutations #719
Conversation
Transactions CostsSizes and execution budgets for Hydra protocol transactions. Note that unlisted parameters are currently using
Script summary
Cost of Init Transaction
Cost of Commit TransactionCurrently only one UTxO per commit allowed (this is about to change soon)
Cost of CollectCom Transaction
Cost of Close Transaction
Cost of Contest Transaction
Cost of Abort TransactionSome variation because of random mixture of still initial and already committed outputs.
Cost of FanOut TransactionInvolves spending head output and burning head tokens. Uses ada-only UTxO for better comparability.
|
Test Results285 tests - 11 279 ✔️ - 11 19m 45s ⏱️ -38s Results for commit 29ac49e. ± Comparison against base commit 32d02fe. This pull request removes 11 tests.
♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's clarify some of the failures further.
b731115
to
2a1128c
Compare
2a1128c
to
e0c85ee
Compare
e0c85ee
to
fd9c157
Compare
fd9c157
to
447c4e0
Compare
b08a808
to
2e15138
Compare
e83bcfe
to
2f9aef7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ask for clarifications if my comments are not clear.
If you disagree with my comments, just ignore them and don’t answer or explain.
Hence I, in advance, approve the P.R.
What I like about this P.R.:
- The effort made to explain the intent of the existing mutations
- Some of the simplifications introduced
For me to find it perfect you would have to:
- reduce the size of the P.R. so that we have a simple and clear commit history to walk through (I can't pretend I've reviewed everything)
- see inline comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last round of inline comments
By using 'const $ healthyClosedState' we are producing an output datum state with snapshotNumber = 3, and because the input datum snapshotNumber = 3, then it fails for the wrong reason TooOldSnapshot, as it needs to be greater to verify: closedSnapshotNumber' > closedSnapshotNumber.
This is redundant because is enough to check that the value is preserved, as we are comming from a valid Closed state having already checked that the ST is present. This is a breaking change so we upgrade hydra-plutus to 0.10.0. Update changelog.
We were changing input using the closed state as script data instad of a contest redeemer.
To be explicit about the transactions being involved.
We were changing input using the open state as script data instad of a close redeemer.
Notes added to apply them in another PR.
We want to enforce the why for a mutation to relevant and not the what is doing. So here we are removing details about the 'what' is failing about the tx. We think that is better to follow a white box approach on mutations, thus labels and error codes should be enough to describe the what.
Break haddock lines on 80 characters makes it easier to read on small or split screens.
On each success, quickcheck increases the size parameter and this will make generators create bigger values (if they honor the size). The genValue generator was creating too big values to pass evaluation within the budget, so we scale them down by a factor of 2.
On each success, quickcheck increases the size parameter and this will make generators create bigger values (if they honor the size). The genValue generator was creating too big values to pass evaluation within the budget, so we scale them down by a factor of 2.
Chances to have this test produce a verification key equal to the healthy one is very unlikely. Anyhow, the changes make the mutation more readable and explicit, thus the reason for this change. This mutation also exists in Close so this also brings consistency to the table between the two.
12e94e8
to
29ac49e
Compare
This contributes to moving #705 forward.
To check before merging: