Skip to content

Deposit value fix #1969

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

Merged
merged 9 commits into from
May 15, 2025
Merged

Deposit value fix #1969

merged 9 commits into from
May 15, 2025

Conversation

ch1bo
Copy link
Member

@ch1bo ch1bo commented Apr 26, 2025

This makes sure that the deposited value as claimed in the datum is actually available at the deposit script output to be incremented.

Related to this paragraph in the specification:

image


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

@ch1bo ch1bo requested a review from a team April 26, 2025 21:13
@ch1bo ch1bo self-assigned this Apr 26, 2025
@ch1bo ch1bo force-pushed the deposit-value-fix branch from 4dc17a4 to d1e24e3 Compare April 26, 2025 21:17
Copy link

github-actions bot commented Apr 26, 2025

Transaction cost differences

No cost or size differences found

Copy link

github-actions bot commented Apr 26, 2025

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 2025-05-15 06:36:02.144858194 UTC
Max. memory units 14000000
Max. CPU units 10000000000
Max. tx size (kB) 16384

Script summary

Name Hash Size (Bytes)
νInitial c8a101a5c8ac4816b0dceb59ce31fc2258e387de828f02961d2f2045 2652
νCommit 61458bc2f297fff3cc5df6ac7ab57cefd87763b0b7bd722146a1035c 685
νHead be6ebc744208c660bf0fdc1cfbb5157477cd305de5b1777e575cbb4c 14665
μHead 1f47a42d1d6edc32ccd834acb19d5db3b2a5232f0bd7eaa8908dc519* 5284
νDeposit ae01dade3a9c346d5c93ae3ce339412b90a0b8f83f94ec6baa24e30c 1102
  • 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 5836 10.44 3.31 0.52
2 6035 12.20 3.85 0.54
3 6236 14.68 4.64 0.58
5 6640 18.38 5.79 0.63
10 7646 29.09 9.17 0.79
43 14279 98.73 30.85 1.80

Commit transaction costs

This uses ada-only outputs for better comparability.

UTxO Tx size % max Mem % max CPU Min fee ₳
1 561 2.44 1.16 0.20
2 740 3.38 1.73 0.22
3 923 4.36 2.33 0.24
5 1279 6.41 3.60 0.28
10 2174 12.13 7.25 0.40
54 10056 98.61 68.52 1.88

CollectCom transaction costs

Parties UTxO (bytes) Tx size % max Mem % max CPU Min fee ₳
1 57 525 24.43 7.12 0.42
2 114 636 32.20 9.36 0.51
3 171 747 41.40 11.95 0.60
4 226 858 48.20 13.99 0.68
5 283 969 55.92 16.29 0.76
6 341 1081 66.15 19.14 0.87
7 393 1192 72.52 21.03 0.94
8 450 1303 90.33 25.80 1.12
9 505 1414 99.42 28.45 1.22

Cost of Increment Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 1811 23.87 7.97 0.48
2 1981 26.43 9.50 0.52
3 2072 26.77 10.22 0.53
5 2479 33.24 13.75 0.63
10 3217 42.19 20.08 0.78
43 7914 99.94 60.61 1.74

Cost of Decrement Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 602 22.73 7.33 0.41
2 749 24.28 8.43 0.44
3 866 24.98 9.27 0.46
5 1227 31.22 12.35 0.55
10 1954 38.50 17.72 0.68
42 6572 96.35 55.14 1.62

Close transaction costs

Parties Tx size % max Mem % max CPU Min fee ₳
1 598 28.33 8.94 0.47
2 841 29.11 10.02 0.49
3 914 32.54 11.67 0.54
5 1231 36.69 14.39 0.60
10 1985 47.19 21.15 0.77
36 6141 98.88 55.90 1.62

Contest transaction costs

Parties Tx size % max Mem % max CPU Min fee ₳
1 702 33.70 10.51 0.53
2 864 36.36 12.08 0.57
3 900 37.10 12.76 0.58
5 1314 42.87 16.17 0.67
10 2004 52.95 22.58 0.83
29 5045 99.37 50.40 1.54

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 5754 26.89 9.01 0.69
2 5912 36.78 12.35 0.80
3 6021 45.37 15.24 0.89
4 5985 49.16 16.36 0.93
5 6323 65.95 22.21 1.12
6 6415 71.41 24.01 1.18
7 6475 79.85 26.70 1.27
8 6630 89.52 30.09 1.38
9 6769 99.48 33.41 1.49

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 ₳
10 1 57 5869 21.67 7.37 0.64
10 20 1136 6510 60.46 22.69 1.09
10 30 1707 6853 79.20 30.17 1.31
10 40 2278 7194 99.72 38.25 1.55

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 2025-05-15 06:39:36.830006724 UTC

Baseline Scenario

Number of nodes 1
Number of txs 300
Avg. Confirmation Time (ms) 4.390839993
P99 5.824176459999995ms
P95 5.142898000000001ms
P50 4.2591245ms
Number of Invalid txs 0

Memory data

Time Used Free
2025-05-15 06:38:14.901501146 UTC 770M 6927M
2025-05-15 06:38:19.901483168 UTC 865M 6789M
2025-05-15 06:38:24.901369414 UTC 871M 6783M
2025-05-15 06:38:29.9014035 UTC 873M 6780M
2025-05-15 06:38:34.901397707 UTC 875M 6779M
2025-05-15 06:38:39.901385394 UTC 875M 6778M

Three local nodes

Number of nodes 3
Number of txs 900
Avg. Confirmation Time (ms) 34.376261433
P99 59.162663559999984ms
P95 51.68715844999999ms
P50 32.211393ms
Number of Invalid txs 0

Memory data

Time Used Free
2025-05-15 06:38:53.717296684 UTC 791M 6897M
2025-05-15 06:38:58.717368974 UTC 1009M 6600M
2025-05-15 06:39:03.717932036 UTC 1069M 6490M
2025-05-15 06:39:08.717958744 UTC 1091M 6413M
2025-05-15 06:39:13.717338732 UTC 1090M 6405M
2025-05-15 06:39:18.717291264 UTC 1091M 6404M
2025-05-15 06:39:23.717349791 UTC 1094M 6400M
2025-05-15 06:39:28.717336811 UTC 1096M 6398M
2025-05-15 06:39:33.717271229 UTC 1096M 6397M

@ch1bo ch1bo force-pushed the deposit-value-fix branch from 1719628 to 14f66be Compare April 26, 2025 22:36
@ch1bo ch1bo force-pushed the deposit-value-fix branch from 14f66be to debbf91 Compare April 27, 2025 18:05
@ch1bo ch1bo moved this from Triage 🏥 to In review 👀 in ☕ Hydra Team Work Apr 27, 2025
@ch1bo ch1bo force-pushed the deposit-value-fix branch from debbf91 to 538d3d1 Compare April 28, 2025 19:07
@ch1bo
Copy link
Member Author

ch1bo commented Apr 29, 2025

Tested and fixed generators now to get this passing consistently. Probably worth a re-review

@ch1bo ch1bo requested review from v0d1ch and a team April 29, 2025 07:21
@ch1bo ch1bo force-pushed the deposit-value-fix branch from 434db29 to b5e188b Compare April 29, 2025 08:57
@ch1bo ch1bo enabled auto-merge April 29, 2025 10:02
@ch1bo ch1bo disabled auto-merge April 29, 2025 10:02
@ch1bo ch1bo added the stacked Stacked diff. Review it, but leave merging to the author. label Apr 29, 2025
@@ -14,6 +14,13 @@ import PlutusLedgerApi.V3 qualified as Plutus

-- * Extras

-- | Check whether left contains right.
containsValue :: Value -> Value -> Bool
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is probably worth a property test or so. At an immediate glance I can't tell if it's correct; so would be nice to check :)

Copy link
Member Author

@ch1bo ch1bo Apr 30, 2025

Choose a reason for hiding this comment

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

Can you state me a property you would like to see here; in english is fine? (I struggled without repeating the implementation)

Copy link
Contributor

Choose a reason for hiding this comment

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

a < b => containsValue b a && not containsValue b a ?

or maybe

one where each individual value is smaller than the requested, but the sum is greater? (does Value work that way?)

basically:

  • what if you forgot to call negateValue ?
  • what if you forgot to write positive ?
  • what if instead of all you wrote any ?

@ch1bo ch1bo linked an issue Apr 30, 2025 that may be closed by this pull request
testNetworkId
(mkHeadId testPolicyId)
CommitBlueprintTx{blueprintTx = txSpendingUTxO healthyDepositUTxO, lookupUTxO = healthyDepositUTxO}
depositDeadline -- TODO: should generate
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to not resolve this todo now? Seems like it should be easy now it's the Num?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not a Num in this PR, but can resolve this, yes.

Copy link
Contributor

@noonio noonio left a comment

Choose a reason for hiding this comment

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

Lovely.

I wonder if it's worth being a bit more explicit with the tests that we write to capture potential attacks; just so we know to be extra extra mindful of those if they fail? I.e. group or tag them in some way?

@ch1bo
Copy link
Member Author

ch1bo commented May 1, 2025

Lovely.

I wonder if it's worth being a bit more explicit with the tests that we write to capture potential attacks; just so we know to be extra extra mindful of those if they fail? I.e. group or tag them in some way?

Aren't you mindful on each failing test? :)

@noonio
Copy link
Contributor

noonio commented May 1, 2025

Aren't you mindful on each failing test? :)

😂

@ch1bo ch1bo force-pushed the deposit-value-fix branch 10 times, most recently from eebbf1a to 63029ec Compare May 14, 2025 07:23
ch1bo added 9 commits May 15, 2025 08:08
Any change to the deposit output value without also updating the
recorded commit in the datum should invalidate the deposit observation.
This makes sure that the deposited value as claimed in the datum is
actually available at the deposit script output to be incremented.
This makes sure we test a wider range of transactions and we can easily
switch to this approach for deposits as the mutations are fully defined
on the mutated tx/utxo.
The ada in values generated by cardano-ledger is often using maxBound of
Word64 and this goes quickly out of bounds when summed up. This changes
genTxOut to only produce realistic ada values (<< max supply).
generateWith uses size 30 and this is too big of utxo to deposit.
Also ensures the minimum lovelace in an output using default pparams.
@ch1bo ch1bo force-pushed the deposit-value-fix branch from 63029ec to 650fd4b Compare May 15, 2025 06:29
@ch1bo ch1bo merged commit 650fd4b into master May 15, 2025
20 checks passed
@ch1bo ch1bo deleted the deposit-value-fix branch May 15, 2025 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stacked Stacked diff. Review it, but leave merging to the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check deposit deadline before incrementing
4 participants