Closed GitGab19 closed 1 month ago
Branch | 1160/merge |
Testbed | sv1 |
Branch | 1160/merge |
Testbed | sv2 |
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,137.00 (+2.82%) | 2,210.45 (96.68%) | 📈 view plot 🚷 view threshold | 473.00 (+0.15%) | 488.86 (96.76%) | 📈 view plot 🚷 view threshold | 732.00 (-0.35%) | 758.70 (96.48%) | 📈 view plot 🚷 view threshold | 8.00 (+38.75%) | 14.13 (56.62%) | 📈 view plot 🚷 view threshold | 39.00 (+3.81%) | 41.33 (94.37%) |
client_sv2_handle_message_mining | 📈 view plot 🚷 view threshold | 8,208.00 (+0.07%) | 8,346.16 (98.34%) | 📈 view plot 🚷 view threshold | 2,137.00 (+0.10%) | 2,173.42 (98.32%) | 📈 view plot 🚷 view threshold | 3,158.00 (+0.05%) | 3,217.43 (98.15%) | 📈 view plot 🚷 view threshold | 37.00 (+1.43%) | 44.56 (83.03%) | 📈 view plot 🚷 view threshold | 139.00 (+0.03%) | 142.32 (97.67%) |
client_sv2_mining_message_submit_standard | 📈 view plot 🚷 view threshold | 6,335.00 (+0.90%) | 6,412.01 (98.80%) | 📈 view plot 🚷 view threshold | 1,750.00 (-0.02%) | 1,766.87 (99.05%) | 📈 view plot 🚷 view threshold | 2,545.00 (-0.34%) | 2,578.59 (98.70%) | 📈 view plot 🚷 view threshold | 23.00 (+35.67%) | 24.43 (94.16%) | 📈 view plot 🚷 view threshold | 105.00 (+0.96%) | 107.72 (97.47%) |
client_sv2_mining_message_submit_standard_serialize | 📈 view plot 🚷 view threshold | 14,668.00 (-0.53%) | 15,052.57 (97.45%) | 📈 view plot 🚷 view threshold | 4,694.00 (-0.01%) | 4,710.87 (99.64%) | 📈 view plot 🚷 view threshold | 6,753.00 (-0.06%) | 6,782.11 (99.57%) | 📈 view plot 🚷 view threshold | 50.00 (+12.46%) | 55.21 (90.56%) | 📈 view plot 🚷 view threshold | 219.00 (-1.31%) | 230.44 (95.04%) |
client_sv2_mining_message_submit_standard_serialize_deserialize | 📈 view plot 🚷 view threshold | 27,484.00 (-0.08%) | 27,890.49 (98.54%) | 📈 view plot 🚷 view threshold | 10,585.00 (+0.16%) | 10,638.78 (99.49%) | 📈 view plot 🚷 view threshold | 15,399.00 (+0.14%) | 15,480.99 (99.47%) | 📈 view plot 🚷 view threshold | 86.00 (+5.34%) | 90.41 (95.13%) | 📈 view plot 🚷 view threshold | 333.00 (-0.56%) | 345.84 (96.29%) |
client_sv2_open_channel | 📈 view plot 🚷 view threshold | 4,413.00 (-0.17%) | 4,667.65 (94.54%) | 📈 view plot 🚷 view threshold | 1,461.00 (+0.01%) | 1,476.34 (98.96%) | 📈 view plot 🚷 view threshold | 2,158.00 (+0.02%) | 2,184.32 (98.80%) | 📈 view plot 🚷 view threshold | 10.00 (+3.23%) | 18.49 (54.09%) | 📈 view plot 🚷 view threshold | 63.00 (-0.43%) | 69.82 (90.23%) |
client_sv2_open_channel_serialize | 📈 view plot 🚷 view threshold | 14,024.00 (-0.57%) | 14,522.41 (96.57%) | 📈 view plot 🚷 view threshold | 5,064.00 (+0.00%) | 5,079.34 (99.70%) | 📈 view plot 🚷 view threshold | 7,324.00 (+0.01%) | 7,351.53 (99.63%) | 📈 view plot 🚷 view threshold | 38.00 (+7.63%) | 43.57 (87.21%) | 📈 view plot 🚷 view threshold | 186.00 (-1.42%) | 200.60 (92.72%) |
client_sv2_open_channel_serialize_deserialize | 📈 view plot 🚷 view threshold | 22,673.00 (+0.15%) | 23,030.76 (98.45%) | 📈 view plot 🚷 view threshold | 8,027.00 (+0.22%) | 8,078.61 (99.36%) | 📈 view plot 🚷 view threshold | 11,673.00 (+0.18%) | 11,755.01 (99.30%) | 📈 view plot 🚷 view threshold | 79.00 (+7.39%) | 84.99 (92.95%) | 📈 view plot 🚷 view threshold | 303.00 (-0.14%) | 314.75 (96.27%) |
client_sv2_setup_connection | 📈 view plot 🚷 view threshold | 4,711.00 (+0.53%) | 4,774.02 (98.68%) | 📈 view plot 🚷 view threshold | 1,502.00 (+0.01%) | 1,517.34 (98.99%) | 📈 view plot 🚷 view threshold | 2,276.00 (-0.09%) | 2,301.83 (98.88%) | 📈 view plot 🚷 view threshold | 11.00 (+21.69%) | 15.04 (73.14%) | 📈 view plot 🚷 view threshold | 68.00 (+0.73%) | 70.06 (97.05%) |
client_sv2_setup_connection_serialize | 📈 view plot 🚷 view threshold | 16,156.00 (-0.24%) | 16,509.38 (97.86%) | 📈 view plot 🚷 view threshold | 5,963.00 (+0.00%) | 5,978.34 (99.74%) | 📈 view plot 🚷 view threshold | 8,661.00 (-0.00%) | 8,691.01 (99.65%) | 📈 view plot 🚷 view threshold | 43.00 (+4.42%) | 54.76 (78.53%) | 📈 view plot 🚷 view threshold | 208.00 (-0.66%) | 217.72 (95.53%) |
client_sv2_setup_connection_serialize_deserialize | 📈 view plot 🚷 view threshold | 35,484.00 (-0.18%) | 35,775.98 (99.18%) | 📈 view plot 🚷 view threshold | 14,855.00 (+0.12%) | 14,907.70 (99.65%) | 📈 view plot 🚷 view threshold | 21,824.00 (+0.14%) | 21,912.16 (99.60%) | 📈 view plot 🚷 view threshold | 93.00 (-1.20%) | 115.85 (80.27%) | 📈 view plot 🚷 view threshold | 377.00 (-0.66%) | 385.09 (97.90%) |
Branch | 1160/merge |
Testbed | sv2 |
Branch | 1160/merge |
Testbed | sv1 |
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,511.00 (+0.75%) | 8,797.24 (96.75%) | 📈 view plot 🚷 view threshold | 3,772.00 (+0.62%) | 3,882.52 (97.15%) | 📈 view plot 🚷 view threshold | 5,296.00 (+0.72%) | 5,441.97 (97.32%) | 📈 view plot 🚷 view threshold | 6.00 (-14.29%) | 11.65 (51.50%) | 📈 view plot 🚷 view threshold | 91.00 (+0.97%) | 95.39 (95.40%) |
get_submit | 📈 view plot 🚷 view threshold | 95,484.00 (+0.01%) | 96,192.79 (99.26%) | 📈 view plot 🚷 view threshold | 59,522.00 (+0.09%) | 59,798.10 (99.54%) | 📈 view plot 🚷 view threshold | 85,504.00 (+0.10%) | 85,878.29 (99.56%) | 📈 view plot 🚷 view threshold | 50.00 (+5.63%) | 70.98 (70.44%) | 📈 view plot 🚷 view threshold | 278.00 (-0.89%) | 289.58 (96.00%) |
get_subscribe | 📈 view plot 🚷 view threshold | 8,034.00 (+0.26%) | 8,362.17 (96.08%) | 📈 view plot 🚷 view threshold | 2,848.00 (+0.39%) | 2,964.07 (96.08%) | 📈 view plot 🚷 view threshold | 3,984.00 (+0.46%) | 4,136.25 (96.32%) | 📈 view plot 🚷 view threshold | 12.00 (-6.23%) | 23.13 (51.88%) | 📈 view plot 🚷 view threshold | 114.00 (+0.17%) | 119.64 (95.28%) |
serialize_authorize | 📈 view plot 🚷 view threshold | 12,286.00 (+0.22%) | 12,638.50 (97.21%) | 📈 view plot 🚷 view threshold | 5,343.00 (+0.44%) | 5,453.52 (97.97%) | 📈 view plot 🚷 view threshold | 7,461.00 (+0.55%) | 7,604.52 (98.11%) | 📈 view plot 🚷 view threshold | 6.00 (-33.51%) | 14.93 (40.18%) | 📈 view plot 🚷 view threshold | 137.00 (+0.03%) | 143.79 (95.28%) |
serialize_deserialize_authorize | 📈 view plot 🚷 view threshold | 24,859.00 (+0.95%) | 25,169.33 (98.77%) | 📈 view plot 🚷 view threshold | 9,920.00 (+0.25%) | 10,053.83 (98.67%) | 📈 view plot 🚷 view threshold | 14,019.00 (+0.37%) | 14,200.32 (98.72%) | 📈 view plot 🚷 view threshold | 33.00 (-5.55%) | 44.21 (74.64%) | 📈 view plot 🚷 view threshold | 305.00 (+1.84%) | 314.01 (97.13%) |
serialize_deserialize_handle_authorize | 📈 view plot 🚷 view threshold | 30,409.00 (+0.46%) | 30,738.49 (98.93%) | 📈 view plot 🚷 view threshold | 12,097.00 (+0.07%) | 12,226.24 (98.94%) | 📈 view plot 🚷 view threshold | 17,139.00 (+0.13%) | 17,308.35 (99.02%) | 📈 view plot 🚷 view threshold | 57.00 (+1.69%) | 68.35 (83.39%) | 📈 view plot 🚷 view threshold | 371.00 (+0.86%) | 381.21 (97.32%) |
serialize_deserialize_handle_submit | 📈 view plot 🚷 view threshold | 126,589.00 (+0.10%) | 127,179.29 (99.54%) | 📈 view plot 🚷 view threshold | 73,363.00 (+0.11%) | 73,677.02 (99.57%) | 📈 view plot 🚷 view threshold | 105,189.00 (+0.13%) | 105,625.63 (99.59%) | 📈 view plot 🚷 view threshold | 115.00 (+4.60%) | 141.99 (80.99%) | 📈 view plot 🚷 view threshold | 595.00 (-0.18%) | 604.68 (98.40%) |
serialize_deserialize_handle_subscribe | 📈 view plot 🚷 view threshold | 27,965.00 (+0.83%) | 28,608.43 (97.75%) | 📈 view plot 🚷 view threshold | 9,666.00 (+0.20%) | 9,780.59 (98.83%) | 📈 view plot 🚷 view threshold | 13,675.00 (+0.22%) | 13,831.72 (98.87%) | 📈 view plot 🚷 view threshold | 65.00 (+1.87%) | 75.80 (85.75%) | 📈 view plot 🚷 view threshold | 399.00 (+1.42%) | 416.65 (95.76%) |
serialize_deserialize_submit | 📈 view plot 🚷 view threshold | 115,491.00 (+0.26%) | 115,918.62 (99.63%) | 📈 view plot 🚷 view threshold | 68,223.00 (+0.19%) | 68,481.06 (99.62%) | 📈 view plot 🚷 view threshold | 97,931.00 (+0.22%) | 98,297.36 (99.63%) | 📈 view plot 🚷 view threshold | 68.00 (+6.50%) | 81.72 (83.21%) | 📈 view plot 🚷 view threshold | 492.00 (+0.32%) | 499.48 (98.50%) |
serialize_deserialize_subscribe | 📈 view plot 🚷 view threshold | 23,393.00 (+1.02%) | 24,018.72 (97.39%) | 📈 view plot 🚷 view threshold | 8,225.00 (+0.29%) | 8,336.38 (98.66%) | 📈 view plot 🚷 view threshold | 11,593.00 (+0.35%) | 11,739.36 (98.75%) | 📈 view plot 🚷 view threshold | 36.00 (-4.20%) | 44.44 (81.01%) | 📈 view plot 🚷 view threshold | 332.00 (+1.78%) | 348.42 (95.29%) |
serialize_submit | 📈 view plot 🚷 view threshold | 99,873.00 (-0.00%) | 100,539.97 (99.34%) | 📈 view plot 🚷 view threshold | 61,566.00 (+0.08%) | 61,846.78 (99.55%) | 📈 view plot 🚷 view threshold | 88,348.00 (+0.10%) | 88,730.37 (99.57%) | 📈 view plot 🚷 view threshold | 51.00 (+4.92%) | 70.14 (72.71%) | 📈 view plot 🚷 view threshold | 322.00 (-0.88%) | 333.31 (96.61%) |
serialize_subscribe | 📈 view plot 🚷 view threshold | 11,363.00 (-0.25%) | 11,760.30 (96.62%) | 📈 view plot 🚷 view threshold | 4,195.00 (+0.27%) | 4,311.07 (97.31%) | 📈 view plot 🚷 view threshold | 5,843.00 (+0.35%) | 5,993.24 (97.49%) | 📈 view plot 🚷 view threshold | 12.00 (-11.32%) | 22.26 (53.90%) | 📈 view plot 🚷 view threshold | 156.00 (-0.75%) | 164.45 (94.86%) |
I took note through normal comments about the constants we will need to remove or modify in the future refactoring. This list will be the input of the tracker issue for it: https://github.com/stratum-mining/stratum/issues/1135
SV2_FRAME_HEADER_LEN_OFFSET
, SV2_FRAME_HEADER_LEN_END
, and NOISE_FRAME_MAX_SIZE
are not used anywhere --> we should just remove them ENCRYPTED_SV2_FRAME_HEADER_SIZE
and NOISE_FRAME_HEADER_SIZE
are declared in sv2-ffi
, and imported in framing_sv2/src/header.rs
, which is then imported into codec_sv2
just for this constant --> we should import them directly into codec_sv2
(cc @rrybarczyk)NOISE_FRAME_HEADER_LEN_OFFSET
is imported in framing_sv2
in the same way here, but then its alias is never used anywhere else --> we should remove itRESPONDER_EXPECTED_HANDSHAKE_MESSAGE_SIZE
is an alias of ELLSWIFT_ENCODING_SIZE
--> we should create the alias where it's used, not hereMAC
it's the same as AEAD_MAC_LEN
--> we should remove itNOISE_SUPPORTED_CIPHERS_MESSAGE
is used to signal AESG support but we're dropping support for AESG --> we should remove it when we will officially remove support from our implementationSV2_TEMPLATE_DISTR_PROTOCOL_DISCRIMINANT
--> I would rename it in V2_TEMPLATE_DISTRIBUTION_PROTOCOL_DISCRIMINANT
MESSAGE_TYPE_OPEN_EXTENDED_MINING_CHANNEL_SUCCES
--> fix typo with MESSAGE_TYPE_OPEN_EXTENDED_MINING_CHANNEL_SUCCESS
MESSAGE_TYPE_RECONNECT
is defined as 0x25
but according to specs it's 0x04
--> we need to move it to 0x04
and shift MESSAGE_TYPE_SET_GROUP_CHANNEL
to 0x25
(we are not specs compliant now)A final consideration could be to group constants in different modules (Mining Protocol, Job Distribution Protocol, Job Declaration Protocol, Common, Encryption, etc) so that it will be more clear also on https://docs.rs/. But this is debatable and not strictly necessary btw
Branch | 1160/merge@8c370643-9a68-46ee-8549-accf24fd4838 |
Testbed | sv1 |
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,398.00 (-0.55%) | 8,791.52 (95.52%) | 📈 view plot 🚷 view threshold | 3,746.00 (-0.06%) | 3,881.43 (96.51%) | 📈 view plot 🚷 view threshold | 5,253.00 (-0.08%) | 5,440.10 (96.56%) | 📈 view plot 🚷 view threshold | 6.00 (-14.48%) | 11.67 (51.43%) | 📈 view plot 🚷 view threshold | 89.00 (-1.19%) | 95.26 (93.43%) |
get_submit | 📈 view plot 🚷 view threshold | 95,365.00 (-0.11%) | 96,195.71 (99.14%) | 📈 view plot 🚷 view threshold | 59,439.00 (-0.05%) | 59,795.52 (99.40%) | 📈 view plot 🚷 view threshold | 85,365.00 (-0.06%) | 85,873.74 (99.41%) | 📈 view plot 🚷 view threshold | 47.00 (-0.95%) | 71.17 (66.04%) | 📈 view plot 🚷 view threshold | 279.00 (-0.58%) | 289.85 (96.26%) |
get_subscribe | 📈 view plot 🚷 view threshold | 8,025.00 (+0.17%) | 8,359.51 (96.00%) | 📈 view plot 🚷 view threshold | 2,841.00 (+0.15%) | 2,963.81 (95.86%) | 📈 view plot 🚷 view threshold | 3,970.00 (+0.12%) | 4,135.69 (95.99%) | 📈 view plot 🚷 view threshold | 13.00 (+0.97%) | 23.16 (56.13%) | 📈 view plot 🚷 view threshold | 114.00 (+0.21%) | 119.55 (95.36%) |
serialize_authorize | 📈 view plot 🚷 view threshold | 12,245.00 (-0.08%) | 12,634.05 (96.92%) | 📈 view plot 🚷 view threshold | 5,317.00 (-0.04%) | 5,452.43 (97.52%) | 📈 view plot 🚷 view threshold | 7,415.00 (-0.06%) | 7,602.50 (97.53%) | 📈 view plot 🚷 view threshold | 7.00 (-22.89%) | 14.91 (46.95%) | 📈 view plot 🚷 view threshold | 137.00 (+0.09%) | 143.69 (95.35%) |
serialize_deserialize_authorize | 📈 view plot 🚷 view threshold | 24,743.00 (+0.52%) | 25,148.27 (98.39%) | 📈 view plot 🚷 view threshold | 9,868.00 (-0.27%) | 10,052.87 (98.16%) | 📈 view plot 🚷 view threshold | 13,928.00 (-0.27%) | 14,197.62 (98.10%) | 📈 view plot 🚷 view threshold | 35.00 (+0.04%) | 44.17 (79.24%) | 📈 view plot 🚷 view threshold | 304.00 (+1.59%) | 313.54 (96.96%) |
serialize_deserialize_handle_authorize | 📈 view plot 🚷 view threshold | 30,372.00 (+0.36%) | 30,724.22 (98.85%) | 📈 view plot 🚷 view threshold | 12,071.00 (-0.15%) | 12,226.40 (98.73%) | 📈 view plot 🚷 view threshold | 17,092.00 (-0.14%) | 17,307.78 (98.75%) | 📈 view plot 🚷 view threshold | 59.00 (+5.27%) | 68.39 (86.27%) | 📈 view plot 🚷 view threshold | 371.00 (+0.91%) | 380.86 (97.41%) |
serialize_deserialize_handle_submit | 📈 view plot 🚷 view threshold | 126,522.00 (+0.05%) | 127,172.65 (99.49%) | 📈 view plot 🚷 view threshold | 73,280.00 (+0.00%) | 73,672.51 (99.47%) | 📈 view plot 🚷 view threshold | 105,052.00 (+0.01%) | 105,617.97 (99.46%) | 📈 view plot 🚷 view threshold | 108.00 (-1.80%) | 142.14 (75.98%) | 📈 view plot 🚷 view threshold | 598.00 (+0.31%) | 604.77 (98.88%) |
serialize_deserialize_handle_subscribe | 📈 view plot 🚷 view threshold | 27,982.00 (+0.94%) | 28,591.41 (97.87%) | 📈 view plot 🚷 view threshold | 9,659.00 (+0.13%) | 9,779.73 (98.77%) | 📈 view plot 🚷 view threshold | 13,662.00 (+0.13%) | 13,830.48 (98.78%) | 📈 view plot 🚷 view threshold | 64.00 (+0.48%) | 75.74 (84.50%) | 📈 view plot 🚷 view threshold | 400.00 (+1.76%) | 416.23 (96.10%) |
serialize_deserialize_submit | 📈 view plot 🚷 view threshold | 115,277.00 (+0.08%) | 115,897.46 (99.46%) | 📈 view plot 🚷 view threshold | 68,057.00 (-0.04%) | 68,472.72 (99.39%) | 📈 view plot 🚷 view threshold | 97,647.00 (-0.06%) | 98,283.15 (99.35%) | 📈 view plot 🚷 view threshold | 68.00 (+6.44%) | 81.74 (83.19%) | 📈 view plot 🚷 view threshold | 494.00 (+0.74%) | 499.42 (98.91%) |
serialize_deserialize_subscribe | 📈 view plot 🚷 view threshold | 23,435.00 (+1.25%) | 24,004.66 (97.63%) | 📈 view plot 🚷 view threshold | 8,211.00 (+0.13%) | 8,335.21 (98.51%) | 📈 view plot 🚷 view threshold | 11,565.00 (+0.12%) | 11,737.22 (98.53%) | 📈 view plot 🚷 view threshold | 36.00 (-4.24%) | 44.40 (81.09%) | 📈 view plot 🚷 view threshold | 334.00 (+2.49%) | 348.08 (95.95%) |
serialize_submit | 📈 view plot 🚷 view threshold | 99,822.00 (-0.05%) | 100,539.78 (99.29%) | 📈 view plot 🚷 view threshold | 61,483.00 (-0.05%) | 61,844.22 (99.42%) | 📈 view plot 🚷 view threshold | 88,207.00 (-0.06%) | 88,725.80 (99.42%) | 📈 view plot 🚷 view threshold | 48.00 (-1.44%) | 70.31 (68.27%) | 📈 view plot 🚷 view threshold | 325.00 (+0.01%) | 333.34 (97.50%) |
serialize_subscribe | 📈 view plot 🚷 view threshold | 11,422.00 (+0.28%) | 11,758.70 (97.14%) | 📈 view plot 🚷 view threshold | 4,188.00 (+0.10%) | 4,310.81 (97.15%) | 📈 view plot 🚷 view threshold | 5,827.00 (+0.08%) | 5,992.64 (97.24%) | 📈 view plot 🚷 view threshold | 13.00 (-4.42%) | 22.24 (58.46%) | 📈 view plot 🚷 view threshold | 158.00 (+0.56%) | 164.41 (96.10%) |
I took note through normal comments about the constants we will need to remove or modify in the future refactoring. This list will be the input of the tracker issue for it: #1135
* `SV2_FRAME_HEADER_LEN_OFFSET`, `SV2_FRAME_HEADER_LEN_END`, and `NOISE_FRAME_MAX_SIZE` are not used anywhere --> we should just remove them * `ENCRYPTED_SV2_FRAME_HEADER_SIZE` and `NOISE_FRAME_HEADER_SIZE` are declared in `sv2-ffi`, and imported in `framing_sv2/src/header.rs`, which is then imported into `codec_sv2` just for this constant --> we should import them directly into `codec_sv2` (cc @rrybarczyk) * `NOISE_FRAME_HEADER_LEN_OFFSET` is imported in `framing_sv2` in the same way [here](https://github.com/stratum-mining/stratum/blob/708e5e4ecb1f67d4e629b68dd4965271ced389c4/protocols/v2/framing-sv2/src/header.rs#L13), but then its alias is never used anywhere else --> we should remove it * `RESPONDER_EXPECTED_HANDSHAKE_MESSAGE_SIZE` is an alias of `ELLSWIFT_ENCODING_SIZE` --> we should create the alias where it's used, not here * `MAC` it's the same as `AEAD_MAC_LEN` --> we should remove it * `NOISE_SUPPORTED_CIPHERS_MESSAGE` is used to signal AESG support but we're dropping support for AESG --> we should remove it when we will officially remove support from our implementation * `SV2_TEMPLATE_DISTR_PROTOCOL_DISCRIMINANT` --> I would rename it in `V2_TEMPLATE_DISTRIBUTION_PROTOCOL_DISCRIMINANT` * `MESSAGE_TYPE_OPEN_EXTENDED_MINING_CHANNEL_SUCCES` --> fix typo with `MESSAGE_TYPE_OPEN_EXTENDED_MINING_CHANNEL_SUCCESS` * `MESSAGE_TYPE_RECONNECT` is defined as `0x25` but according to specs it's `0x04` --> we need to move it to `0x04` and shift `MESSAGE_TYPE_SET_GROUP_CHANNEL` to `0x25` (we are not specs compliant now)
A final consideration could be to group constants in different modules (Mining Protocol, Job Distribution Protocol, Job Declaration Protocol, Common, Encryption, etc) so that it will be more clear also on https://docs.rs/. But this is debatable and not strictly necessary btw
from my experience most of the time is more convenient to keep al the constants of a project in a separate module/lib whatever that group them together.
What do you mean @Fi3? I couldn't fully understand your comment.
What do you mean @Fi3? I couldn't fully understand your comment.
just ignore it, I thought that you were proposing to split the const lib into several libs
What do you mean @Fi3? I couldn't fully understand your comment.
just ignore it, I thought that you were proposing to split the const lib into several libs
No, I was just thinking about grouping constants into modules, here's an example:
/// Constants related to the SV2 frame structure.
pub mod frame {
pub const EXTENSION_TYPE_NO_EXTENSION: u16 = 0;
pub const SV2_FRAME_HEADER_SIZE: usize = 6;
pub const SV2_FRAME_HEADER_LEN_OFFSET: usize = 3;
pub const SV2_FRAME_HEADER_LEN_END: usize = 3;
pub const SV2_FRAME_CHUNK_SIZE: usize = 65535;
pub const AEAD_MAC_LEN: usize = 16;
pub const ENCRYPTED_SV2_FRAME_HEADER_SIZE: usize = SV2_FRAME_HEADER_SIZE + AEAD_MAC_LEN;
}
/// Discriminants for distinct Stratum V2 (sub)protocols.
pub mod protocol {
pub const SV2_MINING_PROTOCOL_DISCRIMINANT: u8 = 0;
pub const SV2_JOB_DECLARATION_PROTOCOL_DISCRIMINANT: u8 = 1;
pub const SV2_TEMPLATE_DISTR_PROTOCOL_DISCRIMINANT: u8 = 2;
pub const MESSAGE_TYPE_SETUP_CONNECTION: u8 = 0x0;
pub const MESSAGE_TYPE_SETUP_CONNECTION_SUCCESS: u8 = 0x1;
pub const MESSAGE_TYPE_SETUP_CONNECTION_ERROR: u8 = 0x2;
pub const MESSAGE_TYPE_CHANNEL_ENDPOINT_CHANGED: u8 = 0x3;
}
/// Mining Protocol message types.
pub mod mining {
pub const MESSAGE_TYPE_OPEN_STANDARD_MINING_CHANNEL: u8 = 0x10;
pub const MESSAGE_TYPE_OPEN_STANDARD_MINING_CHANNEL_SUCCESS: u8 = 0x11;
pub const MESSAGE_TYPE_OPEN_MINING_CHANNEL_ERROR: u8 = 0x12;
pub const MESSAGE_TYPE_OPEN_EXTENDED_MINING_CHANNEL: u8 = 0x13;
pub const MESSAGE_TYPE_OPEN_EXTENDED_MINING_CHANNEL_SUCCES: u8 = 0x14;
pub const MESSAGE_TYPE_NEW_MINING_JOB: u8 = 0x15;
pub const MESSAGE_TYPE_UPDATE_CHANNEL: u8 = 0x16;
pub const MESSAGE_TYPE_UPDATE_CHANNEL_ERROR: u8 = 0x17;
pub const MESSAGE_TYPE_CLOSE_CHANNEL: u8 = 0x18;
pub const MESSAGE_TYPE_SET_EXTRANONCE_PREFIX: u8 = 0x19;
pub const MESSAGE_TYPE_SUBMIT_SHARES_STANDARD: u8 = 0x1a;
pub const MESSAGE_TYPE_SUBMIT_SHARES_EXTENDED: u8 = 0x1b;
pub const MESSAGE_TYPE_SUBMIT_SHARES_SUCCESS: u8 = 0x1c;
pub const MESSAGE_TYPE_SUBMIT_SHARES_ERROR: u8 = 0x1d;
pub const MESSAGE_TYPE_NEW_EXTENDED_MINING_JOB: u8 = 0x1f;
pub const MESSAGE_TYPE_MINING_SET_NEW_PREV_HASH: u8 = 0x20;
pub const MESSAGE_TYPE_SET_TARGET: u8 = 0x21;
pub const MESSAGE_TYPE_SET_CUSTOM_MINING_JOB: u8 = 0x22;
pub const MESSAGE_TYPE_SET_CUSTOM_MINING_JOB_SUCCESS: u8 = 0x23;
pub const MESSAGE_TYPE_SET_CUSTOM_MINING_JOB_ERROR: u8 = 0x24;
pub const MESSAGE_TYPE_RECONNECT: u8 = 0x25;
pub const MESSAGE_TYPE_SET_GROUP_CHANNEL: u8 = 0x26;
}
What do you mean @Fi3? I couldn't fully understand your comment.
just ignore it, I thought that you were proposing to split the const lib into several libs
No, I was just thinking about grouping constants into modules, here's an example:
/// Constants related to the SV2 frame structure. pub mod frame { pub const EXTENSION_TYPE_NO_EXTENSION: u16 = 0; pub const SV2_FRAME_HEADER_SIZE: usize = 6; pub const SV2_FRAME_HEADER_LEN_OFFSET: usize = 3; pub const SV2_FRAME_HEADER_LEN_END: usize = 3; pub const SV2_FRAME_CHUNK_SIZE: usize = 65535; pub const AEAD_MAC_LEN: usize = 16; pub const ENCRYPTED_SV2_FRAME_HEADER_SIZE: usize = SV2_FRAME_HEADER_SIZE + AEAD_MAC_LEN; } /// Discriminants for distinct Stratum V2 (sub)protocols. pub mod protocol { pub const SV2_MINING_PROTOCOL_DISCRIMINANT: u8 = 0; pub const SV2_JOB_DECLARATION_PROTOCOL_DISCRIMINANT: u8 = 1; pub const SV2_TEMPLATE_DISTR_PROTOCOL_DISCRIMINANT: u8 = 2; pub const MESSAGE_TYPE_SETUP_CONNECTION: u8 = 0x0; pub const MESSAGE_TYPE_SETUP_CONNECTION_SUCCESS: u8 = 0x1; pub const MESSAGE_TYPE_SETUP_CONNECTION_ERROR: u8 = 0x2; pub const MESSAGE_TYPE_CHANNEL_ENDPOINT_CHANGED: u8 = 0x3; } /// Mining Protocol message types. pub mod mining { pub const MESSAGE_TYPE_OPEN_STANDARD_MINING_CHANNEL: u8 = 0x10; pub const MESSAGE_TYPE_OPEN_STANDARD_MINING_CHANNEL_SUCCESS: u8 = 0x11; pub const MESSAGE_TYPE_OPEN_MINING_CHANNEL_ERROR: u8 = 0x12; pub const MESSAGE_TYPE_OPEN_EXTENDED_MINING_CHANNEL: u8 = 0x13; pub const MESSAGE_TYPE_OPEN_EXTENDED_MINING_CHANNEL_SUCCES: u8 = 0x14; pub const MESSAGE_TYPE_NEW_MINING_JOB: u8 = 0x15; pub const MESSAGE_TYPE_UPDATE_CHANNEL: u8 = 0x16; pub const MESSAGE_TYPE_UPDATE_CHANNEL_ERROR: u8 = 0x17; pub const MESSAGE_TYPE_CLOSE_CHANNEL: u8 = 0x18; pub const MESSAGE_TYPE_SET_EXTRANONCE_PREFIX: u8 = 0x19; pub const MESSAGE_TYPE_SUBMIT_SHARES_STANDARD: u8 = 0x1a; pub const MESSAGE_TYPE_SUBMIT_SHARES_EXTENDED: u8 = 0x1b; pub const MESSAGE_TYPE_SUBMIT_SHARES_SUCCESS: u8 = 0x1c; pub const MESSAGE_TYPE_SUBMIT_SHARES_ERROR: u8 = 0x1d; pub const MESSAGE_TYPE_NEW_EXTENDED_MINING_JOB: u8 = 0x1f; pub const MESSAGE_TYPE_MINING_SET_NEW_PREV_HASH: u8 = 0x20; pub const MESSAGE_TYPE_SET_TARGET: u8 = 0x21; pub const MESSAGE_TYPE_SET_CUSTOM_MINING_JOB: u8 = 0x22; pub const MESSAGE_TYPE_SET_CUSTOM_MINING_JOB_SUCCESS: u8 = 0x23; pub const MESSAGE_TYPE_SET_CUSTOM_MINING_JOB_ERROR: u8 = 0x24; pub const MESSAGE_TYPE_RECONNECT: u8 = 0x25; pub const MESSAGE_TYPE_SET_GROUP_CHANNEL: u8 = 0x26; }
Can we use enums for the discriminants?
I took note through normal comments about the constants we will need to remove or modify in the future refactoring. This list will be the input of the tracker issue for it: #1135
SV2_FRAME_HEADER_LEN_OFFSET
,SV2_FRAME_HEADER_LEN_END
, andNOISE_FRAME_MAX_SIZE
are not used anywhere --> we should just remove themENCRYPTED_SV2_FRAME_HEADER_SIZE
andNOISE_FRAME_HEADER_SIZE
are declared insv2-ffi
, and imported inframing_sv2/src/header.rs
, which is then imported intocodec_sv2
just for this constant --> we should import them directly intocodec_sv2
(cc @rrybarczyk)NOISE_FRAME_HEADER_LEN_OFFSET
is imported inframing_sv2
in the same way here, but then its alias is never used anywhere else --> we should remove itRESPONDER_EXPECTED_HANDSHAKE_MESSAGE_SIZE
is an alias ofELLSWIFT_ENCODING_SIZE
--> we should create the alias where it's used, not hereMAC
it's the same asAEAD_MAC_LEN
--> we should remove itNOISE_SUPPORTED_CIPHERS_MESSAGE
is used to signal AESG support but we're dropping support for AESG --> we should remove it when we will officially remove support from our implementationSV2_TEMPLATE_DISTR_PROTOCOL_DISCRIMINANT
--> I would rename it inV2_TEMPLATE_DISTRIBUTION_PROTOCOL_DISCRIMINANT
MESSAGE_TYPE_OPEN_EXTENDED_MINING_CHANNEL_SUCCES
--> fix typo withMESSAGE_TYPE_OPEN_EXTENDED_MINING_CHANNEL_SUCCESS
MESSAGE_TYPE_RECONNECT
is defined as0x25
but according to specs it's0x04
--> we need to move it to0x04
and shiftMESSAGE_TYPE_SET_GROUP_CHANNEL
to0x25
(we are not specs compliant now)A final consideration could be to group constants in different modules (Mining Protocol, Job Distribution Protocol, Job Declaration Protocol, Common, Encryption, etc) so that it will be more clear also on https://docs.rs/. But this is debatable and not strictly necessary btw
In our codebase, we're using some numerical literals directly instead of their corresponding constants. For example, NOISE_FRAME_MAX_SIZE
isn't currently used anywhere, but I believe it could be applied where we define the segregation of encrypted and non-encrypted data packets. I'll look into it and get back to you.
@Shourya742 I won't add too many comments regarding refactoring here, since the list I made will be ported to the specific issue (https://github.com/stratum-mining/stratum/issues/1135) as soon as this PR get merged. My intention was just to draft something here for now.
But yeah, we will discuss for sure about these constants which are currently unused to see if they can be used somewhere (e.g. noise_sv2
) before refactoring
I think the docs.rs looks a bit weird with all of the constants in the list, but fixing that might require doing breaking changes which are not expected in this pr, so we can skip this for now I guess.
The line widths, specially in the first 3 paragraphs are not consistent, aligning them to 80 or 100 chars per line makes the docs.rs look much better.
Aligned to 100 chars here: b873c56fe4c91de963f5032e3c167deb90fcdca1. But if I run cargo doc --all-features --open
locally, I wasn't able to see any difference
This PR adds rust docs to
const_sv2
crate. For now it contains a set of comments, which will be resolved after discussions at https://github.com/stratum-mining/stratum/discussions/1159Closes #1016