diff --git a/primitives/core/Cargo.toml b/primitives/core/Cargo.toml index 706a9cb276f71..8242ff8044466 100644 --- a/primitives/core/Cargo.toml +++ b/primitives/core/Cargo.toml @@ -28,13 +28,13 @@ parking_lot = { version = "0.9.0", optional = true } sp-debug-derive = { version = "2.0.0", path = "../debug-derive" } sp-externalities = { version = "2.0.0", optional = true, path = "../externalities" } sp-storage = { version = "2.0.0", default-features = false, path = "../storage" } +libsecp256k1 = { version = "0.3.2", default-features = false } # full crypto ed25519-dalek = { version = "1.0.0-pre.3", default-features = false, features = ["u64_backend", "alloc"], 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.8.5", features = ["preaudit_deprecated", "u64_backend"], default-features = false, optional = true } -libsecp256k1 = { version = "0.3.2", default-features = false, optional = true } sha2 = { version = "0.8.0", default-features = false, optional = true } hex = { version = "0.4", default-features = false, optional = true } twox-hash = { version = "1.5.0", default-features = false, optional = true } @@ -90,7 +90,7 @@ std = [ "schnorrkel/std", "regex", "num-traits/std", - "libsecp256k1", + "libsecp256k1/std", "tiny-keccak", "sp-debug-derive/std", "sp-externalities", @@ -107,7 +107,6 @@ full_crypto = [ "blake2-rfc", "tiny-keccak", "schnorrkel", - "libsecp256k1", "hex", "sha2", "twox-hash", diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index e097d0c5e6fe6..fbdb8a56f75ff 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -46,9 +46,14 @@ use secp256k1::{PublicKey, SecretKey}; #[cfg(feature = "full_crypto")] type Seed = [u8; 32]; -/// The ECDSA 64-byte raw public key. +/// The ECDSA public key. #[derive(Clone, Encode, Decode)] -pub struct Public(pub [u8; 64]); +pub enum Public { + /// A full raw ECDSA public key. + Full([u8; 64]), + /// A compressed ECDSA public key. + Compressed([u8; 33]), +} impl PartialOrd for Public { fn partial_cmp(&self, other: &Self) -> Option { @@ -58,47 +63,113 @@ impl PartialOrd for Public { impl Ord for Public { fn cmp(&self, other: &Self) -> Ordering { - self.0[..].cmp(&other.0[..]) + self.as_ref().cmp(&other.as_ref()) } } impl PartialEq for Public { fn eq(&self, other: &Self) -> bool { - &self.0[..] == &other.0[..] + self.as_ref() == other.as_ref() } } impl Eq for Public {} -impl Default for Public { - fn default() -> Self { - Public([0u8; 64]) +/// An error type for SS58 decoding. +#[cfg(feature = "std")] +#[derive(Clone, Copy, Eq, PartialEq, Debug)] +pub enum PublicError { + /// Bad alphabet. + BadBase58, + /// Bad length. + BadLength, + /// Unknown version. + UnknownVersion, + /// Invalid checksum. + InvalidChecksum, +} + +impl Public { + /// A new instance from the given 64-byte `data`. + /// + /// NOTE: No checking goes on to ensure this is a real public key. Only use it if + /// you are certain that the array actually is a pubkey. GIGO! + pub fn from_raw(data: [u8; 64]) -> Self { + Self::Full(data) + } + + /// A new instance from the given 65-byte `data`. + /// + /// NOTE: No checking goes on to ensure this is a real public key. Only use it if + /// you are certain that the array actually is a pubkey. GIGO! + pub fn from_full(data: [u8; 65]) -> Self { + let raw_key = &data[1..]; + let mut key = [0u8; 64]; + key.copy_from_slice(raw_key); + Self::Full(key) + } + + /// Return in compressed format. + /// + /// Returns an error if `self` is an invalid full public key. + pub fn as_compressed(&self) -> Result<[u8; 33], ()> { + match self { + Self::Full(d) => secp256k1::PublicKey::parse_slice(d, None) + .map(|k| k.serialize_compressed()) + .map_err(|_| ()), + Self::Compressed(d) => Ok(*d), + } + } + + /// Convert `Self` into a compressed public key. + /// + /// Returns an error if `self` is an invalid full public key. + pub fn into_compressed(self) -> Result { + self.as_compressed().map(Self::Compressed) } } -/// A key pair. -#[cfg(feature = "full_crypto")] -#[derive(Clone)] -pub struct Pair { - public: PublicKey, - secret: SecretKey, +impl TraitPublic for Public { + /// A new instance from the given slice that should be 33 bytes long. + /// + /// NOTE: No checking goes on to ensure this is a real public key. Only use it if + /// you are certain that the array actually is a pubkey. GIGO! + fn from_slice(data: &[u8]) -> Self { + if data.len() == 33 { + let mut r = [0u8; 33]; + r.copy_from_slice(data); + Self::Compressed(r) + } else { + let mut r = [0u8; 64]; + r.copy_from_slice(data); + Self::Full(r) + } + } } -impl AsRef<[u8; 64]> for Public { - fn as_ref(&self) -> &[u8; 64] { - &self.0 +impl Derive for Public {} + +impl Default for Public { + fn default() -> Self { + Public::Full([0u8; 64]) } } impl AsRef<[u8]> for Public { fn as_ref(&self) -> &[u8] { - &self.0[..] + match self { + Self::Full(d) => &d[..], + Self::Compressed(d) => &d[..], + } } } impl AsMut<[u8]> for Public { fn as_mut(&mut self) -> &mut [u8] { - &mut self.0[..] + match self { + Self::Full(d) => &mut d[..], + Self::Compressed(d) => &mut d[..], + } } } @@ -106,22 +177,14 @@ impl sp_std::convert::TryFrom<&[u8]> for Public { type Error = (); fn try_from(data: &[u8]) -> Result { - if data.len() == 64 { - let mut inner = [0u8; 64]; - inner.copy_from_slice(data); - Ok(Public(inner)) + if data.len() == 33 || data.len() == 64 { + Ok(Self::from_slice(data)) } else { Err(()) } } } -impl From for [u8; 64] { - fn from(x: Public) -> Self { - x.0 - } -} - #[cfg(feature = "full_crypto")] impl From for Public { fn from(x: Pair) -> Self { @@ -131,7 +194,7 @@ impl From for Public { impl UncheckedFrom<[u8; 64]> for Public { fn unchecked_from(x: [u8; 64]) -> Self { - Public(x) + Public::Full(x) } } @@ -146,7 +209,7 @@ impl std::fmt::Display for Public { impl std::fmt::Debug for Public { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { let s = self.to_ss58check(); - write!(f, "{} ({}...)", crate::hexdisplay::HexDisplay::from(&&self.0[..]), &s[0..8]) + write!(f, "{} ({}...)", crate::hexdisplay::HexDisplay::from(&self.as_ref()), &s[0..8]) } } @@ -168,7 +231,7 @@ impl<'de> Deserialize<'de> for Public { #[cfg(feature = "full_crypto")] impl sp_std::hash::Hash for Public { fn hash(&self, state: &mut H) { - self.0.hash(state); + self.as_ref().hash(state); } } @@ -317,60 +380,6 @@ impl<'a> TryFrom<&'a Signature> for (secp256k1::Signature, secp256k1::RecoveryId } } -/// An error type for SS58 decoding. -#[cfg(feature = "std")] -#[derive(Clone, Copy, Eq, PartialEq, Debug)] -pub enum PublicError { - /// Bad alphabet. - BadBase58, - /// Bad length. - BadLength, - /// Unknown version. - UnknownVersion, - /// Invalid checksum. - InvalidChecksum, -} - -impl Public { - /// A new instance from the given 64-byte `data`. - /// - /// NOTE: No checking goes on to ensure this is a real public key. Only use it if - /// you are certain that the array actually is a pubkey. GIGO! - pub fn from_raw(data: [u8; 64]) -> Self { - Public(data) - } - - /// A new instance from the given 65-byte `data`. - /// - /// NOTE: No checking goes on to ensure this is a real public key. Only use it if - /// you are certain that the array actually is a pubkey. GIGO! - pub fn from_full(data: [u8; 65]) -> Self { - let raw_key = &data[1..]; - let mut key = [0u8; 64]; - key.copy_from_slice(raw_key); - Public(key) - } - - /// Return a slice filled with raw data. - pub fn as_array_ref(&self) -> &[u8; 64] { - self.as_ref() - } -} - -impl TraitPublic for Public { - /// A new instance from the given slice that should be 33 bytes long. - /// - /// NOTE: No checking goes on to ensure this is a real public key. Only use it if - /// you are certain that the array actually is a pubkey. GIGO! - fn from_slice(data: &[u8]) -> Self { - let mut r = [0u8; 64]; - r.copy_from_slice(data); - Public(r) - } -} - -impl Derive for Public {} - /// Derive a single hard junction. #[cfg(feature = "full_crypto")] fn derive_hard_junction(secret_seed: &Seed, cc: &[u8; 32]) -> Seed { @@ -388,6 +397,14 @@ pub enum DeriveError { SoftKeyInPath, } +/// A key pair. +#[cfg(feature = "full_crypto")] +#[derive(Clone)] +pub struct Pair { + public: PublicKey, + secret: SecretKey, +} + #[cfg(feature = "full_crypto")] impl TraitPair for Pair { type Public = Public; @@ -473,7 +490,9 @@ impl TraitPair for Pair { let message = secp256k1::Message::parse(&blake2_256(message.as_ref())); let sig: (_, _) = match sig.try_into() { Ok(x) => x, _ => return false }; match secp256k1::recover(&message, &sig.0, &sig.1) { - Ok(actual) => &pubkey.0[..] == &actual.serialize()[1..], + Ok(actual) => pubkey.as_compressed() + .map(|p| &p[..] == &actual.serialize_compressed()[..]) + .unwrap_or(false), _ => false, } } diff --git a/primitives/core/src/hexdisplay.rs b/primitives/core/src/hexdisplay.rs index 104aaf812e644..83c2363a4c9ec 100644 --- a/primitives/core/src/hexdisplay.rs +++ b/primitives/core/src/hexdisplay.rs @@ -58,7 +58,7 @@ pub trait AsBytesRef { fn as_bytes_ref(&self) -> &[u8]; } -impl<'a> AsBytesRef for &'a [u8] { +impl AsBytesRef for &[u8] { fn as_bytes_ref(&self) -> &[u8] { self } } diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 1e8178f05e168..80ef992f6b928 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -212,7 +212,7 @@ pub enum MultiSigner { Ed25519(ed25519::Public), /// An Sr25519 identity. Sr25519(sr25519::Public), - /// An SECP256k1/ECDSA identity (actually, the Blake2 hash of the pub key). + /// An SECP256k1/ECDSA identity (actually, the Blake2 hash of the compressed pub key). Ecdsa(ecdsa::Public), } @@ -246,7 +246,9 @@ impl traits::IdentifyAccount for MultiSigner { match self { MultiSigner::Ed25519(who) => <[u8; 32]>::from(who).into(), MultiSigner::Sr25519(who) => <[u8; 32]>::from(who).into(), - MultiSigner::Ecdsa(who) => sp_io::hashing::blake2_256(who.as_ref()).into(), + MultiSigner::Ecdsa(who) => sp_io::hashing::blake2_256( + &who.as_compressed().expect("`who` is a valid `ECDSA` public key; qed")[..], + ).into(), } } } @@ -688,8 +690,9 @@ pub fn print(print: impl traits::Printable) { #[cfg(test)] mod tests { - use crate::DispatchError; + use super::*; use codec::{Encode, Decode}; + use sp_core::crypto::Pair; #[test] fn opaque_extrinsic_serialization() { @@ -716,4 +719,20 @@ mod tests { }, ); } + + #[test] + fn multi_signature_ecdsa_verify_works() { + let msg = &b"test-message"[..]; + let (pair, _) = ecdsa::Pair::generate(); + + let signature = pair.sign(&msg); + assert!(ecdsa::Pair::verify(&signature, msg, &pair.public())); + + let multi_sig = MultiSignature::from(signature); + let multi_signer = MultiSigner::from(pair.public()); + assert!(multi_sig.verify(msg, &multi_signer.into_account())); + + let multi_signer = MultiSigner::from(pair.public().into_compressed().unwrap()); + assert!(multi_sig.verify(msg, &multi_signer.into_account())); + } } diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 2b9ea98f05431..a0970accd8740 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -102,7 +102,7 @@ impl Verify for sp_core::ecdsa::Signature { self.as_ref(), &sp_io::hashing::blake2_256(msg.get()), ) { - Ok(pubkey) => >::as_ref(signer) == &pubkey[..], + Ok(pubkey) => signer.as_compressed().map(|s| &s[..] == &pubkey[..]).unwrap_or(false), _ => false, } } @@ -1307,8 +1307,9 @@ pub trait BlockIdTo { #[cfg(test)] mod tests { - use super::AccountIdConversion; + use super::*; use crate::codec::{Encode, Decode, Input}; + use sp_core::{crypto::Pair, ecdsa}; mod t { use sp_core::crypto::KeyTypeId; @@ -1388,4 +1389,16 @@ mod tests { assert_eq!(t.remaining_len(), Ok(None)); assert_eq!(buffer, [0, 0]); } + + #[test] + fn ecdsa_verify_works() { + let msg = &b"test-message"[..]; + let (pair, _) = ecdsa::Pair::generate(); + + let signature = pair.sign(&msg); + assert!(ecdsa::Pair::verify(&signature, msg, &pair.public())); + + assert!(signature.verify(msg, &pair.public())); + assert!(signature.verify(msg, &pair.public().into_compressed().unwrap())); + } }