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

Propose an implementation of noise_sv2 with optional no_std #1238

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Propose an implementation of noise_sv2 with optional no_std #1238

wants to merge 9 commits into from
+332 −38

Conversation

Georges760
Copy link

@Georges760 Georges760 commented Oct 29, 2024 •
edited
Loading

Following @rrybarczyk comment on #1130.

Some equivalent conversion have been done (does not change any functionality, just using the no_std equivalents) :

  • std::ptr -> core::ptr
  • std::boxed::Box -> alloc::boxed::Box
  • std::vec::Vec -> alloc::vec::Vec
  • std::string::{String, ToString} -> alloc::string::{String, ToString}
  • std::convert::TryInto -> core::convert::TryInto
  • std::fmt::{Debug, Formatter, Result} -> core::fmt::{Debug, Formatter, Result}
  • std::time::Duration -> core::time::Duration

To have a no-std version, the --no-default-features must be used.

Current public API (std dependant) is unchanged.
Additional public API for no_std compliance is available using the *_with_rng and *_with_now suffix, and the corresponding arguments. This delegate the choice of the Ramdom Number Generator and the current System Time to the caller, instead of assuming using the ones from std.

  • Initiator::new_with_rng(), Initiator::from_raw_k_with_rng(), Initiator::without_pk_with_rng(), Responder::new_with_rng(), Responder::from_authority_kp_with_rng() and Responder::generate_key_with_rng() take an additional argument: rng implementing rand::Rng + ?Sized
  • SignatureNoiseMessage::sign_with_rng() take an additional argument: rng implementing rand::Rng + rand::CryptoRng
  • Initiator::step_2_with_now() and SignatureNoiseMessage::verify_with_now() take an additional argument: now for the current system time epoch
  • Responder::step_1()_with_now_rng take two additional arguments: rng implementing rand::Rng + rand::CryptoRng and now for the current system time epoch

@Georges760 Georges760 marked this pull request as draft October 29, 2024 10:41
@codecov Codecov
Copy link

codecov bot commented Oct 29, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 63.15789% with 14 lines in your changes missing coverage. Please review.

Project coverage is 19.26%. Comparing base (a58246a) to head (e137377).

Files with missing lines Patch % Lines
protocols/v2/noise-sv2/src/initiator.rs 50.00% 7 Missing ⚠️
protocols/v2/noise-sv2/src/responder.rs 58.82% 7 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1238      +/-   ##
==========================================
- Coverage   19.29%   19.26%   -0.04%     
==========================================
  Files         164      164              
  Lines       10852    10912      +60     
==========================================
+ Hits         2094     2102       +8     
- Misses       8758     8810      +52
Flag Coverage Δ
binary_codec_sv2-coverage 0.00% <0.00%> (ø)
binary_serde_sv2-coverage 3.56% <0.00%> (-0.09%) ⬇️
binary_sv2-coverage 5.36% <0.00%> (-0.13%) ⬇️
bip32_derivation-coverage 0.00% <ø> (ø)
buffer_sv2-coverage 25.02% <ø> (ø)
codec_sv2-coverage 0.01% <0.00%> (-0.01%) ⬇️
common_messages_sv2-coverage 0.13% <0.00%> (-0.01%) ⬇️
const_sv2-coverage 0.00% <0.00%> (ø)
error_handling-coverage 0.00% <ø> (ø)
framing_sv2-coverage 0.28% <0.00%> (-0.01%) ⬇️
jd_client-coverage 0.00% <ø> (ø)
jd_server-coverage 7.79% <ø> (ø)
job_declaration_sv2-coverage 0.00% <0.00%> (ø)
key-utils-coverage 2.39% <ø> (ø)
mining-coverage 2.45% <0.00%> (-0.06%) ⬇️
mining_device-coverage 0.00% <ø> (ø)
mining_proxy_sv2-coverage 0.70% <ø> (ø)
noise_sv2-coverage 4.46% <80.00%> (+0.10%) ⬆️
pool_sv2-coverage 1.38% <ø> (ø)
protocols 24.62% <63.15%> (-0.11%) ⬇️
roles 6.54% <ø> (ø)
roles_logic_sv2-coverage 7.92% <0.00%> (-0.17%) ⬇️
sv2_ffi-coverage 0.00% <0.00%> (ø)
template_distribution_sv2-coverage 0.00% <0.00%> (ø)
translator_sv2-coverage 9.60% <ø> (ø)
utils 25.13% <ø> (ø)
v1-coverage 2.42% <0.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Georges760 Georges760 mentioned this pull request Oct 29, 2024
Merged
@Sjors
Copy link
Collaborator

Sjors commented Dec 6, 2024

Is it possible to run tests on both the regular and no_std build?

@Georges760
Copy link
Author

Georges760 commented Dec 6, 2024 •
edited
Loading

Is it possible to run tests on both the regular and no_std build?

#![no_std] for noise_sv2 (this PR) is a proposal currently, because it require to break its API, so I am waiting for @Shourya742 review of the new proposed API (maybe need to be improved/extended/simplified/etc), then I will work on adapting all depending crate to the new API.

At the end there will not be such a 'std' version of noise_sv2, it will be no_std by nature, which means not dependant to std.

And usage (test included) will have to provide a System Time and a Random Number Generator to the crate, they can comme from std (currently the 'enforced' case), or from a dedicated Hardware (often the case in embedded system aka no_std).
In tests, I will be able to provide them from std, to have the current state, or from a mocked embedded provider to simulat real HW.

@Sjors
Copy link
Collaborator

Sjors commented Dec 6, 2024

I see. But I wonder if it's possible & safer to keep two versions, because IIUC no_std misses various safety features.

https://docs.rust-embedded.org/book/intro/no-std.html

@Georges760
Copy link
Author

Georges760 commented Dec 6, 2024 •
edited
Loading

I see. But I wonder if it's possible & safer to keep two versions, because IIUC no_std misses various safety features.

https://docs.rust-embedded.org/book/intro/no-std.html

I see, you refer to stack overflow protection missing in no_std, I didn't know that, thanks to point it to me.

A double API conditioned by the std feature ? yeah why not ! I will push that this weekend.

@Georges760
Copy link
Author

After reflexion, I think that a #![no_std] lib crate still does not enforce the main crate (the one choosing to link against std or not) so stack overflow protection is only lost if and only if the main crate choose to be #![no_std] (real embedded system).

If the main crate is std aware, even if it use some #![no_std] lib as dep, it will profit from the std security features.

handshake example is a good exemple of a 'main' std crate (using std::rand and std::time) using a #![no_std] lib dep (noise_sv2) and providing the Random Number Generator and System Time (of std) through the lib API so the lib does require std itself.

Breaking the API is a problem for std enviroement, and I fully agree it should be broken. So I will push this dual API (std unchanged, no_std new) according to a std feature enabled by default, in order to keep backward compat.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 7, 2024 •
edited
Loading

🐰 Bencher Report

