`PoolSv2` integration tests #1066

jbesraa · 2024-07-19T07:12:23Z

Blocked by #1155 and #1156

for more context:

github-actions · 2024-07-19T07:16:28Z

Bencher

Report	Wed, September 4, 2024 at 10:48:46 UTC
Project	Stratum V2 (SRI)
Branch	2024-07-15-roles-testing
Testbed	sv2

Click to view all benchmark results

Benchmark	Latency	Latency Results nanoseconds (ns) \| (Δ%)	Latency Lower Boundary nanoseconds (ns) \| (%)	Latency Upper Boundary nanoseconds (ns) \| (%)
client_sv2_handle_message_common	✅ (view plot)	44.06 (-1.28%)	43.50 (98.74%)	45.76 (96.28%)
client_sv2_handle_message_mining	✅ (view plot)	74.43 (+0.42%)	63.60 (85.45%)	84.64 (87.94%)
client_sv2_mining_message_submit_standard	✅ (view plot)	14.66 (-0.03%)	14.59 (99.54%)	14.73 (99.49%)
client_sv2_mining_message_submit_standard_serialize	✅ (view plot)	264.92 (-0.68%)	246.56 (93.07%)	286.89 (92.34%)
client_sv2_mining_message_submit_standard_serialize_deserialize	✅ (view plot)	592.67 (-0.22%)	556.43 (93.88%)	631.57 (93.84%)
client_sv2_open_channel	✅ (view plot)	169.17 (+1.90%)	159.22 (94.12%)	172.82 (97.89%)
client_sv2_open_channel_serialize	✅ (view plot)	288.12 (+2.43%)	265.42 (92.12%)	297.13 (96.97%)
client_sv2_open_channel_serialize_deserialize	✅ (view plot)	373.74 (-0.99%)	324.55 (86.84%)	430.42 (86.83%)
client_sv2_setup_connection	✅ (view plot)	162.55 (-0.47%)	150.02 (92.29%)	176.60 (92.05%)
client_sv2_setup_connection_serialize	✅ (view plot)	468.57 (-0.21%)	412.51 (88.04%)	526.63 (88.98%)
client_sv2_setup_connection_serialize_deserialize	✅ (view plot)	1,049.00 (+6.58%)	883.36 (84.21%)	1,085.16 (96.67%)

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

github-actions · 2024-07-19T07:16:28Z

Bencher Report

Branch	1066/merge
Testbed	sv1

Click to view all benchmark results

Benchmark	Latency	Benchmark Result nanoseconds (ns) (Result Δ%)	Upper Boundary nanoseconds (ns) (Limit %)
client-submit-serialize	📈 view plot 🚷 view threshold	6,752.80 (-2.69%)	8,149.91 (82.86%)
client-submit-serialize-deserialize	📈 view plot 🚷 view threshold	7,806.70 (-1.88%)	9,137.86 (85.43%)
client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle	📈 view plot 🚷 view threshold	8,394.30 (-1.64%)	9,733.03 (86.25%)
client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle	📈 view plot 🚷 view threshold	960.64 (+2.94%)	1,072.16 (89.60%)
client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize	📈 view plot 🚷 view threshold	757.95 (+3.59%)	886.84 (85.47%)
client-sv1-authorize-serialize/client-sv1-authorize-serialize	📈 view plot 🚷 view threshold	277.07 (+8.94%)	308.68 (89.76%)
client-sv1-get-authorize/client-sv1-get-authorize	📈 view plot 🚷 view threshold	158.57 (-0.93%)	181.01 (87.60%)
client-sv1-get-submit	📈 view plot 🚷 view threshold	6,532.00 (-2.39%)	7,675.40 (85.10%)
client-sv1-get-subscribe/client-sv1-get-subscribe	📈 view plot 🚷 view threshold	316.14 (+11.79%)	348.59 (90.69%)
client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle	📈 view plot 🚷 view threshold	805.96 (+2.78%)	939.19 (85.81%)
client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize	📈 view plot 🚷 view threshold	694.25 (+6.50%)	795.02 (87.32%)
client-sv1-subscribe-serialize/client-sv1-subscribe-serialize	📈 view plot 🚷 view threshold	213.09 (+1.14%)	251.73 (84.65%)

🐰 View full continuous benchmarking report in Bencher

github-actions · 2024-07-19T07:16:30Z

Bencher

Report	Wed, September 4, 2024 at 10:49:04 UTC
Project	Stratum V2 (SRI)
Branch	2024-07-15-roles-testing
Testbed	sv2

Click to view all benchmark results

