From 86e5c791c254b67de5fa2f85926b9d1536915573 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 12 Dec 2024 22:02:18 +0400 Subject: [PATCH 1/4] chore: move all secp256k1 helpers to primitives-traits --- crates/primitives-traits/src/crypto.rs | 52 ++++++++++++++++--- .../primitives/src/transaction/signature.rs | 43 ++------------- 2 files changed, 49 insertions(+), 46 deletions(-) diff --git a/crates/primitives-traits/src/crypto.rs b/crates/primitives-traits/src/crypto.rs index cc9c8aaf3bde..7b535f64e45c 100644 --- a/crates/primitives-traits/src/crypto.rs +++ b/crates/primitives-traits/src/crypto.rs @@ -13,15 +13,48 @@ pub const SECP256K1N_HALF: U256 = U256::from_be_bytes([ ]); /// Secp256k1 utility functions. -#[cfg(feature = "secp256k1")] pub mod secp256k1 { - pub use super::impl_secp256k1::*; -} + use super::SECP256K1N_HALF; + use alloy_primitives::PrimitiveSignature as Signature; + use revm_primitives::{Address, B256}; -/// Secp256k1 utility functions. -#[cfg(not(feature = "secp256k1"))] -pub mod secp256k1 { - pub use super::impl_k256::*; + #[cfg(not(feature = "secp256k1"))] + use super::impl_k256 as imp; + #[cfg(feature = "secp256k1")] + use super::impl_secp256k1 as imp; + + pub use imp::{public_key_to_address, sign_message}; + + /// Recover signer from message hash, _without ensuring that the signature has a low `s` + /// value_. + /// + /// Using this for signature validation will succeed, even if the signature is malleable or not + /// compliant with EIP-2. This is provided for compatibility with old signatures which have + /// large `s` values. + pub fn recover_signer_unchecked(signature: &Signature, hash: B256) -> Option
{ + let mut sig: [u8; 65] = [0; 65]; + + sig[0..32].copy_from_slice(&signature.r().to_be_bytes::<32>()); + sig[32..64].copy_from_slice(&signature.s().to_be_bytes::<32>()); + sig[64] = signature.v() as u8; + + // NOTE: we are removing error from underlying crypto library as it will restrain primitive + // errors and we care only if recovery is passing or not. + imp::recover_signer_unchecked(&sig, &hash.0).ok() + } + + /// Recover signer address from message hash. This ensures that the signature S value is + /// greater than `secp256k1n / 2`, as specified in + /// [EIP-2](https://eips.ethereum.org/EIPS/eip-2). + /// + /// If the S value is too large, then this will return `None` + pub fn recover_signer(signature: &Signature, hash: B256) -> Option
{ + if signature.s() > SECP256K1N_HALF { + return None + } + + recover_signer_unchecked(signature, hash) + } } #[cfg(feature = "secp256k1")] @@ -41,7 +74,10 @@ mod impl_secp256k1 { /// /// This does not ensure that the `s` value in the signature is low, and _just_ wraps the /// underlying secp256k1 library. - pub fn recover_signer_unchecked(sig: &[u8; 65], msg: &[u8; 32]) -> Result { + pub(crate) fn recover_signer_unchecked( + sig: &[u8; 65], + msg: &[u8; 32], + ) -> Result { let sig = RecoverableSignature::from_compact(&sig[0..64], RecoveryId::from_i32(sig[64] as i32)?)?; diff --git a/crates/primitives/src/transaction/signature.rs b/crates/primitives/src/transaction/signature.rs index f2850aeca11a..33eb44950d30 100644 --- a/crates/primitives/src/transaction/signature.rs +++ b/crates/primitives/src/transaction/signature.rs @@ -1,8 +1,8 @@ -use crate::transaction::util::secp256k1; use alloy_consensus::transaction::from_eip155_value; -use alloy_primitives::{Address, PrimitiveSignature as Signature, B256, U256}; +use alloy_primitives::{PrimitiveSignature as Signature, U256}; use alloy_rlp::Decodable; -use reth_primitives_traits::crypto::SECP256K1N_HALF; + +pub use reth_primitives_traits::crypto::secp256k1::{recover_signer, recover_signer_unchecked}; pub(crate) fn decode_with_eip155_chain_id( buf: &mut &[u8], @@ -26,43 +26,10 @@ pub(crate) fn decode_with_eip155_chain_id( Ok((Signature::new(r, s, parity), chain_id)) } -/// Recover signer from message hash, _without ensuring that the signature has a low `s` -/// value_. -/// -/// Using this for signature validation will succeed, even if the signature is malleable or not -/// compliant with EIP-2. This is provided for compatibility with old signatures which have -/// large `s` values. -pub fn recover_signer_unchecked(signature: &Signature, hash: B256) -> Option
{ - let mut sig: [u8; 65] = [0; 65]; - - sig[0..32].copy_from_slice(&signature.r().to_be_bytes::<32>()); - sig[32..64].copy_from_slice(&signature.s().to_be_bytes::<32>()); - sig[64] = signature.v() as u8; - - // NOTE: we are removing error from underlying crypto library as it will restrain primitive - // errors and we care only if recovery is passing or not. - secp256k1::recover_signer_unchecked(&sig, &hash.0).ok() -} - -/// Recover signer address from message hash. This ensures that the signature S value is -/// greater than `secp256k1n / 2`, as specified in -/// [EIP-2](https://eips.ethereum.org/EIPS/eip-2). -/// -/// If the S value is too large, then this will return `None` -pub fn recover_signer(signature: &Signature, hash: B256) -> Option
{ - if signature.s() > SECP256K1N_HALF { - return None - } - - recover_signer_unchecked(signature, hash) -} - #[cfg(test)] mod tests { - use crate::transaction::signature::{ - recover_signer, recover_signer_unchecked, SECP256K1N_HALF, - }; - use alloy_eips::eip2718::Decodable2718; + use crate::transaction::signature::{recover_signer, recover_signer_unchecked}; + use alloy_eips::{eip2718::Decodable2718, eip7702::constants::SECP256K1N_HALF}; use alloy_primitives::{hex, Address, PrimitiveSignature as Signature, B256, U256}; use reth_primitives_traits::SignedTransaction; use std::str::FromStr; From 6d4c523ca754b9524bd20f274c575dba2b1e8c39 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 12 Dec 2024 22:05:49 +0400 Subject: [PATCH 2/4] fix --- crates/primitives-traits/src/crypto.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/primitives-traits/src/crypto.rs b/crates/primitives-traits/src/crypto.rs index 7b535f64e45c..0d9649a882e0 100644 --- a/crates/primitives-traits/src/crypto.rs +++ b/crates/primitives-traits/src/crypto.rs @@ -123,7 +123,7 @@ mod impl_k256 { /// /// This does not ensure that the `s` value in the signature is low, and _just_ wraps the /// underlying secp256k1 library. - pub fn recover_signer_unchecked(sig: &[u8; 65], msg: &[u8; 32]) -> Result { + pub(crate) fn recover_signer_unchecked(sig: &[u8; 65], msg: &[u8; 32]) -> Result { let mut signature = k256::ecdsa::Signature::from_slice(&sig[0..64])?; let mut recid = sig[64]; From 2e2abb46b5a47428534d33075cd26d62d72c31d9 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 12 Dec 2024 22:26:51 +0400 Subject: [PATCH 3/4] fmt --- crates/primitives-traits/src/crypto.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/primitives-traits/src/crypto.rs b/crates/primitives-traits/src/crypto.rs index 0d9649a882e0..f123e7f8aa95 100644 --- a/crates/primitives-traits/src/crypto.rs +++ b/crates/primitives-traits/src/crypto.rs @@ -123,7 +123,10 @@ mod impl_k256 { /// /// This does not ensure that the `s` value in the signature is low, and _just_ wraps the /// underlying secp256k1 library. - pub(crate) fn recover_signer_unchecked(sig: &[u8; 65], msg: &[u8; 32]) -> Result { + pub(crate) fn recover_signer_unchecked( + sig: &[u8; 65], + msg: &[u8; 32], + ) -> Result { let mut signature = k256::ecdsa::Signature::from_slice(&sig[0..64])?; let mut recid = sig[64]; From ce8806d09f668326a0dd72f1be0ebbbe4bbe251b Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Thu, 12 Dec 2024 23:34:25 +0400 Subject: [PATCH 4/4] fix --- crates/primitives-traits/src/crypto.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/primitives-traits/src/crypto.rs b/crates/primitives-traits/src/crypto.rs index f123e7f8aa95..aba6107272e3 100644 --- a/crates/primitives-traits/src/crypto.rs +++ b/crates/primitives-traits/src/crypto.rs @@ -14,8 +14,7 @@ pub const SECP256K1N_HALF: U256 = U256::from_be_bytes([ /// Secp256k1 utility functions. pub mod secp256k1 { - use super::SECP256K1N_HALF; - use alloy_primitives::PrimitiveSignature as Signature; + use super::*; use revm_primitives::{Address, B256}; #[cfg(not(feature = "secp256k1"))]