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

Prevent empty decommits #1529

Closed
wants to merge 1 commit into from
Closed

Prevent empty decommits #1529

wants to merge 1 commit into from

Conversation

v0d1ch
Copy link
Contributor

@v0d1ch v0d1ch commented Jul 24, 2024

fix #1502

In order to prevent spamming the hydra-node with empty decommit requests we want to make sure to reject both Decommit client input and ReqDec network messages if the decommit tx outputs are empty.


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

@v0d1ch v0d1ch requested a review from a team July 24, 2024 08:38
@v0d1ch v0d1ch self-assigned this Jul 24, 2024
In order to prevent spamming the hydra-node with empty decommit requests
we want to make sure to reject both Decommit client input and ReqDec
network messages if the decommit is empty.
Copy link

Transaction 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 2024-07-24 08:44:40.089434357 UTC
Max. memory units 14000000
Max. CPU units 10000000000
Max. tx size (kB) 16384

Script summary

Name Hash Size (Bytes)
νInitial 2fac819a1f4f14e29639d1414220d2a18b6abd6b8e444d88d0dda8ff 3799
νCommit 2043a9f1a685bcf491413a5f139ee42e335157c8c6bc8d9e4018669d 1743
νHead 7ae23bc9f0833a5689b9fc812dd92fbe9dac881a632f14b28e8eb8db 10193
μHead 3ebfb5b268e0c94200e2c8a8eeebf704aabf303a769af602edb3603a* 4607
  • The minting policy hash is only usable for comparison. As the script is parameterized, the actual script is unique per head.

Init transaction costs

Parties Tx size % max Mem % max CPU Min fee ₳
1 5186 5.71 2.25 0.44
2 5390 6.99 2.76 0.46
3 5591 8.37 3.30 0.49
5 5993 11.17 4.41 0.54
10 6996 18.06 7.14 0.66
56 16246 81.52 32.24 1.76

Commit transaction costs

This uses ada-only outputs for better comparability.

UTxO Tx size % max Mem % max CPU Min fee ₳
1 557 10.52 4.15 0.29
2 749 13.86 5.65 0.34
3 934 17.33 7.20 0.38
5 1307 24.65 10.44 0.48
10 2245 45.22 19.36 0.75
20 4119 95.99 40.76 1.40

CollectCom transaction costs

Parties UTxO (bytes) Tx size % max Mem % max CPU Min fee ₳
1 56 548 21.46 8.41 0.41
2 114 659 33.93 13.42 0.55
3 171 769 46.21 18.48 0.69
4 227 879 62.39 25.10 0.87
5 284 994 76.63 31.11 1.04
6 338 1100 90.05 36.93 1.19

Cost of Decrement Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 616 17.96 7.88 0.38
2 829 20.18 9.47 0.42
3 852 19.24 9.80 0.41
5 1188 22.73 12.61 0.48
10 1870 30.49 19.28 0.62
50 7920 98.20 74.73 1.83

Close transaction costs

Parties Tx size % max Mem % max CPU Min fee ₳
1 646 20.98 9.39 0.42
2 853 22.86 11.17 0.45
3 908 23.84 12.17 0.47
5 1297 27.54 15.65 0.54
10 2346 37.40 24.87 0.74
49 7913 96.84 82.08 1.87

Contest transaction costs

Parties Tx size % max Mem % max CPU Min fee ₳
1 697 27.18 11.69 0.48
2 903 29.36 13.57 0.53
3 978 31.02 14.98 0.55
5 1263 34.55 18.02 0.61
10 2205 44.93 27.02 0.81
38 6308 99.13 72.91 1.75

Abort transaction costs

There is some variation due to the random mixture of initial and already committed outputs.

Parties Tx size % max Mem % max CPU Min fee ₳
1 4979 13.95 5.82 0.52
2 5164 24.90 10.79 0.66
3 5271 37.72 16.49 0.81
4 5431 54.12 23.87 1.00
5 5553 71.62 31.68 1.20
6 5613 88.74 39.31 1.40

FanOut transaction costs

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 5022 7.95 3.36 0.46
5 1 57 5056 9.08 4.08 0.48
5 5 285 5193 13.41 6.84 0.54
5 10 569 5361 18.87 10.31 0.61
5 20 1140 5702 30.58 17.60 0.78
5 30 1705 6039 41.90 24.72 0.93
5 40 2275 6380 53.23 31.84 1.09
5 50 2842 6716 64.17 38.80 1.24
5 81 4611 7772 99.92 61.18 1.74

End-to-end benchmark results

This page is intended to collect the latest end-to-end benchmark results produced by Hydra's continuous integration (CI) system from the latest master code.

Please note that these results are approximate as they are currently produced from limited cloud VMs and not controlled hardware. Rather than focusing on the absolute results, the emphasis should be on relative results, such as how the timings for a scenario evolve as the code changes.

Generated at 2024-07-24 08:47:53.106568631 UTC

Baseline Scenario

Number of nodes 1
Number of txs 3000
Avg. Confirmation Time (ms) 4.249089262
P99 8.701609799999995ms
P95 5.488250949999998ms
P50 4.0219925ms
Number of Invalid txs 0

Three local nodes

Number of nodes 3
Number of txs 9000
Avg. Confirmation Time (ms) 23.309280357
P99 112.13197182ms
P95 32.46403915ms
P50 20.765465ms
Number of Invalid txs 0

Copy link

Test Results

466 tests   454 ✅  15m 50s ⏱️
148 suites   12 💤
  5 files      0 ❌

Results for commit b62bae8.

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.

See my thought whether this maybe not even an issue?

, decommitInvalidReason =
ServerOutput.DecommitTxInvalid{localUTxO, validationError}
}
if utxoFromTx decommitTx == mempty
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of checking it here, I would suggest doing it as "a step before" like we do in other places. See also the TODO in line 735/744 which mentions the #1502 issue.

Do we need to update the spec?

{ headId
, decommitTx
, decommitInvalidReason =
ServerOutput.DecommitTxInvalid{localUTxO, validationError = emptyDecommitError}
Copy link
Collaborator

Choose a reason for hiding this comment

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

It feels ugly that you "shoehorn" the hard-coded emptyDecommitError into a ledger validationError field here.

But this makes me think.. is this whole thing even an issue if we use a real ledger? i.e. wouldn't the cardanoLedger make empty decommits impossible by requiring transactions to be balanced?

Must: Validate whether the #1502 issue is even real by having a test that uses real Tx trying to do decommit nothing. What's the behavior without these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ledger prevents empty tx outputs with ValueNotConserved and in case you don't provide any inputs then it is TxBodyEmptyTxIns. So this is actually not an issue we thought it would be 👍

@v0d1ch
Copy link
Contributor Author

v0d1ch commented Jul 25, 2024

Closing because this seems to be a non-issue #1529 (comment)

@v0d1ch v0d1ch closed this Jul 25, 2024
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.

Prevent empty decommits
2 participants