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

Rust docs for protocols::v2::const-sv2 #1160

Merged
merged 3 commits into from
Oct 4, 2024
Merged

Rust docs for protocols::v2::const-sv2 #1160

merged 3 commits into from
Oct 4, 2024

Conversation

GitGab19
Copy link
Collaborator

@GitGab19 GitGab19 commented Sep 6, 2024

This PR adds rust docs to const_sv2 crate.
For now it contains a set of comments, which will be resolved after discussions at #1159

Closes #1016

@GitGab19 GitGab19 marked this pull request as draft September 6, 2024 00:14
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 6, 2024 •
edited
Loading

🐰 Bencher Report

Branch1160/merge
Testbedsv1
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
nanoseconds (ns)
(Result Δ%)		Upper Boundary
nanoseconds (ns)
(Limit %)
client-submit-serialize📈 view plot
🚷 view threshold		6,625.40
(-3.57%)		7,953.40
(83.30%)
client-submit-serialize-deserialize📈 view plot
🚷 view threshold		7,691.60
(-2.45%)		8,918.18
(86.25%)
client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle📈 view plot
🚷 view threshold		8,112.60
(-3.83%)		9,674.24
(83.86%)
client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle📈 view plot
🚷 view threshold		859.91
(-6.53%)		1,083.89
(79.34%)
client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize📈 view plot
🚷 view threshold		716.62
(-0.63%)		807.22
(88.78%)
client-sv1-authorize-serialize/client-sv1-authorize-serialize📈 view plot
🚷 view threshold		253.54
(+0.48%)		296.11
(85.62%)
client-sv1-get-authorize/client-sv1-get-authorize📈 view plot
🚷 view threshold		146.97
(-7.88%)		193.56
(75.93%)
client-sv1-get-submit📈 view plot
🚷 view threshold		6,284.50
(-5.09%)		7,638.79
(82.27%)
client-sv1-get-subscribe/client-sv1-get-subscribe📈 view plot
🚷 view threshold		257.89
(-7.20%)		328.20
(78.58%)
client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle📈 view plot
🚷 view threshold		725.21
(-5.72%)		895.11
(81.02%)
client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize📈 view plot
🚷 view threshold		649.03
(+1.31%)		715.22
(90.75%)
client-sv1-subscribe-serialize/client-sv1-subscribe-serialize📈 view plot
🚷 view threshold		192.64
(-7.46%)		262.60
(73.36%)
🐰 View full continuous benchmarking report in Bencher

@GitGab19 GitGab19 mentioned this pull request Sep 6, 2024
Open
78 tasks
@GitGab19 GitGab19 force-pushed the add-rust-docs-const-sv2 branch from e97ea55 to 7598bf6 Compare September 26, 2024 16:16
@GitGab19 GitGab19 marked this pull request as ready for review September 26, 2024 16:17
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 26, 2024 •
edited
Loading

🐰 Bencher Report

Branch1160/merge
Testbedsv2
Click to view all benchmark results
BenchmarkEstimated CyclesBenchmark Result
estimated cycles
(Result Δ%)		Upper Boundary
estimated cycles
(Limit %)		InstructionsBenchmark Result
instructions
(Result Δ%)		Upper Boundary
instructions
(Limit %)		L1 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)		L2 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)		RAM AccessesBenchmark 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%)
🐰 View full continuous benchmarking report in Bencher

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 26, 2024 •
edited
Loading

🐰 Bencher Report

Branch1160/merge
Testbedsv2
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
nanoseconds (ns)
(Result Δ%)		Upper Boundary
nanoseconds (ns)
(Limit %)
client_sv2_handle_message_common📈 view plot
🚷 view threshold		44.41
(-0.62%)		46.96
(94.57%)
client_sv2_handle_message_mining📈 view plot
🚷 view threshold		75.13
(-2.54%)		101.62
(73.93%)
client_sv2_mining_message_submit_standard📈 view plot
🚷 view threshold		14.79
(+0.36%)		14.96
(98.84%)
client_sv2_mining_message_submit_standard_serialize📈 view plot
🚷 view threshold		266.53
(-0.53%)		291.54
(91.42%)
client_sv2_mining_message_submit_standard_serialize_deserialize📈 view plot
🚷 view threshold		594.22
(+0.22%)		645.43
(92.07%)
client_sv2_open_channel📈 view plot
🚷 view threshold		165.60
(-0.81%)		182.80
(90.59%)
client_sv2_open_channel_serialize📈 view plot
🚷 view threshold		275.59
(+1.08%)		287.70
(95.79%)
client_sv2_open_channel_serialize_deserialize📈 view plot
🚷 view threshold		382.78
(+0.72%)		434.85
(88.02%)
client_sv2_setup_connection📈 view plot
🚷 view threshold		164.04
(+0.42%)		186.30
(88.05%)
client_sv2_setup_connection_serialize📈 view plot
🚷 view threshold		445.89
(-1.44%)		520.21
(85.71%)
client_sv2_setup_connection_serialize_deserialize📈 view plot
🚷 view threshold		952.81
(-2.43%)		1,153.11
(82.63%)
🐰 View full continuous benchmarking report in Bencher

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 26, 2024 •
edited
Loading

