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

Do allow contest only once #680

Merged
merged 31 commits into from
Feb 6, 2023
Merged

Do allow contest only once #680

merged 31 commits into from
Feb 6, 2023

Conversation

ffakenz
Copy link
Contributor

@ffakenz ffakenz commented Jan 21, 2023

⚡ add closedContesters :: [Plutus.PubKeyHash] attribute to:

  • Closed on-chain state for HeadState datum.
  • ClosedThreadOutput in ClosedState direct-chain state.
  • ClosedState off-chain state in HeadLogic.

⚡ update transactions:

  • closeTx output datum to include a list of contesters to start tracking.
  • contestTx output datum to include the signer in the list of contesters.

⚡ update Head script:

  • checkClose verifies the list of contesters is empty in the output datum.
  • checkContest verifies the signer of contestTx did not contest in the past.

⚡ update transaction observations:

  • observeClose to init the list of contesters as empty.
  • observeContest to update the state using the new list of contesters.

⚡ add mutation in Close and Contest to test the new Head validator checks.

⚡ update log json-schema for ClosedState.

⚡ add orphan instances of Plutus.PubKeyHash for JSON and Arbitrary.

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

@ffakenz ffakenz changed the title Contest only once Do allow contest only once Jan 21, 2023
@ffakenz ffakenz self-assigned this Jan 21, 2023
@github-actions
Copy link

github-actions bot commented Jan 21, 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-02-06 17:32:54.260800504 UTC
Max. memory units 14000000
Max. CPU units 10000000000
Max. tx size (kB) 16384

Script summary

Name Hash Size (Bytes)
νInitial 6e4ce2bd32260424babefe087c2a5ee0a414745fe0281d5608ae9bf8 5530
νCommit 03f2e29fd352f8c2961ad2f61d87a48f88bf01ea6c079b46a3184aee 2547
νHead 86771aaa801dba45c81c0a9662c5d92214df0ae8c76ef4df9f9978ac 9519

Cost of Init Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 5016 9.22 3.63 0.47
2 5221 14.40 5.71 0.54
3 5426 15.94 6.29 0.56
5 5836 19.24 7.55 0.62
10 6862 29.57 11.54 0.77
46 14242 99.94 38.63 1.86

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 630 21.10 8.53 0.41

Cost of CollectCom Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 12801 22.51 8.97 0.96
2 13223 38.92 15.67 1.16
3 13577 56.77 23.04 1.37
4 13997 80.26 32.72 1.65

Cost of Close Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 10113 14.36 5.72 0.75
2 10311 16.44 6.67 0.78
3 10446 17.48 7.19 0.80
5 10843 21.04 8.84 0.86
10 11634 27.40 11.95 0.97
30 15016 56.74 25.96 1.46
61 16101 74.73 27.69 1.66

Cost of Contest Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 10136 15.28 6.05 0.76
2 10342 16.77 6.76 0.79
3 10499 18.47 7.55 0.82
5 10838 20.98 8.79 0.86
10 11697 28.39 12.32 0.98
30 15034 56.95 26.00 1.46
38 16359 68.71 31.61 1.65

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 14720 29.33 12.47 1.12
2 14546 37.05 15.39 1.20
3 14898 57.39 24.30 1.44
4 14769 69.15 29.06 1.57
5 15118 94.93 40.41 1.87

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 14530 11.44 4.92 0.92
2 14371 11.53 5.24 0.91
3 14470 13.43 6.27 0.94
5 14544 16.78 8.15 0.98
10 14854 25.41 12.95 1.10
20 15082 39.35 21.24 1.28
58 16383 96.94 54.59 2.05

@github-actions
Copy link

github-actions bot commented Jan 22, 2023

Test Results

274 tests  +3   268 ✔️ +3   12m 30s ⏱️ -8s
  93 suites +1       6 💤 ±0 
    4 files   ±0       0 ±0 

Results for commit aa222e9. ± Comparison against base commit e7dfd26.

♻️ This comment has been updated with latest results.

@ffakenz ffakenz force-pushed the task/contest_only_once branch from 329269a to bbe28c5 Compare January 24, 2023 10:41
@ffakenz ffakenz marked this pull request as ready for review January 24, 2023 15:09
@ch1bo ch1bo force-pushed the task/contest_only_once branch from 6cfe659 to 28da8b7 Compare January 25, 2023 15:54
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.

We should not use Cardano-specific types in the HeadLogic. I think we can remove it for now. Then, we don't need to regenerate golden files for HeadState and (Timed)ServerOutput.