Branchnoise_sv2_no_std
Testbedsv1
Click to view all benchmark results
BenchmarkEstimated CyclesBenchmark Result
estimated cycles
(Result Δ%)		Upper Boundary
1e3 x estimated cycles
(Limit %)		InstructionsBenchmark Result
instructions
(Result Δ%)		Upper Boundary
1e3 x instructions
(Limit %)		L1 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
1e3 x 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.42
(-0.39%)		8.67
(97.08%)		📈 view plot
🚷 view threshold		3.69
(-1.12%)		3.86
(95.54%)		📈 view plot
🚷 view threshold		5.16
(-1.31%)		5.45
(94.69%)		📈 view plot
🚷 view threshold		9.00
(+10.45%)		15.61
(57.64%)		📈 view plot
🚷 view threshold		92.00
(+0.97%)		96.15
(95.69%)
get_submit📈 view plot
🚷 view threshold		95.29
(-0.08%)		95.66
(99.61%)		📈 view plot
🚷 view threshold		59.35
(-0.12%)		59.70
(99.40%)		📈 view plot
🚷 view threshold		85.22
(-0.13%)		85.81
(99.31%)		📈 view plot
🚷 view threshold		46.00
(+3.79%)		59.74
(77.00%)		📈 view plot
🚷 view threshold		281.00
(+0.23%)		290.33
(96.79%)
get_subscribe📈 view plot
🚷 view threshold		7.94
(-0.82%)		8.23
(96.37%)		📈 view plot
🚷 view threshold		2.77
(-1.89%)		2.94
(93.94%)		📈 view plot
🚷 view threshold		3.85
(-2.21%)		4.14
(92.82%)		📈 view plot
🚷 view threshold		13.00
(+6.33%)		20.27
(64.12%)		📈 view plot
🚷 view threshold		115.00
(+0.44%)		117.82
(97.60%)
serialize_authorize📈 view plot
🚷 view threshold		12.14
(-1.00%)		12.51
(97.03%)		📈 view plot
🚷 view threshold		5.27
(-0.71%)		5.43
(97.05%)		📈 view plot
🚷 view threshold		7.33
(-0.82%)		7.60
(96.41%)		📈 view plot
🚷 view threshold		10.00
(+0.79%)		18.42
(54.28%)		📈 view plot
🚷 view threshold		136.00
(-1.29%)		142.44
(95.48%)
serialize_deserialize_authorize📈 view plot
🚷 view threshold		24.70
(-0.05%)		25.18
(98.09%)		📈 view plot
🚷 view threshold		9.84
(-0.27%)		10.01
(98.24%)		📈 view plot
🚷 view threshold		13.88
(-0.33%)		14.18
(97.88%)		📈 view plot
🚷 view threshold		37.00
(+3.70%)		45.52
(81.27%)		📈 view plot
🚷 view threshold		304.00
(+0.26%)		313.07
(97.10%)
serialize_deserialize_handle_authorize📈 view plot
🚷 view threshold		30.21
(-0.38%)		30.72
(98.33%)		📈 view plot
🚷 view threshold		12.02
(-0.35%)		12.19
(98.57%)		📈 view plot
🚷 view threshold		17.00
(-0.43%)		17.30
(98.27%)		📈 view plot
🚷 view threshold		60.00
(+7.79%)		67.10
(89.42%)		📈 view plot
🚷 view threshold		369.00
(-0.48%)		379.15
(97.32%)
serialize_deserialize_handle_submit📈 view plot
🚷 view threshold		126.30
(-0.12%)		126.78
(99.62%)		📈 view plot
🚷 view threshold		73.20
(-0.08%)		73.53
(99.56%)		📈 view plot
🚷 view threshold		104.91
(-0.10%)		105.49
(99.45%)		📈 view plot
🚷 view threshold		113.00
(+6.49%)		125.16
(90.28%)		📈 view plot
🚷 view threshold		595.00
(-0.39%)		608.20
(97.83%)
serialize_deserialize_handle_subscribe📈 view plot
🚷 view threshold		27.81
(-0.35%)		28.41
(97.91%)		📈 view plot
🚷 view threshold		9.58
(-0.54%)		9.76
(98.21%)		📈 view plot
🚷 view threshold		13.53
(-0.67%)		13.84
(97.81%)		📈 view plot
🚷 view threshold		70.00
(+8.92%)		77.40
(90.44%)		📈 view plot
🚷 view threshold		398.00
(-0.24%)		410.31
(97.00%)
serialize_deserialize_submit📈 view plot
🚷 view threshold		115.16
(-0.07%)		115.71
(99.53%)		📈 view plot
🚷 view threshold		68.06
(-0.01%)		68.41
(99.49%)		📈 view plot
🚷 view threshold		97.65
(-0.02%)		98.27
(99.37%)		📈 view plot
🚷 view threshold		65.00
(+0.86%)		85.62
(75.91%)		📈 view plot
🚷 view threshold		491.00
(-0.36%)		501.92
(97.82%)
serialize_deserialize_subscribe📈 view plot
🚷 view threshold		23.24
(-0.35%)		23.83
(97.53%)		📈 view plot
🚷 view threshold		8.14
(-0.59%)		8.31
(97.93%)		📈 view plot
🚷 view threshold		11.45
(-0.70%)		11.74
(97.50%)		📈 view plot
🚷 view threshold		41.00
(+6.26%)		50.36
(81.42%)		📈 view plot
🚷 view threshold		331.00
(-0.12%)		342.64
(96.60%)
serialize_submit📈 view plot
🚷 view threshold		99.67
(-0.13%)		100.13
(99.54%)		📈 view plot
🚷 view threshold		61.41
(-0.10%)		61.73
(99.48%)		📈 view plot
🚷 view threshold		88.08
(-0.11%)		88.64
(99.37%)		📈 view plot
🚷 view threshold		49.00
(+3.19%)		65.82
(74.45%)		📈 view plot
🚷 view threshold		324.00
(-0.32%)		335.66
(96.53%)
serialize_subscribe📈 view plot
🚷 view threshold		11.38
(-0.17%)		11.60
(98.10%)		📈 view plot
🚷 view threshold		4.12
(-1.18%)		4.28
(96.13%)		📈 view plot
🚷 view threshold		5.71
(-1.46%)		6.00
(95.15%)		📈 view plot
🚷 view threshold		15.00
(+10.92%)		23.25
(64.52%)		📈 view plot
🚷 view threshold		160.00
(+1.05%)		163.67
(97.76%)
🐰 View full continuous benchmarking report in Bencher

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 7, 2024 •
edited
Loading