Benchmark	Estimated Cycles	Estimated Cycles Results estimated cycles \| (Δ%)	Estimated Cycles Lower Boundary estimated cycles \| (%)	Estimated Cycles Upper Boundary estimated cycles \| (%)	Instructions	Instructions Results instructions \| (Δ%)	Instructions Lower Boundary instructions \| (%)	Instructions Upper Boundary instructions \| (%)	L1 Accesses	L1 Accesses Results accesses \| (Δ%)	L1 Accesses Lower Boundary accesses \| (%)	L1 Accesses Upper Boundary accesses \| (%)	L2 Accesses	L2 Accesses Results accesses \| (Δ%)	L2 Accesses Lower Boundary accesses \| (%)	L2 Accesses Upper Boundary accesses \| (%)	RAM Accesses	RAM Accesses Results accesses \| (Δ%)	RAM Accesses Lower Boundary accesses \| (%)	RAM Accesses Upper Boundary accesses \| (%)
client_sv2_handle_message_common	✅ (view plot)	2,091.00 (+1.04%)	1,973.26 (94.37%)	2,165.54 (96.56%)	✅ (view plot)	473.00 (+0.33%)	457.50 (96.72%)	485.42 (97.44%)	✅ (view plot)	736.00 (+0.38%)	712.12 (96.76%)	754.38 (97.56%)	✅ (view plot)	5.00 (-19.35%)	-0.50 (-10.10%)	12.90 (38.75%)	✅ (view plot)	38.00 (+1.90%)	34.63 (91.13%)	39.95 (95.12%)
client_sv2_handle_message_mining	✅ (view plot)	8,146.00 (-0.67%)	8,077.03 (99.15%)	8,325.23 (97.85%)	✅ (view plot)	2,137.00 (+0.21%)	2,093.47 (97.96%)	2,171.45 (98.41%)	✅ (view plot)	3,166.00 (+0.41%)	3,089.90 (97.60%)	3,215.96 (98.45%)	✅ (view plot)	30.00 (-18.28%)	29.64 (98.79%)	43.78 (68.52%)	✅ (view plot)	138.00 (-0.71%)	136.20 (98.70%)	141.78 (97.34%)
client_sv2_mining_message_submit_standard	✅ (view plot)	6,243.00 (-0.58%)	6,169.82 (98.83%)	6,388.88 (97.72%)	✅ (view plot)	1,750.00 (-0.01%)	1,736.93 (99.25%)	1,763.39 (99.24%)	✅ (view plot)	2,553.00 (-0.02%)	2,532.57 (99.20%)	2,574.43 (99.17%)	✅ (view plot)	17.00 (-1.39%)	11.19 (65.83%)	23.29 (73.00%)	✅ (view plot)	103.00 (-0.95%)	101.01 (98.07%)	106.97 (96.29%)
client_sv2_mining_message_submit_standard_serialize	✅ (view plot)	14,632.00 (-0.88%)	14,522.97 (99.25%)	15,001.69 (97.54%)	✅ (view plot)	4,694.00 (-0.00%)	4,680.93 (99.72%)	4,707.39 (99.72%)	✅ (view plot)	6,762.00 (+0.08%)	6,736.50 (99.62%)	6,776.46 (99.79%)	✅ (view plot)	41.00 (-8.75%)	36.59 (89.26%)	53.27 (76.97%)	✅ (view plot)	219.00 (-1.49%)	215.61 (98.45%)	229.03 (95.62%)
client_sv2_mining_message_submit_standard_serialize_deserialize	✅ (view plot)	27,490.00 (-0.06%)	27,185.28 (98.89%)	27,828.12 (98.78%)	✅ (view plot)	10,585.00 (+0.22%)	10,506.82 (99.26%)	10,616.30 (99.71%)	✅ (view plot)	15,405.00 (+0.24%)	15,288.63 (99.24%)	15,447.87 (99.72%)	✅ (view plot)	79.00 (-3.56%)	74.91 (94.82%)	88.93 (88.83%)	✅ (view plot)	334.00 (-0.33%)	326.13 (97.64%)	344.09 (97.07%)
client_sv2_open_channel	✅ (view plot)	4,371.00 (-1.52%)	4,252.44 (97.29%)	4,624.04 (94.53%)	✅ (view plot)	1,461.00 (+0.05%)	1,447.53 (99.08%)	1,472.99 (99.19%)	✅ (view plot)	2,161.00 (+0.25%)	2,135.53 (98.82%)	2,175.55 (99.33%)	✅ (view plot)	8.00 (-24.31%)	4.71 (58.85%)	16.43 (48.69%)	✅ (view plot)	62.00 (-2.68%)	58.72 (94.72%)	68.70 (90.25%)
client_sv2_open_channel_serialize	✅ (view plot)	13,958.00 (-1.30%)	13,839.00 (99.15%)	14,443.44 (96.64%)	✅ (view plot)	5,064.00 (+0.01%)	5,050.53 (99.73%)	5,075.99 (99.76%)	✅ (view plot)	7,333.00 (+0.15%)	7,299.50 (99.54%)	7,343.94 (99.85%)	✅ (view plot)	30.00 (-15.01%)	28.70 (95.67%)	41.90 (71.60%)	✅ (view plot)	185.00 (-2.53%)	181.38 (98.04%)	198.22 (93.33%)
client_sv2_open_channel_serialize_deserialize	✅ (view plot)	22,589.00 (-0.24%)	22,317.12 (98.80%)	22,968.94 (98.35%)	✅ (view plot)	8,027.00 (+0.30%)	7,949.34 (99.03%)	8,055.98 (99.64%)	✅ (view plot)	11,679.00 (+0.32%)	11,563.24 (99.01%)	11,720.82 (99.64%)	✅ (view plot)	75.00 (+2.00%)	64.31 (85.74%)	82.75 (90.63%)	✅ (view plot)	301.00 (-0.92%)	294.62 (97.88%)	313.00 (96.17%)
client_sv2_setup_connection	✅ (view plot)	4,669.00 (-0.42%)	4,614.24 (98.83%)	4,763.28 (98.02%)	✅ (view plot)	1,502.00 (+0.05%)	1,488.53 (99.10%)	1,513.99 (99.21%)	✅ (view plot)	2,279.00 (+0.09%)	2,256.55 (99.01%)	2,297.17 (99.21%)	✅ (view plot)	9.00 (-4.15%)	4.12 (45.83%)	14.66 (61.41%)	✅ (view plot)	67.00 (-0.84%)	65.45 (97.68%)	69.69 (96.14%)
client_sv2_setup_connection_serialize	✅ (view plot)	16,136.00 (-0.49%)	15,971.56 (98.98%)	16,459.48 (98.03%)	✅ (view plot)	5,963.00 (+0.01%)	5,949.53 (99.77%)	5,974.99 (99.80%)	✅ (view plot)	8,666.00 (+0.08%)	8,635.20 (99.64%)	8,682.44 (99.81%)	✅ (view plot)	38.00 (-9.76%)	31.60 (83.17%)	52.62 (72.22%)	✅ (view plot)	208.00 (-0.90%)	203.38 (97.78%)	216.40 (96.12%)
client_sv2_setup_connection_serialize_deserialize	✅ (view plot)	35,548.00 (+0.01%)	35,356.61 (99.46%)	35,734.63 (99.48%)	✅ (view plot)	14,855.00 (+0.17%)	14,775.66 (99.47%)	14,884.56 (99.80%)	✅ (view plot)	21,823.00 (+0.18%)	21,692.88 (99.40%)	21,873.46 (99.77%)	✅ (view plot)	92.00 (-3.17%)	77.39 (84.12%)	112.63 (81.68%)	✅ (view plot)	379.00 (-0.17%)	375.25 (99.01%)	384.03 (98.69%)

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

github-actions · 2024-07-19T07:16:32Z

Bencher

Report	Wed, September 4, 2024 at 10:48:57 UTC
Project	Stratum V2 (SRI)
Branch	2024-07-15-roles-testing
Testbed	sv1

Click to view all benchmark results

Benchmark	Estimated Cycles	Estimated Cycles Results estimated cycles \| (Δ%)	Estimated Cycles Lower Boundary estimated cycles \| (%)	Estimated Cycles Upper Boundary estimated cycles \| (%)	Instructions	Instructions Results instructions \| (Δ%)	Instructions Lower Boundary instructions \| (%)	Instructions Upper Boundary instructions \| (%)	L1 Accesses	L1 Accesses Results accesses \| (Δ%)	L1 Accesses Lower Boundary accesses \| (%)	L1 Accesses Upper Boundary accesses \| (%)	L2 Accesses	L2 Accesses Results accesses \| (Δ%)	L2 Accesses Lower Boundary accesses \| (%)	L2 Accesses Upper Boundary accesses \| (%)	RAM Accesses	RAM Accesses Results accesses \| (Δ%)	RAM Accesses Lower Boundary accesses \| (%)	RAM Accesses Upper Boundary accesses \| (%)
get_authorize	✅ (view plot)	8,545.00 (+1.24%)	8,159.42 (95.49%)	8,721.39 (97.98%)	✅ (view plot)	3,772.00 (+0.67%)	3,637.25 (96.43%)	3,856.67 (97.80%)	✅ (view plot)	5,295.00 (+0.76%)	5,104.82 (96.41%)	5,405.60 (97.95%)	✅ (view plot)	6.00 (-18.64%)	4.32 (71.92%)	10.43 (57.50%)	✅ (view plot)	92.00 (+2.28%)	85.89 (93.36%)	94.01 (97.86%)
get_submit	✅ (view plot)	95,442.00 (-0.06%)	94,920.89 (99.45%)	96,082.84 (99.33%)	✅ (view plot)	59,522.00 (+0.09%)	59,202.38 (99.46%)	59,738.96 (99.64%)	✅ (view plot)	85,507.00 (+0.11%)	85,037.11 (99.45%)	85,791.14 (99.67%)	✅ (view plot)	48.00 (-3.37%)	33.17 (69.10%)	66.18 (72.53%)	✅ (view plot)	277.00 (-1.47%)	273.88 (98.87%)	288.37 (96.06%)
get_subscribe	✅ (view plot)	8,064.00 (+0.78%)	7,714.93 (95.67%)	8,288.41 (97.29%)	✅ (view plot)	2,848.00 (+0.46%)	2,729.94 (95.85%)	2,940.02 (96.87%)	✅ (view plot)	3,984.00 (+0.54%)	3,822.38 (95.94%)	4,103.08 (97.10%)	✅ (view plot)	11.00 (-19.27%)	6.12 (55.59%)	21.13 (52.05%)	✅ (view plot)	115.00 (+1.36%)	108.77 (94.59%)	118.13 (97.35%)
serialize_authorize	✅ (view plot)	12,328.00 (+0.69%)	11,937.07 (96.83%)	12,550.24 (98.23%)	✅ (view plot)	5,343.00 (+0.47%)	5,208.25 (97.48%)	5,427.67 (98.44%)	✅ (view plot)	7,458.00 (+0.55%)	7,266.83 (97.44%)	7,567.98 (98.55%)	✅ (view plot)	8.00 (-16.80%)	5.58 (69.81%)	13.65 (58.62%)	✅ (view plot)	138.00 (+1.08%)	131.26 (95.11%)	141.78 (97.33%)
serialize_deserialize_authorize	✅ (view plot)	24,931.00 (+1.45%)	24,191.53 (97.03%)	24,956.36 (99.90%)	✅ (view plot)	9,920.00 (+0.22%)	9,771.33 (98.50%)	10,025.98 (98.94%)	✅ (view plot)	14,016.00 (+0.33%)	13,783.28 (98.34%)	14,156.24 (99.01%)	✅ (view plot)	34.00 (-5.15%)	29.45 (86.62%)	42.24 (80.49%)	✅ (view plot)	307.00 (+3.07%)	288.30 (93.91%)	307.41 (99.87%)
serialize_deserialize_handle_authorize	✅ (view plot)	30,431.00 (+0.67%)	29,896.45 (98.24%)	30,560.82 (99.58%)	✅ (view plot)	12,097.00 (+0.04%)	11,979.66 (99.03%)	12,204.65 (99.12%)	✅ (view plot)	17,141.00 (+0.13%)	16,961.17 (98.95%)	17,275.33 (99.22%)	✅ (view plot)	54.00 (-5.90%)	49.91 (92.43%)	64.86 (83.26%)	✅ (view plot)	372.00 (+1.53%)	357.53 (96.11%)	375.24 (99.14%)
serialize_deserialize_handle_submit	✅ (view plot)	126,463.00 (+0.01%)	125,862.45 (99.53%)	127,037.79 (99.55%)	✅ (view plot)	73,363.00 (+0.13%)	72,944.06 (99.43%)	73,594.90 (99.68%)	✅ (view plot)	105,198.00 (+0.16%)	104,563.73 (99.40%)	105,498.81 (99.71%)	✅ (view plot)	109.00 (-3.12%)	89.58 (82.18%)	135.44 (80.48%)	✅ (view plot)	592.00 (-0.65%)	589.28 (99.54%)	602.50 (98.26%)
serialize_deserialize_handle_subscribe	✅ (view plot)	27,971.00 (+1.20%)	27,032.16 (96.64%)	28,246.15 (99.03%)	✅ (view plot)	9,666.00 (+0.25%)	9,533.49 (98.63%)	9,750.93 (99.13%)	✅ (view plot)	13,681.00 (+0.31%)	13,488.04 (98.59%)	13,789.11 (99.22%)	✅ (view plot)	58.00 (-10.12%)	55.78 (96.17%)	73.28 (79.15%)	✅ (view plot)	400.00 (+2.35%)	374.92 (93.73%)	406.68 (98.36%)
serialize_deserialize_submit	✅ (view plot)	115,445.00 (+0.25%)	114,592.21 (99.26%)	115,728.83 (99.75%)	✅ (view plot)	68,223.00 (+0.21%)	67,769.39 (99.34%)	68,393.34 (99.75%)	✅ (view plot)	97,935.00 (+0.25%)	97,231.04 (99.28%)	98,159.23 (99.77%)	✅ (view plot)	65.00 (-0.25%)	52.35 (80.54%)	77.97 (83.36%)	✅ (view plot)	491.00 (+0.27%)	483.17 (98.40%)	496.24 (98.94%)
serialize_deserialize_subscribe	✅ (view plot)	23,469.00 (+1.75%)	22,459.72 (95.70%)	23,669.47 (99.15%)	✅ (view plot)	8,225.00 (+0.34%)	8,087.05 (98.32%)	8,306.49 (99.02%)	✅ (view plot)	11,589.00 (+0.37%)	11,394.83 (98.32%)	11,696.67 (99.08%)	✅ (view plot)	38.00 (-0.68%)	32.87 (86.51%)	43.64 (87.07%)	✅ (view plot)	334.00 (+3.20%)	308.34 (92.32%)	338.94 (98.54%)
serialize_submit	✅ (view plot)	99,835.00 (-0.05%)	99,344.10 (99.51%)	100,435.24 (99.40%)	✅ (view plot)	61,566.00 (+0.08%)	61,242.93 (99.48%)	61,786.99 (99.64%)	✅ (view plot)	88,350.00 (+0.10%)	87,874.59 (99.46%)	88,642.16 (99.67%)	✅ (view plot)	50.00 (-1.68%)	36.56 (73.12%)	65.15 (76.74%)	✅ (view plot)	321.00 (-1.25%)	318.46 (99.21%)	331.65 (96.79%)
serialize_subscribe	✅ (view plot)	11,405.00 (+0.24%)	11,072.39 (97.08%)	11,682.22 (97.63%)	✅ (view plot)	4,195.00 (+0.31%)	4,076.94 (97.19%)	4,287.02 (97.85%)	✅ (view plot)	5,840.00 (+0.35%)	5,679.27 (97.25%)	5,960.06 (97.99%)	✅ (view plot)	14.00 (-2.87%)	8.70 (62.17%)	20.12 (69.57%)	✅ (view plot)	157.00 (+0.17%)	150.78 (96.04%)	162.69 (96.51%)

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

roles/tests-integration/src/main.rs

roles/jd-client/src/lib/mod.rs

roles/pool/config-examples/integration-tests.toml

roles/tests-integration/tests/common/mod.rs

roles/pool/src/lib/mod.rs

plebhash · 2024-10-03T21:34:34Z

roles/tests-integration/tests/pool_integration.rs

+async fn success_pool_template_provider_connection() {
+    let sniffer_addr = common::get_available_address();
+    let tp_addr = common::get_available_address();
+    let pool_addr = common::get_available_address();
+    let _tp = common::start_template_provider(tp_addr.port()).await;
+    let sniffer = common::start_sniffer(tp_addr, sniffer_addr).await;
+    let _ = common::start_pool(Some(pool_addr), Some(sniffer_addr)).await;
+    assert!(sniffer.expect_downstream_message(MESSAGE_TYPE_SETUP_CONNECTION));
+    assert!(sniffer.expect_upstream_message(MESSAGE_TYPE_SETUP_CONNECTION_SUCCESS));
+    assert!(sniffer.expect_downstream_message(MESSAGE_TYPE_COINBASE_OUTPUT_DATA_SIZE));
+    assert!(sniffer.expect_upstream_message(MESSAGE_TYPE_NEW_TEMPLATE));
+    assert!(sniffer.expect_upstream_message(MESSAGE_TYPE_SET_NEW_PREV_HASH));
+}


let's also use next_downstream_message and next_upstream_message here so we can get a feeling of how the user experience is going to be like

Maybe it is worth doing that in a test where it is needed and not here?

ok here's a very simple test where I think this makes sense.

I couldn't find it as a MG test but honestly we should have had this in our CI for a long time.

start TP

start Pool (connected to TP)

start SV2 CPU Miner (connected to Pool)

we can assert that:

Pool connects to TP and receives template (basically what's already being done on success_pool_template_provider_connection)

SV2 CPU Miner connects to Pool (SetupConnection / SetupConnection.Success)

SV2 CPU Miner opens standard channel with Pool (OpenStandardMiningChannel / OpenStandardMiningChannel.Success)

Pool sends SetTarget to SV2 CPU Miner (SetTarget)

Pool sends Standard Job to SV2 CPU Miner (NewMiningJob)

SV2 CPU Miner sends share to Pool (SubmitSharesStandard)

Pool confirms receiving share (SubmitShares.Success)

we can assert things like:channel ids and job ids

So, I was able to test the above but I think it requires some discussion/changes in mining-device crate. Currently mining-device calculates the difficulty in a way that does not always work. On my machine, the difficulty target was never hit when trying to send shares, even after running the test for 60 seconds. When lowering the difficulty target manually the mining-device sends the SubmitShare message succesfully, but it send many of them, and it will probably behave differently on different machines. I don't have a good solution for this yet, need to sleep on it.

To try to unblock this pr, made the following changes 338cc02#diff-2a0b80af72100f3ef6475d64910e57bfa2194a668b3a7692391a53f964340fb4R21 to give an example of how to use next_x_message. Also made small doc/var name changes here e99e36b, will fixup this one once it is reviewed.

Edit: opened #1188

plebhash · 2024-10-03T21:35:57Z

roles/tests-integration/tests/common/sniffer.rs

+    }
+}
+
+impl Drop for Sniffer {


maybe worth adding a comment to this impl to highlight the fact that every test needs to drain the entire queue before it finishes?

roles/tests-integration/tests/common/sniffer.rs

jbesraa · 2024-10-04T09:51:26Z

Addressed above comments.

pavlenex · 2024-10-04T09:57:01Z

I believe this will also close #106

Handle errors in `start` function for better user experience and to be able to catch errors in test environment, for example without introducing error handling, we do not get a proper response if the provided `coinbase_output` in the config is valid.

Sniffer allows to intercept messages sent between two roles. The sniffer sets between the downstream and upstream roles, aggregates messages sent by the downstream and upstream roles in two separate FIFO queues. The messages are then forwarded to the target role without any modification. It is useful for testing purposes, as it allows to assert that the roles have sent specific messages in a specific order and to inspect the messages details.

A utility struct that wraps the original `PoolSv2` and allows to start the pool role in testing env.

An abstraction above the `TemplateProvider` struct to start TP and mine needed blocks in a single function call.

Stopps the TP running instance in case of test failure.

Starts a Template Provider and Pool and checks that both roles send the corrects messages on first connection.

Align `tests-integration` rust version with the other roles inside the `roles` workspace, otherwise weird problems pops in CI.

GitGab19

LGTM.
This is a great PR, I'm excited to see how many tests we will add thanks to this.
Just left some minor comments

GitGab19 · 2024-10-08T13:22:57Z

roles/tests-integration/tests/common/mod.rs

 };
 use tar::Archive;

+// prevents get_available_port from ever returning the same port twice
+static UNIQUE_PORTS: Lazy<Mutex<HashSet<u16>>> = Lazy::new(|| Mutex::new(HashSet::new()));
+
 const VERSION_TP: &str = "0.1.7";


I would use the latest version release, which seems the most stable, 0.1.9: https://github.com/Sjors/bitcoin/releases/tag/sv2-tp-0.1.9

GitGab19 · 2024-10-08T13:30:10Z

roles/tests-integration/tests/common/mod.rs

+    tokio::task::spawn(async move {
+        assert!(pool_clone.start().await.is_ok());
+    });
+    tokio::time::sleep(std::time::Duration::from_secs(1)).await;


Why there's this sleep here?

To give the pool a second to send/receive the messages, otherwise we start asserting for messages before the pool received a response on some of them

maybe add a comment to make it clear?

GitGab19 · 2024-10-08T13:31:44Z

roles/tests-integration/tests/common/sniffer.rs

+
+/// Allows to intercept messages sent between two roles.
+///
+/// The downstream(or client) role connects to the [`Sniffer`] `listening_address` and the


Suggested change

/// The downstream(or client) role connects to the [`Sniffer`] `listening_address` and the

/// The downstream (or client) role connects to the [`Sniffer`] `listening_address` and the

GitGab19 · 2024-10-08T13:32:32Z

roles/tests-integration/tests/common/sniffer.rs

+/// the upstream role. And when a response is received it is saved in `upstream_messages` and
+/// forwareded to the downstream role. Both `downstream_messages` and `upstream_messages` can be


Suggested change

/// the upstream role. And when a response is received it is saved in `upstream_messages` and

/// forwareded to the downstream role. Both `downstream_messages` and `upstream_messages` can be

/// the upstream role. When a response is received it is saved in `upstream_messages` and

/// forwarded to the downstream role. Both `downstream_messages` and `upstream_messages` can be

GitGab19 · 2024-10-08T13:33:27Z

roles/tests-integration/tests/common/sniffer.rs

