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

Externally commit regular utxo #887

Merged
merged 52 commits into from
Jun 7, 2023
Merged

Conversation

v0d1ch
Copy link
Contributor

@v0d1ch v0d1ch commented May 22, 2023

This PR adds the option for our users to commit a regular utxo to the Head externally. Note that this is the initial chunk of work and we are still working on enabling script utxos to be spent in the Head too.

Why

We want to allow our users to commit externally.

What

Create a HTTP endpoint that can accept utxo user wants to commit to a head. Respond with a draft commit transaction that needs to be signed and submitted to the network by the user.

🌴 Added DraftCommitTxRequest to obtain client utxo in our server

🌴 Added DraftCommitTxResponse to return draft tx to the user

🌴 Added APIRestInputReceived to the logs so we can observe client contents sent

🌴 Added new PostTxError called FailedToDraftTxNotInitializing in case we experience a failure while drafting a tx


  • 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 review from ch1bo, pgrange and a user May 22, 2023 15:07
@github-actions
Copy link

github-actions bot commented May 22, 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-06-07 10:05:58.390426852 UTC
Max. memory units 14000000
Max. CPU units 10000000000
Max. tx size (kB) 16384

Script summary

Name Hash Size (Bytes)
νInitial 2212a4ee618434b9b2f366d7c330dbdfb5c7072e793a850fd0de6ddd 4294
νCommit 69e1ccf9ad73dc6d37a5bc8de5aec86f3c4c1710fe5fd334e0e16b18 2124
νHead 8ae095dca4d14a1b8edffb37faa6c84ec60340fbf389a62f027e0b76 9355
μHead 33642a45c7fbb955ce1704ef09229bb211bf9af9980530db28c6aafe* 4148
  • 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 4746 13.21 5.18 0.50
2 4951 14.44 5.60 0.53
3 5152 18.37 7.14 0.58
5 5562 23.29 9.00 0.65
10 6588 33.57 12.82 0.80
37 12124 98.12 37.19 1.74

Cost of Commit Transaction

This is using ada-only outputs for better comparability.

UTxO Tx size % max Mem % max CPU Min fee ₳
1 599 14.98 5.74 0.34
2 782 19.66 7.73 0.40
3 974 24.75 9.88 0.46
5 1345 36.07 14.56 0.61
10 2284 71.73 28.85 1.04
13 2843 98.11 39.18 1.35

Cost of CollectCom Transaction

Parties UTxO (bytes) Tx size % max Mem % max CPU Min fee ₳
1 57 814 27.89 10.83 0.49
2 113 1135 43.76 17.13 0.68
3 171 1455 61.60 24.27 0.89
4 224 1772 82.57 32.67 1.13

Cost of Close Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 639 18.58 8.31 0.39
2 804 20.27 9.68 0.42
3 977 21.67 10.94 0.45
5 1300 24.46 13.45 0.50
10 2124 31.12 19.62 0.64
50 8725 87.17 69.99 1.74

Cost of Contest Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 709 25.27 11.00 0.46
2 848 26.49 12.01 0.49
3 1005 27.77 13.21 0.51
5 1335 31.63 16.12 0.58
10 2153 40.20 22.97 0.74
45 7945 99.74 70.74 1.82

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 4854 22.40 9.38 0.61
2 5176 36.67 15.56 0.79
3 5354 49.09 20.77 0.93
4 5817 73.45 31.60 1.23
5 6138 96.03 41.46 1.49

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 4764 8.66 3.57 0.46
5 1 57 4801 10.06 4.39 0.47
5 5 285 4940 15.64 7.69 0.55
5 10 568 5122 22.61 11.82 0.64
5 20 1140 5485 36.56 20.07 0.83
5 30 1709 5842 50.52 28.33 1.02
5 40 2278 6205 64.49 36.60 1.21
5 50 2843 6559 78.45 44.87 1.40
5 65 3701 7102 99.42 57.28 1.68

@v0d1ch v0d1ch force-pushed the external-commits-regular-utxo branch from b6d16eb to 8dc60f2 Compare May 23, 2023 06:58
@github-actions
Copy link

github-actions bot commented May 23, 2023

Test Results

324 tests  +16   318 ✔️ +16   22m 55s ⏱️ + 1m 41s
110 suites +  6       6 💤 ±  0 
    6 files   +  1       0 ±  0 

Results for commit d8a1942. ± Comparison against base commit 38787da.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@pgrange pgrange left a comment

Choose a reason for hiding this comment

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

I see the following issues with this feature:

  1. the hydra-node is paying the fees for the client, is it ok in the long run?
  2. the hydra-node could spend the same UTxO twice for paying fees if it receives two draftTx in a row. Is it ok ?
  3. the client could ask the hydra-node to spend a UTxO that is owned by the node and not the client. Is it ok in the long run?