🐰 Bencher Report

Branchnoise_sv2_no_std
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,521.40
(-0.94%)		6,911.94
(94.35%)
client-submit-serialize-deserialize📈 view plot
🚷 view threshold		7,313.60
(-1.55%)		7,834.75
(93.35%)
client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle📈 view plot
🚷 view threshold		7,921.10
(-2.18%)		9,255.74
(85.58%)
client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle📈 view plot
🚷 view threshold		892.72
(+2.96%)		941.22
(94.85%)
client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize📈 view plot
🚷 view threshold		679.13
(+0.77%)		715.48
(94.92%)
client-sv1-authorize-serialize/client-sv1-authorize-serialize📈 view plot
🚷 view threshold		249.09
(-0.02%)		269.51
(92.42%)
client-sv1-get-authorize/client-sv1-get-authorize📈 view plot
🚷 view threshold		157.34
(-0.07%)		166.06
(94.75%)
client-sv1-get-submit📈 view plot
🚷 view threshold		6,272.20
(-1.35%)		6,763.73
(92.73%)
client-sv1-get-subscribe/client-sv1-get-subscribe📈 view plot
🚷 view threshold		277.67
(-1.59%)		323.14
(85.93%)
client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle📈 view plot
🚷 view threshold		740.22
(+1.73%)		775.39
(95.46%)
client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize📈 view plot
🚷 view threshold		590.72
(+0.20%)		624.69
(94.56%)
client-sv1-subscribe-serialize/client-sv1-subscribe-serialize📈 view plot
🚷 view threshold		205.06
(-0.59%)		223.42
(91.78%)
🐰 View full continuous benchmarking report in Bencher

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 7, 2024 •
edited
Loading

🐰 Bencher Report

Branchnoise_sv2_no_std
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.17
(-2.14%)		58.25
(75.84%)
client_sv2_handle_message_mining📈 view plot
🚷 view threshold		74.48
(-4.18%)		103.79
(71.76%)
client_sv2_mining_message_submit_standard📈 view plot
🚷 view threshold		14.64
(-0.11%)		14.71
(99.54%)
client_sv2_mining_message_submit_standard_serialize📈 view plot
🚷 view threshold		260.91
(-1.91%)		289.23
(90.21%)
client_sv2_mining_message_submit_standard_serialize_deserialize📈 view plot
🚷 view threshold		624.25
(+1.79%)		647.10
(96.47%)
client_sv2_open_channel📈 view plot
🚷 view threshold		164.38
(-0.25%)		175.64
(93.59%)
client_sv2_open_channel_serialize📈 view plot
🚷 view threshold		284.60
(-0.23%)		308.27
(92.32%)
client_sv2_open_channel_serialize_deserialize📈 view plot
🚷 view threshold		379.70
(-0.98%)		407.68
(93.14%)
client_sv2_setup_connection📈 view plot
🚷 view threshold		157.98
(-1.18%)		170.58
(92.61%)
client_sv2_setup_connection_serialize📈 view plot
🚷 view threshold		445.56
(-5.12%)		552.11
(80.70%)
client_sv2_setup_connection_serialize_deserialize📈 view plot
🚷 view threshold		994.36
(-0.56%)		1,139.33
(87.28%)
🐰 View full continuous benchmarking report in Bencher

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 7, 2024 •
edited
Loading

