-
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
Allow commit txs with multiple UTxO #774
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 TransactionThis is using ada-only outputs for better comparability.
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 Results307 tests - 11 301 ✔️ - 11 19m 32s ⏱️ +33s Results for commit 3e4eb95. ± Comparison against base commit ed0c22d. 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.
This looks very good! 💯
What is left for me is:
- a test to validate now multiple commits are allowed.
- have a mutation to validate if any of the multiple commits is not healthy.
- update api documentation to mention now we support multiple commits.
- maybe explain and mention the limits around this.
Also what is not clear to me is how, from API can I commit multiple UTxO.
Seems the user must provide a big one instead of multiple, is that intended?
3d95dc4
to
270b957
Compare
82c032f
to
f903bc3
Compare
664d298
to
f722d9a
Compare
This PR works well for us at |
168ab40
to
7814a8e
Compare
2709d99
to
1e5032f
Compare
This changes the Redeemer and Datum types of commitTx to be [TxOutRef] instead of Maybe TxOutRef and generalize the checks on- and off-chain to work on a list of committed UTxO instead of an optional, single one.
Now that we allow multiple commits, we should improve the benchmark to show the (sad) cost of this.
This change is mostly fixing compilation issues. Not all tests are testing the fact that there now may be more than one commit.
This also requires generating new golden files which had this error included.
The picture for commitTx was not yet updated in this.
The limit of comitted UTxO is not in place anymore. Also, reorder two bullets to group known issues related to commits.
Still ada-only (for better control of size), but within a range of 0-5 outputs are committed. The scenario is static per compilation (constant seed, but depends on generators), but currently UTxO with lengths of [0,2,4] is collected from three parties.
Often we only modify a single one of them.
This is a bit artifical, but ensures we check for completeness of recorded commits.
116081f
to
6b18f81
Compare
6b18f81
to
3e4eb95
Compare
🔢 Extends the specification and the on-chain protocol to record any number of outputs committed in a commit tx into a Hydra head.
🔢 Changes (and simplifies) multiple call sites from
Maybe Commit
to[Commit]
.🔢 Adds a mutation test where a commit is tried to be left out.
🔢 This theoretically increases the likelihood that some party locks up too many UTxO and makes the head not even abortable (see #699). Practically, this issue already exists with reference scripts and big datums.
🔢 Removes the
MoreThanOneUTxOCommitted
error as it's not needed anymore.TODO: