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 HeadLogic: Not validate tx on NewTx #745

Merged
merged 8 commits into from
Feb 27, 2023
Merged

Conversation

ch1bo
Copy link
Member

@ch1bo ch1bo commented Feb 23, 2023

When updating the spec we realized we do not need an applicability to the confirmed (last snapshot) UTxO set on NewTx. This is reason enough to re-think whether this makes even sense.

  • This PR is now dropping TxSeen and TxExpired in favor of TxValid and TxInvalid. See commit description for rationale.

  • Although it fixes a bug int he ttl handling, this PR does not change the behavior. Notably, the ttl == 0 cases for ReqSn and AckSn are not defined. They will just fall out of the queue. We might want to emit a SnapshotExpired or HeadLikelyStuck message.


  • CHANGELOG updated
  • Documentation updated
  • Added and/or updated haddocks
  • No new TODOs introduced or explained herafter

@ch1bo ch1bo changed the title Not validate tx on NewTx Align HeadLogic: Not validate tx on NewTx Feb 23, 2023
@ch1bo ch1bo force-pushed the ch1bo/not-validate-on-newtx branch from 2630eea to 93cb1af Compare February 23, 2023 14:03
@ch1bo ch1bo self-assigned this Feb 23, 2023
@ch1bo ch1bo force-pushed the ch1bo/not-validate-on-newtx branch from 93cb1af to 681179a Compare February 24, 2023 12:26
@ch1bo ch1bo force-pushed the ch1bo/not-validate-on-newtx branch from 681179a to 08dc5f6 Compare February 24, 2023 19:41
@ch1bo ch1bo marked this pull request as ready for review February 24, 2023 19:41
@ch1bo ch1bo requested review from ffakenz, abailly-iohk, pgrange and v0d1ch and removed request for ffakenz February 24, 2023 19:41
@ch1bo ch1bo force-pushed the ch1bo/not-validate-on-newtx branch from 08dc5f6 to 1e3e9e3 Compare February 24, 2023 19:43
@ch1bo ch1bo removed their assignment Feb 24, 2023
@ch1bo ch1bo force-pushed the ch1bo/not-validate-on-newtx branch from 1e3e9e3 to 62f4885 Compare February 24, 2023 20:28
@ch1bo ch1bo linked an issue Feb 24, 2023 that may be closed by this pull request
20 tasks
@ch1bo ch1bo force-pushed the ch1bo/not-validate-on-newtx branch from 62f4885 to 39a9858 Compare February 24, 2023 20:41
Base automatically changed from ch1bo/verify-multisig to master February 24, 2023 20:46
This is a somewhat reasonable timeout (12 seconds on my machine) and the
test is not appearing to hang forever.

It's not perfect still, as we are presented with a slew of Tick-related
via the traceInIOSim and shouldRunInSim.
This should capture the behavior as I read it from the spec: Somewhat
robust behavior against out-of-order requesting of NewTx (up to some delay).
This is now more important as we only keep transactions pending for
waitDealy * defaultTTL. That is, currently 0.5 seconds.
This means all transactions result in a ReqTx and allows out-of-order
ingestion of transactions into the Hydra Head.

This also adds TODOs/FIXMEs for now redundant TxInvalid et al
The server outputs TxValid and TxInvalid shall be used instead.

Rationale:

Before we were checking against the confirmed ledger on NewTx and yield
a TxValid if ok, but that is a useless check as it needs to be
applicable to the seen ledger to continue in the protocol. Instead we
now only yield TxValid if it was applicable against the seen ledger.

TxExpired was not giving any error information why it was expired.
Similarly, TxInvalid had no real value as it was in response of checking
against the confirmed ledger, while the seen ledger might already have
advanced and the transaction is actually not applicable. Instead we now
return a TxInvalid in this case with the last validationError that we
saw.
@ch1bo ch1bo force-pushed the ch1bo/not-validate-on-newtx branch from 39a9858 to cf45fb7 Compare February 24, 2023 20:47
@github-actions
Copy link

github-actions bot commented Feb 24, 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-02-24 20:55:06.652088709 UTC
Max. memory units 14000000
Max. CPU units 10000000000
Max. tx size (kB) 16384

Script summary

Name Hash Size (Bytes)
μHead N/A 4600
νInitial 7bb44e248f92ae5d2c4c744244b469d59a5bfa8a5d8ce9b6b27e5750 5530
νCommit 70c545fc894ada81ad46715bac1a11fa755b3e3a1d94f03254a4d397 2473
νHead 58cef84ce7e80b1b3089a10d825da2f41ec1cb5ad923d9f39d842f3c 9998

Cost of Init Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 5198 15.32 6.07 0.55
2 5399 16.74 6.58 0.57
3 5611 20.38 7.99 0.62
5 6017 25.18 9.80 0.69
10 7046 37.41 14.44 0.87
36 12374 99.20 37.77 1.77

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.95 8.45 0.41

Cost of CollectCom Transaction

Parties UTxO (bytes) Tx size % max Mem % max CPU Min fee ₳
1 57 13260 21.89 8.77 0.97
2 114 13604 37.14 15.01 1.16
3 170 13957 54.10 22.04 1.36
4 227 14309 74.92 30.67 1.61
5 284 14660 97.76 40.22 1.88

Cost of Close Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 10634 16.83 7.37 0.81
2 10777 17.73 7.78 0.82
3 10942 19.21 8.58 0.85
5 11300 22.59 10.60 0.91
10 12165 29.95 14.69 1.03
43 14801 58.67 27.18 1.47

Cost of Contest Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 10641 21.47 8.96 0.86
2 10841 23.33 10.08 0.89
3 10978 24.47 10.56 0.91
5 11301 28.02 12.38 0.96
10 12152 37.56 17.34 1.11
35 16321 80.90 39.86 1.81

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 15306 27.81 11.83 1.13
2 15203 35.93 15.02 1.22
3 16085 68.59 29.80 1.63
4 15667 73.13 31.22 1.66

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 15182 9.70 4.01 0.93
5 1 57 15222 10.97 4.79 0.94
5 5 285 15358 16.83 8.21 1.02
5 10 570 15542 24.95 12.82 1.13
5 20 1139 15903 39.71 21.42 1.33
5 30 1707 16262 54.98 30.24 1.53
5 33 1879 16370 59.81 32.98 1.59

@github-actions
Copy link

Test Results

292 tests  +12   286 ✔️ +12   19m 36s ⏱️ + 3m 49s
  98 suites +  4       6 💤 ±  0 
    5 files   +  1       0 ±  0 

Results for commit cf45fb7. ± Comparison against base commit 427c97c.

This pull request removes 10 and adds 22 tests. Note that renamed tests count towards both.
Hydra.Behavior/in an open head ‑ can be finalized by all parties after contestation period
Hydra.Behavior/in an open head ‑ contest automatically when detecting closing with old snapshot
Hydra.Behavior/in an open head ‑ multiple transactions get snapshotted
Hydra.Behavior/in an open head ‑ outputs utxo from confirmed snapshot when client requests it
Hydra.Behavior/in an open head ‑ reports transactions as seen only when they validate (against the confirmed ledger)
Hydra.Behavior/in an open head ‑ sees the head closed by other nodes
Hydra.Behavior/in an open head ‑ sending two conflicting transactions should lead one being confirmed and one expired
Hydra.Behavior/in an open head ‑ valid new transactions are seen by all parties
Hydra.Behavior/in an open head ‑ valid new transactions get snapshotted
Hydra.HeadLogic/Coordinated Head Protocol ‑ rejects if a requested tx is expired
Hydra.Behavior/Two participant Head ‑ can be finalized by all parties after contestation period
Hydra.Behavior/Two participant Head ‑ contest automatically when detecting closing with old snapshot
Hydra.Behavior/Two participant Head/in an open head ‑ depending transactions expire if not applicable in time
Hydra.Behavior/Two participant Head/in an open head ‑ depending transactions stay pending and are confirmed in order
Hydra.Behavior/Two participant Head/in an open head ‑ multiple transactions get snapshotted
Hydra.Behavior/Two participant Head/in an open head ‑ outputs utxo from confirmed snapshot when client requests it
Hydra.Behavior/Two participant Head/in an open head ‑ sees the head closed by other nodes
Hydra.Behavior/Two participant Head/in an open head ‑ sending two conflicting transactions should lead one being confirmed and one expired
Hydra.Behavior/Two participant Head/in an open head ‑ valid new transactions are seen by all parties
Hydra.Behavior/Two participant Head/in an open head ‑ valid new transactions get snapshotted
…
This pull request removes 1 skipped test and adds 1 skipped test. Note that renamed tests count towards both.
Hydra.Behavior/in an open head ‑ multiple transactions get snapshotted
Hydra.Behavior/Two participant Head/in an open head ‑ multiple transactions get snapshotted

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.

LGTM!

@ch1bo ch1bo merged commit 54d6e45 into master Feb 27, 2023
@ch1bo ch1bo deleted the ch1bo/not-validate-on-newtx branch February 27, 2023 17:09
@pgrange pgrange mentioned this pull request Mar 1, 2023
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.

Align implementation with HeadV1 protocol specification
2 participants