chore: integrate rust-secp256k1 #1915
Conversation
crates/precompile/src/secp256k1.rs
Outdated
| @@ -45,20 +45,23 @@ mod secp256k1 { | |||
| #[allow(clippy::module_inception)] | |||
| mod secp256k1 { | |||
| use primitives::{alloy_primitives::B512, keccak256, B256}; | |||
| use secp256k1::{ | |||
| use rust_secp256k1::{ | |||
There was a problem hiding this comment.
Hey @stevencartavia, the idea is not to replace it but to add a new option with a feature flag for rust_secp256k1.
I would see to try cfg_if (there are a few examples of its usage in the repo)
crates/precompile/src/secp256k1.rs
Outdated
| use secp256k1 as _; | ||
|
|
||
| pub fn ecrecover(sig: &B512, recid: u8, msg: &B256) -> Result<B256, rust_secp256k1::Error> { | ||
| let engine = Secp256k1::new(); |
There was a problem hiding this comment.
does rust_secp256k1 have something as a static context? for secp256k1 this was one of the performance degradation and that is why we use SECP256K1.recover_ecdsa as it is faster
|
I haven't found a way to fix that warning, if I add |
crates/precompile/Cargo.toml
Outdated
| @@ -78,7 +79,7 @@ serde_json = "1.0" | |||
| serde_derive = "1.0" | |||
|
|
|||
| [features] | |||
| default = ["std", "c-kzg", "secp256k1", "portable", "blst"] | |||
| default = ["std", "c-kzg", "secp256k1", "portable", "blst", "libsecp256k1"] | |||
There was a problem hiding this comment.
| default = ["std", "c-kzg", "secp256k1", "portable", "blst", "libsecp256k1"] | |
| default = ["std", "c-kzg", "secp256k1", "portable", "blst"] |
|
Have restructured the code a little, and left comments for visibility. Will merge after ci |
| pub mod bitcoin_secp256k1; | ||
| pub mod k256; | ||
| #[cfg(feature = "libsecp256k1")] | ||
| pub mod parity_libsecp256k1; |
There was a problem hiding this comment.
Moved it to a separate file so it is easier to navigate. and all of them can be enabled in same time
| let res = parity_libsecp256k1::ecrecover(sig, recid, msg); | ||
| } else { | ||
| let res = k256::ecrecover(sig, recid, msg); | ||
| } |
There was a problem hiding this comment.
This is were switch is happening, priority is secp256k1 (bitcoin lib), parity (if enabled) and by default k256.
CodSpeed Performance ReportMerging #1915 will not alter performanceComparing Summary
|
should close #1914