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.

Sign up for GitHub

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

Jump to bottom

MovePoolSv2 lib code out of main.rs #1097

Merged
merged 2 commits into from
Aug 30, 2024
Merged

MovePoolSv2 lib code out of main.rs #1097

merged 2 commits into from
Aug 30, 2024

Conversation

jbesraa
Copy link
Contributor

@jbesraa jbesraa commented Aug 13, 2024 •
edited
Loading

part of #1093

@jbesraa jbesraa mentioned this pull request Aug 13, 2024
Open
5 tasks
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 13, 2024 •
edited
Loading

🐰Bencher

ReportFri, August 30, 2024 at 15:32:50 UTC
ProjectStratum v2 (SRI)
Branch1097/merge
Testbedsv1
Click to view all benchmark results
BenchmarkLatencyLatency Results
nanoseconds (ns) | (Δ%)		Latency Upper Boundary
nanoseconds (ns) | (%)
client-submit-serialize✅ (view plot)6,880.20 (+0.60%)7,302.61 (94.22%)
client-submit-serialize-deserialize✅ (view plot)7,780.10 (-0.00%)8,282.77 (93.93%)
client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle✅ (view plot)8,432.80 (+0.88%)8,788.07 (95.96%)
client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle✅ (view plot)934.15 (+3.21%)943.36 (99.02%)
client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize✅ (view plot)731.13 (+3.88%)733.99 (99.61%)
client-sv1-authorize-serialize/client-sv1-authorize-serialize✅ (view plot)248.96 (-0.06%)255.76 (97.34%)
client-sv1-get-authorize/client-sv1-get-authorize✅ (view plot)158.10 (+0.59%)162.19 (97.48%)
client-sv1-get-submit✅ (view plot)6,600.10 (-0.20%)7,060.85 (93.47%)
client-sv1-get-subscribe/client-sv1-get-subscribe✅ (view plot)271.98 (-1.86%)292.70 (92.92%)
client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle✅ (view plot)782.49 (+3.72%)790.73 (98.96%)
client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize✅ (view plot)617.67 (+0.17%)642.32 (96.16%)
client-sv1-subscribe-serialize/client-sv1-subscribe-serialize✅ (view plot)204.58 (-1.33%)218.58 (93.60%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 13, 2024 •
edited
Loading

🐰Bencher

ReportFri, August 30, 2024 at 15:32:49 UTC
ProjectStratum v2 (SRI)
Branch2024-08-13-refactor-poolsv2
Testbedsv2

🚨 1 ALERT: Threshold Boundary Limit exceeded!
BenchmarkMeasure (units)ViewValueLower BoundaryUpper Boundary
client_sv2_mining_message_submit_standardL2 Accesses (accesses)🚨 (view plot | view alert)23.00 (+32.43%)22.91 (100.40%)

Click to view all benchmark results
BenchmarkEstimated CyclesEstimated Cycles Results
estimated cycles | (Δ%)		Estimated Cycles Upper Boundary
estimated cycles | (%)		InstructionsInstructions Results
instructions | (Δ%)		Instructions Upper Boundary
instructions | (%)		L1 AccessesL1 Accesses Results
accesses | (Δ%)		L1 Accesses Upper Boundary
accesses | (%)		L2 AccessesL2 Accesses Results
accesses | (Δ%)		L2 Accesses Upper Boundary
accesses | (%)		RAM AccessesRAM Accesses Results
accesses | (Δ%)		RAM Accesses Upper Boundary
accesses | (%)
client_sv2_handle_message_common✅ (view plot)2,065.00 (+0.04%)2,146.25 (96.21%)✅ (view plot)473.00 (+0.39%)484.22 (97.68%)✅ (view plot)735.00 (+0.31%)752.29 (97.70%)✅ (view plot)7.00 (+7.98%)12.29 (56.98%)✅ (view plot)37.00 (-0.31%)39.36 (94.00%)
client_sv2_handle_message_mining✅ (view plot)8,200.00 (-0.05%)8,319.02 (98.57%)✅ (view plot)2,137.00 (+0.24%)2,168.44 (98.55%)✅ (view plot)3,160.00 (+0.27%)3,210.74 (98.42%)✅ (view plot)35.00 (-5.73%)43.11 (81.19%)✅ (view plot)139.00 (-0.04%)141.63 (98.14%)
client_sv2_mining_message_submit_standard✅ (view plot)6,267.00 (-0.21%)6,380.70 (98.22%)✅ (view plot)1,750.00 (-0.01%)1,762.59 (99.29%)✅ (view plot)2,547.00 (-0.26%)2,573.22 (98.98%)🚨 (view plot | view alert)23.00 (+32.43%)22.91 (100.40%)✅ (view plot)103.00 (-0.95%)106.75 (96.48%)
client_sv2_mining_message_submit_standard_serialize✅ (view plot)14,720.00 (-0.34%)14,988.29 (98.21%)✅ (view plot)4,694.00 (-0.00%)4,706.59 (99.73%)✅ (view plot)6,755.00 (-0.01%)6,774.31 (99.71%)✅ (view plot)46.00 (+0.96%)52.44 (87.71%)✅ (view plot)221.00 (-0.67%)228.67 (96.65%)
client_sv2_mining_message_submit_standard_serialize_deserialize✅ (view plot)27,506.00 (+0.01%)27,804.84 (98.93%)✅ (view plot)10,585.00 (+0.26%)10,605.44 (99.81%)✅ (view plot)15,401.00 (+0.25%)15,431.45 (99.80%)✅ (view plot)83.00 (+0.81%)88.42 (93.87%)✅ (view plot)334.00 (-0.33%)343.55 (97.22%)
client_sv2_open_channel✅ (view plot)4,345.00 (-2.27%)4,614.66 (94.16%)✅ (view plot)1,461.00 (+0.06%)1,472.04 (99.25%)✅ (view plot)2,160.00 (+0.25%)2,172.85 (99.41%)✅ (view plot)10.00 (-9.19%)15.91 (62.83%)✅ (view plot)61.00 (-4.53%)68.46 (89.11%)
client_sv2_open_channel_serialize✅ (view plot)14,012.00 (-1.04%)14,423.45 (97.15%)✅ (view plot)5,064.00 (+0.02%)5,075.04 (99.78%)✅ (view plot)7,327.00 (+0.09%)7,340.45 (99.82%)✅ (view plot)35.00 (-1.96%)41.36 (84.63%)✅ (view plot)186.00 (-2.25%)197.69 (94.09%)
client_sv2_open_channel_serialize_deserialize✅ (view plot)22,639.00 (-0.02%)22,950.62 (98.64%)✅ (view plot)8,027.00 (+0.35%)8,044.58 (99.78%)✅ (view plot)11,674.00 (+0.32%)11,702.80 (99.75%)✅ (view plot)79.00 (+7.12%)82.23 (96.08%)✅ (view plot)302.00 (-0.65%)312.57 (96.62%)
client_sv2_setup_connection✅ (view plot)4,647.00 (-0.90%)4,756.52 (97.70%)✅ (view plot)1,502.00 (+0.06%)1,513.04 (99.27%)✅ (view plot)2,277.00 (+0.02%)2,295.57 (99.19%)✅ (view plot)12.00 (+26.85%)14.39 (83.41%)✅ (view plot)66.00 (-2.35%)69.52 (94.93%)
client_sv2_setup_connection_serialize✅ (view plot)16,110.00 (-0.71%)16,447.61 (97.95%)✅ (view plot)5,963.00 (+0.02%)5,974.04 (99.82%)✅ (view plot)8,665.00 (+0.09%)8,678.46 (99.84%)✅ (view plot)40.00 (-6.88%)51.55 (77.60%)✅ (view plot)207.00 (-1.48%)216.11 (95.78%)
client_sv2_setup_connection_serialize_deserialize✅ (view plot)35,552.00 (+0.03%)35,714.91 (99.54%)✅ (view plot)14,855.00 (+0.19%)14,872.90 (99.88%)✅ (view plot)21,822.00 (+0.21%)21,853.70 (99.85%)✅ (view plot)93.00 (-3.11%)111.40 (83.48%)✅ (view plot)379.00 (-0.15%)383.61 (98.80%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 13, 2024 •
edited
Loading

🐰Bencher

ReportFri, August 30, 2024 at 15:32:53 UTC
ProjectStratum v2 (SRI)
Branch2024-08-13-refactor-poolsv2
Testbedsv1
Click to view all benchmark results
BenchmarkEstimated CyclesEstimated Cycles Results
estimated cycles | (Δ%)		Estimated Cycles Upper Boundary
estimated cycles | (%)		InstructionsInstructions Results
instructions | (Δ%)		Instructions Upper Boundary
instructions | (%)		L1 AccessesL1 Accesses Results
accesses | (Δ%)		L1 Accesses Upper Boundary
accesses | (%)		L2 AccessesL2 Accesses Results
accesses | (Δ%)		L2 Accesses Upper Boundary
accesses | (%)		RAM AccessesRAM Accesses Results
accesses | (Δ%)		RAM Accesses Upper Boundary
accesses | (%)
get_authorize✅ (view plot)8,402.00 (-0.40%)8,694.15 (96.64%)✅ (view plot)3,746.00 (+0.02%)3,847.82 (97.35%)✅ (view plot)5,252.00 (-0.02%)5,392.95 (97.39%)✅ (view plot)7.00 (-6.05%)10.25 (68.28%)✅ (view plot)89.00 (-0.98%)93.52 (95.17%)
get_submit✅ (view plot)95,345.00 (-0.18%)96,056.13 (99.26%)✅ (view plot)59,439.00 (-0.05%)59,720.43 (99.53%)✅ (view plot)85,370.00 (-0.05%)85,761.99 (99.54%)✅ (view plot)42.00 (-17.06%)64.87 (64.74%)✅ (view plot)279.00 (-0.91%)287.81 (96.94%)
get_subscribe✅ (view plot)8,025.00 (+0.36%)8,264.62 (97.10%)✅ (view plot)2,841.00 (+0.26%)2,932.29 (96.89%)✅ (view plot)3,970.00 (+0.24%)4,092.04 (97.02%)✅ (view plot)13.00 (-8.01%)20.53 (63.31%)✅ (view plot)114.00 (+0.62%)117.58 (96.95%)
serialize_authorize✅ (view plot)12,249.00 (+0.10%)12,521.92 (97.82%)✅ (view plot)5,317.00 (+0.01%)5,418.82 (98.12%)✅ (view plot)7,414.00 (-0.01%)7,555.26 (98.13%)✅ (view plot)8.00 (-18.20%)13.39 (59.73%)✅ (view plot)137.00 (+0.46%)141.24 (97.00%)
serialize_deserialize_authorize✅ (view plot)24,683.00 (+0.58%)24,833.43 (99.39%)✅ (view plot)9,868.00 (-0.32%)10,017.68 (98.51%)✅ (view plot)13,928.00 (-0.30%)14,142.22 (98.49%)✅ (view plot)37.00 (+3.06%)41.81 (88.49%)✅ (view plot)302.00 (+1.71%)303.82 (99.40%)
serialize_deserialize_handle_authorize✅ (view plot)30,388.00 (+0.61%)30,481.78 (99.69%)✅ (view plot)12,071.00 (-0.19%)12,198.69 (98.95%)✅ (view plot)17,088.00 (-0.18%)17,265.28 (98.97%)✅ (view plot)63.00 (+8.91%)64.12 (98.26%)✅ (view plot)371.00 (+1.47%)372.64 (99.56%)
serialize_deserialize_handle_submit✅ (view plot)126,518.00 (+0.06%)126,999.03 (99.62%)✅ (view plot)73,280.00 (+0.02%)73,564.78 (99.61%)✅ (view plot)105,053.00 (+0.03%)105,448.25 (99.63%)✅ (view plot)107.00 (-6.01%)133.76 (80.00%)✅ (view plot)598.00 (+0.34%)601.87 (99.36%)
serialize_deserialize_handle_subscribe✅ (view plot)28,010.00 (+1.52%)28,081.25 (99.75%)✅ (view plot)9,659.00 (+0.20%)9,740.36 (99.16%)✅ (view plot)13,655.00 (+0.15%)13,773.18 (99.14%)✅ (view plot)71.00 (+9.18%)72.60 (97.80%)✅ (view plot)400.00 (+2.71%)402.08 (99.48%)
serialize_deserialize_submit✅ (view plot)115,177.00 (+0.03%)115,653.62 (99.59%)✅ (view plot)68,057.00 (-0.02%)68,357.31 (99.56%)✅ (view plot)97,657.00 (-0.03%)98,098.82 (99.55%)✅ (view plot)60.00 (-8.70%)77.10 (77.82%)✅ (view plot)492.00 (+0.52%)495.39 (99.32%)
serialize_deserialize_subscribe✅ (view plot)23,379.00 (+1.59%)23,499.49 (99.49%)✅ (view plot)8,211.00 (+0.21%)8,295.57 (98.98%)✅ (view plot)11,564.00 (+0.20%)11,680.86 (99.00%)✅ (view plot)39.00 (+1.55%)43.37 (89.93%)✅ (view plot)332.00 (+3.01%)334.37 (99.29%)
serialize_submit✅ (view plot)99,814.00 (-0.09%)100,410.44 (99.41%)✅ (view plot)61,483.00 (-0.05%)61,768.32 (99.54%)✅ (view plot)88,209.00 (-0.05%)88,613.07 (99.54%)✅ (view plot)46.00 (-10.77%)64.24 (71.61%)✅ (view plot)325.00 (-0.10%)331.06 (98.17%)
serialize_subscribe✅ (view plot)11,426.00 (+0.47%)11,658.01 (98.01%)✅ (view plot)4,188.00 (+0.18%)4,279.29 (97.87%)✅ (view plot)5,826.00 (+0.14%)5,949.23 (97.93%)✅ (view plot)14.00 (-5.14%)19.72 (70.98%)✅ (view plot)158.00 (+0.89%)162.12 (97.46%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 13, 2024 •
edited
Loading

🐰Bencher

ReportFri, August 30, 2024 at 15:32:48 UTC
ProjectStratum v2 (SRI)
Branch2024-08-13-refactor-poolsv2
Testbedsv2

🚨 1 ALERT: Threshold Boundary Limit exceeded!
BenchmarkMeasure (units)ViewValueLower BoundaryUpper Boundary
client_sv2_setup_connectionLatency (nanoseconds (ns))🚨 (view plot | view alert)178.27 (+8.95%)175.68 (101.47%)

Click to view all benchmark results
BenchmarkLatencyLatency Results
nanoseconds (ns) | (Δ%)		Latency Upper Boundary
nanoseconds (ns) | (%)
client_sv2_handle_message_common✅ (view plot)45.41 (+1.74%)45.57 (99.63%)
client_sv2_handle_message_mining✅ (view plot)72.80 (-1.21%)82.08 (88.69%)
client_sv2_mining_message_submit_standard✅ (view plot)14.64 (-0.13%)14.71 (99.52%)
client_sv2_mining_message_submit_standard_serialize✅ (view plot)271.43 (+1.84%)284.88 (95.28%)
client_sv2_mining_message_submit_standard_serialize_deserialize✅ (view plot)587.93 (-0.99%)627.89 (93.64%)
client_sv2_open_channel✅ (view plot)169.37 (+2.15%)171.77 (98.60%)
client_sv2_open_channel_serialize✅ (view plot)271.55 (-3.59%)294.34 (92.26%)
client_sv2_open_channel_serialize_deserialize✅ (view plot)370.28 (-2.06%)428.08 (86.50%)
client_sv2_setup_connection🚨 (view plot | view alert)178.27 (+8.95%)175.68 (101.47%)
client_sv2_setup_connection_serialize✅ (view plot)449.55 (-4.59%)511.38 (87.91%)
client_sv2_setup_connection_serialize_deserialize✅ (view plot)981.29 (-0.02%)1,072.65 (91.48%)

Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help

@jbesraa jbesraa changed the title Refactor PoolSv2 MovePoolSv2 lib code out of main.rs Aug 13, 2024
@jbesraa jbesraa marked this pull request as ready for review August 13, 2024 17:02
@marathon-gary
Copy link
Contributor

[[bin]]??
#1095 (review)

marathon-gary
marathon-gary reviewed Aug 14, 2024
View reviewed changes
roles/pool/src/lib/mining_pool/mod.rs
@@ -66,6 +66,15 @@ pub struct CoinbaseOutput {
output_script_value: String,
}

impl CoinbaseOutput {
Copy link
Contributor

Choose a reason for hiding this comment

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

This struct appears all over. I'll create an issue from this comment to consolidate to a location in the code base that makes sense.
#1066 (comment)
#1066 (comment)
#1066 (comment)

marathon-gary
marathon-gary reviewed Aug 14, 2024
View reviewed changes
Copy link
Contributor

@marathon-gary marathon-gary left a comment

Choose a reason for hiding this comment

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

LGTM.
Seems naming of some variables drifts from conventions I've seen elsewhere but that's outside the scope of this PR.

roles/pool/src/lib/mining_pool/mod.rs Show resolved Hide resolved
roles/pool/src/lib/mod.rs
impl PoolSv2 {
pub async fn start(config: Configuration) {
let (status_tx, status_rx) = unbounded();
let (s_new_t, r_new_t) = bounded(10);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: naming conventions for channels should be consistent with the rest of the code base.

Shourya742
Shourya742 suggested changes Aug 18, 2024
View reviewed changes
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. Same nits as @marathon-gary mentioned. I am still facing the shutdown signalling error mentioned here: #1095 (comment)

@jbesraa jbesraa mentioned this pull request Aug 19, 2024
Closed
@jbesraa jbesraa force-pushed the 2024-08-13-refactor-poolsv2 branch from b5abc57 to 6a9218c Compare August 19, 2024 16:54
@plebhash plebhash linked an issue Aug 20, 2024 that may be closed by this pull request
Restructure Roles as a prepration for integration tests #1093
Open
5 tasks
@plebhash plebhash mentioned this pull request Aug 20, 2024
Merged
@jbesraa jbesraa force-pushed the 2024-08-13-refactor-poolsv2 branch from 6a9218c to 70d7cda Compare August 21, 2024 12:20
plebhash
plebhash reviewed Aug 21, 2024
View reviewed changes
roles/pool/src/lib/mod.rs Outdated Show resolved Hide resolved
plebhash
plebhash reviewed Aug 21, 2024
View reviewed changes
roles/pool/src/lib/mod.rs Outdated Show resolved Hide resolved
@jbesraa jbesraa force-pushed the 2024-08-13-refactor-poolsv2 branch from 70d7cda to d4a4470 Compare August 23, 2024 10:30
@plebhash plebhash mentioned this pull request Aug 23, 2024
Merged
@jbesraa
Move code out of main.rs to lib
8158398 
..This would allow us to use the crate in other enviornements.
@jbesraa jbesraa force-pushed the 2024-08-13-refactor-poolsv2 branch from d4a4470 to 8158398 Compare August 26, 2024 08:09
@jbesraa jbesraa mentioned this pull request Aug 27, 2024
Closed
@plebhash plebhash changed the base branch from dev to main August 28, 2024 19:18
@plebhash plebhash added the ready-to-be-merged triggers auto rebase bot label Aug 30, 2024
@plebhash plebhash merged commit 1848fc6 into stratum-mining:main Aug 30, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marathon-gary marathon-gary marathon-gary left review comments

@Shourya742 Shourya742 Shourya742 requested changes

@plebhash plebhash plebhash approved these changes

Assignees
No one assigned
Labels
ready-to-be-merged triggers auto rebase bot
Projects
SV2 Roadmap 🛣️
Status: Done ✅
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Restructure Roles as a prepration for integration tests
4 participants
@jbesraa @marathon-gary @Shourya742 @plebhash