I don't think those 3 points are ok. I would need to ensure that we keep track of them for future evolutions.

Copy link
Member

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

Comments from our discussion today.

hydra-cluster/config/credentials/external.sk Outdated Show resolved Hide resolved
hydra-cluster/src/Hydra/Cluster/Scenarios.hs Outdated Show resolved Hide resolved
hydra-cluster/src/Hydra/Cluster/Scenarios.hs Show resolved Hide resolved
hydra-cluster/src/Hydra/Cluster/Scenarios.hs Show resolved Hide resolved
hydra-cluster/hydra-cluster.cabal Outdated Show resolved Hide resolved
hydra-node/src/Hydra/API/Server.hs Outdated Show resolved Hide resolved
hydra-node/src/Hydra/API/Server.hs Outdated Show resolved Hide resolved
hydra-node/src/Hydra/Chain.hs Show resolved Hide resolved
hydra-node/src/Hydra/Chain/Direct/Handlers.hs Outdated Show resolved Hide resolved
hydra-node/src/Hydra/Ledger/Cardano.hs Outdated Show resolved Hide resolved
@v0d1ch v0d1ch force-pushed the external-commits-regular-utxo branch 4 times, most recently from 86fce02 to d05ce7d Compare May 23, 2023 15:33
@v0d1ch v0d1ch requested a review from ch1bo May 23, 2023 15:34
@v0d1ch
Copy link
Contributor Author

v0d1ch commented May 23, 2023

@pgrange I think your comments were addressed in the grooming session.

@v0d1ch v0d1ch requested a review from pgrange May 23, 2023 15:34
@ffakenz ffakenz force-pushed the external-commits-regular-utxo branch 4 times, most recently from 29cad13 to c828a59 Compare June 1, 2023 08:06
@ffakenz ffakenz force-pushed the external-commits-regular-utxo branch from c828a59 to 602d762 Compare June 5, 2023 09:14
hydra-node/json-schemas/api.yaml Outdated Show resolved Hide resolved
hydra-node/json-schemas/api.yaml Outdated Show resolved Hide resolved
hydra-node/json-schemas/api.yaml Outdated Show resolved Hide resolved
hydra-node/json-schemas/api.yaml Outdated Show resolved Hide resolved
hydra-node/src/Hydra/API/ClientInput.hs Outdated Show resolved Hide resolved
hydra-node/src/Hydra/Chain/Direct/Handlers.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/RestServerSpec.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/BehaviorSpec.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/LoggingSpec.hs Outdated Show resolved Hide resolved
@v0d1ch v0d1ch force-pushed the external-commits-regular-utxo branch 5 times, most recently from 5a204be to 408bbb4 Compare June 6, 2023 07:50
hydra-node/json-schemas/api.yaml Outdated Show resolved Hide resolved
hydra-node/json-schemas/api.yaml Outdated Show resolved Hide resolved
summary: Hydra node drafted commit transaction responses.
operationId: draftCommitTxResponse
message:
$ref: "#/components/messages/DraftCommitTxResponse"
Copy link
Member

Choose a reason for hiding this comment

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

Should: Document the possible error responses with status codes.

Could: inline response and error schemas as they likely are not going to be re-used

v0d1ch and others added 22 commits June 7, 2023 09:44
- Remove not needed pragma
- Use consumeRequestBodyStrict
- Rename router to chain
- Decide on what errors we should throw in a Chain handle
- Improve haddocks
- update api version to 0.11.0
- update  api description and operation ids
- upgrade  npm package to fix yarn run v1.22.19
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. cmd
- add http bindings to api /commit channel
- add tag to response to make yarn run v1.22.19
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. happy, but it is still broken for request
The query field represents a Schema object containing the definitions for each query parameter.
We are not considering any query params, but just have a request body.
The response does use a query parameters either, but only a response body.
@v0d1ch v0d1ch force-pushed the external-commits-regular-utxo branch from 9a691df to f37c49a Compare June 7, 2023 07:47
@ch1bo ch1bo unassigned v0d1ch and ffakenz Jun 7, 2023
Copy link
Member

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

Good enough to continue with the other work streams

@v0d1ch v0d1ch force-pushed the external-commits-regular-utxo branch from 0490cd3 to d8a1942 Compare June 7, 2023 09:56
@v0d1ch v0d1ch merged commit 6645da2 into master Jun 7, 2023
@v0d1ch v0d1ch deleted the external-commits-regular-utxo branch June 7, 2023 10:19
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.

4 participants