🐰 Bencher Report

Branchnoise_sv2_no_std
Testbedsv2
Click to view all benchmark results
BenchmarkEstimated CyclesBenchmark Result
estimated cycles
(Result Δ%)		Upper Boundary
1e3 x 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.17
(+2.71%)		2.23
(97.23%)		📈 view plot
🚷 view threshold		473.00
(-0.05%)		488.92
(96.74%)		📈 view plot
🚷 view threshold		732.00
(-0.49%)		757.93
(96.58%)		📈 view plot
🚷 view threshold		7.00
(+40.88%)		11.89
(58.87%)		📈 view plot
🚷 view threshold		40.00
(+3.75%)		41.61
(96.14%)
client_sv2_handle_message_mining📈 view plot
🚷 view threshold		8.26
(+0.50%)		8.39
(98.53%)		📈 view plot
🚷 view threshold		2,137.00
(-0.01%)		2,140.01
(99.86%)		📈 view plot
🚷 view threshold		3,152.00
(-0.22%)		3,169.05
(99.46%)		📈 view plot
🚷 view threshold		42.00
(+18.00%)		42.09
(99.78%)		📈 view plot
🚷 view threshold		140.00
(+0.33%)		144.09
(97.16%)
client_sv2_mining_message_submit_standard📈 view plot
🚷 view threshold		6.32
(+0.40%)		6.44
(98.15%)		📈 view plot
🚷 view threshold		1,750.00
(-0.02%)		1,766.07
(99.09%)		📈 view plot
🚷 view threshold		2,548.00
(-0.15%)		2,574.44
(98.97%)		📈 view plot
🚷 view threshold		20.00
(+17.11%)		24.26
(82.44%)		📈 view plot
🚷 view threshold		105.00
(+0.39%)		108.61
(96.68%)
client_sv2_mining_message_submit_standard_serialize📈 view plot
🚷 view threshold		14.75
(+0.20%)		14.93
(98.81%)		📈 view plot
🚷 view threshold		4,694.00
(-0.01%)		4,710.07
(99.66%)		📈 view plot
🚷 view threshold		6,747.00
(-0.14%)		6,785.68
(99.43%)		📈 view plot
🚷 view threshold		54.00
(+20.17%)		61.05
(88.46%)		📈 view plot
🚷 view threshold		221.00
(-0.08%)		226.28
(97.66%)
client_sv2_mining_message_submit_standard_serialize_deserialize📈 view plot
🚷 view threshold		27.76
(+0.63%)		28.01
(99.11%)		📈 view plot
🚷 view threshold		10,645.00
(+0.40%)		10,688.15
(99.60%)		📈 view plot
🚷 view threshold		15,503.00
(+0.45%)		15,576.70
(99.53%)		📈 view plot
🚷 view threshold		92.00
(+10.56%)		98.06
(93.82%)		📈 view plot
🚷 view threshold		337.00
(+0.51%)		342.45
(98.41%)
client_sv2_open_channel📈 view plot
🚷 view threshold		4.52
(+2.84%)		4.56
(99.13%)		📈 view plot
🚷 view threshold		1,461.00
(-0.02%)		1,476.92
(98.92%)		📈 view plot
🚷 view threshold		2,154.00
(-0.30%)		2,183.67
(98.64%)		📈 view plot
🚷 view threshold		11.00
(+36.30%)		13.45
(81.77%)		📈 view plot
🚷 view threshold		66.00
(+5.32%)		67.22
(98.19%)
client_sv2_open_channel_serialize📈 view plot
🚷 view threshold		14.11
(+0.65%)		14.19
(99.40%)		📈 view plot
🚷 view threshold		5,064.00
(-0.00%)		5,079.92
(99.69%)		📈 view plot
🚷 view threshold		7,318.00
(-0.11%)		7,351.39
(99.55%)		📈 view plot
🚷 view threshold		42.00
(+16.57%)		47.11
(89.15%)		📈 view plot
🚷 view threshold		188.00
(+1.06%)		190.97
(98.44%)
client_sv2_open_channel_serialize_deserialize📈 view plot
🚷 view threshold		22.87
(+0.78%)		23.01
(99.39%)		📈 view plot
🚷 view threshold		8,040.00
(+0.11%)		8,054.11
(99.82%)		📈 view plot
🚷 view threshold		11,686.00
(+0.02%)		11,711.31
(99.78%)		📈 view plot
🚷 view threshold		87.00
(+15.46%)		88.84
(97.93%)		📈 view plot
🚷 view threshold		307.00
(+1.10%)		311.19
(98.65%)
client_sv2_setup_connection📈 view plot
🚷 view threshold		4.74
(+1.13%)		4.79
(99.05%)		📈 view plot
🚷 view threshold		1,502.00
(-0.02%)		1,517.92
(98.95%)		📈 view plot
🚷 view threshold		2,276.00
(-0.10%)		2,299.53
(98.98%)		📈 view plot
🚷 view threshold		10.00
(+6.31%)		15.39
(64.96%)		📈 view plot
🚷 view threshold		69.00
(+2.21%)		70.06
(98.48%)
client_sv2_setup_connection_serialize📈 view plot
🚷 view threshold		16.24
(+0.56%)		16.31
(99.57%)		📈 view plot
🚷 view threshold		5,963.00
(-0.00%)		5,978.92
(99.73%)		📈 view plot
🚷 view threshold		8,655.00
(-0.10%)		8,691.57
(99.58%)		📈 view plot
🚷 view threshold		47.00
(+17.59%)		54.00
(87.03%)		📈 view plot
🚷 view threshold		210.00
(+0.88%)		211.96
(99.08%)
client_sv2_setup_connection_serialize_deserialize📈 view plot
🚷 view threshold		35.80
(+0.61%)		35.91
(99.71%)		📈 view plot
🚷 view threshold		14,888.00
(+0.16%)		14,913.03
(99.83%)		📈 view plot
🚷 view threshold		21,869.00
(+0.13%)		21,911.78
(99.80%)		📈 view plot
🚷 view threshold		106.00
(+13.65%)		116.62
(90.90%)		📈 view plot
🚷 view threshold		383.00
(+0.94%)		385.17
(99.44%)
🐰 View full continuous benchmarking report in Bencher

