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

Align specification and code #988

Merged
merged 16 commits into from
Jul 21, 2023
Merged

Align specification and code #988

merged 16 commits into from
Jul 21, 2023

Conversation

pgrange
Copy link
Contributor

@pgrange pgrange commented Jul 20, 2023

Fixes #974

As also described in the issue, this

  • Reverts the removal of transactions from allTxs upon TxInvalid

  • Updates the off-chain protocol section in the spec to match what is currently implemented

  • Moves the removal of transactions from allTxs when handling a ReqSn (code + spec)

  • Rename terms: seenTxs -> localTxs, seenUTxO -> localUTxO, allTxs stays the same

  • Kept note of discussion points in code or follow-up items:

    • Coherence of TxInvalid messages, e.g. when receiving invalid txs it's unintuitive to get informed about them; on the other hand, we would like to be informed if transactions are not valid to our local view

    • There is no equivalence for storing the fact that we requested a snapshot in the spec. Do we really need it? If yes, we should have a mention of it in the spec or update the formalism accordingly.


  • CHANGELOG updated or not needed
  • Documentation updated or not needed
  • Haddocks updated or not needed
  • No new TODOs introduced or explained herafter

@pgrange pgrange force-pushed the ensemble/align-spec branch from 8d59a8b to 36f2676 Compare July 20, 2023 13:14
@github-actions
Copy link

github-actions bot commented Jul 20, 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-07-21 12:32:55.008770701 UTC
Max. memory units 14000000
Max. CPU units 10000000000
Max. tx size (kB) 16384

Script summary

Name Hash Size (Bytes)
νInitial 7ceb53f05e444cfdabfd0a37a0590090066da457a1f1db30d613b8bd 4289
νCommit 70e70fc13217bfde96932956656c1d540a743b1588c845ca09dc3723 2124
νHead cda51d313c1c8285b6925ce2413def012db27f544e2bbd79b8173000 9185
μHead 1c0b665fc49bc2e9e2ce4e8252c8f37fe84dd75bd8e086abfdb92685* 4149
  • The minting policy hash is only usable for comparison. As the script is parameterized, the actual script is unique per Head.

Cost of Init Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 4740 13.06 5.09 0.50
2 4947 14.53 5.63 0.53
3 5152 18.04 6.98 0.57
5 5563 22.20 8.53 0.64
10 6585 34.93 13.34 0.82
36 11920 96.46 36.50 1.72

Cost of Commit Transaction

This is using ada-only outputs for better comparability.

UTxO Tx size % max Mem % max CPU Min fee ₳
1 596 15.06 5.76 0.34
2 781 19.75 7.76 0.40
3 971 24.85 9.91 0.47
5 1348 36.19 14.60 0.61
10 2281 71.91 28.91 1.04
13 2850 98.24 39.22 1.35

Cost of CollectCom Transaction

Parties UTxO (bytes) Tx size % max Mem % max CPU Min fee ₳
1 57 815 27.96 10.85 0.49
2 113 1135 43.81 17.13 0.68
3 171 1464 62.12 24.44 0.89
4 227 1774 83.30 32.91 1.14

Cost of Close Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 635 18.68 8.34 0.39
2 801 20.38 9.71 0.42
3 969 21.78 10.97 0.45
5 1299 24.27 13.37 0.50
10 2124 31.57 19.78 0.64
50 8725 87.25 69.99 1.74

Cost of Contest Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 668 24.44 10.50 0.45
2 840 26.59 12.04 0.49
3 1006 27.88 13.24 0.52
5 1335 31.75 16.15 0.58
10 2161 40.34 23.01 0.74
44 7771 98.37 69.48 1.79

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 4931 23.72 9.97 0.63
2 5178 36.96 15.65 0.79
3 5498 53.94 23.02 0.99
4 5817 74.69 32.01 1.24
5 6137 97.61 41.99 1.51

Cost of FanOut Transaction

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

