Add test for unknown message fields #1259
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 13 commits
November 16, 2024 10:27
Coinbase signature is not part of the Sv2 protocol, some pool maybe want to use it other not. The first part of the extranonce could also be reserved for things that are not a pool signature. This pr rename the pool_signature field of the channel factory into additional_coinbase_script_data and change the type from Strgin to Vec<u8>, since can be anything.
The coinbase input script additional data should be sent as part of the extranonce_prefix and not as part of the coinbase_prefix. So that a JDC can see what the pool want as coinbase input script additional data without the need to observ the coinbase prefix in job constructed by the pool.
Update the pool to use an extranonce of 16 bytes rather then 32 so that there is enaugh space to add the additional coinbase input script data.
Right now the channel factory only support one active job at time. That means that if we receive a share for a job right after we sent downstream a new job that share will be invalid. Now the channel factory keep track of the last 3 jobs, so we give time to the dowstream to receive the job and propagate it down before stop accepting shares for older job. This is useful, and the system can be more responsive: as soon as we change the coinbase additional input script data we can send a new job dowsntream without worrying of invalidating miner's shares. When the pool receive a prev hash it immidiatly invalidate all the previous jobs, we still want to refuse shares for stale jobs. The client can easly handle this situation: when a pool refuse a share it should start a timer and if do not receive a new prev hash (or already have) within n seconds it change pool.
This commit fix 2 miner things: When we calculate the coinbase_prefix (what we need to put in the extended job) we need to account also for the coinbase input script additional data that is part of the extranonce. When we create pool channel facotry we pass an extranonce creator and an pool signature. If the signature + extranonce are bigger then 32 bytes we have to return an error. That cause in sv2 the extranonce can not be longer than 32 bytes.
The translator normalize the coinbase and remove the segwit data from the coinbase prefix and suffix. In order to do that it need to know the extranonce len, we used a default value of 322 bytes, but the pool could use also smalle extranonces.
Add a method that the pool can use in order to change coinbase_script_additional_data of an already opened channel and send downstream the new extranonce_prefix
…e data then expected
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1259 +/- ##
==========================================
+ Coverage 19.30% 19.88% +0.58%
==========================================
Files 164 164
Lines 10849 11013 +164
==========================================
+ Hits 2094 2190 +96
- Misses 8755 8823 +68
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Bencher Report
|
| Branch | AddTestForUnknownMessageFields |
| Testbed | sv2 |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result nanoseconds (ns) (Result Δ%) | Upper Boundary nanoseconds (ns) (Limit %) |
|---|---|---|---|
| client_sv2_handle_message_common | 📈 view plot 🚷 view threshold | 44.59 (-0.07%) | 47.41 (94.04%) |
| client_sv2_handle_message_mining | 📈 view plot 🚷 view threshold | 74.95 (-0.76%) | 85.30 (87.88%) |
| client_sv2_mining_message_submit_standard | 📈 view plot 🚷 view threshold | 14.66 (-0.03%) | 14.71 (99.62%) |
| client_sv2_mining_message_submit_standard_serialize | 📈 view plot 🚷 view threshold | 275.79 (+2.88%) | 321.04 (85.90%) |
| client_sv2_mining_message_submit_standard_serialize_deserialize | 📈 view plot 🚷 view threshold | 622.27 (+3.82%) | 726.86 (85.61%) |
| client_sv2_open_channel | 📈 view plot 🚷 view threshold | 147.58 (-0.02%) | 157.94 (93.44%) |
| client_sv2_open_channel_serialize | 📈 view plot 🚷 view threshold | 283.23 (-0.57%) | 318.21 (89.01%) |
| client_sv2_open_channel_serialize_deserialize | 📈 view plot 🚷 view threshold | 379.74 (+0.70%) | 411.18 (92.35%) |
| client_sv2_setup_connection | 📈 view plot 🚷 view threshold | 161.20 (-0.11%) | 176.60 (91.28%) |
| client_sv2_setup_connection_serialize | 📈 view plot 🚷 view threshold | 504.37 (+7.30%) | 603.30 (83.60%) |
| client_sv2_setup_connection_serialize_deserialize | 📈 view plot 🚷 view threshold | 977.29 (+0.67%) | 1,065.59 (91.71%) |
|
| Branch | AddTestForUnknownMessageFields |
| Testbed | sv1 |
Click to view all benchmark results
| Benchmark | Estimated Cycles | Benchmark Result estimated cycles (Result Δ%) | Upper Boundary estimated cycles (Limit %) | Instructions | Benchmark Result instructions (Result Δ%) | Upper Boundary instructions (Limit %) | L1 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | L2 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | RAM Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| get_authorize | 📈 view plot 🚷 view threshold | 8,372.00 (-0.98%) | 8,831.90 (94.79%) | 📈 view plot 🚷 view threshold | 3,663.00 (-2.23%) | 3,894.75 (94.05%) | 📈 view plot 🚷 view threshold | 5,112.00 (-2.70%) | 5,467.02 (93.51%) | 📈 view plot 🚷 view threshold | 8.00 (+11.18%) | 12.99 (61.57%) | 📈 view plot 🚷 view threshold | 92.00 (+1.73%) | 96.22 (95.62%) |
| get_submit | 📈 view plot 🚷 view threshold | 95,258.00 (-0.17%) | 95,989.85 (99.24%) | 📈 view plot 🚷 view threshold | 59,263.00 (-0.33%) | 59,715.76 (99.24%) | 📈 view plot 🚷 view threshold | 85,083.00 (-0.37%) | 85,791.41 (99.17%) | 📈 view plot 🚷 view threshold | 40.00 (-11.06%) | 64.01 (62.49%) | 📈 view plot 🚷 view threshold | 285.00 (+1.84%) | 289.41 (98.48%) |
| get_subscribe | 📈 view plot 🚷 view threshold | 7,935.00 (-0.97%) | 8,388.89 (94.59%) | 📈 view plot 🚷 view threshold | 2,758.00 (-2.63%) | 2,973.93 (92.74%) | 📈 view plot 🚷 view threshold | 3,830.00 (-3.27%) | 4,158.92 (92.09%) | 📈 view plot 🚷 view threshold | 16.00 (+33.07%) | 20.87 (76.65%) | 📈 view plot 🚷 view threshold | 115.00 (+0.79%) | 120.17 (95.70%) |
| serialize_authorize | 📈 view plot 🚷 view threshold | 12,119.00 (-1.25%) | 12,657.37 (95.75%) | 📈 view plot 🚷 view threshold | 5,240.00 (-1.46%) | 5,464.29 (95.90%) | 📈 view plot 🚷 view threshold | 7,284.00 (-1.78%) | 7,627.79 (95.49%) | 📈 view plot 🚷 view threshold | 8.00 (-11.88%) | 15.92 (50.26%) | 📈 view plot 🚷 view threshold | 137.00 (-0.32%) | 143.64 (95.38%) |
| serialize_deserialize_authorize | 📈 view plot 🚷 view threshold | 24,538.00 (-0.59%) | 25,280.80 (97.06%) | 📈 view plot 🚷 view threshold | 9,786.00 (-1.04%) | 10,059.86 (97.28%) | 📈 view plot 🚷 view threshold | 13,788.00 (-1.24%) | 14,218.84 (96.97%) | 📈 view plot 🚷 view threshold | 36.00 (+3.43%) | 44.39 (81.11%) | 📈 view plot 🚷 view threshold | 302.00 (+0.21%) | 315.61 (95.69%) |
| serialize_deserialize_handle_authorize | 📈 view plot 🚷 view threshold | 30,325.00 (+0.02%) | 30,789.10 (98.49%) | 📈 view plot 🚷 view threshold | 11,989.00 (-0.75%) | 12,229.79 (98.03%) | 📈 view plot 🚷 view threshold | 16,950.00 (-0.91%) | 17,327.08 (97.82%) | 📈 view plot 🚷 view threshold | 57.00 (+2.27%) | 68.30 (83.45%) | 📈 view plot 🚷 view threshold | 374.00 (+1.21%) | 381.88 (97.94%) |
| serialize_deserialize_handle_submit | 📈 view plot 🚷 view threshold | 126,266.00 (-0.15%) | 127,085.18 (99.36%) | 📈 view plot 🚷 view threshold | 73,117.00 (-0.22%) | 73,600.03 (99.34%) | 📈 view plot 🚷 view threshold | 104,771.00 (-0.27%) | 105,551.63 (99.26%) | 📈 view plot 🚷 view threshold | 106.00 (-1.02%) | 133.34 (79.50%) | 📈 view plot 🚷 view threshold | 599.00 (+0.47%) | 605.58 (98.91%) |
| serialize_deserialize_handle_subscribe | 📈 view plot 🚷 view threshold | 27,893.00 (+0.23%) | 28,645.49 (97.37%) | 📈 view plot 🚷 view threshold | 9,577.00 (-0.72%) | 9,792.54 (97.80%) | 📈 view plot 🚷 view threshold | 13,518.00 (-0.92%) | 13,855.70 (97.56%) | 📈 view plot 🚷 view threshold | 68.00 (+7.10%) | 76.09 (89.37%) | 📈 view plot 🚷 view threshold | 401.00 (+1.20%) | 417.19 (96.12%) |
| serialize_deserialize_submit | 📈 view plot 🚷 view threshold | 114,849.00 (-0.33%) | 115,936.41 (99.06%) | 📈 view plot 🚷 view threshold | 67,894.00 (-0.30%) | 68,441.26 (99.20%) | 📈 view plot 🚷 view threshold | 97,379.00 (-0.35%) | 98,275.08 (99.09%) | 📈 view plot 🚷 view threshold | 57.00 (-9.07%) | 78.31 (72.79%) | 📈 view plot 🚷 view threshold | 491.00 (0.00%) | 499.69 (98.26%) |
| serialize_deserialize_subscribe | 📈 view plot 🚷 view threshold | 23,134.00 (-0.48%) | 24,069.29 (96.11%) | 📈 view plot 🚷 view threshold | 8,129.00 (-0.88%) | 8,349.35 (97.36%) | 📈 view plot 🚷 view threshold | 11,429.00 (-1.07%) | 11,763.15 (97.16%) | 📈 view plot 🚷 view threshold | 38.00 (+1.80%) | 44.67 (85.07%) | 📈 view plot 🚷 view threshold | 329.00 (+0.07%) | 348.87 (94.30%) |
| serialize_submit | 📈 view plot 🚷 view threshold | 99,563.00 (-0.27%) | 100,391.63 (99.17%) | 📈 view plot 🚷 view threshold | 61,325.00 (-0.29%) | 61,753.86 (99.31%) | 📈 view plot 🚷 view threshold | 87,948.00 (-0.34%) | 88,629.54 (99.23%) | 📈 view plot 🚷 view threshold | 41.00 (-12.39%) | 64.73 (63.34%) | 📈 view plot 🚷 view threshold | 326.00 (+0.47%) | 334.01 (97.60%) |
| serialize_subscribe | 📈 view plot 🚷 view threshold | 11,266.00 (-1.12%) | 11,770.97 (95.71%) | 📈 view plot 🚷 view threshold | 4,111.00 (-1.65%) | 4,319.39 (95.18%) | 📈 view plot 🚷 view threshold | 5,696.00 (-2.07%) | 6,013.79 (94.72%) | 📈 view plot 🚷 view threshold | 15.00 (+16.79%) | 20.64 (72.68%) | 📈 view plot 🚷 view threshold | 157.00 (-0.33%) | 164.15 (95.64%) |
|
| Branch | AddTestForUnknownMessageFields |
| Testbed | sv2 |
🚨 11 Alerts
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Upper Boundary (Limit %) |
|---|---|---|---|---|
| client_sv2_handle_message_mining | L2 Accesses accesses | 📈 plot 🚨 alert (🔔) 🚷 threshold | 43.00 (+20.73%) | 42.57 (101.00%) |
| client_sv2_mining_message_submit_standard_serialize | L2 Accesses accesses | 📈 plot 🚨 alert (🔔) 🚷 threshold | 56.00 (+28.46%) | 54.07 (103.57%) |
| client_sv2_mining_message_submit_standard_serialize_deserialize | Estimated Cycles estimated cycles | 📈 plot 🚨 alert (🔔) 🚷 threshold | 27,766.00 (+0.86%) | 27,756.72 (100.03%) |
| client_sv2_mining_message_submit_standard_serialize_deserialize | L2 Accesses accesses | 📈 plot 🚨 alert (🔔) 🚷 threshold | 94.00 (+15.39%) | 90.68 (103.66%) |
| client_sv2_mining_message_submit_standard_serialize_deserialize | L1 Accesses accesses | 📈 plot 🚨 alert (🔔) 🚷 threshold | 15,501.00 (+0.68%) | 15,498.72 (100.01%) |
| client_sv2_open_channel_serialize_deserialize | Estimated Cycles estimated cycles | 📈 plot 🚨 alert (🔔) 🚷 threshold | 22,958.00 (+1.30%) | 22,912.13 (100.20%) |
| client_sv2_open_channel_serialize_deserialize | L2 Accesses accesses | 📈 plot 🚨 alert (🔔) 🚷 threshold | 93.00 (+24.87%) | 86.31 (107.75%) |
| client_sv2_setup_connection | L2 Accesses accesses | 📈 plot 🚨 alert (🔔) 🚷 threshold | 17.00 (+80.58%) | 16.26 (104.53%) |
| client_sv2_setup_connection_serialize | L2 Accesses accesses | 📈 plot 🚨 alert (🔔) 🚷 threshold | 56.00 (+39.75%) | 54.23 (103.27%) |
| client_sv2_setup_connection_serialize_deserialize | Estimated Cycles estimated cycles | 📈 plot 🚨 alert (🔔) 🚷 threshold | 35,918.00 (+1.03%) | 35,771.03 (100.41%) |
| client_sv2_setup_connection_serialize_deserialize | RAM Accesses accesses | 📈 plot 🚨 alert (🔔) 🚷 threshold | 386.00 (+1.77%) | 384.95 (100.27%) |
Click to view all benchmark results
| Benchmark | Estimated Cycles | Benchmark Result estimated cycles (Result Δ%) | Upper Boundary estimated cycles (Limit %) | Instructions | Benchmark Result instructions (Result Δ%) | Upper Boundary instructions (Limit %) | L1 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | L2 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | RAM Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| client_sv2_handle_message_common | 📈 view plot 🚷 view threshold | 2,179.00 (+4.07%) | 2,208.84 (98.65%) | 📈 view plot 🚷 view threshold | 473.00 (-0.05%) | 488.63 (96.80%) | 📈 view plot 🚷 view threshold | 729.00 (-0.92%) | 757.61 (96.22%) | 📈 view plot 🚷 view threshold | 10.00 (+82.34%) | 14.27 (70.08%) | 📈 view plot 🚷 view threshold | 40.00 (+5.22%) | 41.22 (97.04%) |
| client_sv2_handle_message_mining | 📈 view plot 🚷 view threshold | 8,232.00 (+0.33%) | 8,288.27 (99.32%) | 📈 view plot 🚷 view threshold | 2,137.00 (-0.03%) | 2,143.08 (99.72%) | 📈 view plot 🚷 view threshold | 3,152.00 (-0.28%) | 3,170.82 (99.41%) | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 43.00 (+20.73%) | 42.57 (101.00%) | 📈 view plot 🚷 view threshold | 139.00 (-0.02%) | 140.92 (98.64%) |
| client_sv2_mining_message_submit_standard | 📈 view plot 🚷 view threshold | 6,263.00 (-0.25%) | 6,388.83 (98.03%) | 📈 view plot 🚷 view threshold | 1,750.00 (-0.05%) | 1,767.00 (99.04%) | 📈 view plot 🚷 view threshold | 2,548.00 (-0.22%) | 2,575.89 (98.92%) | 📈 view plot 🚷 view threshold | 22.00 (+29.35%) | 24.75 (88.88%) | 📈 view plot 🚷 view threshold | 103.00 (-0.95%) | 107.03 (96.23%) |
| client_sv2_mining_message_submit_standard_serialize | 📈 view plot 🚷 view threshold | 14,828.00 (+0.66%) | 14,964.01 (99.09%) | 📈 view plot 🚷 view threshold | 4,694.00 (-0.02%) | 4,711.00 (99.64%) | 📈 view plot 🚷 view threshold | 6,743.00 (-0.23%) | 6,782.65 (99.42%) | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 56.00 (+28.46%) | 54.07 (103.57%) | 📈 view plot 🚷 view threshold | 223.00 (+0.66%) | 227.64 (97.96%) |
| client_sv2_mining_message_submit_standard_serialize_deserialize | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 27,766.00 (+0.86%) | 27,756.72 (100.03%) | 📈 view plot 🚷 view threshold | 10,645.00 (+0.61%) | 10,645.64 (99.99%) | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 15,501.00 (+0.68%) | 15,498.72 (100.01%) | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 94.00 (+15.39%) | 90.68 (103.66%) | 📈 view plot 🚷 view threshold | 337.00 (+0.59%) | 341.28 (98.75%) |
| client_sv2_open_channel | 📈 view plot 🚷 view threshold | 4,459.00 (+1.41%) | 4,610.00 (96.72%) | 📈 view plot 🚷 view threshold | 1,461.00 (-0.02%) | 1,476.63 (98.94%) | 📈 view plot 🚷 view threshold | 2,154.00 (-0.26%) | 2,184.94 (98.58%) | 📈 view plot 🚷 view threshold | 13.00 (+44.82%) | 17.69 (73.51%) | 📈 view plot 🚷 view threshold | 64.00 (+2.17%) | 68.33 (93.67%) |
| client_sv2_open_channel_serialize | 📈 view plot 🚷 view threshold | 14,214.00 (+1.10%) | 14,443.68 (98.41%) | 📈 view plot 🚷 view threshold | 5,064.00 (-0.01%) | 5,079.63 (99.69%) | 📈 view plot 🚷 view threshold | 7,314.00 (-0.16%) | 7,352.73 (99.47%) | 📈 view plot 🚷 view threshold | 43.00 (+22.26%) | 43.65 (98.51%) | 📈 view plot 🚷 view threshold | 191.00 (+1.93%) | 198.40 (96.27%) |
| client_sv2_open_channel_serialize_deserialize | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 22,958.00 (+1.30%) | 22,912.13 (100.20%) | 📈 view plot 🚷 view threshold | 8,040.00 (+0.25%) | 8,074.46 (99.57%) | 📈 view plot 🚷 view threshold | 11,678.00 (+0.09%) | 11,754.01 (99.35%) | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 93.00 (+24.87%) | 86.31 (107.75%) | 📈 view plot 🚷 view threshold | 309.00 (+1.80%) | 311.68 (99.14%) |
| client_sv2_setup_connection | 📈 view plot 🚷 view threshold | 4,769.00 (+1.80%) | 4,775.94 (99.85%) | 📈 view plot 🚷 view threshold | 1,502.00 (-0.02%) | 1,517.63 (98.97%) | 📈 view plot 🚷 view threshold | 2,269.00 (-0.41%) | 2,299.43 (98.68%) | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 17.00 (+80.58%) | 16.26 (104.53%) | 📈 view plot 🚷 view threshold | 69.00 (+2.36%) | 69.85 (98.78%) |
| client_sv2_setup_connection_serialize | 📈 view plot 🚷 view threshold | 16,344.00 (+1.06%) | 16,467.07 (99.25%) | 📈 view plot 🚷 view threshold | 5,963.00 (-0.00%) | 5,978.63 (99.74%) | 📈 view plot 🚷 view threshold | 8,644.00 (-0.22%) | 8,692.60 (99.44%) | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 56.00 (+39.75%) | 54.23 (103.27%) | 📈 view plot 🚷 view threshold | 212.00 (+1.51%) | 216.45 (97.95%) |
| client_sv2_setup_connection_serialize_deserialize | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 35,918.00 (+1.03%) | 35,771.03 (100.41%) | 📈 view plot 🚷 view threshold | 14,888.00 (+0.27%) | 14,907.31 (99.87%) | 📈 view plot 🚷 view threshold | 21,863.00 (+0.23%) | 21,918.56 (99.75%) | 📈 view plot 🚷 view threshold | 109.00 (+17.39%) | 114.37 (95.31%) | 📈 view plot 🚨 view alert (🔔) 🚷 view threshold | 386.00 (+1.77%) | 384.95 (100.27%) |
Add exmaples on ping pong with and without noise, of how we can handle messages that have additional field. In the without noise example, the receiver is aware of the additional field and can receive both messages with an wihout the additional field. In order to do that it try **before** to parse the message with the additional field, and if it fail it try to parse it without. In the with noise example, we have a receiver that is only aware of the message without the additional field, and a sender that send it with the additional field, the sender ignore the additional field without failing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
to merge after #1248