In general, the added mutation test is not worth much and I would like to see an end-to-end test specifically ensuring that "Allow contest only once" is true.

BTW I would suggest to start with such a test next time it can be specified so clearly!

hydra-node/src/Hydra/HeadLogic.hs Outdated Show resolved Hide resolved
hydra-node/src/Hydra/Chain/Direct/Tx.hs Outdated Show resolved Hide resolved
hydra-node/src/Hydra/Party.hs Outdated Show resolved Hide resolved
hydra-node/json-schemas/logs.yaml Outdated Show resolved Hide resolved
hydra-node/golden/ChainState.json Outdated Show resolved Hide resolved
hydra-node/test/Hydra/Chain/Direct/Contract/Close.hs Outdated Show resolved Hide resolved
@ch1bo ch1bo force-pushed the task/contest_only_once branch from 52a50b3 to d879537 Compare January 30, 2023 15:05
@ffakenz ffakenz force-pushed the task/contest_only_once branch from 241821b to 8ec0845 Compare January 31, 2023 12:44
@v0d1ch v0d1ch requested a review from ch1bo February 1, 2023 09:02
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.

Left some comments

hydra-node/src/Hydra/Chain/Direct/Handlers.hs Outdated Show resolved Hide resolved
hydra-node/golden/HeadState SimpleTx.json Outdated Show resolved Hide resolved
hydra-node/test/Hydra/Chain/Direct/Contract/Close.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/Chain/Direct/Contract/Close.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/Chain/Direct/Contract/Contest.hs Outdated Show resolved Hide resolved
hydra-plutus/src/Hydra/Contract/Head.hs Outdated Show resolved Hide resolved
hydra-plutus/src/Hydra/Contract/Head.hs Outdated Show resolved Hide resolved
hydra-plutus/src/Hydra/Contract/Head.hs Outdated Show resolved Hide resolved
hydra-plutus/src/Plutus/Orphans.hs Show resolved Hide resolved
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.

Left some comments

@ffakenz ffakenz force-pushed the task/contest_only_once branch from 4102087 to e7870c6 Compare February 1, 2023 19:33
@ffakenz ffakenz force-pushed the task/contest_only_once branch 2 times, most recently from a3f7069 to 46aaf61 Compare February 3, 2023 18:08
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.

Left some comments

hydra-node/src/Hydra/Chain/Direct/Tx.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/Chain/Direct/Contract/Close.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/Chain/Direct/Contract/Contest.hs Outdated Show resolved Hide resolved
hydra-plutus/src/Hydra/Contract/Head.hs Outdated Show resolved Hide resolved
hydra-plutus/src/Plutus/Orphans.hs Show resolved Hide resolved
@ch1bo ch1bo force-pushed the task/contest_only_once branch from f2fe66b to 40f3987 Compare February 6, 2023 08:06
@ffakenz ffakenz force-pushed the task/contest_only_once branch from 40f3987 to 6be501b Compare February 6, 2023 14:22
@ch1bo ch1bo force-pushed the task/contest_only_once branch from 6f38e19 to 4b877e8 Compare February 6, 2023 16:40
ch1bo added 2 commits February 6, 2023 17:41
Ideally though we would have used the cardano-api ScriptExecutionError,
but conversion from ledger types to this is actually not exported and we
want to avoid vendoring this conversion code just for identifying this
case. It's a long standing TODO to switch to the cardano-api types and
balancing code in the wallet anyways.
We have replaced this case with the other two, more granular cases for
no fuel UTxO and not enough fuel some time ago.
@ch1bo ch1bo force-pushed the task/contest_only_once branch from 4b877e8 to 3d3fff6 Compare February 6, 2023 16:41
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.

This looks good now!

hydra-node/src/Hydra/Chain/Direct/Handlers.hs Show resolved Hide resolved
@ffakenz ffakenz merged commit 829332b into master Feb 6, 2023
@ffakenz ffakenz deleted the task/contest_only_once branch February 6, 2023 19:27
@@ -389,7 +398,7 @@ forAllFanout action =
in action utxo tx
& label ("Fanout size: " <> prettyLength (countAssets $ txOuts' tx))
where
maxSupported = 39
maxSupported = 30
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change was not needed and got merged by mistake.

Copy link
Contributor Author

@ffakenz ffakenz Feb 6, 2023

Choose a reason for hiding this comment

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

change reverted in following PR by this commit.

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.

3 participants