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.

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

remove usage of libsecp256k1 for secp256k1 in claims pallet #5841

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
7 changes: 4 additions & 3 deletions 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 @@ -1213,7 +1213,7 @@ schemars = { version = "0.8.13", default-features = false }
schnellru = { version = "0.2.3" }
schnorrkel = { version = "0.11.4", default-features = false }
seccompiler = { version = "0.4.0" }
secp256k1 = { version = "0.28.0", default-features = false }
secp256k1 = { version = "0.30.0", default-features = false }
secrecy = { version = "0.8.0", default-features = false }
serde = { version = "1.0.214", default-features = false }
serde-big-array = { version = "0.3.2" }
Expand Down
4 changes: 4 additions & 0 deletions polkadot/runtime/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pallet-babe = { optional = true, workspace = true }
libsecp256k1 = { workspace = true }
polkadot-primitives = { workspace = true }
polkadot-runtime-parachains = { workspace = true }
secp256k1 = { workspace = true }

slot-range-helper = { workspace = true }
xcm = { workspace = true }
Expand All @@ -69,6 +70,7 @@ libsecp256k1 = { workspace = true, default-features = true }
pallet-babe = { workspace = true, default-features = true }
pallet-treasury = { workspace = true, default-features = true }
polkadot-primitives-test-helpers = { workspace = true }
secp256k1 = { workspace = true, default-features = true }
serde_json = { workspace = true, default-features = true }
sp-keyring = { workspace = true, default-features = true }
sp-keystore = { workspace = true, default-features = true }
Expand Down Expand Up @@ -103,6 +105,8 @@ std = [
"polkadot-runtime-parachains/std",
"rustc-hex/std",
"scale-info/std",
"secp256k1/global-context",
"secp256k1/std",
"serde/std",
"slot-range-helper/std",
"sp-api/std",
Expand Down
28 changes: 19 additions & 9 deletions polkadot/runtime/common/src/claims/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use frame_support::{
traits::UnfilteredDispatchable,
};
use frame_system::RawOrigin;
use secp256k1::SecretKey;
use secp_utils::*;
use sp_runtime::{
traits::{DispatchTransaction, ValidateUnsigned},
Expand All @@ -37,7 +38,8 @@ const MAX_CLAIMS: u32 = 10_000;
const VALUE: u32 = 1_000_000;

fn create_claim<T: Config>(input: u32) -> DispatchResult {
let secret_key = libsecp256k1::SecretKey::parse(&keccak_256(&input.encode())).unwrap();
let secret_key = SecretKey::from_byte_array(&keccak_256(&input.encode()))
.expect("32 bytes, within curve order");
let eth_address = eth(&secret_key);
let vesting = Some((100_000u32.into(), 1_000u32.into(), 100u32.into()));
super::Pallet::<T>::mint_claim(
Expand All @@ -51,7 +53,8 @@ fn create_claim<T: Config>(input: u32) -> DispatchResult {
}

fn create_claim_attest<T: Config>(input: u32) -> DispatchResult {
let secret_key = libsecp256k1::SecretKey::parse(&keccak_256(&input.encode())).unwrap();
let secret_key = SecretKey::from_byte_array(&keccak_256(&input.encode()))
.expect("32 bytes, within curve order");
let eth_address = eth(&secret_key);
let vesting = Some((100_000u32.into(), 1_000u32.into(), 100u32.into()));
super::Pallet::<T>::mint_claim(
Expand Down Expand Up @@ -82,7 +85,8 @@ mod benchmarks {
create_claim::<T>(c)?;
create_claim_attest::<T>(u32::MAX - c)?;
}
let secret_key = libsecp256k1::SecretKey::parse(&keccak_256(&c.encode())).unwrap();
let secret_key = SecretKey::from_byte_array(&keccak_256(&c.encode()))
.expect("32 bytes, within curve order");
let eth_address = eth(&secret_key);
let account: T::AccountId = account("user", c, SEED);
let vesting = Some((100_000u32.into(), 1_000u32.into(), 100u32.into()));
Expand Down Expand Up @@ -142,7 +146,8 @@ mod benchmarks {
}
// Crate signature
let attest_c = u32::MAX - c;
let secret_key = libsecp256k1::SecretKey::parse(&keccak_256(&attest_c.encode())).unwrap();
let secret_key = SecretKey::from_byte_array(&keccak_256(&attest_c.encode()))
.expect("32 bytes, within curve order");
let eth_address = eth(&secret_key);
let account: T::AccountId = account("user", c, SEED);
let vesting = Some((100_000u32.into(), 1_000u32.into(), 100u32.into()));
Expand Down Expand Up @@ -186,7 +191,8 @@ mod benchmarks {
create_claim_attest::<T>(u32::MAX - c)?;
}
let attest_c = u32::MAX - c;
let secret_key = libsecp256k1::SecretKey::parse(&keccak_256(&attest_c.encode())).unwrap();
let secret_key = SecretKey::from_byte_array(&keccak_256(&attest_c.encode()))
.expect("32 bytes, within curve order");
let eth_address = eth(&secret_key);
let account: T::AccountId = account("user", c, SEED);
let vesting = Some((100_000u32.into(), 1_000u32.into(), 100u32.into()));
Expand Down Expand Up @@ -218,11 +224,13 @@ mod benchmarks {
create_claim_attest::<T>(u32::MAX - c)?;
}
let attest_c = u32::MAX - c;
let secret_key = libsecp256k1::SecretKey::parse(&keccak_256(&attest_c.encode())).unwrap();
let secret_key = SecretKey::from_byte_array(&keccak_256(&attest_c.encode()))
.expect("32 bytes, within curve order");
let eth_address = eth(&secret_key);

let new_secret_key =
libsecp256k1::SecretKey::parse(&keccak_256(&(u32::MAX / 2).encode())).unwrap();
secp256k1::SecretKey::from_byte_array(&keccak_256(&(u32::MAX / 2).encode()))
.expect("32 bytes, within curve order");
let new_eth_address = eth(&new_secret_key);

let account: T::AccountId = account("user", c, SEED);
Expand Down Expand Up @@ -256,7 +264,8 @@ mod benchmarks {
#[benchmark(extra)]
fn eth_recover(i: Linear<0, 1_000>) {
// Crate signature
let secret_key = libsecp256k1::SecretKey::parse(&keccak_256(&i.encode())).unwrap();
let secret_key = SecretKey::from_byte_array(&keccak_256(&i.encode()))
.expect("32 bytes, within curve order");
let account: T::AccountId = account("user", i, SEED);
let signature = sig::<T>(&secret_key, &account.encode(), &[][..]);
let data = account.using_encoded(to_ascii_hex);
Expand All @@ -282,7 +291,8 @@ mod benchmarks {
let call: <T as frame_system::Config>::RuntimeCall = call.into();
let info = call.get_dispatch_info();
let attest_c = u32::MAX - c;
let secret_key = libsecp256k1::SecretKey::parse(&keccak_256(&attest_c.encode())).unwrap();
let secret_key = SecretKey::from_byte_array(&keccak_256(&attest_c.encode()))
.expect("32 bytes, within curve order");
let eth_address = eth(&secret_key);
let account: T::AccountId = account("user", c, SEED);
let vesting = Some((100_000u32.into(), 1_000u32.into(), 100u32.into()));
Expand Down
22 changes: 12 additions & 10 deletions polkadot/runtime/common/src/claims/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ use frame_support::{derive_impl, ord_parameter_types, parameter_types, traits::W
use pallet_balances;
use sp_runtime::{traits::Identity, BuildStorage};

use secp256k1::SecretKey;

type Block = frame_system::mocking::MockBlock<Test>;

frame_support::construct_runtime!(
Expand Down Expand Up @@ -86,20 +88,20 @@ impl Config for Test {
type WeightInfo = TestWeightInfo;
}

pub fn alice() -> libsecp256k1::SecretKey {
libsecp256k1::SecretKey::parse(&keccak_256(b"Alice")).unwrap()
pub fn alice() -> secp256k1::SecretKey {
SecretKey::from_byte_array(&keccak_256(b"Alice")).expect("32 bytes, within curve order")
}
pub fn bob() -> libsecp256k1::SecretKey {
libsecp256k1::SecretKey::parse(&keccak_256(b"Bob")).unwrap()
pub fn bob() -> secp256k1::SecretKey {
SecretKey::from_byte_array(&keccak_256(b"Bob")).expect("32 bytes, within curve order")
}
pub fn dave() -> libsecp256k1::SecretKey {
libsecp256k1::SecretKey::parse(&keccak_256(b"Dave")).unwrap()
pub fn dave() -> secp256k1::SecretKey {
SecretKey::from_byte_array(&keccak_256(b"Dave")).expect("32 bytes, within curve order")
}
pub fn eve() -> libsecp256k1::SecretKey {
libsecp256k1::SecretKey::parse(&keccak_256(b"Eve")).unwrap()
pub fn eve() -> secp256k1::SecretKey {
SecretKey::from_byte_array(&keccak_256(b"Eve")).expect("32 bytes, within curve order")
}
pub fn frank() -> libsecp256k1::SecretKey {
libsecp256k1::SecretKey::parse(&keccak_256(b"Frank")).unwrap()
pub fn frank() -> secp256k1::SecretKey {
SecretKey::from_byte_array(&keccak_256(b"Frank")).expect("32 bytes, within curve order")
}

// This function basically just builds a genesis storage key/value store according to
Expand Down
21 changes: 13 additions & 8 deletions polkadot/runtime/common/src/claims/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ impl<T: Config> Pallet<T> {

let vesting = Vesting::<T>::get(&signer);
if vesting.is_some() && T::VestingSchedule::vesting_balance(&dest).is_some() {
return Err(Error::<T>::VestedBalanceExists.into())
return Err(Error::<T>::VestedBalanceExists.into());
}

// We first need to deposit the balance to ensure that the account exists.
Expand Down Expand Up @@ -698,28 +698,33 @@ where
#[cfg(any(test, feature = "runtime-benchmarks"))]
mod secp_utils {
use super::*;
use secp256k1::{Message, Secp256k1, SECP256K1};

pub fn public(secret: &libsecp256k1::SecretKey) -> libsecp256k1::PublicKey {
libsecp256k1::PublicKey::from_secret_key(secret)
pub fn public(secret: &secp256k1::SecretKey) -> secp256k1::PublicKey {
let secp = Secp256k1::new();
secp256k1::PublicKey::from_secret_key(&secp, &secret)
}
pub fn eth(secret: &libsecp256k1::SecretKey) -> EthereumAddress {
pub fn eth(secret: &secp256k1::SecretKey) -> EthereumAddress {
let mut res = EthereumAddress::default();
res.0.copy_from_slice(&keccak_256(&public(secret).serialize()[1..65])[12..]);
res
}
pub fn sig<T: Config>(
secret: &libsecp256k1::SecretKey,
secret: &secp256k1::SecretKey,
what: &[u8],
extra: &[u8],
) -> EcdsaSignature {
let msg = keccak_256(&super::Pallet::<T>::ethereum_signable_message(
&to_ascii_hex(what)[..],
extra,
));
let (sig, recovery_id) = libsecp256k1::sign(&libsecp256k1::Message::parse(&msg), secret);

let message = Message::from_digest_slice(&msg).unwrap();
let (recovery_id, sig) =
SECP256K1.sign_ecdsa_recoverable(&message, &secret).serialize_compact();
let mut r = [0u8; 65];
r[0..64].copy_from_slice(&sig.serialize()[..]);
r[64] = recovery_id.serialize();
r[0..64].copy_from_slice(&sig[..]);
r[64] = recovery_id as u8;
EcdsaSignature(r)
}
}
Expand Down
7 changes: 7 additions & 0 deletions prdoc/pr_5841.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
title: 'remove usage of libsecp256k1 for newer secp256k1 in claims pallet'
doc:
- audience: Runtime User
description: Remove libsecp256k1 for a newer and maintained secp256k1 dependency instead
crates:
- name: polkadot-runtime-common
bump: patch
4 changes: 2 additions & 2 deletions substrate/primitives/core/src/ecdsa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl Signature {
pub fn recover_prehashed(&self, message: &[u8; 32]) -> Option<Public> {
#[cfg(feature = "std")]
{
let rid = RecoveryId::from_i32(self.0[64] as i32).ok()?;
let rid = RecoveryId::try_from(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 a 32 bytes hash; qed");
Expand Down Expand Up @@ -142,7 +142,7 @@ impl From<RecoverableSignature> for Signature {
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] = recid as u8;
r
}
}
Expand Down
6 changes: 3 additions & 3 deletions substrate/primitives/io/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ pub trait Crypto {
use ed25519_dalek::Verifier;

let Ok(public_key) = ed25519_dalek::VerifyingKey::from_bytes(&pub_key.0) else {
return false
return false;
};

let sig = ed25519_dalek::Signature::from_bytes(&sig.0);
Expand Down Expand Up @@ -1147,7 +1147,7 @@ pub trait Crypto {
sig: &[u8; 65],
msg: &[u8; 32],
) -> Result<[u8; 64], EcdsaVerifyError> {
let rid = RecoveryId::from_i32(if sig[64] > 26 { sig[64] - 27 } else { sig[64] } as i32)
let rid = RecoveryId::try_from(if sig[64] > 26 { sig[64] - 27 } else { sig[64] } as i32)
.map_err(|_| EcdsaVerifyError::BadV)?;
let sig = RecoverableSignature::from_compact(&sig[..64], rid)
.map_err(|_| EcdsaVerifyError::BadRS)?;
Expand Down Expand Up @@ -1193,7 +1193,7 @@ pub trait Crypto {
sig: &[u8; 65],
msg: &[u8; 32],
) -> Result<[u8; 33], EcdsaVerifyError> {
let rid = RecoveryId::from_i32(if sig[64] > 26 { sig[64] - 27 } else { sig[64] } as i32)
let rid = RecoveryId::try_from(if sig[64] > 26 { sig[64] - 27 } else { sig[64] } as i32)
.map_err(|_| EcdsaVerifyError::BadV)?;
let sig = RecoverableSignature::from_compact(&sig[..64], rid)
.map_err(|_| EcdsaVerifyError::BadRS)?;
Expand Down
Loading