Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ crc32fast = { opt-level = 3 }
crossbeam-deque = { opt-level = 3 }
crypto-mac = { opt-level = 3 }
curve25519-dalek = { opt-level = 3 }
ed25519-dalek = { opt-level = 3 }
ed25519-zebra = { opt-level = 3 }
flate2 = { opt-level = 3 }
futures-channel = { opt-level = 3 }
hashbrown = { opt-level = 3 }
Expand Down
9 changes: 3 additions & 6 deletions primitives/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,7 @@ dyn-clonable = { version = "0.9.0", optional = true }
thiserror = { version = "1.0.21", optional = true }

# full crypto
ed25519-dalek = { version = "1.0.1", default-features = false, features = [
"u64_backend",
"alloc",
], optional = true }
ed25519-zebra = { version = "2.2.0", default-features = false, optional = true}
blake2-rfc = { version = "0.2.18", default-features = false, optional = true }
tiny-keccak = { version = "2.0.1", features = ["keccak"], optional = true }
schnorrkel = { version = "0.9.1", features = [
Expand Down Expand Up @@ -102,7 +99,7 @@ std = [
"serde",
"twox-hash/std",
"blake2-rfc/std",
"ed25519-dalek/std",
"ed25519-zebra",
"hex/std",
"base58",
"substrate-bip39",
Expand Down Expand Up @@ -130,7 +127,7 @@ std = [
# or Intel SGX.
# For the regular wasm runtime builds this should not be used.
full_crypto = [
"ed25519-dalek",
"ed25519-zebra",
"blake2-rfc",
"tiny-keccak",
"schnorrkel",
Expand Down
51 changes: 28 additions & 23 deletions primitives/core/src/ed25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use bip39::{Language, Mnemonic, MnemonicType};
#[cfg(feature = "full_crypto")]
use core::convert::TryFrom;
#[cfg(feature = "full_crypto")]
use ed25519_dalek::{Signer as _, Verifier as _};
use ed25519_zebra::{SigningKey, VerificationKey};
#[cfg(feature = "std")]
use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
use sp_runtime_interface::pass_by::PassByInner;
Expand All @@ -63,16 +63,18 @@ pub struct Public(pub [u8; 32]);

/// A key pair.
#[cfg(feature = "full_crypto")]
pub struct Pair(ed25519_dalek::Keypair);
pub struct Pair {
public: VerificationKey,
secret: SigningKey,
}

#[cfg(feature = "full_crypto")]
impl Clone for Pair {
fn clone(&self) -> Self {
Pair(ed25519_dalek::Keypair {
public: self.0.public,
secret: ed25519_dalek::SecretKey::from_bytes(self.0.secret.as_bytes())
.expect("key is always the correct size; qed"),
})
Pair {
public: self.public.clone(),
secret: self.secret.clone(),
}
}
}

Expand Down Expand Up @@ -484,10 +486,11 @@ impl TraitPair for Pair {
///
/// You should never need to use this; generate(), generate_with_phrase
fn from_seed_slice(seed_slice: &[u8]) -> Result<Pair, SecretStringError> {
let secret = ed25519_dalek::SecretKey::from_bytes(seed_slice)
// does try_into consume the seed? can I consume seed_slice here? I think not right?
Copy link
Contributor

@gilescope gilescope Sep 3, 2021

Choose a reason for hiding this comment

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

You can't consume the seed in this fn but have raised a PR to make from_seed (above) take ownership of the Seed. ( #9683 )

That then allows SigningKey::from(seed) in the from_seed function.

Copy link
Contributor Author

@jakehemmerle jakehemmerle Sep 8, 2021

Choose a reason for hiding this comment

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

This was my first rust ticket and I hadn't wrapped my head around ownership yet :)

I also agree with your point in that PR that the seed should be consumed; will update this if that gets merged

let secret = SigningKey::try_from(seed_slice)
.map_err(|_| SecretStringError::InvalidSeedLength)?;
let public = ed25519_dalek::PublicKey::from(&secret);
Ok(Pair(ed25519_dalek::Keypair { secret, public }))
let public = VerificationKey::from(&secret);
Ok(Pair {secret, public})
}

/// Derive a child key from a series of given junctions.
Expand All @@ -496,7 +499,7 @@ impl TraitPair for Pair {
path: Iter,
_seed: Option<Seed>,
) -> Result<(Pair, Option<Seed>), DeriveError> {
let mut acc = self.0.secret.to_bytes();
let mut acc: [u8; 32] = self.secret.into();
for j in path {
match j {
DeriveJunction::Soft(_cc) => return Err(DeriveError::SoftKeyInPath),
Expand All @@ -508,15 +511,14 @@ impl TraitPair for Pair {

/// Get the public key.
fn public(&self) -> Public {
let mut r = [0u8; 32];
let pk = self.0.public.as_bytes();
r.copy_from_slice(pk);
Public(r)
// does this consume public? Is that why we used copy_from_slice?
let pk: [u8; 32] = self.public.into();
Public(pk)
}

/// Sign a message.
fn sign(&self, message: &[u8]) -> Signature {
let r = self.0.sign(message).to_bytes();
let r: [u8; 64] = self.secret.sign(message).into();
Signature::from_raw(r)
}

Expand All @@ -530,17 +532,20 @@ impl TraitPair for Pair {
/// This doesn't use the type system to ensure that `sig` and `pubkey` are the correct
/// size. Use it only if you're coming from byte buffers and need the speed.
fn verify_weak<P: AsRef<[u8]>, M: AsRef<[u8]>>(sig: &[u8], message: M, pubkey: P) -> bool {
let public_key = match ed25519_dalek::PublicKey::from_bytes(pubkey.as_ref()) {
let public_key = match VerificationKey::try_from(pubkey.as_ref()) {
Ok(pk) => pk,
Err(_) => return false,
};

let sig = match ed25519_dalek::Signature::try_from(sig) {
let sig = match ed25519_zebra::Signature::try_from(sig) {
Ok(s) => s,
Err(_) => return false,
};

public_key.verify(message.as_ref(), &sig).is_ok()
match public_key.verify(&sig, message.as_ref()) {
Ok(_) => true,
_ => false,
}
}

/// Return a vec filled with raw data.
Expand All @@ -552,8 +557,8 @@ impl TraitPair for Pair {
#[cfg(feature = "full_crypto")]
impl Pair {
/// Get the seed for this key.
pub fn seed(&self) -> &Seed {
self.0.secret.as_bytes()
pub fn seed(&self) -> Seed {
self.secret.into()
}

/// Exactly as `from_string` except that if no matches are found then, the the first 32
Expand Down Expand Up @@ -605,12 +610,12 @@ mod test {
fn seed_and_derive_should_work() {
let seed = hex!("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60");
let pair = Pair::from_seed(&seed);
assert_eq!(pair.seed(), &seed);
assert_eq!(pair.seed(), seed);
let path = vec![DeriveJunction::Hard([0u8; 32])];
let derived = pair.derive(path.into_iter(), None).ok().unwrap().0;
assert_eq!(
derived.seed(),
&hex!("ede3354e133f9c8e337ddd6ee5415ed4b4ffe5fc7d21e933f4930a3730e5b21c")
hex!("ede3354e133f9c8e337ddd6ee5415ed4b4ffe5fc7d21e933f4930a3730e5b21c")
);
}

Expand Down
30 changes: 15 additions & 15 deletions primitives/io/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1619,15 +1619,18 @@ mod tests {
ext.register_extension(TaskExecutorExt::new(TaskExecutor::new()));
ext.execute_with(|| {
let pair = sr25519::Pair::generate_with_phrase(None).0;
let pair_unused = sr25519::Pair::generate_with_phrase(None).0;
crypto::start_batch_verify();
for it in 0..70 {
let msg = format!("Schnorrkel {}!", it);
let signature = pair.sign(msg.as_bytes());
crypto::sr25519_batch_verify(&signature, msg.as_bytes(), &pair.public());
}

// push invlaid
crypto::sr25519_batch_verify(&Default::default(), &Vec::new(), &Default::default());
// push invalid
let msg = format!("asdf!");
let signature = pair.sign(msg.as_bytes());
crypto::sr25519_batch_verify(&signature, msg.as_bytes(), &pair_unused.public());
assert!(!crypto::finish_batch_verify());

crypto::start_batch_verify();
Expand All @@ -1645,11 +1648,6 @@ mod tests {
let mut ext = BasicExternalities::default();
ext.register_extension(TaskExecutorExt::new(TaskExecutor::new()));
ext.execute_with(|| {
// invalid ed25519 signature
crypto::start_batch_verify();
crypto::ed25519_batch_verify(&Default::default(), &Vec::new(), &Default::default());
assert!(!crypto::finish_batch_verify());

// 2 valid ed25519 signatures
crypto::start_batch_verify();

Expand All @@ -1668,12 +1666,13 @@ mod tests {
// 1 valid, 1 invalid ed25519 signature
crypto::start_batch_verify();

let pair = ed25519::Pair::generate_with_phrase(None).0;
let pair1 = ed25519::Pair::generate_with_phrase(None).0;
let pair2 = ed25519::Pair::generate_with_phrase(None).0;
let msg = b"Important message";
let signature = pair.sign(msg);
crypto::ed25519_batch_verify(&signature, msg, &pair.public());
let signature = pair1.sign(msg);

crypto::ed25519_batch_verify(&Default::default(), &Vec::new(), &Default::default());
crypto::ed25519_batch_verify(&signature, msg, &pair1.public());
crypto::ed25519_batch_verify(&signature, msg, &pair2.public());

assert!(!crypto::finish_batch_verify());

Expand All @@ -1700,12 +1699,13 @@ mod tests {
// 1 valid sr25519, 1 invalid sr25519
crypto::start_batch_verify();

let pair = sr25519::Pair::generate_with_phrase(None).0;
let pair1 = sr25519::Pair::generate_with_phrase(None).0;
let pair2 = sr25519::Pair::generate_with_phrase(None).0;
let msg = b"Schnorrkcel!";
let signature = pair.sign(msg);
crypto::sr25519_batch_verify(&signature, msg, &pair.public());

crypto::sr25519_batch_verify(&Default::default(), &Vec::new(), &Default::default());

crypto::sr25519_batch_verify(&signature, msg, &pair1.public());
crypto::sr25519_batch_verify(&signature, msg, &pair2.public());

assert!(!crypto::finish_batch_verify());
});
Expand Down