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

check commits are reimbursed in nu head #670

Merged
merged 25 commits into from
Jan 13, 2023

Conversation

ffakenz
Copy link
Contributor

@ffakenz ffakenz commented Jan 5, 2023

This PR is a step into the right direction but there will be a follow-up one to address a gap in the checkAbort where we should check that two identical commits can not be reimbursed by one output.

To check before merging:

  • CHANGELOG is up to date
  • Documentation is up to date

@github-actions
Copy link

github-actions bot commented Jan 5, 2023

Transactions Costs

Sizes and execution budgets for Hydra protocol transactions. Note that unlisted parameters are currently using arbitrary values and results are not fully deterministic and comparable to previous runs.

Metadata
Generated at 2023-01-13 10:22:09.039463532 UTC
Max. memory units 14000000
Max. CPU units 10000000000
Max. tx size (kB) 16384

Cost of Init Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 4988 10.02 3.96 0.48
2 5188 13.82 5.47 0.53
3 5394 13.96 5.48 0.54
5 5808 20.11 7.91 0.63
10 6829 27.85 10.83 0.75
45 14005 99.14 38.35 1.84

Cost of Commit Transaction

Currently only one UTxO per commit allowed (this is about to change soon)

UTxO Tx size % max Mem % max CPU Min fee ₳
1 633 20.88 8.46 0.41

Cost of CollectCom Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 12516 23.54 9.26 0.96
2 12939 43.83 17.31 1.20
3 13292 66.23 26.29 1.46
4 13645 92.87 37.02 1.77

Cost of Close Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 9384 7.29 2.90 0.64
2 9651 9.05 3.88 0.68
3 9814 9.84 4.34 0.69
5 9851 9.92 3.91 0.69
10 10970 15.37 7.54 0.81
30 14352 32.29 17.14 1.17
68 16133 40.38 15.48 1.30

Cost of Contest Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 9446 7.70 3.19 0.65
2 9611 8.49 3.65 0.67
3 9778 9.28 4.11 0.69
5 10142 11.23 5.17 0.72
10 11001 15.51 7.59 0.81
30 14341 31.95 17.00 1.16
42 16284 41.06 22.34 1.36

Cost of Abort Transaction

Some variation because of random mixture of still initial and already committed outputs.

Parties Tx size % max Mem % max CPU Min fee ₳
1 13704 22.77 9.46 1.01
2 14057 38.98 16.59 1.20
3 14695 66.85 30.44 1.56
4 14280 70.66 30.22 1.57

Cost of FanOut Transaction

Involves spending head output and burning head tokens. Uses ada-only UTxO for better comparability.

UTxO Tx size % max Mem % max CPU Min fee ₳
1 13601 10.16 4.43 0.86
2 13703 11.82 5.35 0.89
3 13804 14.02 6.50 0.92
5 13940 17.15 8.29 0.96
10 13991 24.26 12.50 1.05
50 15362 84.64 47.50 1.85
59 15748 98.76 55.58 2.04

Copy link
Contributor

@v0d1ch v0d1ch left a comment

Choose a reason for hiding this comment

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

🚢

@ffakenz ffakenz force-pushed the ensemble/check-commits-are-reimbursed-in-nu-head branch 2 times, most recently from 73432c2 to 8cb9984 Compare January 5, 2023 15:17
Copy link
Collaborator

@ch1bo ch1bo left a comment

Choose a reason for hiding this comment

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

Interestingly the DropOneCommitOutput mutation fails now. A possible solution could be to check that all PTs are burned in the abortTx (e.g. by the head validator).

hydra-plutus/src/Hydra/Contract/Commit.hs Outdated Show resolved Hide resolved
hydra-plutus/src/Hydra/Contract/Commit.hs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@pgrange pgrange force-pushed the ensemble/check-commits-are-reimbursed-in-nu-head branch from b6445f6 to 2e19048 Compare January 10, 2023 15:06
@pgrange pgrange requested a review from ch1bo January 10, 2023 15:19
@v0d1ch v0d1ch force-pushed the ensemble/check-commits-are-reimbursed-in-nu-head branch 2 times, most recently from eabf27b to f0356a6 Compare January 11, 2023 16:12
@pgrange pgrange removed the request for review from ch1bo January 12, 2023 10:22
@github-actions
Copy link

github-actions bot commented Jan 12, 2023

Test Results

268 tests   - 11   262 ✔️  - 11   13m 22s ⏱️ - 1m 38s
  91 suites  -   4       6 💤 ±  0 
    4 files    -   1       0 ±  0 

Results for commit dc6d7d0. ± Comparison against base commit 7f5e25c.

This pull request removes 11 tests.
Hydra.TUI.Options ‑ parses --cardano-signing-key option
Hydra.TUI.Options ‑ parses --connect option
Hydra.TUI.Options ‑ parses --network-id option
Hydra.TUI.Options ‑ parses --node-socket option
Hydra.TUI/end-to-end smoke tests ‑ display feedback long enough
Hydra.TUI/end-to-end smoke tests ‑ doesn't allow multiple initializations
Hydra.TUI/end-to-end smoke tests ‑ starts & renders
Hydra.TUI/end-to-end smoke tests ‑ supports the full Head life cycle
Hydra.TUI/end-to-end smoke tests ‑ supports the init & abort Head life cycle
Hydra.TUI/text rendering errors ‑ should show not enough fuel message and suggestion
…

♻️ This comment has been updated with latest results.

@pgrange pgrange requested a review from ch1bo January 12, 2023 10:28
@v0d1ch v0d1ch force-pushed the ensemble/check-commits-are-reimbursed-in-nu-head branch from 9d00143 to 3372c46 Compare January 12, 2023 12:33
abailly and others added 14 commits January 13, 2023 09:22
We should check the output of abort with the inputs of the transaction
and not the output.
Not all the inputs we explore are commits, some may be intials so
we tolerate for the absence of datum as intial have no datum.
We explicitly check that one can not try to spend an initial in
a collectCom
Head datum check has been removed from the commit validator
@ch1bo ch1bo force-pushed the ensemble/check-commits-are-reimbursed-in-nu-head branch from 3372c46 to fedcb8c Compare January 13, 2023 08:22
newInput <- arbitrary
-- XXX: Ideally we should need to modify the PT to simulate a proper new commit
-- FIXME: this shouldn't be green
pure $ AddInput newInput output
Copy link
Collaborator

Choose a reason for hiding this comment

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

@v0d1ch and me added this to highlight that this PR is not completely solving the gap. We need to make sure the commits are completely reimbursed, i.e. not two identical commits can be reimbursed by one output. A follow-up PR will address this (and the corresponding FIXME in the script)

@v0d1ch v0d1ch merged commit 9cb4e1b into master Jan 13, 2023
@v0d1ch v0d1ch deleted the ensemble/check-commits-are-reimbursed-in-nu-head branch January 13, 2023 12:07
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.

5 participants