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

const_sv2: remove unused dep (secp256k1) #1237

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

Georges760
Copy link

const_sv2 does depend on secp256k1 anymore

@Georges760 Georges760 marked this pull request as ready for review October 26, 2024 06:58
Copy link

codecov bot commented Oct 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 19.36%. Comparing base (b8e7715) to head (0743f82).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1237   +/-   ##
=======================================
  Coverage   19.36%   19.36%           
=======================================
  Files         164      164           
  Lines       10811    10811           
=======================================
  Hits         2094     2094           
  Misses       8717     8717           
Flag Coverage Δ
binary_codec_sv2-coverage 0.00% <ø> (ø)
binary_serde_sv2-coverage 3.65% <ø> (ø)
binary_sv2-coverage 5.46% <ø> (ø)
bip32_derivation-coverage 0.00% <ø> (ø)
buffer_sv2-coverage 25.02% <ø> (ø)
codec_sv2-coverage 0.01% <ø> (ø)
common_messages_sv2-coverage 0.13% <ø> (ø)
const_sv2-coverage 0.00% <ø> (ø)
error_handling-coverage 0.00% <ø> (ø)
framing_sv2-coverage 0.29% <ø> (ø)
jd_client-coverage 0.00% <ø> (ø)
jd_server-coverage 8.13% <ø> (ø)
job_declaration_sv2-coverage 0.00% <ø> (ø)
key-utils-coverage 2.39% <ø> (ø)
mining-coverage 2.49% <ø> (-0.02%) ⬇️
mining_device-coverage 0.00% <ø> (ø)
mining_proxy_sv2-coverage 0.70% <ø> (ø)
noise_sv2-coverage 4.35% <ø> (ø)
pool_sv2-coverage 1.38% <ø> (ø)
protocols 24.72% <ø> (ø)
roles 6.63% <ø> (ø)
roles_logic_sv2-coverage 8.06% <ø> (ø)
sv1-mining-device-coverage 0.00% <ø> (ø)
sv2_ffi-coverage 0.00% <ø> (ø)
template_distribution_sv2-coverage 0.00% <ø> (ø)
translator_sv2-coverage 9.61% <ø> (ø)
utils 25.13% <ø> (ø)
v1-coverage 2.47% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

🐰 Bencher Report

Branch1237/merge
Testbedsv1

