Snapshot number and utxo hash need not be in the redeemer on close/contest #677

ffakenz · 2023-01-16T18:22:17Z

⛰️ Remove snapshot number and utxo hash from Contest and Close redeemers

the output datum needs to include them and we can check using that
only the signature is enough

⛰️ BTW this improves the coverage for the case where we close with the initialSnapshot

To check before merging:

CHANGELOG is up to date
Documentation is up to date

github-actions · 2023-01-16T18:41:35Z

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-01 16:36:05.646474015 UTC
Max. memory units	14000000
Max. CPU units	10000000000
Max. tx size (kB)	16384

Script summary

Name	Hash	Size (Bytes)
νInitial	bbed691fc7fdce8d95616cf16b01e5ed2abb42f20c0a5de6882bb860	5429
νCommit	08bb32edf316341c4fa7b0e3ed0e77bccb7f1d3b687c10d1b1e48969	2961
νHead	e4c9e128a7674f68247a796da72265727c782e54d93e1285b4066c60	9835

Cost of Init Transaction

Parties	Tx size	% max Mem	% max CPU	Min fee ₳
1	4984	12.19	4.86	0.50
2	5187	11.26	4.42	0.50
3	5397	13.20	5.17	0.53
5	5804	18.57	7.27	0.61
10	6829	29.99	11.71	0.78
45	14005	98.23	37.98	1.83

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.90	8.47	0.41

Cost of CollectCom Transaction

Parties	Tx size	% max Mem	% max CPU	Min fee ₳
1	13425	22.01	8.61	0.98
2	13884	42.11	16.67	1.22
3	14235	63.92	25.45	1.48
4	14659	92.72	36.96	1.81

Cost of Close Transaction

Parties	Tx size	% max Mem	% max CPU	Min fee ₳
1	10427	14.87	5.84	0.77
2	10463	13.72	5.25	0.76
3	10761	17.52	7.13	0.82
5	11158	20.61	8.60	0.87
10	11989	27.43	11.92	0.99
30	15292	54.04	24.90	1.44
60	16321	68.76	25.49	1.61

Cost of Contest Transaction

Parties	Tx size	% max Mem	% max CPU	Min fee ₳
1	10426	14.43	5.66	0.77
2	10624	16.08	6.44	0.79
3	10816	17.78	7.23	0.82
5	11086	19.72	8.25	0.86
10	12015	27.78	12.05	0.99
30	15313	54.09	24.91	1.44
36	16271	61.76	28.68	1.57

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	15004	29.47	12.52	1.14
2	15072	42.12	17.79	1.28
3	15181	57.52	24.34	1.46
4	15367	77.15	32.80	1.69

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	14616	10.20	4.44	0.91
2	14652	11.47	5.22	0.93
3	14815	14.05	6.51	0.96
5	14760	16.04	7.86	0.99
10	15135	25.05	12.81	1.11
20	15429	39.96	21.48	1.31
48	16299	81.92	45.86	1.86

github-actions · 2023-01-17T15:58:19Z

Test Results

282 tests +13 276 ✔️ +13 16m 1s ⏱️ + 2m 40s
  96 suites +  5     6 💤 ±  0
    5 files +  1     0 ❌ ±  0

Results for commit 005ade2. ± Comparison against base commit 179df2f.

♻️ This comment has been updated with latest results.

v0d1ch

Looks good to me overall @ffakenz, I just have a concern @ch1bo will be able to resolve which is related to constructing the expected datum.

hydra-plutus/src/Hydra/Contract/Head.hs

ch1bo

As @v0d1ch mentioned, the on-chain code can and should be simplified before we merge this.

CHANGELOG.md

hydra-node/test/Hydra/Chain/Direct/Contract/Close.hs

hydra-plutus/src/Hydra/Contract/Head.hs

pgrange

Code duplication when fetching datum from the tx in close and contest. See proposed code change to remove duplication.

Side note: I'm having trouble with naming this function. Maybe you can find a better or, maybe, this is a smell that something's odd here and could be improved but I can't find it.

hydra-plutus/src/Hydra/Contract/Head.hs