@Georges760
Copy link
Author

Thanks to @Sjors good comment, the current API is unchanged and an addition one allow no_std.

This is the way secp256k1 crate is doing it too, see sign_schnorr_with_rng.

@Georges760 Georges760 marked this pull request as ready for review December 7, 2024 11:01
Shourya742
Shourya742 suggested changes Dec 7, 2024
View reviewed changes
Copy link
Contributor

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

utack 9f2a908. Will test the PR once with the whole setup up and running. Rest looks ok on first glance with minor nits

protocols/v2/noise-sv2/src/handshake.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/signature_message.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/responder.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/responder.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/handshake.rs Show resolved Hide resolved
@Georges760
Copy link
Author

utack 9f2a908. Will test the PR once with the whole setup up and running. Rest looks ok on first glance with minor nits

PR good to merge (for me) nothing more to add, you can test it

Shourya742
Shourya742 suggested changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

tack feeb225. Just a few minor nits left, and we’re good to go. The handshake messages look as expected.

protocols/v2/noise-sv2/src/handshake.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/initiator.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/initiator.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/initiator.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/responder.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/responder.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/responder.rs Show resolved Hide resolved
protocols/v2/noise-sv2/src/signature_message.rs Show resolved Hide resolved
protocols/v2/noise-sv2/Cargo.toml Outdated Show resolved Hide resolved
@Shourya742
Copy link
Contributor

