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

Allow clients configure API features #794

Merged
merged 9 commits into from Apr 1, 2023

Conversation

v0d1ch
Copy link
Contributor

@v0d1ch v0d1ch commented Mar 27, 2023

fix #380 #379 #371

Why

🌴 Some clients don't want to observe a historical server outputs.
🌴 Some want to have the transactions in the output displayed as CBOR.

What

Using the query parameters decide on the api server level what kind of output to serve.


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

@v0d1ch v0d1ch added this to the 0.10.0 milestone Mar 27, 2023
@v0d1ch v0d1ch self-assigned this Mar 27, 2023
@v0d1ch v0d1ch force-pushed the allow-api-client-and-server-to-negotiate-features branch 4 times, most recently from cfaa843 to 02371fb Compare March 27, 2023 15:58
@v0d1ch v0d1ch requested review from ch1bo, abailly-iohk and ffakenz and removed request for ffakenz March 27, 2023 16:00
@github-actions
Copy link

github-actions bot commented Mar 27, 2023

Test Results

299 tests  +2   293 ✔️ +2   21m 8s ⏱️ -25s
100 suites ±0       6 💤 ±0 
    5 files   ±0       0 ±0 

Results for commit 4abaead. ± Comparison against base commit e0e96b5.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Mar 27, 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-04-01 07:29:40.546839138 UTC
Max. memory units 14000000
Max. CPU units 10000000000
Max. tx size (kB) 16384

Script summary

Name Hash Size (Bytes)
νInitial 9492414f8f96e8483a0b8ee268fc06a954641cb2cbaa9a8b093c2c9b 4621
νCommit 5d3f107aaa56d06188cf231941cf8163e777236a9cfdc48fd4bbfa23 2422
νHead 82f16b51e2d81c6f4d42dd7398b4713a445464902f63dfd86ffe754e 8954
μHead 4083fa7081a0f4b4092fb02867c9ac594bb0e8bab8110ab242ba5a72* 4458
  • 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 5058 12.82 5.04 0.51
2 5264 18.66 7.37 0.59
3 5468 19.59 7.67 0.60
5 5875 23.03 8.92 0.66
10 6900 37.51 14.48 0.86
37 12437 99.65 37.87 1.78

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 603 15.83 6.22 0.35

Cost of CollectCom Transaction

Parties UTxO (bytes) Tx size % max Mem % max CPU Min fee ₳
1 57 817 24.72 10.03 0.46
2 113 1138 40.75 16.66 0.65
3 168 1459 59.31 24.40 0.87
4 225 1782 80.67 33.34 1.12

Cost of Close Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 640 16.99 7.31 0.37
2 667 16.25 6.95 0.36
3 970 19.91 8.90 0.42
5 1300 22.95 10.54 0.47
10 2124 29.96 14.40 0.59
50 8726 86.05 45.32 1.56

Cost of Contest Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 676 21.73 9.11 0.42
2 841 23.49 10.01 0.45
3 1027 25.61 11.22 0.48
5 1336 29.19 12.88 0.54
10 2169 37.29 17.11 0.67
46 8103 99.32 49.03 1.66

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 5249 30.33 13.03 0.72
2 5494 48.08 20.77 0.93
3 5674 65.12 28.11 1.13
4 6140 95.55 41.69 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 5078 10.59 4.43 0.49
5 1 57 5115 12.12 5.31 0.51
5 5 284 5257 18.22 8.83 0.59
5 10 570 5437 25.85 13.24 0.69
5 20 1140 5794 41.11 22.05 0.90
5 30 1708 6157 56.37 30.86 1.10
5 40 2276 6511 71.64 39.68 1.30
5 50 2844 6874 86.92 48.50 1.50
5 58 3296 7161 99.14 55.56 1.67

hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
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.

This review tries to apply the perfection game protocol

I will describe what I like about this P.R. and then what improvement I can think of to make it perfect.
Ask for clarifications if my comments are not clear.

If you disagree with my comments, just ignore them and don’t answer or explain.
Hence I, in advance, approve the P.R.

What I like about this P.R.:

For me to find it perfect you would have to:

  • I would make two distinct pull requests to separate the history topic from the formatting topic
  • I would polish the git history to make the review easier to make. For instance, I would remove the commit Finish with the implementation and test and integrates the changes their to the appropriate commits
  • I would remove commit Introduce a test that should prevent history display to avoid having tests that don't pass in the git history and commit both this test and the production code in the same commit
  • see other improvement proposals inline

hydra-node/src/Hydra/API/Server.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
hydra-node/src/Hydra/API/ServerOutput.hs Outdated Show resolved Hide resolved
docs/core-concepts/behavior.md Outdated Show resolved Hide resolved
@v0d1ch v0d1ch force-pushed the allow-api-client-and-server-to-negotiate-features branch from ec63cfc to 90da78b Compare March 30, 2023 08:44
@ch1bo ch1bo changed the title Allow api client and server to negotiate features Allow clients configure API features Mar 30, 2023
@ch1bo ch1bo linked an issue Mar 30, 2023 that may be closed by this pull request
@ch1bo ch1bo removed a link to an issue Mar 30, 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.

Have not looked at the tests yet, but the api.yaml must document the query parameters!

docs/core-concepts/behavior.md Show resolved Hide resolved
docs/core-concepts/behavior.md Outdated Show resolved Hide resolved
hydra-node/json-schemas/api.yaml 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/API/ServerOutput.hs Outdated Show resolved Hide resolved
hydra-node/src/Hydra/API/ServerOutput.hs Outdated Show resolved Hide resolved
hydra-node/src/Hydra/API/ServerOutput.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
@v0d1ch v0d1ch force-pushed the allow-api-client-and-server-to-negotiate-features branch from f048716 to 88d8363 Compare March 30, 2023 15:07
@ch1bo ch1bo removed this from the 0.10.0 milestone Mar 31, 2023
@v0d1ch v0d1ch force-pushed the allow-api-client-and-server-to-negotiate-features branch 3 times, most recently from 8d7a6ce to 7a268ac Compare March 31, 2023 12:19
@v0d1ch v0d1ch requested review from ch1bo and ffakenz March 31, 2023 12:21
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.

Looks good, only minor improvements left (feel free to merge also without them).

hydra-node/src/Hydra/API/Server.hs Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
hydra-node/test/Hydra/API/ServerSpec.hs Outdated Show resolved Hide resolved
@v0d1ch v0d1ch force-pushed the allow-api-client-and-server-to-negotiate-features branch from 7a268ac to 384dd7f Compare April 1, 2023 07:10
@v0d1ch v0d1ch force-pushed the allow-api-client-and-server-to-negotiate-features branch from 384dd7f to 4abaead Compare April 1, 2023 07:16
@v0d1ch v0d1ch merged commit df3f848 into master Apr 1, 2023
19 checks passed
@v0d1ch v0d1ch deleted the allow-api-client-and-server-to-negotiate-features branch April 1, 2023 08:24
@pgrange pgrange added this to the 0.10.0 milestone May 11, 2023
@ch1bo ch1bo removed this from the 0.10.0 milestone May 11, 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.

Allow API client to control the server output
4 participants