pgrange · 2023-01-23T12:56:01Z

Some tests are failing since what looks like the first commits of the branch.

~~This build fails since this commit (which has been overwritten since then by some rebase I guess).~~

This build make me believe this commit introduced the issue.

I'm moving this P.R. back to in progress to investigate and fix.

hydra-node/hydra-node.cabal

hydra-node/exe/tx-cost/TxCost.hs

Extract mutateClosedContestationDeadline to use in both close and closeInitial mutations

We introduce replaceSomething to ease the mutation of a head state. This functions are exposed by Mutation.hs as it seems to be the better place to me right now but maybe we can do better? Also, we replace some functions which did that and failed when the state was not an expected one. Now we just return the state unchanged if the field we want to change does not exist for this state. It may be a good idea... it also may introduce difficulties in diagnostic test errors. That is not clear at this point. If ist becomes a problem, we can sill introduce the errors back easily in the code.

The removed test used to check that the validator would fail for a snapshot with a negative value. But due to recent changes, it is now OK to have a negative snapshot number. Although it should fail if we try to use a negative snaphot number with a non-intiail snapshot. So we amend the previous test we checked for 0 value with a generated negative or 0 value.

The 0 case can be captured by ensuring the deadline generator covers this as well.

There is a problem with the way we test validity bounds mutations. Most of the arbitrarily generated values will make the validator fail but there are no reason to not generate valid transaction with what we had. To fix that, we distinguish between 3 situations: 1. The lower bound is infinite 2. The upper bound is infinite 3. The upper bound is too high given the contestation deadline

No need to call this "healthy" as it cannot be used outside of the healthyCloseInitialTx context.

One could have tried to change the head parameters during a close, like change the headId or the parties.

ch1bo

Only a very minor thing left and the tests seem to not agree on the maxSupported assets as it fails with 45 utxos on the fanout.

Feel free to merge after fixing this so I approve.

hydra-node/test/Hydra/Chain/Direct/Contract/Close.hs

ffakenz mentioned this pull request Jan 16, 2023

Align implementation with HeadV1 protocol specification #452

Closed

20 tasks

ffakenz force-pushed the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch from 172a1bc to 2f0b7e2 Compare January 16, 2023 19:04

ffakenz marked this pull request as ready for review January 17, 2023 16:12

ch1bo requested review from ch1bo and v0d1ch January 18, 2023 10:39

ch1bo assigned ffakenz and ch1bo and unassigned ffakenz Jan 19, 2023

v0d1ch approved these changes Jan 19, 2023

View reviewed changes

hydra-plutus/src/Hydra/Contract/Head.hs Outdated Show resolved Hide resolved

hydra-plutus/src/Hydra/Contract/Head.hs Outdated Show resolved Hide resolved

ch1bo requested changes Jan 20, 2023

View reviewed changes

v0d1ch force-pushed the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch from 9f1d99a to a3174f0 Compare January 20, 2023 10:24

ch1bo assigned v0d1ch Jan 20, 2023

pgrange suggested changes Jan 23, 2023

View reviewed changes

hydra-plutus/src/Hydra/Contract/Head.hs 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

ch1bo force-pushed the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch from 92ba27c to ef28398 Compare January 24, 2023 08:50

ch1bo assigned pgrange and ffakenz and unassigned v0d1ch and ch1bo Jan 24, 2023

pgrange force-pushed the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch from 5910e4a to 2f235fc Compare January 24, 2023 15:25

pgrange reviewed Jan 24, 2023

View reviewed changes

hydra-node/hydra-node.cabal Show resolved Hide resolved

ch1bo reviewed Jan 25, 2023

View reviewed changes

hydra-node/exe/tx-cost/TxCost.hs Outdated Show resolved Hide resolved

ch1bo self-assigned this Jan 25, 2023

pgrange force-pushed the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch 2 times, most recently from ff00dec to 3f9add4 Compare January 25, 2023 15:41

ch1bo force-pushed the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch from fb80c1e to ed4b111 Compare January 26, 2023 09:08

