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

Combine blueprint and commit tx metadata #1409

Merged
merged 17 commits into from
May 16, 2024
Merged

Combine blueprint and commit tx metadata #1409

merged 17 commits into from
May 16, 2024

Conversation

v0d1ch
Copy link
Contributor

@v0d1ch v0d1ch commented Apr 30, 2024

Why

We have a user report that the commit auxiliary data hashes are invalid when submitting the commit transaction to blockfrost, responding ConflictingMetadataHash.

What

  • Extends the property tests on commitTx construction to also check the auxiliary data hash of the resulting blueprint transaction is correct.

  • Fixes the auxiliary data hash computation and refactors commit tx construction

  • Resolves a warning of missing txSpendingUTxO for the Payment tx type

  • Simplifies definition of toLedgerTx and fromLedgerTx in hydra-cardano-api


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

@v0d1ch v0d1ch self-assigned this Apr 30, 2024
Copy link

github-actions bot commented Apr 30, 2024

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 2024-05-16 10:25:56.985288997 UTC
Max. memory units 14000000
Max. CPU units 10000000000
Max. tx size (kB) 16384

Script summary

Name Hash Size (Bytes)
νInitial bccf2a430c016bc960fbf31b02694011cd399d20da8882aac9d33611 4110
νCommit 56b0f0b597150e619c76bed60683f3b1e42d7bc0685ed951b882bfc5 1975
νHead 86bff95ba20e9d1d1b34899a56d86bbacc9fed999260b27dcc92d128 9351
μHead 88f533cf67cd0fc93d7d9ccf0a8b1d69ffd1208a825efbebbc1d36ba* 4213
  • 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 4792 8.68 3.31 0.46
2 4999 10.66 4.07 0.49
3 5198 12.60 4.82 0.52
5 5600 16.43 6.29 0.57
10 6605 26.22 10.05 0.73
48 14244 99.97 38.37 1.86

Cost of Commit Transaction

This is using ada-only outputs for better comparability.

UTxO Tx size % max Mem % max CPU Min fee ₳
1 559 10.24 4.04 0.29
2 746 13.88 5.64 0.34
3 937 17.66 7.29 0.39
5 1304 25.66 10.74 0.49
10 2241 48.19 20.30 0.78
19 3930 97.83 40.79 1.41

Cost of CollectCom Transaction

Parties UTxO (bytes) Tx size % max Mem % max CPU Min fee ₳
1 57 544 16.93 6.68 0.36
2 114 654 27.48 10.93 0.48
3 170 764 39.82 15.97 0.62
4 226 874 52.84 21.36 0.77
5 282 988 65.90 26.86 0.92
6 341 1099 86.63 35.31 1.15

Cost of Close Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 604 8.34 4.62 0.28
2 765 9.41 5.99 0.30
3 920 10.16 7.08 0.32
5 1374 12.87 10.27 0.39
10 1962 16.06 15.33 0.48
50 8248 51.44 64.14 1.39

Cost of Contest Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 631 8.65 4.74 0.28
2 816 9.88 6.17 0.31
3 847 9.70 6.54 0.31
5 1231 11.76 9.29 0.37
10 2210 18.15 16.87 0.52
50 7825 49.74 61.95 1.34

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 4584 13.93 5.76 0.51
2 4874 28.95 12.70 0.69
3 4924 40.77 17.86 0.83
4 4977 52.24 22.77 0.96
5 5178 73.41 32.17 1.20

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 4628 7.85 3.28 0.44
5 1 57 4661 9.00 3.99 0.46
5 5 285 4798 13.61 6.84 0.52
5 10 569 4966 19.37 10.40 0.60
5 20 1139 5306 30.69 17.44 0.76
5 30 1707 5646 42.43 24.67 0.92
5 40 2277 5986 53.96 31.81 1.08
5 50 2841 6320 65.71 39.04 1.24
5 79 4496 7308 99.00 59.68 1.70

End-To-End Benchmark Results