⚠️ WARNING: The following Measures do not have a Threshold. Without a Threshold, no Alerts will ever be generated!

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results
BenchmarkEstimated Cyclesestimated cyclesInstructionsinstructionsL1 AccessesaccessesL2 AccessesaccessesRAM Accessesaccesses
get_authorize📈 view plot
⚠️ NO THRESHOLD
8,316.00📈 view plot
⚠️ NO THRESHOLD
3,663.00📈 view plot
⚠️ NO THRESHOLD
5,111.00📈 view plot
⚠️ NO THRESHOLD
11.00📈 view plot
⚠️ NO THRESHOLD
90.00
get_submit📈 view plot
⚠️ NO THRESHOLD
95,164.00📈 view plot
⚠️ NO THRESHOLD
59,263.00📈 view plot
⚠️ NO THRESHOLD
85,084.00📈 view plot
⚠️ NO THRESHOLD
42.00📈 view plot
⚠️ NO THRESHOLD
282.00
get_subscribe📈 view plot
⚠️ NO THRESHOLD
7,879.00📈 view plot
⚠️ NO THRESHOLD
2,758.00📈 view plot
⚠️ NO THRESHOLD
3,829.00📈 view plot
⚠️ NO THRESHOLD
19.00📈 view plot
⚠️ NO THRESHOLD
113.00
serialize_authorize📈 view plot
⚠️ NO THRESHOLD
12,071.00📈 view plot
⚠️ NO THRESHOLD
5,240.00📈 view plot
⚠️ NO THRESHOLD
7,281.00📈 view plot
⚠️ NO THRESHOLD
13.00📈 view plot
⚠️ NO THRESHOLD
135.00
serialize_deserialize_authorize📈 view plot
⚠️ NO THRESHOLD
24,512.00📈 view plot
⚠️ NO THRESHOLD
9,786.00📈 view plot
⚠️ NO THRESHOLD
13,787.00📈 view plot
⚠️ NO THRESHOLD
38.00📈 view plot
⚠️ NO THRESHOLD
301.00
serialize_deserialize_handle_authorize📈 view plot
⚠️ NO THRESHOLD
30,323.00📈 view plot
⚠️ NO THRESHOLD
11,989.00📈 view plot
⚠️ NO THRESHOLD
16,943.00📈 view plot
⚠️ NO THRESHOLD
65.00📈 view plot
⚠️ NO THRESHOLD
373.00
serialize_deserialize_handle_submit📈 view plot
⚠️ NO THRESHOLD
126,180.00📈 view plot
⚠️ NO THRESHOLD
73,117.00📈 view plot
⚠️ NO THRESHOLD
104,770.00📈 view plot
⚠️ NO THRESHOLD
110.00📈 view plot
⚠️ NO THRESHOLD
596.00
serialize_deserialize_handle_subscribe📈 view plot
⚠️ NO THRESHOLD
27,871.00📈 view plot
⚠️ NO THRESHOLD
9,577.00📈 view plot
⚠️ NO THRESHOLD
13,516.00📈 view plot
⚠️ NO THRESHOLD
71.00📈 view plot
⚠️ NO THRESHOLD
400.00
serialize_deserialize_submit📈 view plot
⚠️ NO THRESHOLD
114,789.00📈 view plot
⚠️ NO THRESHOLD
67,894.00📈 view plot
⚠️ NO THRESHOLD
97,379.00📈 view plot
⚠️ NO THRESHOLD
59.00📈 view plot
⚠️ NO THRESHOLD
489.00
serialize_deserialize_subscribe📈 view plot
⚠️ NO THRESHOLD
23,116.00📈 view plot
⚠️ NO THRESHOLD
8,129.00📈 view plot
⚠️ NO THRESHOLD
11,426.00📈 view plot
⚠️ NO THRESHOLD
42.00📈 view plot
⚠️ NO THRESHOLD
328.00
serialize_submit📈 view plot
⚠️ NO THRESHOLD
99,473.00📈 view plot
⚠️ NO THRESHOLD
61,325.00📈 view plot
⚠️ NO THRESHOLD
87,948.00📈 view plot
⚠️ NO THRESHOLD
44.00📈 view plot
⚠️ NO THRESHOLD
323.00
serialize_subscribe📈 view plot
⚠️ NO THRESHOLD
11,214.00📈 view plot
⚠️ NO THRESHOLD
4,111.00📈 view plot
⚠️ NO THRESHOLD
5,694.00📈 view plot
⚠️ NO THRESHOLD
19.00📈 view plot
⚠️ NO THRESHOLD
155.00
🐰 View full continuous benchmarking report in Bencher

Copy link
Contributor

🐰 Bencher Report

Branch1237/merge
Testbedsv1

⚠️ WARNING: The following Measure does not have a Threshold. Without a Threshold, no Alerts will ever be generated!

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
client-submit-serialize📈 view plot
⚠️ NO THRESHOLD
6,489.80
client-submit-serialize-deserialize📈 view plot
⚠️ NO THRESHOLD
7,567.00
client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle📈 view plot
⚠️ NO THRESHOLD
8,054.80
client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle📈 view plot
⚠️ NO THRESHOLD
862.68
client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize📈 view plot
⚠️ NO THRESHOLD
676.56
client-sv1-authorize-serialize/client-sv1-authorize-serialize📈 view plot
⚠️ NO THRESHOLD
252.41
client-sv1-get-authorize/client-sv1-get-authorize📈 view plot
⚠️ NO THRESHOLD
157.41
client-sv1-get-submit📈 view plot
⚠️ NO THRESHOLD
6,251.10
client-sv1-get-subscribe/client-sv1-get-subscribe📈 view plot
⚠️ NO THRESHOLD
277.21
client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle📈 view plot
⚠️ NO THRESHOLD
725.82
client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize📈 view plot
⚠️ NO THRESHOLD
590.51
client-sv1-subscribe-serialize/client-sv1-subscribe-serialize📈 view plot
⚠️ NO THRESHOLD
204.73
🐰 View full continuous benchmarking report in Bencher