pgrange force-pushed the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch 2 times, most recently from 7eae215 to ef08385 Compare January 30, 2023 10:31

pgrange added 2 commits January 31, 2023 18:40

Reduce duplication

4efed8e

Extract mutateClosedContestationDeadline to use in both close and closeInitial mutations

pgrange force-pushed the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch from fcea1e7 to d511963 Compare January 31, 2023 17:40

pgrange mentioned this pull request Feb 1, 2023

Async issues with tracer #690

Merged

2 tasks

abailly-iohk marked this pull request as draft February 1, 2023 11:13

ch1bo marked this pull request as ready for review February 1, 2023 11:15

pgrange marked this pull request as draft February 1, 2023 11:16

pgrange force-pushed the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch from ae00b16 to 8dd1029 Compare February 1, 2023 14:13

pgrange and others added 13 commits February 1, 2023 15:12

Generate interesting / mutated deadlines and unify mutation cases

75a36be

The 0 case can be captured by ensuring the deadline generator covers this as well.

Refer to spec from close mutation labels

7adb1e6

Try to restructure code a bit

b795ee6

Keep a note that fromBuiltinData is increasing script size

ed5eb65

Won't FIX in this P.R.

a0e535d

This is not a generally available stuff

4b7bf7c

No need to call this "healthy" as it cannot be used outside of the healthyCloseInitialTx context.

Explain why we would have two healthy close cases to test

ec11c64

Explain why we keep this single mutation for initial close

6d49e60

FIX one cound change head parameters

e0dad73

One could have tried to change the head parameters during a close, like change the headId or the parties.

Same potential performance issue here

742805a

Remove suchThat and use choose to generate tx validity values

d8250d6

pgrange force-pushed the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch from 51d1c66 to d8250d6 Compare February 1, 2023 15:18

pgrange marked this pull request as ready for review February 1, 2023 15:21

ch1bo approved these changes Feb 1, 2023

View reviewed changes

ch1bo mentioned this pull request Feb 1, 2023

Do allow contest only once #680

Merged

2 tasks

Back to working value

005ade2

pgrange merged commit 2c04ccf into master Feb 1, 2023

pgrange deleted the gap/move_attributes_from_redeemer_to_datum_for_close_and_contest_tx branch February 1, 2023 16:38

ch1bo unassigned pgrange and ch1bo Feb 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Snapshot number and utxo hash need not be in the redeemer on close/contest #677

Snapshot number and utxo hash need not be in the redeemer on close/contest #677

ffakenz commented Jan 16, 2023 •

edited by pgrange

Loading

github-actions bot commented Jan 16, 2023 •

edited

Loading

github-actions bot commented Jan 17, 2023 •

edited

Loading

v0d1ch left a comment

ch1bo left a comment

pgrange left a comment

pgrange commented Jan 23, 2023 •

edited

Loading

ch1bo left a comment

Snapshot number and utxo hash need not be in the redeemer on close/contest #677

Snapshot number and utxo hash need not be in the redeemer on close/contest #677

Conversation

ffakenz commented Jan 16, 2023 • edited by pgrange Loading

github-actions bot commented Jan 16, 2023 • edited Loading

Transactions Costs

Script summary

Cost of Init Transaction

Cost of Commit Transaction

Cost of CollectCom Transaction

Cost of Close Transaction

Cost of Contest Transaction

Cost of Abort Transaction

Cost of FanOut Transaction

github-actions bot commented Jan 17, 2023 • edited Loading

Test Results

v0d1ch left a comment

Choose a reason for hiding this comment

ch1bo left a comment

Choose a reason for hiding this comment

pgrange left a comment

Choose a reason for hiding this comment

pgrange commented Jan 23, 2023 • edited Loading

ch1bo left a comment

Choose a reason for hiding this comment

ffakenz commented Jan 16, 2023 •

edited by pgrange

Loading

github-actions bot commented Jan 16, 2023 •

edited

Loading

github-actions bot commented Jan 17, 2023 •

edited

Loading

pgrange commented Jan 23, 2023 •

edited

Loading