From 80035c3e7b4bbce76cab802d4f6844c812e42c5b Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Fri, 13 Dec 2024 00:19:18 +0400 Subject: [PATCH 1/2] chore: move all secp256k1 helpers to `primitives-traits` (#13363) --- crates/primitives-traits/src/crypto.rs | 56 ++++++++++++++++--- .../primitives/src/transaction/signature.rs | 43 ++------------ 2 files changed, 52 insertions(+), 47 deletions(-) diff --git a/crates/primitives-traits/src/crypto.rs b/crates/primitives-traits/src/crypto.rs index cc9c8aaf3bde..aba6107272e3 100644 --- a/crates/primitives-traits/src/crypto.rs +++ b/crates/primitives-traits/src/crypto.rs @@ -13,15 +13,47 @@ 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::*; + 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 +73,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)?)?; @@ -87,7 +122,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 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]; 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 1535664cd769f2f1381037bd4bc5ebb832873d5b Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 12 Dec 2024 21:37:28 +0100 Subject: [PATCH 2/2] chore: update holocene fns (#13366) --- Cargo.lock | 29 ++++++++-------- Cargo.toml | 10 +++--- crates/optimism/chainspec/Cargo.toml | 1 + crates/optimism/chainspec/src/lib.rs | 47 ++++---------------------- crates/optimism/evm/src/lib.rs | 5 +-- crates/optimism/payload/src/payload.rs | 4 +-- 6 files changed, 32 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 917a4f242175..47e25c632c4a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5411,9 +5411,9 @@ checksum = "b410bbe7e14ab526a0e86877eb47c6996a2bd7746f027ba551028c925390e4e9" [[package]] name = "op-alloy-consensus" -version = "0.8.1" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5085cc8be65a2da9c04e9a904eccfe38eb69ecc3bc6c6485ce0f1af879f3abe" +checksum = "24aaf487dd59beed72931e31b11b305cdcb6a20651a1cccf992a20706a54cc3b" dependencies = [ "alloy-consensus", "alloy-eips", @@ -5429,9 +5429,9 @@ dependencies = [ [[package]] name = "op-alloy-genesis" -version = "0.8.1" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "64f5aa1201d83af3b0ebffbfc28fdc7e772d7e44f4dea9e41c51162f84412edf" +checksum = "42ad6c33c2711611e19092a7d17c076542e27687ef975f67bf57039fa3d57e06" dependencies = [ "alloy-consensus", "alloy-eips", @@ -5444,9 +5444,9 @@ dependencies = [ [[package]] name = "op-alloy-network" -version = "0.8.1" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d12eafaad5b89792de75f1344d42634dd5271945fd36256e4e5d766cf32107e" +checksum = "8273ddf3a0a8cb891abbc625289f8094fbeab1329e0874b38e14bf1670402b7c" dependencies = [ "alloy-consensus", "alloy-network", @@ -5459,9 +5459,9 @@ dependencies = [ [[package]] name = "op-alloy-protocol" -version = "0.8.1" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cf8c05e1b7ed20c4af6f72a54cc389225d2c6af6fcf932ef6481cfdfcb540ac" +checksum = "ccb98f90d0101cdaabb739c44f4d4b2d0ad0ad1a3dd68ce525683ffafd97dd13" dependencies = [ "alloc-no-stdlib", "alloy-consensus", @@ -5483,9 +5483,9 @@ dependencies = [ [[package]] name = "op-alloy-rpc-jsonrpsee" -version = "0.8.1" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b6edd6fb56f23ab45c704ad0c598641086e07b3a55d74890acaa01226ffc3e2" +checksum = "44a41095f6aa4f39e3d8927ac8a903a45e76ddb533f466ef592853f9a49fe8cf" dependencies = [ "alloy-eips", "alloy-primitives", @@ -5496,9 +5496,9 @@ dependencies = [ [[package]] name = "op-alloy-rpc-types" -version = "0.8.1" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b44194f44faef3db1edd17fc8e0b1309d377c6c7a4ba74a02d78c13d5f2ed90d" +checksum = "21d0ada6356ac7818d4b1ba48c37116662732b97f18a3684f08727dafbef2437" dependencies = [ "alloy-consensus", "alloy-eips", @@ -5515,9 +5515,9 @@ dependencies = [ [[package]] name = "op-alloy-rpc-types-engine" -version = "0.8.1" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c005d0a4431fbd9cd16eb0c2f67306de4245f76f311fa1628a68b79e7ea5aa0e" +checksum = "7c63ba3f50dba410d1ce18aa16242c198f91ad44da5221f7347c1525958a9b09" dependencies = [ "alloy-eips", "alloy-primitives", @@ -8362,6 +8362,7 @@ dependencies = [ "alloy-primitives", "derive_more", "once_cell", + "op-alloy-consensus", "op-alloy-rpc-types", "reth-chainspec", "reth-ethereum-forks", diff --git a/Cargo.toml b/Cargo.toml index 80233cdb4397..0c9f94bcf55a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -477,11 +477,11 @@ alloy-transport-ipc = { version = "0.8.0", default-features = false } alloy-transport-ws = { version = "0.8.0", default-features = false } # op -op-alloy-rpc-types = "0.8.1" -op-alloy-rpc-types-engine = "0.8.1" -op-alloy-rpc-jsonrpsee = "0.8.1" -op-alloy-network = "0.8.1" -op-alloy-consensus = "0.8.1" +op-alloy-rpc-types = "0.8.2" +op-alloy-rpc-types-engine = "0.8.2" +op-alloy-rpc-jsonrpsee = "0.8.2" +op-alloy-network = "0.8.2" +op-alloy-consensus = "0.8.2" # misc aquamarine = "0.6" diff --git a/crates/optimism/chainspec/Cargo.toml b/crates/optimism/chainspec/Cargo.toml index 5ccf26607094..818f2cd0d45c 100644 --- a/crates/optimism/chainspec/Cargo.toml +++ b/crates/optimism/chainspec/Cargo.toml @@ -30,6 +30,7 @@ alloy-eips.workspace = true # op op-alloy-rpc-types.workspace = true +op-alloy-consensus.workspace = true # io serde_json.workspace = true diff --git a/crates/optimism/chainspec/src/lib.rs b/crates/optimism/chainspec/src/lib.rs index a1a08dd3b09a..6670aef1d01d 100644 --- a/crates/optimism/chainspec/src/lib.rs +++ b/crates/optimism/chainspec/src/lib.rs @@ -29,6 +29,7 @@ pub use dev::OP_DEV; #[cfg(not(feature = "std"))] pub(crate) use once_cell::sync::Lazy as LazyLock; pub use op::OP_MAINNET; +use op_alloy_consensus::{decode_holocene_extra_data, EIP1559ParamError}; pub use op_sepolia::OP_SEPOLIA; use reth_chainspec::{ BaseFeeParams, BaseFeeParamsKind, ChainSpec, ChainSpecBuilder, DepositContract, EthChainSpec, @@ -201,8 +202,8 @@ impl OpChainSpec { &self, parent: &Header, timestamp: u64, - ) -> Result { - let (denominator, elasticity) = decode_holocene_1559_params(&parent.extra_data)?; + ) -> Result { + let (denominator, elasticity) = decode_holocene_extra_data(&parent.extra_data)?; let base_fee = if elasticity == 0 && denominator == 0 { parent .next_block_base_fee(self.base_fee_params_at_timestamp(timestamp)) @@ -221,13 +222,11 @@ impl OpChainSpec { &self, parent: &Header, timestamp: u64, - ) -> Result { + ) -> Result { // > if Holocene is active in parent_header.timestamp, then the parameters from // > parent_header.extraData are used. - let is_holocene_activated = self.inner.is_fork_active_at_timestamp( - reth_optimism_forks::OpHardfork::Holocene, - parent.timestamp, - ); + let is_holocene_activated = + self.inner.is_fork_active_at_timestamp(OpHardfork::Holocene, parent.timestamp); // If we are in the Holocene, we need to use the base fee params // from the parent block's extra data. @@ -244,40 +243,6 @@ impl OpChainSpec { } } -#[derive(Clone, Debug, Display, Eq, PartialEq)] -/// Error type for decoding Holocene 1559 parameters -pub enum DecodeError { - #[display("Insufficient data to decode")] - /// Insufficient data to decode - InsufficientData, - #[display("Invalid denominator parameter")] - /// Invalid denominator parameter - InvalidDenominator, - #[display("Invalid elasticity parameter")] - /// Invalid elasticity parameter - InvalidElasticity, -} - -impl core::error::Error for DecodeError { - fn source(&self) -> Option<&(dyn core::error::Error + 'static)> { - // None of the errors have sub-errors - None - } -} - -/// Extracts the Holcene 1599 parameters from the encoded form: -/// -pub fn decode_holocene_1559_params(extra_data: &[u8]) -> Result<(u32, u32), DecodeError> { - if extra_data.len() < 9 { - return Err(DecodeError::InsufficientData); - } - let denominator: [u8; 4] = - extra_data[1..5].try_into().map_err(|_| DecodeError::InvalidDenominator)?; - let elasticity: [u8; 4] = - extra_data[5..9].try_into().map_err(|_| DecodeError::InvalidElasticity)?; - Ok((u32::from_be_bytes(denominator), u32::from_be_bytes(elasticity))) -} - impl EthChainSpec for OpChainSpec { type Header = Header; diff --git a/crates/optimism/evm/src/lib.rs b/crates/optimism/evm/src/lib.rs index 740eb92e6171..f99d2f9def35 100644 --- a/crates/optimism/evm/src/lib.rs +++ b/crates/optimism/evm/src/lib.rs @@ -15,8 +15,9 @@ extern crate alloc; use alloc::{sync::Arc, vec::Vec}; use alloy_consensus::Header; use alloy_primitives::{Address, U256}; +use op_alloy_consensus::EIP1559ParamError; use reth_evm::{ConfigureEvm, ConfigureEvmEnv, NextBlockEnvAttributes}; -use reth_optimism_chainspec::{DecodeError, OpChainSpec}; +use reth_optimism_chainspec::OpChainSpec; use reth_primitives::{transaction::FillTxEnv, Head, TransactionSigned}; use reth_revm::{ inspector_handle_register, @@ -58,7 +59,7 @@ impl OpEvmConfig { impl ConfigureEvmEnv for OpEvmConfig { type Header = Header; type Transaction = TransactionSigned; - type Error = DecodeError; + type Error = EIP1559ParamError; fn fill_tx_env(&self, tx_env: &mut TxEnv, transaction: &TransactionSigned, sender: Address) { transaction.fill_tx_env(tx_env, sender); diff --git a/crates/optimism/payload/src/payload.rs b/crates/optimism/payload/src/payload.rs index e243745cea68..1a8655bd7333 100644 --- a/crates/optimism/payload/src/payload.rs +++ b/crates/optimism/payload/src/payload.rs @@ -7,7 +7,7 @@ use alloy_eips::{ use alloy_primitives::{keccak256, Address, Bytes, B256, B64, U256}; use alloy_rlp::Encodable; use alloy_rpc_types_engine::{ExecutionPayloadEnvelopeV2, ExecutionPayloadV1, PayloadId}; -use op_alloy_consensus::{decode_holocene_extra_data, EIP1559ParamError}; +use op_alloy_consensus::{encode_holocene_extra_data, EIP1559ParamError}; /// Re-export for use in downstream arguments. pub use op_alloy_rpc_types_engine::OpPayloadAttributes; use op_alloy_rpc_types_engine::{OpExecutionPayloadEnvelopeV3, OpExecutionPayloadEnvelopeV4}; @@ -45,7 +45,7 @@ impl OpPayloadBuilderAttributes { default_base_fee_params: BaseFeeParams, ) -> Result { self.eip_1559_params - .map(|params| decode_holocene_extra_data(params, default_base_fee_params)) + .map(|params| encode_holocene_extra_data(params, default_base_fee_params)) .ok_or(EIP1559ParamError::NoEIP1559Params)? } }