Copy link
Contributor

🐰 Bencher Report

Branch1237/merge
Testbedsv2

⚠️ WARNING: The following Measures do not have a Threshold. Without a Threshold, no Alerts will ever be generated!

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results
BenchmarkEstimated Cyclesestimated cyclesInstructionsinstructionsL1 AccessesaccessesL2 AccessesaccessesRAM Accessesaccesses
client_sv2_handle_message_common📈 view plot
⚠️ NO THRESHOLD
2,095.00📈 view plot
⚠️ NO THRESHOLD
473.00📈 view plot
⚠️ NO THRESHOLD
735.00📈 view plot
⚠️ NO THRESHOLD
6.00📈 view plot
⚠️ NO THRESHOLD
38.00
client_sv2_handle_message_mining📈 view plot
⚠️ NO THRESHOLD
8,158.00📈 view plot
⚠️ NO THRESHOLD
2,137.00📈 view plot
⚠️ NO THRESHOLD
3,163.00📈 view plot
⚠️ NO THRESHOLD
33.00📈 view plot
⚠️ NO THRESHOLD
138.00
client_sv2_mining_message_submit_standard📈 view plot
⚠️ NO THRESHOLD
6,251.00📈 view plot
⚠️ NO THRESHOLD
1,750.00📈 view plot
⚠️ NO THRESHOLD
2,551.00📈 view plot
⚠️ NO THRESHOLD
19.00📈 view plot
⚠️ NO THRESHOLD
103.00
client_sv2_mining_message_submit_standard_serialize📈 view plot
⚠️ NO THRESHOLD
14,606.00📈 view plot
⚠️ NO THRESHOLD
4,694.00📈 view plot
⚠️ NO THRESHOLD
6,761.00📈 view plot
⚠️ NO THRESHOLD
43.00📈 view plot
⚠️ NO THRESHOLD
218.00
client_sv2_mining_message_submit_standard_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
27,438.00📈 view plot
⚠️ NO THRESHOLD
10,585.00📈 view plot
⚠️ NO THRESHOLD
15,403.00📈 view plot
⚠️ NO THRESHOLD
83.00📈 view plot
⚠️ NO THRESHOLD
332.00
client_sv2_open_channel📈 view plot
⚠️ NO THRESHOLD
4,375.00📈 view plot
⚠️ NO THRESHOLD
1,461.00📈 view plot
⚠️ NO THRESHOLD
2,160.00📈 view plot
⚠️ NO THRESHOLD
9.00📈 view plot
⚠️ NO THRESHOLD
62.00
client_sv2_open_channel_serialize📈 view plot
⚠️ NO THRESHOLD
13,974.00📈 view plot
⚠️ NO THRESHOLD
5,064.00📈 view plot
⚠️ NO THRESHOLD
7,329.00📈 view plot
⚠️ NO THRESHOLD
34.00📈 view plot
⚠️ NO THRESHOLD
185.00
client_sv2_open_channel_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
22,651.00📈 view plot
⚠️ NO THRESHOLD
8,027.00📈 view plot
⚠️ NO THRESHOLD
11,671.00📈 view plot
⚠️ NO THRESHOLD
82.00📈 view plot
⚠️ NO THRESHOLD
302.00
client_sv2_setup_connection📈 view plot
⚠️ NO THRESHOLD
4,673.00📈 view plot
⚠️ NO THRESHOLD
1,502.00📈 view plot
⚠️ NO THRESHOLD
2,278.00📈 view plot
⚠️ NO THRESHOLD
10.00📈 view plot
⚠️ NO THRESHOLD
67.00
client_sv2_setup_connection_serialize📈 view plot
⚠️ NO THRESHOLD
16,106.00📈 view plot
⚠️ NO THRESHOLD
5,963.00📈 view plot
⚠️ NO THRESHOLD
8,666.00📈 view plot
⚠️ NO THRESHOLD
39.00📈 view plot
⚠️ NO THRESHOLD
207.00
client_sv2_setup_connection_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
35,446.00📈 view plot
⚠️ NO THRESHOLD
14,855.00📈 view plot
⚠️ NO THRESHOLD
21,826.00📈 view plot
⚠️ NO THRESHOLD
92.00📈 view plot
⚠️ NO THRESHOLD
376.00
🐰 View full continuous benchmarking report in Bencher