+#[derive(Debug, Clone)]
+pub struct Sniffer {
+    listening_address: SocketAddr,
+    upstream: SocketAddr,


Why not call this upstream_address? For consistency

GitGab19 · 2024-10-08T13:41:17Z

roles/tests-integration/tests/common/sniffer.rs

+        if let Ok((stream, _)) = listner.accept().await {
+            stream
+        } else {
+            panic!("Impossible to accept dowsntream connetion")


Suggested change

panic!("Impossible to accept dowsntream connetion")

panic!("Impossible to accept dowsntream connection")

GitGab19 · 2024-10-08T13:41:46Z

roles/tests-integration/tests/common/sniffer.rs

+// This is useful to ensure that the test has checked all exchanged messages between the roles.
+impl Drop for Sniffer {
+    fn drop(&mut self) {
+        // Dont print backtrace on panic


Suggested change

// Dont print backtrace on panic

// Don't print backtrace on panic

GitGab19 · 2024-10-08T13:41:57Z

roles/tests-integration/tests/common/sniffer.rs

+        }));
+        if !self.downstream_messages.is_empty() {
+            println!(
+                "You didnt handle all downstream messages: {:?}",


Suggested change

"You didnt handle all downstream messages: {:?}",

"You didn't handle all downstream messages: {:?}",

GitGab19 · 2024-10-08T13:42:08Z

roles/tests-integration/tests/common/sniffer.rs

+        }
+        if !self.upstream_messages.is_empty() {
+            println!(
+                "You didnt handle all upstream messages: {:?}",


Suggested change

"You didnt handle all upstream messages: {:?}",

"You didn't handle all upstream messages: {:?}",

GitGab19 · 2024-10-08T13:43:37Z

roles/tests-integration/tests/pool_integration.rs

+    parsers::{CommonMessages, PoolMessages},
+};
+
+#[tokio::test]


Shouldn't we add a description of the test? I think it's a good practice since we are going to have a lot of them

jbesraa · 2024-10-08T14:05:26Z

Thanks @GitGab19
Addressed your review in 5e2d3fc and 506bc88 and 4538991 .

Will fixup the first two once reviewed again.

plebhash · 2024-10-08T15:03:59Z

I believe this will also close #106

@pavlenex hmm not really

as we start addressing #1121 we might add a few unit tests to pool, which will indeed fall under #106

but that doesn't necessarily mean that the scope of #106 will be fully addressed, since MG tests are not comprehensive

ideally we should eventually jump into #106 as a task on its own... but I'd leave that for some time in the future

Shourya742

Looks good at first glance. I'll provide a detailed review by tomorrow.

Shourya742 · 2024-10-08T12:12:02Z

roles/tests-integration/Cargo.toml

@@ -1,13 +1,24 @@
 [package]
 name = "integration-test"
 version = "0.1.0"
-edition = "2021"
+edition = "2018"


Shourya742 · 2024-10-08T12:31:07Z

roles/tests-integration/tests/common/sniffer.rs

+    async fn create_upstream(
+        stream: TcpStream,
+    ) -> Option<(Receiver<MessageFrame>, Sender<MessageFrame>)> {
+        let initiator = Initiator::without_pk().expect("This fn call can not fail");


any specific reason for without_pk?

roles/tests-integration/tests/common/sniffer.rs

Shourya742 · 2024-10-08T12:48:43Z

roles/tests-integration/tests/common/sniffer.rs

+            .safe_lock(|messages| messages.is_empty())
+            .unwrap()
+    }
+


Can we add a front method as well. Because sometime we might not want to consume the element to refer.

hmm can you elaborate?

under which circumstances do you see it being useful to look at the message without popping it from the FIFO?

plebhash · 2024-10-08T15:47:27Z

roles/tests-integration/tests/pool_integration.rs

+    let _tp = common::start_template_provider(tp_addr.port()).await;
+    let sniffer = common::start_sniffer(sniffer_addr, tp_addr).await;
+    let _ = common::start_pool(Some(pool_addr), Some(sniffer_addr)).await;
+    match sniffer.next_downstream_message() {


since this is the first ever test, we will be using it as a reference to write new tests in the future
so it makes sense to add some extra comments to guide us in the future

Suggested change

match sniffer.next_downstream_message() {

// assert message type as well as specific message fields

match sniffer.next_downstream_message() {

I guess we discussed something along these lines on PR Review call last week:

I wonder if we can encapsulate this match logic into some reusable pattern via macro magic?

just to avoid all this match verbosity inside the tests

just a high level suggestion though, I'm not very good with macros so I'm not sure what would be really possible here

First thing which comes in my mind is to have a function like:
expect_downstream_message_field which gets msg_type, msg_field, and msg_field_value as parameters, and then it compares the next message got from the queue with the value passed as parameter.

First thing which comes in my mind is to have a function like: expect_downstream_message_field which gets msg_type, msg_field, and msg_field_value as parameters, and then it compares the next message got from the queue with the value passed as parameter.

this will probably be hard to achieve since we don't know the type for msg_field_value before hand

that's why I believe macro is the only viable option

anyways my suggestion above doesn't need to be implemented on this PR, I can live with the match statements for now

as @jbesraa proposed, we should aim to merge this and move on to #1121

as we make progress on #1121, we can still come back and make adjustments to #1122

Totally agree. To me this one is good to go

before merging, I still thing it would be good to add the comments on the suggestion that started this thread

plebhash · 2024-10-08T15:47:47Z

roles/tests-integration/tests/pool_integration.rs

+        }
+        _ => panic!("Pool did not send a message to template provider"),
+    };
+    assert!(sniffer.expect_upstream_message(MESSAGE_TYPE_SETUP_CONNECTION_SUCCESS));


Suggested change

assert!(sniffer.expect_upstream_message(MESSAGE_TYPE_SETUP_CONNECTION_SUCCESS));

// only assert message type

assert!(sniffer.expect_upstream_message(MESSAGE_TYPE_SETUP_CONNECTION_SUCCESS));

roles/tests-integration/tests/common/sniffer.rs

jbesraa changed the title ~~2024 07 15 roles testing~~ Roles integration tests Jul 19, 2024

jbesraa force-pushed the 2024-07-15-roles-testing branch from 0dde2c8 to 9dc8a6c Compare July 19, 2024 09:00

jbesraa mentioned this pull request Jul 19, 2024

Graceful shutdown for mining-proxy #1021

Merged

jbesraa force-pushed the 2024-07-15-roles-testing branch 3 times, most recently from 8ace1ac to e90b991 Compare July 24, 2024 09:36

pavlenex added this to the 1.1.0 milestone Jul 30, 2024

plebhash reviewed Jul 30, 2024

View reviewed changes

roles/tests-integration/src/main.rs Outdated Show resolved Hide resolved

plebhash reviewed Jul 30, 2024

View reviewed changes

roles/jd-client/src/lib/mod.rs Outdated Show resolved Hide resolved

plebhash reviewed Jul 30, 2024

View reviewed changes

roles/jd-client/src/lib/mod.rs Outdated Show resolved Hide resolved

plebhash reviewed Jul 30, 2024

View reviewed changes

roles/pool/config-examples/integration-tests.toml Outdated Show resolved Hide resolved

plebhash mentioned this pull request Jul 31, 2024

issues lacking CI #1077

Open

jbesraa force-pushed the 2024-07-15-roles-testing branch 2 times, most recently from c0dbde4 to fbb8464 Compare August 5, 2024 14:20

plebhash mentioned this pull request Aug 5, 2024

Translator restart if disconnected from upstream #1001

Merged

jbesraa force-pushed the 2024-07-15-roles-testing branch from 56c26c0 to 6cfc1c7 Compare August 6, 2024 15:29

plebhash reviewed Aug 6, 2024

View reviewed changes

roles/tests-integration/tests/common/mod.rs Show resolved Hide resolved

plebhash reviewed Aug 7, 2024

View reviewed changes

roles/pool/src/lib/mod.rs Outdated Show resolved Hide resolved

jbesraa force-pushed the 2024-07-15-roles-testing branch 8 times, most recently from a3f154b to 0be5cc6 Compare August 11, 2024 15:25

plebhash reviewed Oct 3, 2024

View reviewed changes

roles/tests-integration/tests/common/sniffer.rs Outdated Show resolved Hide resolved

jbesraa force-pushed the 2024-07-15-roles-testing branch from 2047b03 to c0b499f Compare October 4, 2024 09:35

jbesraa added 7 commits October 7, 2024 12:46

Remove unused fn

15a09ca

Add networking/port test utils fn(s)

fe94154

Add TestPoolSv2

151ecae

A utility struct that wraps the original `PoolSv2` and allows to start the pool role in testing env.

Add start_template_provider

db27d44

An abstraction above the `TemplateProvider` struct to start TP and mine needed blocks in a single function call.

Impl Drop for TemplateProvider

8a59fda

Stopps the TP running instance in case of test failure.

jbesraa force-pushed the 2024-07-15-roles-testing branch from c0b499f to d3fa7c3 Compare October 7, 2024 13:39

jbesraa added 2 commits October 7, 2024 16:40

Test connection setup between pool and template_provider

338cc02

Starts a Template Provider and Pool and checks that both roles send the corrects messages on first connection.

Change version in tests-integration

d1126aa

Align `tests-integration` rust version with the other roles inside the `roles` workspace, otherwise weird problems pops in CI.

jbesraa force-pushed the 2024-07-15-roles-testing branch from d3fa7c3 to e99e36b Compare October 7, 2024 13:40

f sniffer

f4e1cb0

jbesraa force-pushed the 2024-07-15-roles-testing branch from e99e36b to f4e1cb0 Compare October 7, 2024 13:53

GitGab19 reviewed Oct 8, 2024

View reviewed changes

jbesraa added 3 commits October 8, 2024 16:54

Upgrade TP test module version to 0.1.9

4538991

f sniffer

5e2d3fc

f test docs

506bc88

plebhash mentioned this pull request Oct 8, 2024

mining-device cpu miner does not hit the target #1188

Closed

Shourya742 suggested changes Oct 8, 2024

View reviewed changes

plebhash reviewed Oct 8, 2024

View reviewed changes

Shourya742 reviewed Oct 9, 2024

View reviewed changes

roles/tests-integration/tests/common/sniffer.rs Show resolved Hide resolved

	/// The downstream(or client) role connects to the [`Sniffer`] `listening_address` and the
	/// The downstream (or client) role connects to the [`Sniffer`] `listening_address` and the

		/// the upstream role. And when a response is received it is saved in `upstream_messages` and
		/// forwareded to the downstream role. Both `downstream_messages` and `upstream_messages` can be

	panic!("Impossible to accept dowsntream connetion")
	panic!("Impossible to accept dowsntream connection")

	// Dont print backtrace on panic
	// Don't print backtrace on panic

	"You didnt handle all downstream messages: {:?}",
	"You didn't handle all downstream messages: {:?}",

	"You didnt handle all upstream messages: {:?}",
	"You didn't handle all upstream messages: {:?}",

	match sniffer.next_downstream_message() {
	// assert message type as well as specific message fields
	match sniffer.next_downstream_message() {

	assert!(sniffer.expect_upstream_message(MESSAGE_TYPE_SETUP_CONNECTION_SUCCESS));
	// only assert message type
	assert!(sniffer.expect_upstream_message(MESSAGE_TYPE_SETUP_CONNECTION_SUCCESS));

PoolSv2 integration tests #1066

Are you sure you want to change the base?

PoolSv2 integration tests #1066

Conversation

jbesraa commented Jul 19, 2024 • edited Loading

github-actions bot commented Jul 19, 2024 • edited Loading

Bencher

github-actions bot commented Jul 19, 2024 • edited Loading

github-actions bot commented Jul 19, 2024 • edited Loading

Bencher

github-actions bot commented Jul 19, 2024 • edited Loading

Bencher

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbesraa Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbesraa commented Oct 4, 2024

pavlenex commented Oct 4, 2024

GitGab19 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbesraa Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbesraa commented Oct 8, 2024

plebhash commented Oct 8, 2024 • edited Loading

Shourya742 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

plebhash Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

`PoolSv2` integration tests #1066

`PoolSv2` integration tests #1066

jbesraa commented Jul 19, 2024 •

edited

Loading

github-actions bot commented Jul 19, 2024 •

edited

Loading

github-actions bot commented Jul 19, 2024 •

edited

Loading

github-actions bot commented Jul 19, 2024 •

edited

Loading

github-actions bot commented Jul 19, 2024 •

edited

Loading

jbesraa Oct 7, 2024 •

edited

Loading

jbesraa Oct 8, 2024 •

edited

Loading

plebhash commented Oct 8, 2024 •

edited

Loading

plebhash Oct 8, 2024 •

edited

Loading