-
Notifications
You must be signed in to change notification settings - Fork 666
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.
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
core::crypto
: features cleanup in tests
#1983
Changes from 1 commit
df41de0
11708ad
4b0111f
e3fa4a5
aac6793
01b0bdd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -535,13 +535,20 @@ impl CryptoType for Pair { | |
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
use crate::crypto::{ | ||
set_default_ss58_version, PublicError, Ss58AddressFormat, Ss58AddressFormatRegistry, | ||
DEV_PHRASE, | ||
}; | ||
|
||
#[cfg(feature = "std")] | ||
use crate::crypto::set_default_ss58_version; | ||
#[cfg(feature = "std")] | ||
use crate::crypto::DEV_PHRASE; | ||
#[cfg(all(feature = "serde", feature = "full_crypto"))] | ||
use crate::crypto::{PublicError, Ss58AddressFormat, Ss58AddressFormatRegistry}; | ||
#[cfg(feature = "serde")] | ||
use serde_json; | ||
#[cfg(feature = "full_crypto")] | ||
use sp_std::vec; | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn default_phrase_should_be_used() { | ||
assert_eq!( | ||
Pair::from_string("//Alice///password", None).unwrap().public(), | ||
|
@@ -552,6 +559,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "full_crypto")] | ||
fn seed_and_derive_should_work() { | ||
let seed = array_bytes::hex2array_unchecked( | ||
"9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60", | ||
|
@@ -569,6 +577,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we only run tests in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I want e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But you don't run this anymore? And serde is mainly about testing that it compiles? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to run it locally. I want to have ability to test the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have trait methods definition under
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These features are not changing anything in the implementation, they only unlock different kind of functions etc. When you run everything using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because you are using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO the tests should be runnable for any set of features provided by this module. The point of this PR was to check if tests are fine for particular features (or combinations of them). If we always want to test with default features, then this PR does not make sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean checking that it compiles make sense. (without the tests). To ensure that these features compile on their own. |
||
fn test_vector_should_work() { | ||
let pair = Pair::from_seed(&array_bytes::hex2array_unchecked( | ||
"9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60", | ||
|
@@ -588,6 +597,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn test_vector_by_string_should_work() { | ||
let pair = Pair::from_string( | ||
"0x9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60", | ||
|
@@ -609,6 +619,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn generated_pair_should_work() { | ||
let (pair, _) = Pair::generate(); | ||
let public = pair.public(); | ||
|
@@ -619,6 +630,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn seeded_pair_should_work() { | ||
let pair = Pair::from_seed(b"12345678901234567890123456789012"); | ||
let public = pair.public(); | ||
|
@@ -630,12 +642,12 @@ mod test { | |
); | ||
let message = array_bytes::hex2bytes_unchecked("2f8c6129d816cf51c374bc7f08c3e63ed156cf78aefb4a6550d97b87997977ee00000000000000000200d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a4500000000000000"); | ||
let signature = pair.sign(&message[..]); | ||
println!("Correct signature: {:?}", signature); | ||
assert!(Pair::verify(&signature, &message[..], &public)); | ||
assert!(!Pair::verify(&signature, "Other message", &public)); | ||
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn generate_with_phrase_recovery_possible() { | ||
let (pair1, phrase, _) = Pair::generate_with_phrase(None); | ||
let (pair2, _) = Pair::from_phrase(&phrase, None).unwrap(); | ||
|
@@ -644,6 +656,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn generate_with_password_phrase_recovery_possible() { | ||
let (pair1, phrase, _) = Pair::generate_with_phrase(Some("password")); | ||
let (pair2, _) = Pair::from_phrase(&phrase, Some("password")).unwrap(); | ||
|
@@ -652,6 +665,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn password_does_something() { | ||
let (pair1, phrase, _) = Pair::generate_with_phrase(Some("password")); | ||
let (pair2, _) = Pair::from_phrase(&phrase, None).unwrap(); | ||
|
@@ -660,16 +674,17 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(all(feature = "full_crypto", feature = "serde"))] | ||
fn ss58check_roundtrip_works() { | ||
let pair = Pair::from_seed(b"12345678901234567890123456789012"); | ||
let public = pair.public(); | ||
let s = public.to_ss58check(); | ||
println!("Correct: {}", s); | ||
let cmp = Public::from_ss58check(&s).unwrap(); | ||
assert_eq!(cmp, public); | ||
} | ||
|
||
#[test] | ||
#[cfg(all(feature = "full_crypto", feature = "serde"))] | ||
fn ss58check_format_check_works() { | ||
let pair = Pair::from_seed(b"12345678901234567890123456789012"); | ||
let public = pair.public(); | ||
|
@@ -679,6 +694,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(all(feature = "full_crypto", feature = "serde"))] | ||
fn ss58check_full_roundtrip_works() { | ||
let pair = Pair::from_seed(b"12345678901234567890123456789012"); | ||
let public = pair.public(); | ||
|
@@ -696,6 +712,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn ss58check_custom_format_works() { | ||
// We need to run this test in its own process to not interfere with other tests running in | ||
// parallel and also relying on the ss58 version. | ||
|
@@ -730,6 +747,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(all(feature = "full_crypto", feature = "serde"))] | ||
fn signature_serialization_works() { | ||
let pair = Pair::from_seed(b"12345678901234567890123456789012"); | ||
let message = b"Something important"; | ||
|
@@ -742,6 +760,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "serde")] | ||
fn signature_serialization_doesnt_panic() { | ||
fn deserialize_signature(text: &str) -> Result<Signature, serde_json::error::Error> { | ||
serde_json::from_str(text) | ||
|
@@ -753,6 +772,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn sign_prehashed_works() { | ||
let (pair, _, _) = Pair::generate_with_phrase(Some("password")); | ||
|
||
|
@@ -777,6 +797,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn verify_prehashed_works() { | ||
let (pair, _, _) = Pair::generate_with_phrase(Some("password")); | ||
|
||
|
@@ -791,6 +812,7 @@ mod test { | |
} | ||
|
||
#[test] | ||
#[cfg(feature = "std")] | ||
fn recover_prehashed_works() { | ||
let (pair, _, _) = Pair::generate_with_phrase(Some("password")); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably should not be here. However without this compilation fails:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this is wrong and needs to be removed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Substrate currently assumed that
no_std == arch::wasm32
. Shitty, I know, but that is currently the assumption.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, ideally that assert should be changed to check for
wasm32
and not forno_std
, and then you won't need to disable it....as Basti said, the current assumption everywhere is that no_std equals WASM, which, uh, I guess I'll have to fix soon-ish since I'll be wanting to get the RISC-V based runtimes working. :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we remove this one too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good idea.