Parties UTxO UTxO (bytes) Tx size % max Mem % max CPU Min fee ₳
5 0 0 4766 8.72 3.59 0.46
5 1 56 4801 10.12 4.41 0.48
5 5 283 4944 15.71 7.72 0.55
5 10 568 5123 22.69 11.84 0.64
5 20 1137 5483 36.67 20.11 0.83
5 30 1709 5848 50.65 28.38 1.02
5 40 2278 6205 64.63 36.65 1.21
5 50 2846 6563 78.62 44.92 1.40
5 65 3699 7101 99.62 57.34 1.68

@github-actions
Copy link

github-actions bot commented Jul 20, 2023

Test Results

334 tests   - 15   329 ✔️  - 15   21m 32s ⏱️ - 1m 57s
111 suites  -   4       5 💤 ±  0 
    5 files    -   1       0 ±  0 

Results for commit 1ba305c. ± Comparison against base commit 8330b37.

This pull request removes 16 and adds 1 tests. Note that renamed tests count towards both.
Hydra.HeadLogic/Coordinated Head Protocol/Tracks Transaction Ids ‑ keeps transactions in allTxs given it receives a ReqSn
Hydra.HeadLogic/Coordinated Head Protocol/Tracks Transaction Ids ‑ removes transactions from allTxs when included in a acked snapshot
Hydra.HeadLogic/Coordinated Head Protocol/Tracks Transaction Ids ‑ removes transactions from allTxs when ttl expires
Hydra.TUI.Options ‑ no arguments yield default options
Hydra.TUI.Options ‑ parses --cardano-signing-key option
Hydra.TUI.Options ‑ parses --connect option
Hydra.TUI.Options ‑ parses --node-socket option
Hydra.TUI.Options ‑ parses --testnet-magic option
Hydra.TUI.Options ‑ parses --version option
Hydra.TUI/end-to-end smoke tests ‑ display feedback long enough
…
Hydra.HeadLogic/Coordinated Head Protocol/Tracks Transaction Ids ‑ removes transactions in allTxs given it receives a ReqSn

♻️ This comment has been updated with latest results.

@ch1bo ch1bo force-pushed the ensemble/align-spec branch 3 times, most recently from 3cddf34 to ea46f1a Compare July 21, 2023 09:10
This resulted in some discussion especially about when to drop
transactions from the allTxs in conflicting transaction scenarios.
@ch1bo ch1bo force-pushed the ensemble/align-spec branch from 41632cd to 7dce929 Compare July 21, 2023 11:48
ch1bo and others added 13 commits July 21, 2023 13:50
This was a single pass via a set comprehension before, now we are have
an iterative/mutation algorithm corresponding to the fold in the
functional Haskell implementation.
This is still done in ackSn like in the code.
Transactions included in a snapshot can be removed from allTxs.

Before, it was done when the snapshot was fully validated but, in
fact, as soon as we receiver a request for snapshot, we can not
unsee this snapshot so, anyway, we're commited to, either, integrate
these transactions in this snapshot and validate it or kill the
head liveness.
…way.

If we do so, timex txs which are suppose to get pruned are not.

Also removed blue coloring as now the impl follows the spec.
The difference between `seenTxs` and `allTxs` is not always clear so
we rename things to make it more obvious to read:

* seenTxs -> localTxs
* seenUTxO -> localUTxO
* allTxs stays the same
This also removes the ^ from the Tall variable in spec and code.
@ch1bo ch1bo force-pushed the ensemble/align-spec branch from 7dce929 to d39cb73 Compare July 21, 2023 11:50
We could not express a behavior test which would motivate this
change and it's not covered by the specification. However, as we can
formulate a situation where we would not want them removed, we should
keep them for now.

If performance because of a growing allTxs becomes an issue, we should
put an upper bound on how many invalid txs we keep in allTxs.
@ch1bo ch1bo marked this pull request as ready for review July 21, 2023 12:05
@ch1bo ch1bo changed the title Ensemble/align spec Align specification and code Jul 21, 2023
@ch1bo ch1bo requested a review from v0d1ch July 21, 2023 12:15
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.

Nice!

@v0d1ch v0d1ch merged commit a01d7f5 into master Jul 21, 2023
@v0d1ch v0d1ch deleted the ensemble/align-spec branch July 21, 2023 13:10
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.

Make the spec consistent with #904
4 participants