🐰 Bencher Report

Branch1160/merge
Testbedsv1
Click to view all benchmark results
BenchmarkEstimated CyclesBenchmark Result
estimated cycles
(Result Δ%)		Upper Boundary
estimated cycles
(Limit %)		InstructionsBenchmark Result
instructions
(Result Δ%)		Upper Boundary
instructions
(Limit %)		L1 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)		L2 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)		RAM AccessesBenchmark 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%)
🐰 View full continuous benchmarking report in Bencher

@stratum-mining stratum-mining deleted a comment from github-actions bot Sep 26, 2024
@stratum-mining stratum-mining deleted a comment from github-actions bot Sep 26, 2024
@stratum-mining stratum-mining deleted a comment from github-actions bot Sep 26, 2024
@GitGab19
Copy link
Collaborator Author

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, 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

@github-actions GitHub Actions
Copy link
Contributor

🐰 Bencher Report

Branch1160/merge@8c370643-9a68-46ee-8549-accf24fd4838
Testbedsv1
Click to view all benchmark results
BenchmarkEstimated CyclesBenchmark Result
estimated cycles
(Result Δ%)		Upper Boundary
estimated cycles
(Limit %)		InstructionsBenchmark Result
instructions
(Result Δ%)		Upper Boundary
instructions
(Limit %)		L1 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)		L2 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)		RAM AccessesBenchmark 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%)
🐰 View full continuous benchmarking report in Bencher

@GitGab19 GitGab19 force-pushed the add-rust-docs-const-sv2 branch from ecbe5fd to 4771a06 Compare September 26, 2024 17:31
@Fi3
Copy link
Collaborator

Fi3 commented Sep 27, 2024

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.

@GitGab19
Copy link
Collaborator Author

What do you mean @Fi3? I couldn't fully understand your comment.

@Fi3
Copy link
Collaborator

Fi3 commented Sep 27, 2024

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

@GitGab19
Copy link
Collaborator Author

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;
}

@Shourya742
Copy link
Contributor

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?

@Shourya742
Copy link
Contributor

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, 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

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.

@GitGab19
Copy link
Collaborator Author

@Shourya742 I won't add too many comments regarding refactoring here, since the list I made will be ported to the specific issue (#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

@GitGab19 GitGab19 force-pushed the add-rust-docs-const-sv2 branch 2 times, most recently from 8c7ff08 to d84c53c Compare October 2, 2024 09:22
jbesraa
jbesraa reviewed Oct 3, 2024
View reviewed changes
Copy link
Contributor

@jbesraa jbesraa left a comment

Choose a reason for hiding this comment

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

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.

@GitGab19 GitGab19 force-pushed the add-rust-docs-const-sv2 branch from d84c53c to 0232da4 Compare October 3, 2024 17:39
@GitGab19
Copy link
Collaborator Author

GitGab19 commented Oct 3, 2024 •
edited
Loading

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: b873c56. But if I run cargo doc --all-features --open locally, I wasn't able to see any difference

@GitGab19 GitGab19 force-pushed the add-rust-docs-const-sv2 branch from 0232da4 to b873c56 Compare October 3, 2024 17:57
@rrybarczyk rrybarczyk mentioned this pull request Oct 3, 2024
Closed
@GitGab19 GitGab19 added the ready-to-be-merged triggers auto rebase bot label Oct 4, 2024
@pavlenex pavlenex force-pushed the add-rust-docs-const-sv2 branch from b873c56 to a3f7b26 Compare October 4, 2024 14:27
@plebhash plebhash merged commit ebf4b05 into stratum-mining:main Oct 4, 2024
34 checks passed
@rrybarczyk rrybarczyk removed the ready-to-be-merged triggers auto rebase bot label Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@plebhash plebhash plebhash approved these changes

@jbesraa jbesraa jbesraa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Need Rust docs on protocols::v2::const-sv2
6 participants
@GitGab19 @Fi3 @Shourya742 @jbesraa @plebhash @rrybarczyk