@Georges760 You’ll need to bump the major version to make CI happy.

@Georges760
Copy link
Author

@Georges760 You’ll need to bump the major version to make CI happy.

do you want me to do it in this PR ?

also should I rebase on main ?

Shourya742
Shourya742 suggested changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

These doc comments are exactly similar std counterparts. Could you add a quick note on why we need this with the random number generator? instead of this. Also, we usually skip 'Arguments' and 'Return type' sections in our docs, so if you can update those too, that’d be great!

Shourya742
Shourya742 approved these changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

ACK

@Georges760
Copy link
Author

The CI fails because of infra, need to be re-run when CI depending server/files are accessible again

@Shourya742
Copy link
Contributor

The CI fails because of infra, need to be re-run when CI depending server/files are accessible again

Ya I noticed it now... I have opened a PR to address this, #1293

@Georges760 Georges760 force-pushed the noise_sv2_no_std branch 2 times, most recently from 50c9902 to 27403d2 Compare December 15, 2024 14:30
@Sjors Sjors mentioned this pull request Dec 16, 2024
Closed
@Georges760 Georges760 force-pushed the noise_sv2_no_std branch 3 times, most recently from 25cbb20 to 3df4528 Compare December 18, 2024 08:27
@Georges760
Copy link
Author

CI is still randomly failing :

  • on last dev call, the re-run made it pass
  • after my last reabse, it PASS
  • after my README last push, it FAIL again (i am unable to re-run the job)

@Georges760 Georges760 force-pushed the noise_sv2_no_std branch 5 times, most recently from 7176f7d to dda9233 Compare December 19, 2024 15:08
This was referenced Dec 20, 2024
Open
Draft
Georges Palauqui and others added 9 commits December 26, 2024 14:48
noise_sv2: is a optional no_std crate, std by default
9b91f93 
- std::ptr -> core::ptr
- std::boxed::Box -> alloc::boxed::Box
- std::vec::Vec -> alloc::vec::Vec
- std::string::{String, ToString} -> alloc::string::{String, ToString}
- std::convert::TryInto -> core::convert::TryInto
- std::fmt::{Debug, Formatter, Result} -> core::fmt::{Debug, Formatter, Result}
- std::time::Duration -> core::time::Duration

To have a `no-std` version, the `--no-default-features` must be used
public API changed to be able to be `no_std` (delegate the choice of the Ramdom Number Generator and the current System Time to the caller, instead of assuming using the ones from `std`) :
- `Initiator::new()`, `Initiator::from_raw_k()`, `Initiator::without_pk()`, `Responder::new()`, `Responder::from_authority_kp()` and `Responder::generate_key()` take an additional argument implementing rand::Rng + ?Sized
- `Responder::step_1()` and `SignatureNoiseMessage::sign()` take an additional argument implementing rand::Rng + rand::CryptoRng
- `Initiator::step_2()`, `Responder::step_1()`, `Responder::step_2()` and `SignatureNoiseMessage::verify()` take an additional argument for the current system time epoch
noise_sv2: keep current (std) API unchanged, add a no_std compliant A…
feadcd3 
…PI with `*_with_rng` and `*_with_now`
utils/key-utils: revert changes, doesn't have to be changed in this PR
157cfc8
fix no-default-features tests
192aee9
Shourya742 review
8db277f
inline macro and rust-doc
ee0527d
@Georges760 @Shourya742
panic strategy for different build profiles
e0dbbea 
Co-authored-by: bit-aloo <[email protected]>
refactor rust-docs on new functions
2425377
add a no_std note in the README
e137377
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shourya742 Shourya742 Shourya742 approved these changes

At least 1 approving review is required to merge this pull request.

Assignees

@Georges760 Georges760

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Georges760 @Sjors @Shourya742