This page is intended to collect the latest end-to-end benchmarks results produced by Hydra's Continuous Integration system from the latest master code.

Please take those results with a grain of salt as they are currently produced from very limited cloud VMs and not controlled hardware. Instead of focusing on the absolute results, the emphasis should be on relative results, eg. how the timings for a scenario evolve as the code changes.

Generated at 2024-05-16 10:28:19.129531788 UTC

Baseline Scenario

Number of nodes 1
Number of txs 3000
Avg. Confirmation Time (ms) 4.567642475
P99 11.236068089999875ms
P95 4.67230825ms
P50 3.8052265ms
Number of Invalid txs 0

Three local nodes

Number of nodes 3
Number of txs 9000
Avg. Confirmation Time (ms) 21.759153328
P99 113.14955837ms
P95 30.412064799999992ms
P50 19.2489915ms
Number of Invalid txs 0

@ch1bo ch1bo marked this pull request as draft April 30, 2024 16:53
@ch1bo ch1bo self-assigned this May 3, 2024
Copy link

github-actions bot commented May 3, 2024

Test Results

427 tests  ±0   419 ✅ ±0   14m 43s ⏱️ -31s
139 suites ±0     8 💤 ±0 
  2 files   ±0     0 ❌ ±0 

Results for commit f3ba138. ± Comparison against base commit a08ab18.

♻️ This comment has been updated with latest results.

@ch1bo ch1bo marked this pull request as ready for review May 3, 2024 17:08
@ch1bo ch1bo requested a review from a team May 3, 2024 17:12
@ch1bo
Copy link
Collaborator

ch1bo commented May 3, 2024

@v0d1ch Your fix was already correct and I merely refactored and cleaned up the changes. I'd encourage you to double check my refactorings.

@locallycompact @ffakenz You are kindly asked to review the changes as I'm involved now and can't review anymore.

Copy link
Contributor Author

@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.

I'd approve but I can't 😀 Probably because I am the author.

@ch1bo ch1bo added this to the 0.17.0 milestone May 13, 2024
@v0d1ch v0d1ch force-pushed the validate-tx-metadata branch 3 times, most recently from 0a3ab8b to a3dabaa Compare May 13, 2024 13:12
hydra-node/src/Hydra/Chain/Direct/Tx.hs Outdated Show resolved Hide resolved
hydra-cardano-api/src/Hydra/Cardano/Api/Tx.hs Outdated Show resolved Hide resolved
@v0d1ch v0d1ch force-pushed the validate-tx-metadata branch 2 times, most recently from 99515d7 to 4feb75d Compare May 14, 2024 07:10
@ch1bo ch1bo removed their assignment May 15, 2024
v0d1ch and others added 12 commits May 16, 2024 12:04
The genMatadata' generator seems reasonable for our use case here.
This is hopefully a bit clearer to read and maintain going forward.
Reducing noise in the code and providing more counter examples by DRYing
things up a bit.
This was not covered before and seems like we have failing transactions now.
We suspect that toLedgerTx is incomplete if the cardano-api Tx was
modified in it's auxiliary data.
On each conversion we need to recalculate the aux data hash since
cardano-api does not provide a way to set the data hash and on top of
it it seems that it removes it completely.

Add a roundtrip test to assert the metadata is correctly converted.
The cardano-api Tx type is just a wrapper around the ledger and we
should be using that data constructor to do this "conversion".

This also drops a test for "self-healing" auxiliary data hashes, but we
did not require that in the first place: if you modify a transaction
using the ledger-api, make sure to re-compute the auxDataHashTxBodyL.
@ch1bo
Copy link
Collaborator

ch1bo commented May 16, 2024

It turned out that the inconsistency on the auxiliary data hash was on the users side. Nontheless, this PR holds a cleaner implementation of the commit tx construction and some refactorings.

@v0d1ch v0d1ch merged commit 68d157c into master May 16, 2024
21 checks passed
@v0d1ch v0d1ch deleted the validate-tx-metadata branch May 16, 2024 10:39
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