From fa7589cca304fca036011e83aff02b8b6e2e47d6 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Thu, 29 Feb 2024 17:15:59 +0100 Subject: [PATCH 01/18] core: crypto/ecdsa switch to k256 --- substrate/primitives/core/Cargo.toml | 5 +- substrate/primitives/core/src/ecdsa.rs | 99 +++++++++----------------- 2 files changed, 35 insertions(+), 69 deletions(-) diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml index 8fcabfeb2384..a41b0c9f3b01 100644 --- a/substrate/primitives/core/Cargo.toml +++ b/substrate/primitives/core/Cargo.toml @@ -52,9 +52,9 @@ blake2 = { version = "0.10.4", default-features = false, optional = true } libsecp256k1 = { version = "0.7", default-features = false, features = ["static-context"], optional = true } schnorrkel = { version = "0.11.4", features = ["preaudit_deprecated"], default-features = false } merlin = { version = "3.0", default-features = false } -secp256k1 = { version = "0.28.0", default-features = false, features = ["alloc", "recovery"], optional = true } sp-crypto-hashing = { path = "../crypto/hashing", default-features = false, optional = true } sp-runtime-interface = { path = "../runtime-interface", default-features = false } +k256 = { version = "0.13.3", features = ["ecdsa"] } # bls crypto w3f-bls = { version = "0.1.3", default-features = false, optional = true } @@ -105,8 +105,6 @@ std = [ "rand", "scale-info/std", "schnorrkel/std", - "secp256k1/global-context", - "secp256k1/std", "secrecy/alloc", "serde/std", "sp-crypto-hashing/std", @@ -147,7 +145,6 @@ full_crypto = [ "blake2", "ed25519-zebra", "libsecp256k1", - "secp256k1", "sp-crypto-hashing", "sp-runtime-interface/disable_target_static_assertions", ] diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index f172b3a7d02c..2245aafdde05 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -28,15 +28,7 @@ use crate::crypto::{ }; #[cfg(feature = "full_crypto")] use crate::crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretStringError}; -#[cfg(all(feature = "full_crypto", not(feature = "std")))] -use secp256k1::Secp256k1; -#[cfg(feature = "std")] -use secp256k1::SECP256K1; -#[cfg(feature = "full_crypto")] -use secp256k1::{ - ecdsa::{RecoverableSignature, RecoveryId}, - Message, PublicKey, SecretKey, -}; +use k256::ecdsa::{RecoveryId, SigningKey, VerifyingKey}; #[cfg(feature = "serde")] use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; #[cfg(all(not(feature = "std"), feature = "serde"))] @@ -53,7 +45,7 @@ pub const PUBLIC_KEY_SERIALIZED_SIZE: usize = 33; /// The byte length of signature pub const SIGNATURE_SERIALIZED_SIZE: usize = 65; -/// A secret seed (which is bytewise essentially equivalent to a SecretKey). +/// A secret seed (which is bytewise essentially equivalent to a SigningKey). /// /// We need it as a different type because `Seed` is expected to be AsRef<[u8]>. #[cfg(feature = "full_crypto")] @@ -103,11 +95,11 @@ impl Public { let mut tagged_full = [0u8; 65]; tagged_full[0] = 0x04; tagged_full[1..].copy_from_slice(full); - secp256k1::PublicKey::from_slice(&tagged_full) + VerifyingKey::from_sec1_bytes(&tagged_full) } else { - secp256k1::PublicKey::from_slice(full) + VerifyingKey::from_sec1_bytes(full) }; - pubkey.map(|k| Self(k.serialize())).map_err(|_| ()) + pubkey.map(|k| k.into()).map_err(|_| ()) } } @@ -131,6 +123,16 @@ impl AsMut<[u8]> for Public { } } +impl From for Public { + fn from(pubkey: VerifyingKey) -> Self { + Self::unchecked_from( + pubkey.to_sec1_bytes()[..] + .try_into() + .expect("valid key is serializable to [u8,33]. qed."), + ) + } +} + impl TryFrom<&[u8]> for Public { type Error = (); @@ -331,30 +333,19 @@ impl Signature { /// Recover the public key from this signature and a pre-hashed message. #[cfg(feature = "full_crypto")] pub fn recover_prehashed(&self, message: &[u8; 32]) -> Option { - let rid = RecoveryId::from_i32(self.0[64] as i32).ok()?; - let sig = RecoverableSignature::from_compact(&self.0[..64], rid).ok()?; - let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed"); - - #[cfg(feature = "std")] - let context = SECP256K1; - #[cfg(not(feature = "std"))] - let context = Secp256k1::verification_only(); + let rid = RecoveryId::from_byte(self.0[64])?; + let sig = k256::ecdsa::Signature::from_bytes((&self.0[..64]).into()).ok()?; - context - .recover_ecdsa(&message, &sig) - .ok() - .map(|pubkey| Public(pubkey.serialize())) + VerifyingKey::recover_from_prehash(message, &sig, rid).map(|p| p.into()).ok() } } #[cfg(feature = "full_crypto")] -impl From for Signature { - fn from(recsig: RecoverableSignature) -> Signature { +impl From<(k256::ecdsa::Signature, RecoveryId)> for Signature { + fn from(recsig: (k256::ecdsa::Signature, RecoveryId)) -> Signature { let mut r = Self::default(); - let (recid, sig) = recsig.serialize_compact(); - r.0[..64].copy_from_slice(&sig); - // This is safe due to the limited range of possible valid ids. - r.0[64] = recid.to_i32() as u8; + r.0[..64].copy_from_slice(&recsig.0.to_bytes()); + r.0[64] = recsig.1.to_byte(); r } } @@ -370,7 +361,7 @@ fn derive_hard_junction(secret_seed: &Seed, cc: &[u8; 32]) -> Seed { #[derive(Clone)] pub struct Pair { public: Public, - secret: SecretKey, + secret: SigningKey, } #[cfg(feature = "full_crypto")] @@ -385,16 +376,8 @@ impl TraitPair for Pair { /// You should never need to use this; generate(), generate_with_phrase fn from_seed_slice(seed_slice: &[u8]) -> Result { let secret = - SecretKey::from_slice(seed_slice).map_err(|_| SecretStringError::InvalidSeedLength)?; - - #[cfg(feature = "std")] - let context = SECP256K1; - #[cfg(not(feature = "std"))] - let context = Secp256k1::signing_only(); - - let public = PublicKey::from_secret_key(&context, &secret); - let public = Public(public.serialize()); - Ok(Pair { public, secret }) + SigningKey::from_slice(seed_slice).map_err(|_| SecretStringError::InvalidSeedLength)?; + Ok(Pair { public: VerifyingKey::from(&secret).into(), secret }) } /// Derive a child key from a series of given junctions. @@ -438,7 +421,7 @@ impl TraitPair for Pair { impl Pair { /// Get the seed for this key. pub fn seed(&self) -> Seed { - self.secret.secret_bytes() + self.secret.to_bytes().into() } /// Exactly as `from_string` except that if no matches are found then, the the first 32 @@ -455,14 +438,10 @@ impl Pair { /// Sign a pre-hashed message pub fn sign_prehashed(&self, message: &[u8; 32]) -> Signature { - let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed"); - - #[cfg(feature = "std")] - let context = SECP256K1; - #[cfg(not(feature = "std"))] - let context = Secp256k1::signing_only(); - - context.sign_ecdsa_recoverable(&message, &self.secret).into() + self.secret + .sign_prehash_recoverable(message) + .expect("signing may not fail (???). qed.") + .into() } /// Verify a signature on a pre-hashed message. Return `true` if the signature is valid @@ -498,18 +477,6 @@ impl Pair { } } -// The `secp256k1` backend doesn't implement cleanup for their private keys. -// Currently we should take care of wiping the secret from memory. -// NOTE: this solution is not effective when `Pair` is moved around memory. -// The very same problem affects other cryptographic backends that are just using -// `zeroize`for their secrets. -#[cfg(feature = "full_crypto")] -impl Drop for Pair { - fn drop(&mut self) { - self.secret.non_secure_erase() - } -} - impl CryptoType for Public { #[cfg(feature = "full_crypto")] type Pair = Pair; @@ -770,8 +737,10 @@ mod test { let msg = [0u8; 32]; let sig1 = pair.sign_prehashed(&msg); let sig2: Signature = { - let message = Message::from_digest_slice(&msg).unwrap(); - SECP256K1.sign_ecdsa_recoverable(&message, &pair.secret).into() + pair.secret + .sign_prehash_recoverable(&msg) + .expect("signing may not fail (???). qed.") + .into() }; assert_eq!(sig1, sig2); From d6cf147ee9969300d683397aba6f55256ea17be5 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Thu, 29 Feb 2024 17:18:56 +0100 Subject: [PATCH 02/18] Cargo.lock --- Cargo.lock | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 991541ee84ee..2331f3cfd4d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4930,9 +4930,9 @@ checksum = "a26ae43d7bcc3b814de94796a5e736d4029efb0ee900c12e2d54c993ad1a1e07" [[package]] name = "elliptic-curve" -version = "0.13.5" +version = "0.13.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "968405c8fdc9b3bf4df0a6638858cc0b52462836ab6b1c87377785dd09cf1c0b" +checksum = "b5e6043086bf7973472e0c7dff2142ea0b680d30e18d9cc40f267efbf222bd47" dependencies = [ "base16ct", "crypto-bigint", @@ -7035,15 +7035,16 @@ dependencies = [ [[package]] name = "k256" -version = "0.13.1" +version = "0.13.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cadb76004ed8e97623117f3df85b17aaa6626ab0b0831e6573f104df16cd1bcc" +checksum = "956ff9b67e26e1a6a866cb758f12c6f8746208489e3e4a4b5580802f2f0a587b" dependencies = [ "cfg-if", "ecdsa", "elliptic-curve", "once_cell", "sha2 0.10.7", + "signature", ] [[package]] @@ -18658,6 +18659,7 @@ dependencies = [ "hash256-std-hasher", "impl-serde", "itertools 0.10.5", + "k256", "lazy_static", "libsecp256k1", "log", @@ -18670,7 +18672,6 @@ dependencies = [ "regex", "scale-info", "schnorrkel 0.11.4", - "secp256k1", "secrecy", "serde", "serde_json", @@ -22637,9 +22638,9 @@ dependencies = [ [[package]] name = "zeroize" -version = "1.6.0" +version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a0956f1ba7c7909bfb66c2e9e4124ab6f6482560f6628b5aaeba39207c9aad9" +checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" dependencies = [ "zeroize_derive", ] From 682ddf06db720c5b3066ba2ac26048f49752ddc7 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Thu, 29 Feb 2024 18:07:41 +0100 Subject: [PATCH 03/18] fixes for no-std --- substrate/primitives/core/Cargo.toml | 4 +++- substrate/primitives/core/src/ecdsa.rs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml index a41b0c9f3b01..0cde9085c671 100644 --- a/substrate/primitives/core/Cargo.toml +++ b/substrate/primitives/core/Cargo.toml @@ -54,7 +54,7 @@ schnorrkel = { version = "0.11.4", features = ["preaudit_deprecated"], default-f merlin = { version = "3.0", default-features = false } sp-crypto-hashing = { path = "../crypto/hashing", default-features = false, optional = true } sp-runtime-interface = { path = "../runtime-interface", default-features = false } -k256 = { version = "0.13.3", features = ["ecdsa"] } +k256 = { version = "0.13.3", features = ["alloc", "ecdsa"], default-features = false } # bls crypto w3f-bls = { version = "0.1.3", default-features = false, optional = true } @@ -94,6 +94,7 @@ std = [ "hash256-std-hasher/std", "impl-serde/std", "itertools", + "k256/std", "libsecp256k1/std", "log/std", "merlin/std", @@ -135,6 +136,7 @@ serde = [ "secrecy/alloc", "sp-crypto-hashing", "sp-storage/serde", + "k256/serde" ] # This feature enables all crypto primitives for `no_std` builds like microcontrollers diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 2245aafdde05..c463b3cf885b 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -28,7 +28,9 @@ use crate::crypto::{ }; #[cfg(feature = "full_crypto")] use crate::crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretStringError}; -use k256::ecdsa::{RecoveryId, SigningKey, VerifyingKey}; +use k256::ecdsa::{VerifyingKey}; +#[cfg(feature = "full_crypto")] +use k256::ecdsa::{RecoveryId, SigningKey}; #[cfg(feature = "serde")] use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; #[cfg(all(not(feature = "std"), feature = "serde"))] From 277068b46b500e52930ef4382126829d768e6fa6 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Thu, 29 Feb 2024 18:12:26 +0100 Subject: [PATCH 04/18] Cargo.lock --- Cargo.lock | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 2331f3cfd4d5..a64c4cdd35ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4864,6 +4864,7 @@ dependencies = [ "digest 0.10.7", "elliptic-curve", "rfc6979", + "serdect", "signature", "spki", ] @@ -4943,6 +4944,7 @@ dependencies = [ "pkcs8", "rand_core 0.6.4", "sec1", + "serdect", "subtle 2.5.0", "zeroize", ] @@ -7043,8 +7045,8 @@ dependencies = [ "ecdsa", "elliptic-curve", "once_cell", + "serdect", "sha2 0.10.7", - "signature", ] [[package]] @@ -17262,6 +17264,7 @@ dependencies = [ "der", "generic-array 0.14.7", "pkcs8", + "serdect", "subtle 2.5.0", "zeroize", ] @@ -17520,6 +17523,16 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "serdect" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a84f14a19e9a014bb9f4512488d9829a68e04ecabffb0f9904cd1ace94598177" +dependencies = [ + "base16ct", + "serde", +] + [[package]] name = "serial_test" version = "2.0.0" From 80b133e3a80bf078170ade3d998221f5d0a80716 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Thu, 29 Feb 2024 17:23:34 +0000 Subject: [PATCH 05/18] ".git/.scripts/commands/fmt/fmt.sh" --- substrate/primitives/core/Cargo.toml | 2 +- substrate/primitives/core/src/ecdsa.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml index 0cde9085c671..c59b9589594a 100644 --- a/substrate/primitives/core/Cargo.toml +++ b/substrate/primitives/core/Cargo.toml @@ -131,12 +131,12 @@ serde = [ "bs58/alloc", "dep:serde", "impl-serde", + "k256/serde", "primitive-types/serde_no_std", "scale-info/serde", "secrecy/alloc", "sp-crypto-hashing", "sp-storage/serde", - "k256/serde" ] # This feature enables all crypto primitives for `no_std` builds like microcontrollers diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index c463b3cf885b..3b9a2568dd0b 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -28,7 +28,7 @@ use crate::crypto::{ }; #[cfg(feature = "full_crypto")] use crate::crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretStringError}; -use k256::ecdsa::{VerifyingKey}; +use k256::ecdsa::VerifyingKey; #[cfg(feature = "full_crypto")] use k256::ecdsa::{RecoveryId, SigningKey}; #[cfg(feature = "serde")] From 35878b20bbb7ac70a56b3bc16aeccdab27130e65 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Fri, 1 Mar 2024 09:08:50 +0100 Subject: [PATCH 06/18] seed doc improved --- substrate/primitives/core/src/ecdsa.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 3b9a2568dd0b..8b42c4d1d398 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -47,9 +47,9 @@ pub const PUBLIC_KEY_SERIALIZED_SIZE: usize = 33; /// The byte length of signature pub const SIGNATURE_SERIALIZED_SIZE: usize = 65; -/// A secret seed (which is bytewise essentially equivalent to a SigningKey). +/// The secret seed. /// -/// We need it as a different type because `Seed` is expected to be AsRef<[u8]>. +/// The raw secret seed, which can be used to create the `Pair`. #[cfg(feature = "full_crypto")] type Seed = [u8; 32]; From e63fbac7f77ca5ef9eea7f3f6c1b3a672495aefa Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 4 Mar 2024 17:38:25 +0100 Subject: [PATCH 07/18] secp256k1 is used on host Two features added: - backend_secp256k1 - backend_k256 This allows to choose a secp256k1 implementation backend for crypto/ecdsa module. backend_secp256k1 is by default enabled in std. --- substrate/primitives/core/Cargo.toml | 11 +- substrate/primitives/core/src/ecdsa.rs | 166 +++++++++++++++++++++---- 2 files changed, 155 insertions(+), 22 deletions(-) diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml index c59b9589594a..764007de336a 100644 --- a/substrate/primitives/core/Cargo.toml +++ b/substrate/primitives/core/Cargo.toml @@ -55,6 +55,7 @@ merlin = { version = "3.0", default-features = false } sp-crypto-hashing = { path = "../crypto/hashing", default-features = false, optional = true } sp-runtime-interface = { path = "../runtime-interface", default-features = false } k256 = { version = "0.13.3", features = ["alloc", "ecdsa"], default-features = false } +secp256k1 = { version = "0.28.0", default-features = false, features = ["alloc", "recovery"], optional = true } # bls crypto w3f-bls = { version = "0.1.3", default-features = false, optional = true } @@ -75,7 +76,15 @@ harness = false bench = false [features] -default = ["std"] +default = ["backend_secp256k1", "std"] + +# use secp256k1 crate, better performance, intended to be used on host side +backend_secp256k1 = [ + "secp256k1/global-context", + "secp256k1/std", +] +# use k256 crate, better portability, intended to be used in substrate-runtimes +backend_k256 = [] std = [ "array-bytes", "bandersnatch_vrfs?/std", diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 8b42c4d1d398..23a862959b13 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -28,9 +28,20 @@ use crate::crypto::{ }; #[cfg(feature = "full_crypto")] use crate::crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretStringError}; + +#[cfg(feature = "backend_k256")] use k256::ecdsa::VerifyingKey; -#[cfg(feature = "full_crypto")] +#[cfg(all(feature = "backend_k256", feature = "full_crypto"))] use k256::ecdsa::{RecoveryId, SigningKey}; +#[cfg(all(feature = "backend_secp256k1", feature = "full_crypto", not(feature = "std")))] +use secp256k1::Secp256k1; +#[cfg(all(feature = "backend_secp256k1", feature = "std"))] +use secp256k1::SECP256K1; +#[cfg(all(feature = "backend_secp256k1", feature = "full_crypto"))] +use secp256k1::{ + ecdsa::{RecoverableSignature, RecoveryId}, + Message, PublicKey, SecretKey, +}; #[cfg(feature = "serde")] use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; #[cfg(all(not(feature = "std"), feature = "serde"))] @@ -92,14 +103,24 @@ impl Public { /// This will convert the full public key into the compressed format. #[cfg(feature = "std")] pub fn from_full(full: &[u8]) -> Result { - let pubkey = if full.len() == 64 { + let mut tagged_full = [0u8; 65]; + let full = if full.len() == 64 { // Tag it as uncompressed public key. - let mut tagged_full = [0u8; 65]; tagged_full[0] = 0x04; tagged_full[1..].copy_from_slice(full); - VerifyingKey::from_sec1_bytes(&tagged_full) + &tagged_full } else { - VerifyingKey::from_sec1_bytes(full) + full + }; + let pubkey = { + #[cfg(feature = "backend_secp256k1")] + { + secp256k1::PublicKey::from_slice(&full) + } + #[cfg(feature = "backend_k256")] + { + VerifyingKey::from_sec1_bytes(&full) + } }; pubkey.map(|k| k.into()).map_err(|_| ()) } @@ -125,6 +146,14 @@ impl AsMut<[u8]> for Public { } } +#[cfg(feature = "backend_secp256k1")] +impl From for Public { + fn from(pubkey: secp256k1::PublicKey) -> Self { + Self(pubkey.serialize()) + } +} + +#[cfg(feature = "backend_k256")] impl From for Public { fn from(pubkey: VerifyingKey) -> Self { Self::unchecked_from( @@ -335,14 +364,34 @@ impl Signature { /// Recover the public key from this signature and a pre-hashed message. #[cfg(feature = "full_crypto")] pub fn recover_prehashed(&self, message: &[u8; 32]) -> Option { - let rid = RecoveryId::from_byte(self.0[64])?; - let sig = k256::ecdsa::Signature::from_bytes((&self.0[..64]).into()).ok()?; + #[cfg(feature = "backend_secp256k1")] + { + let rid = RecoveryId::from_i32(self.0[64] as i32).ok()?; + let sig = RecoverableSignature::from_compact(&self.0[..64], rid).ok()?; + let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed"); + + #[cfg(feature = "std")] + let context = SECP256K1; + #[cfg(not(feature = "std"))] + let context = Secp256k1::verification_only(); + + context + .recover_ecdsa(&message, &sig) + .ok() + .map(|pubkey| Public(pubkey.serialize())) + } - VerifyingKey::recover_from_prehash(message, &sig, rid).map(|p| p.into()).ok() + #[cfg(feature = "backend_k256")] + { + let rid = RecoveryId::from_byte(self.0[64])?; + let sig = k256::ecdsa::Signature::from_bytes((&self.0[..64]).into()).ok()?; + + VerifyingKey::recover_from_prehash(message, &sig, rid).map(|p| p.into()).ok() + } } } -#[cfg(feature = "full_crypto")] +#[cfg(all(feature = "backend_k256", feature = "full_crypto"))] impl From<(k256::ecdsa::Signature, RecoveryId)> for Signature { fn from(recsig: (k256::ecdsa::Signature, RecoveryId)) -> Signature { let mut r = Self::default(); @@ -352,6 +401,18 @@ impl From<(k256::ecdsa::Signature, RecoveryId)> for Signature { } } +#[cfg(all(feature = "backend_secp256k1", feature = "full_crypto"))] +impl From for Signature { + fn from(recsig: RecoverableSignature) -> Signature { + let mut r = Self::default(); + let (recid, sig) = recsig.serialize_compact(); + r.0[..64].copy_from_slice(&sig); + // This is safe due to the limited range of possible valid ids. + r.0[64] = recid.to_i32() as u8; + r + } +} + /// Derive a single hard junction. #[cfg(feature = "full_crypto")] fn derive_hard_junction(secret_seed: &Seed, cc: &[u8; 32]) -> Seed { @@ -363,6 +424,9 @@ fn derive_hard_junction(secret_seed: &Seed, cc: &[u8; 32]) -> Seed { #[derive(Clone)] pub struct Pair { public: Public, + #[cfg(feature = "backend_secp256k1")] + secret: SecretKey, + #[cfg(feature = "backend_k256")] secret: SigningKey, } @@ -377,9 +441,27 @@ impl TraitPair for Pair { /// /// You should never need to use this; generate(), generate_with_phrase fn from_seed_slice(seed_slice: &[u8]) -> Result { - let secret = - SigningKey::from_slice(seed_slice).map_err(|_| SecretStringError::InvalidSeedLength)?; - Ok(Pair { public: VerifyingKey::from(&secret).into(), secret }) + #[cfg(feature = "backend_secp256k1")] + { + let secret = SecretKey::from_slice(seed_slice) + .map_err(|_| SecretStringError::InvalidSeedLength)?; + + #[cfg(feature = "std")] + let context = SECP256K1; + #[cfg(not(feature = "std"))] + let context = Secp256k1::signing_only(); + + let public = PublicKey::from_secret_key(&context, &secret); + let public = Public(public.serialize()); + Ok(Pair { public, secret }) + } + + #[cfg(feature = "backend_k256")] + { + let secret = SigningKey::from_slice(seed_slice) + .map_err(|_| SecretStringError::InvalidSeedLength)?; + Ok(Pair { public: VerifyingKey::from(&secret).into(), secret }) + } } /// Derive a child key from a series of given junctions. @@ -423,7 +505,14 @@ impl TraitPair for Pair { impl Pair { /// Get the seed for this key. pub fn seed(&self) -> Seed { - self.secret.to_bytes().into() + #[cfg(feature = "backend_secp256k1")] + { + self.secret.secret_bytes() + } + #[cfg(feature = "backend_k256")] + { + self.secret.to_bytes().into() + } } /// Exactly as `from_string` except that if no matches are found then, the the first 32 @@ -440,10 +529,25 @@ impl Pair { /// Sign a pre-hashed message pub fn sign_prehashed(&self, message: &[u8; 32]) -> Signature { - self.secret - .sign_prehash_recoverable(message) - .expect("signing may not fail (???). qed.") - .into() + #[cfg(feature = "backend_secp256k1")] + { + let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed"); + + #[cfg(feature = "std")] + let context = SECP256K1; + #[cfg(not(feature = "std"))] + let context = Secp256k1::signing_only(); + + context.sign_ecdsa_recoverable(&message, &self.secret).into() + } + + #[cfg(feature = "backend_k256")] + { + self.secret + .sign_prehash_recoverable(message) + .expect("signing may not fail (???). qed.") + .into() + } } /// Verify a signature on a pre-hashed message. Return `true` if the signature is valid @@ -479,6 +583,18 @@ impl Pair { } } +// The `secp256k1` backend doesn't implement cleanup for their private keys. +// Currently we should take care of wiping the secret from memory. +// NOTE: this solution is not effective when `Pair` is moved around memory. +// The very same problem affects other cryptographic backends that are just using +// `zeroize`for their secrets. +#[cfg(all(feature = "backend_secp256k1", feature = "full_crypto"))] +impl Drop for Pair { + fn drop(&mut self) { + self.secret.non_secure_erase() + } +} + impl CryptoType for Public { #[cfg(feature = "full_crypto")] type Pair = Pair; @@ -739,10 +855,18 @@ mod test { let msg = [0u8; 32]; let sig1 = pair.sign_prehashed(&msg); let sig2: Signature = { - pair.secret - .sign_prehash_recoverable(&msg) - .expect("signing may not fail (???). qed.") - .into() + #[cfg(feature = "backend_secp256k1")] + { + let message = Message::from_digest_slice(&msg).unwrap(); + SECP256K1.sign_ecdsa_recoverable(&message, &pair.secret).into() + } + #[cfg(feature = "backend_k256")] + { + pair.secret + .sign_prehash_recoverable(&msg) + .expect("signing may not fail (???). qed.") + .into() + } }; assert_eq!(sig1, sig2); From 7db5674d6c11b0633d48d0fdb6a8becdae438fa3 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 4 Mar 2024 17:38:50 +0100 Subject: [PATCH 08/18] Cargo.lock --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index a64c4cdd35ca..3ea49a925dd7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -18685,6 +18685,7 @@ dependencies = [ "regex", "scale-info", "schnorrkel 0.11.4", + "secp256k1", "secrecy", "serde", "serde_json", From 93086ecea82b238a0dc503d50e310c533d12f0ef Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 4 Mar 2024 17:47:54 +0100 Subject: [PATCH 09/18] features fix --- substrate/primitives/core/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml index 764007de336a..19f82c24f275 100644 --- a/substrate/primitives/core/Cargo.toml +++ b/substrate/primitives/core/Cargo.toml @@ -54,7 +54,7 @@ schnorrkel = { version = "0.11.4", features = ["preaudit_deprecated"], default-f merlin = { version = "3.0", default-features = false } sp-crypto-hashing = { path = "../crypto/hashing", default-features = false, optional = true } sp-runtime-interface = { path = "../runtime-interface", default-features = false } -k256 = { version = "0.13.3", features = ["alloc", "ecdsa"], default-features = false } +k256 = { version = "0.13.3", features = ["alloc", "ecdsa"], default-features = false, optional = true } secp256k1 = { version = "0.28.0", default-features = false, features = ["alloc", "recovery"], optional = true } # bls crypto @@ -84,7 +84,7 @@ backend_secp256k1 = [ "secp256k1/std", ] # use k256 crate, better portability, intended to be used in substrate-runtimes -backend_k256 = [] +backend_k256 = ["k256"] std = [ "array-bytes", "bandersnatch_vrfs?/std", From 42bb70c25fedb6535ab9da1126594e19ed26ea67 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 4 Mar 2024 18:20:54 +0100 Subject: [PATCH 10/18] make clippy happy --- substrate/primitives/core/src/ecdsa.rs | 38 ++++++++++++++------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 23a862959b13..7189ea6dda9e 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -29,10 +29,15 @@ use crate::crypto::{ #[cfg(feature = "full_crypto")] use crate::crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretStringError}; -#[cfg(feature = "backend_k256")] +//note: "not" is required to satisfy clippy --all-features. +#[cfg(all( + feature = "backend_k256", + not(feature = "backend_secp256k1"), + feature = "full_crypto" +))] +use k256::ecdsa::SigningKey as SecretKey; +#[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] use k256::ecdsa::VerifyingKey; -#[cfg(all(feature = "backend_k256", feature = "full_crypto"))] -use k256::ecdsa::{RecoveryId, SigningKey}; #[cfg(all(feature = "backend_secp256k1", feature = "full_crypto", not(feature = "std")))] use secp256k1::Secp256k1; #[cfg(all(feature = "backend_secp256k1", feature = "std"))] @@ -117,7 +122,7 @@ impl Public { { secp256k1::PublicKey::from_slice(&full) } - #[cfg(feature = "backend_k256")] + #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] { VerifyingKey::from_sec1_bytes(&full) } @@ -153,7 +158,7 @@ impl From for Public { } } -#[cfg(feature = "backend_k256")] +#[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] impl From for Public { fn from(pubkey: VerifyingKey) -> Self { Self::unchecked_from( @@ -381,9 +386,9 @@ impl Signature { .map(|pubkey| Public(pubkey.serialize())) } - #[cfg(feature = "backend_k256")] + #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] { - let rid = RecoveryId::from_byte(self.0[64])?; + let rid = k256::ecdsa::RecoveryId::from_byte(self.0[64])?; let sig = k256::ecdsa::Signature::from_bytes((&self.0[..64]).into()).ok()?; VerifyingKey::recover_from_prehash(message, &sig, rid).map(|p| p.into()).ok() @@ -391,9 +396,9 @@ impl Signature { } } -#[cfg(all(feature = "backend_k256", feature = "full_crypto"))] -impl From<(k256::ecdsa::Signature, RecoveryId)> for Signature { - fn from(recsig: (k256::ecdsa::Signature, RecoveryId)) -> Signature { +#[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1"), feature = "full_crypto"))] +impl From<(k256::ecdsa::Signature, k256::ecdsa::RecoveryId)> for Signature { + fn from(recsig: (k256::ecdsa::Signature, k256::ecdsa::RecoveryId)) -> Signature { let mut r = Self::default(); r.0[..64].copy_from_slice(&recsig.0.to_bytes()); r.0[64] = recsig.1.to_byte(); @@ -424,10 +429,7 @@ fn derive_hard_junction(secret_seed: &Seed, cc: &[u8; 32]) -> Seed { #[derive(Clone)] pub struct Pair { public: Public, - #[cfg(feature = "backend_secp256k1")] secret: SecretKey, - #[cfg(feature = "backend_k256")] - secret: SigningKey, } #[cfg(feature = "full_crypto")] @@ -456,9 +458,9 @@ impl TraitPair for Pair { Ok(Pair { public, secret }) } - #[cfg(feature = "backend_k256")] + #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] { - let secret = SigningKey::from_slice(seed_slice) + let secret = SecretKey::from_slice(seed_slice) .map_err(|_| SecretStringError::InvalidSeedLength)?; Ok(Pair { public: VerifyingKey::from(&secret).into(), secret }) } @@ -509,7 +511,7 @@ impl Pair { { self.secret.secret_bytes() } - #[cfg(feature = "backend_k256")] + #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] { self.secret.to_bytes().into() } @@ -541,7 +543,7 @@ impl Pair { context.sign_ecdsa_recoverable(&message, &self.secret).into() } - #[cfg(feature = "backend_k256")] + #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] { self.secret .sign_prehash_recoverable(message) @@ -860,7 +862,7 @@ mod test { let message = Message::from_digest_slice(&msg).unwrap(); SECP256K1.sign_ecdsa_recoverable(&message, &pair.secret).into() } - #[cfg(feature = "backend_k256")] + #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] { pair.secret .sign_prehash_recoverable(&msg) From 0fc72014bc02bb918c7d761549d3c0b713eacebf Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 4 Mar 2024 19:08:22 +0100 Subject: [PATCH 11/18] backend features removed: std/no-std used --- substrate/primitives/core/Cargo.toml | 15 +++---- substrate/primitives/core/src/ecdsa.rs | 59 +++++++++++--------------- 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml index 19f82c24f275..7d33b82e2d7c 100644 --- a/substrate/primitives/core/Cargo.toml +++ b/substrate/primitives/core/Cargo.toml @@ -54,7 +54,9 @@ schnorrkel = { version = "0.11.4", features = ["preaudit_deprecated"], default-f merlin = { version = "3.0", default-features = false } sp-crypto-hashing = { path = "../crypto/hashing", default-features = false, optional = true } sp-runtime-interface = { path = "../runtime-interface", default-features = false } -k256 = { version = "0.13.3", features = ["alloc", "ecdsa"], default-features = false, optional = true } +# k256 crate, better portability, intended to be used in substrate-runtimes (no-std) +k256 = { version = "0.13.3", features = ["alloc", "ecdsa"], default-features = false } +# secp256k1 crate, better performance, intended to be used on host side (std) secp256k1 = { version = "0.28.0", default-features = false, features = ["alloc", "recovery"], optional = true } # bls crypto @@ -76,15 +78,8 @@ harness = false bench = false [features] -default = ["backend_secp256k1", "std"] +default = ["std"] -# use secp256k1 crate, better performance, intended to be used on host side -backend_secp256k1 = [ - "secp256k1/global-context", - "secp256k1/std", -] -# use k256 crate, better portability, intended to be used in substrate-runtimes -backend_k256 = ["k256"] std = [ "array-bytes", "bandersnatch_vrfs?/std", @@ -115,6 +110,8 @@ std = [ "rand", "scale-info/std", "schnorrkel/std", + "secp256k1/global-context", + "secp256k1/std", "secrecy/alloc", "serde/std", "sp-crypto-hashing/std", diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 7189ea6dda9e..670b4505a81b 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -30,19 +30,13 @@ use crate::crypto::{ use crate::crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretStringError}; //note: "not" is required to satisfy clippy --all-features. -#[cfg(all( - feature = "backend_k256", - not(feature = "backend_secp256k1"), - feature = "full_crypto" -))] +#[cfg(all(not(feature = "std"), feature = "full_crypto"))] use k256::ecdsa::SigningKey as SecretKey; -#[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] +#[cfg(not(feature = "std"))] use k256::ecdsa::VerifyingKey; -#[cfg(all(feature = "backend_secp256k1", feature = "full_crypto", not(feature = "std")))] -use secp256k1::Secp256k1; -#[cfg(all(feature = "backend_secp256k1", feature = "std"))] +#[cfg(all(feature = "std"))] use secp256k1::SECP256K1; -#[cfg(all(feature = "backend_secp256k1", feature = "full_crypto"))] +#[cfg(all(feature = "std", feature = "full_crypto"))] use secp256k1::{ ecdsa::{RecoverableSignature, RecoveryId}, Message, PublicKey, SecretKey, @@ -106,7 +100,6 @@ impl Public { /// Create a new instance from the given full public key. /// /// This will convert the full public key into the compressed format. - #[cfg(feature = "std")] pub fn from_full(full: &[u8]) -> Result { let mut tagged_full = [0u8; 65]; let full = if full.len() == 64 { @@ -118,11 +111,11 @@ impl Public { full }; let pubkey = { - #[cfg(feature = "backend_secp256k1")] + #[cfg(feature = "std")] { secp256k1::PublicKey::from_slice(&full) } - #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] + #[cfg(not(feature = "std"))] { VerifyingKey::from_sec1_bytes(&full) } @@ -151,14 +144,14 @@ impl AsMut<[u8]> for Public { } } -#[cfg(feature = "backend_secp256k1")] +#[cfg(feature = "std")] impl From for Public { fn from(pubkey: secp256k1::PublicKey) -> Self { Self(pubkey.serialize()) } } -#[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] +#[cfg(not(feature = "std"))] impl From for Public { fn from(pubkey: VerifyingKey) -> Self { Self::unchecked_from( @@ -369,7 +362,7 @@ impl Signature { /// Recover the public key from this signature and a pre-hashed message. #[cfg(feature = "full_crypto")] pub fn recover_prehashed(&self, message: &[u8; 32]) -> Option { - #[cfg(feature = "backend_secp256k1")] + #[cfg(feature = "std")] { let rid = RecoveryId::from_i32(self.0[64] as i32).ok()?; let sig = RecoverableSignature::from_compact(&self.0[..64], rid).ok()?; @@ -380,23 +373,20 @@ impl Signature { #[cfg(not(feature = "std"))] let context = Secp256k1::verification_only(); - context - .recover_ecdsa(&message, &sig) - .ok() - .map(|pubkey| Public(pubkey.serialize())) + context.recover_ecdsa(&message, &sig).ok().map(Public::from) } - #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] + #[cfg(not(feature = "std"))] { let rid = k256::ecdsa::RecoveryId::from_byte(self.0[64])?; let sig = k256::ecdsa::Signature::from_bytes((&self.0[..64]).into()).ok()?; - VerifyingKey::recover_from_prehash(message, &sig, rid).map(|p| p.into()).ok() + VerifyingKey::recover_from_prehash(message, &sig, rid).map(Public::from).ok() } } } -#[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1"), feature = "full_crypto"))] +#[cfg(not(feature = "std"))] impl From<(k256::ecdsa::Signature, k256::ecdsa::RecoveryId)> for Signature { fn from(recsig: (k256::ecdsa::Signature, k256::ecdsa::RecoveryId)) -> Signature { let mut r = Self::default(); @@ -406,7 +396,7 @@ impl From<(k256::ecdsa::Signature, k256::ecdsa::RecoveryId)> for Signature { } } -#[cfg(all(feature = "backend_secp256k1", feature = "full_crypto"))] +#[cfg(all(feature = "std", feature = "full_crypto"))] impl From for Signature { fn from(recsig: RecoverableSignature) -> Signature { let mut r = Self::default(); @@ -443,7 +433,7 @@ impl TraitPair for Pair { /// /// You should never need to use this; generate(), generate_with_phrase fn from_seed_slice(seed_slice: &[u8]) -> Result { - #[cfg(feature = "backend_secp256k1")] + #[cfg(feature = "std")] { let secret = SecretKey::from_slice(seed_slice) .map_err(|_| SecretStringError::InvalidSeedLength)?; @@ -453,12 +443,11 @@ impl TraitPair for Pair { #[cfg(not(feature = "std"))] let context = Secp256k1::signing_only(); - let public = PublicKey::from_secret_key(&context, &secret); - let public = Public(public.serialize()); + let public = PublicKey::from_secret_key(&context, &secret).into(); Ok(Pair { public, secret }) } - #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] + #[cfg(not(feature = "std"))] { let secret = SecretKey::from_slice(seed_slice) .map_err(|_| SecretStringError::InvalidSeedLength)?; @@ -507,11 +496,11 @@ impl TraitPair for Pair { impl Pair { /// Get the seed for this key. pub fn seed(&self) -> Seed { - #[cfg(feature = "backend_secp256k1")] + #[cfg(feature = "std")] { self.secret.secret_bytes() } - #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] + #[cfg(not(feature = "std"))] { self.secret.to_bytes().into() } @@ -531,7 +520,7 @@ impl Pair { /// Sign a pre-hashed message pub fn sign_prehashed(&self, message: &[u8; 32]) -> Signature { - #[cfg(feature = "backend_secp256k1")] + #[cfg(feature = "std")] { let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed"); @@ -543,7 +532,7 @@ impl Pair { context.sign_ecdsa_recoverable(&message, &self.secret).into() } - #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] + #[cfg(not(feature = "std"))] { self.secret .sign_prehash_recoverable(message) @@ -590,7 +579,7 @@ impl Pair { // NOTE: this solution is not effective when `Pair` is moved around memory. // The very same problem affects other cryptographic backends that are just using // `zeroize`for their secrets. -#[cfg(all(feature = "backend_secp256k1", feature = "full_crypto"))] +#[cfg(all(feature = "std", feature = "full_crypto"))] impl Drop for Pair { fn drop(&mut self) { self.secret.non_secure_erase() @@ -857,12 +846,12 @@ mod test { let msg = [0u8; 32]; let sig1 = pair.sign_prehashed(&msg); let sig2: Signature = { - #[cfg(feature = "backend_secp256k1")] + #[cfg(feature = "std")] { let message = Message::from_digest_slice(&msg).unwrap(); SECP256K1.sign_ecdsa_recoverable(&message, &pair.secret).into() } - #[cfg(all(feature = "backend_k256", not(feature = "backend_secp256k1")))] + #[cfg(not(feature = "std"))] { pair.secret .sign_prehash_recoverable(&msg) From 0ff4aa2e217463c958dfba156547af2cdf50006c Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 4 Mar 2024 20:12:22 +0100 Subject: [PATCH 12/18] Apply suggestions from code review --- substrate/primitives/core/src/ecdsa.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 670b4505a81b..54efa41d5ce9 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -29,7 +29,6 @@ use crate::crypto::{ #[cfg(feature = "full_crypto")] use crate::crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretStringError}; -//note: "not" is required to satisfy clippy --all-features. #[cfg(all(not(feature = "std"), feature = "full_crypto"))] use k256::ecdsa::SigningKey as SecretKey; #[cfg(not(feature = "std"))] @@ -368,10 +367,7 @@ impl Signature { let sig = RecoverableSignature::from_compact(&self.0[..64], rid).ok()?; let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed"); - #[cfg(feature = "std")] let context = SECP256K1; - #[cfg(not(feature = "std"))] - let context = Secp256k1::verification_only(); context.recover_ecdsa(&message, &sig).ok().map(Public::from) } @@ -438,10 +434,7 @@ impl TraitPair for Pair { let secret = SecretKey::from_slice(seed_slice) .map_err(|_| SecretStringError::InvalidSeedLength)?; - #[cfg(feature = "std")] let context = SECP256K1; - #[cfg(not(feature = "std"))] - let context = Secp256k1::signing_only(); let public = PublicKey::from_secret_key(&context, &secret).into(); Ok(Pair { public, secret }) @@ -524,10 +517,7 @@ impl Pair { { let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed"); - #[cfg(feature = "std")] let context = SECP256K1; - #[cfg(not(feature = "std"))] - let context = Secp256k1::signing_only(); context.sign_ecdsa_recoverable(&message, &self.secret).into() } From 1ab98f6cc70ae97c8b24ad9e8df68d227c6a05bb Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 4 Mar 2024 19:20:55 +0100 Subject: [PATCH 13/18] taplo for frame/support/Cargo.toml --- substrate/frame/support/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/support/Cargo.toml b/substrate/frame/support/Cargo.toml index fb9cb3d45119..113af41751ed 100644 --- a/substrate/frame/support/Cargo.toml +++ b/substrate/frame/support/Cargo.toml @@ -95,9 +95,9 @@ std = [ "sp-staking/std", "sp-state-machine/std", "sp-std/std", + "sp-timestamp/std", "sp-tracing/std", "sp-weights/std", - "sp-timestamp/std" ] runtime-benchmarks = [ "frame-system/runtime-benchmarks", From 6797ec4c41ee1a53ca23c951d298d8b9f98f00d7 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Mon, 4 Mar 2024 20:20:28 +0100 Subject: [PATCH 14/18] fixes --- substrate/primitives/core/src/ecdsa.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 54efa41d5ce9..9931e17217e3 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -366,10 +366,7 @@ impl Signature { let rid = RecoveryId::from_i32(self.0[64] as i32).ok()?; let sig = RecoverableSignature::from_compact(&self.0[..64], rid).ok()?; let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed"); - - let context = SECP256K1; - - context.recover_ecdsa(&message, &sig).ok().map(Public::from) + SECP256K1.recover_ecdsa(&message, &sig).ok().map(Public::from) } #[cfg(not(feature = "std"))] @@ -433,10 +430,7 @@ impl TraitPair for Pair { { let secret = SecretKey::from_slice(seed_slice) .map_err(|_| SecretStringError::InvalidSeedLength)?; - - let context = SECP256K1; - - let public = PublicKey::from_secret_key(&context, &secret).into(); + let public = PublicKey::from_secret_key(&SECP256K1, &secret).into(); Ok(Pair { public, secret }) } @@ -516,10 +510,7 @@ impl Pair { #[cfg(feature = "std")] { let message = Message::from_digest_slice(message).expect("Message is 32 bytes; qed"); - - let context = SECP256K1; - - context.sign_ecdsa_recoverable(&message, &self.secret).into() + SECP256K1.sign_ecdsa_recoverable(&message, &self.secret).into() } #[cfg(not(feature = "std"))] From 461998e1acdd2e6b069ee86732c7c06380c97884 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 5 Mar 2024 08:38:56 +0100 Subject: [PATCH 15/18] minor fixes --- substrate/primitives/core/src/ecdsa.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 9931e17217e3..6f707ad7e05a 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -33,12 +33,10 @@ use crate::crypto::{DeriveError, DeriveJunction, Pair as TraitPair, SecretString use k256::ecdsa::SigningKey as SecretKey; #[cfg(not(feature = "std"))] use k256::ecdsa::VerifyingKey; -#[cfg(all(feature = "std"))] -use secp256k1::SECP256K1; #[cfg(all(feature = "std", feature = "full_crypto"))] use secp256k1::{ ecdsa::{RecoverableSignature, RecoveryId}, - Message, PublicKey, SecretKey, + Message, PublicKey, SecretKey, SECP256K1, }; #[cfg(feature = "serde")] use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; @@ -112,7 +110,7 @@ impl Public { let pubkey = { #[cfg(feature = "std")] { - secp256k1::PublicKey::from_slice(&full) + PublicKey::from_slice(&full) } #[cfg(not(feature = "std"))] { @@ -144,8 +142,8 @@ impl AsMut<[u8]> for Public { } #[cfg(feature = "std")] -impl From for Public { - fn from(pubkey: secp256k1::PublicKey) -> Self { +impl From for Public { + fn from(pubkey: PublicKey) -> Self { Self(pubkey.serialize()) } } @@ -373,7 +371,6 @@ impl Signature { { let rid = k256::ecdsa::RecoveryId::from_byte(self.0[64])?; let sig = k256::ecdsa::Signature::from_bytes((&self.0[..64]).into()).ok()?; - VerifyingKey::recover_from_prehash(message, &sig, rid).map(Public::from).ok() } } @@ -430,8 +427,7 @@ impl TraitPair for Pair { { let secret = SecretKey::from_slice(seed_slice) .map_err(|_| SecretStringError::InvalidSeedLength)?; - let public = PublicKey::from_secret_key(&SECP256K1, &secret).into(); - Ok(Pair { public, secret }) + Ok(Pair { public: PublicKey::from_secret_key(&SECP256K1, &secret).into(), secret }) } #[cfg(not(feature = "std"))] From c6d3bf2661838ca65796f6fc35a61934d17c2d25 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Fri, 8 Mar 2024 17:50:53 +0100 Subject: [PATCH 16/18] Update substrate/primitives/core/src/ecdsa.rs Co-authored-by: Davide Galassi --- substrate/primitives/core/src/ecdsa.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/substrate/primitives/core/src/ecdsa.rs b/substrate/primitives/core/src/ecdsa.rs index 6f707ad7e05a..e6bd2c7eb57d 100644 --- a/substrate/primitives/core/src/ecdsa.rs +++ b/substrate/primitives/core/src/ecdsa.rs @@ -107,16 +107,10 @@ impl Public { } else { full }; - let pubkey = { - #[cfg(feature = "std")] - { - PublicKey::from_slice(&full) - } - #[cfg(not(feature = "std"))] - { - VerifyingKey::from_sec1_bytes(&full) - } - }; + #[cfg(feature = "std")] + let pubkey = PublicKey::from_slice(&full); + #[cfg(not(feature = "std"))] + let pubkey = VerifyingKey::from_sec1_bytes(&full); pubkey.map(|k| k.into()).map_err(|_| ()) } } From 38de881ad93b76f53274b5bea1d42b5d46c2f556 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Fri, 8 Mar 2024 21:38:49 +0100 Subject: [PATCH 17/18] Update substrate/primitives/core/Cargo.toml --- substrate/primitives/core/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml index f7cc2b4fcf78..192a216273a6 100644 --- a/substrate/primitives/core/Cargo.toml +++ b/substrate/primitives/core/Cargo.toml @@ -98,7 +98,6 @@ std = [ "hash256-std-hasher/std", "impl-serde/std", "itertools", - "k256/std", "libsecp256k1/std", "log/std", "merlin/std", From b7b215add431cb881614d2c61418ef1f8ddc38c8 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Fri, 8 Mar 2024 21:57:38 +0100 Subject: [PATCH 18/18] Revert "Update substrate/primitives/core/Cargo.toml" Bring back k256/std. This reverts commit 38de881ad93b76f53274b5bea1d42b5d46c2f556. --- substrate/primitives/core/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml index 192a216273a6..f7cc2b4fcf78 100644 --- a/substrate/primitives/core/Cargo.toml +++ b/substrate/primitives/core/Cargo.toml @@ -98,6 +98,7 @@ std = [ "hash256-std-hasher/std", "impl-serde/std", "itertools", + "k256/std", "libsecp256k1/std", "log/std", "merlin/std",