Copy link
Contributor

🐰 Bencher Report

Branch1237/merge
Testbedsv2

⚠️ WARNING: The following Measure does not have a Threshold. Without a Threshold, no Alerts will ever be generated!

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
client_sv2_handle_message_common📈 view plot
⚠️ NO THRESHOLD
44.08
client_sv2_handle_message_mining📈 view plot
⚠️ NO THRESHOLD
74.93
client_sv2_mining_message_submit_standard📈 view plot
⚠️ NO THRESHOLD
14.66
client_sv2_mining_message_submit_standard_serialize📈 view plot
⚠️ NO THRESHOLD
262.50
client_sv2_mining_message_submit_standard_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
590.76
client_sv2_open_channel📈 view plot
⚠️ NO THRESHOLD
148.44
client_sv2_open_channel_serialize📈 view plot
⚠️ NO THRESHOLD
282.63
client_sv2_open_channel_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
402.10
client_sv2_setup_connection📈 view plot
⚠️ NO THRESHOLD
158.78
client_sv2_setup_connection_serialize📈 view plot
⚠️ NO THRESHOLD
489.64
client_sv2_setup_connection_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
945.48
🐰 View full continuous benchmarking report in Bencher

Copy link
Contributor

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

ACK 0743f82

@plebhash
Copy link
Collaborator

plebhash commented Oct 26, 2024

hey @Georges760 thanks for this contribution

can you elaborate on the rationale that motivated this contribution?

@plebhash plebhash requested a review from GitGab19 October 26, 2024 14:30
@Georges760
Copy link
Author

Georges760 commented Oct 26, 2024

#1130 ask to no_std noise_sv2, so I asked in Discord dev channel if I can give it a try. Doing so, I first had to understand the dependencies graph so I wrote a mermaid graph that ended in #1233 and used #1230 to unify the no_std nature of protocol/sv2/* crates.
The top level crate (the one all other depends on) is const_sv2, which is just a bunch of 'pub const' declaration. And it has a dependency not used anywhere, so I asked about it on the Discord and @Fi3 reply me I could remove it. It was first part of #1230 (like the dependencies graph) then I made single PR for both of them. And here we are.

@plebhash
Copy link
Collaborator

#1130 ask to no_std noise_sv2, so I asked in Discord dev channel if I can give it a try. Doing so, I first had to understand the dependencies graph so I wrote a mermaid graph that ended in #1233 and used #1230 to unify the no_std nature of protocol/sv2/* crates. The top level crate (the one all other depends on) is const_sv2, which is just a bunch of 'pub const' declaration. And it has a dependency not used anywhere, so I asked about it on the Discord and @Fi3 reply me I could remove it. It was first part of #1230 (like the dependencies graph) then I made single PR for both of them. And here we are.

ah ok! sorry I missed that discussion on Discord while I was away for a few days

thank you for the context, I just caught up on that discussion

Copy link
Collaborator

@GitGab19 GitGab19 left a comment

Choose a reason for hiding this comment

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

Since we are going to refactor a bit this crate (look at #1135), it's ok to merge this PR now, but I want to underline that we are not going to bump the version of this crate for this tiny change. We will do it after the refactoring.

@GitGab19 GitGab19 merged commit 0a8c17e into stratum-mining:main Oct 28, 2024
38 checks passed
@Georges760 Georges760 deleted the const-sv2-simplify-deps branch October 28, 2024 10:02
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.

5 participants