search

alloy-rs / core

High-performance, well-tested & documented core libraries for Ethereum, in Rust
https://alloy.rs
Apache License 2.0
758 stars 131 forks source link

feat: expose Signature<T> #689

Closed leruaa closed 1 month ago

leruaa commented 1 month ago

Motivation

Needed for https://github.com/alloy-rs/alloy/issues/1077 and https://github.com/paradigmxyz/reth/pull/9593

Solution

PR Checklist

leruaa commented 1 month ago

While attempting to apply this refactoring to alloy itself, I start to understand why the k256 feature is not additive. That to avoid to impl traits like SignableTransaction for multiple Signature types leading to the "multiple impls ... found" error.

I'm trying to find a workaround.

mattsse commented 1 month ago

@leruaa perhaps we can introduce a helper trait in alloy for the required rsv function and just that for the alloy encode signature fns instead?

mattsse commented 1 month ago

but perhaps we should consider removing the generic entirely, I find this very useless because this is effectively just duplicating the rsv again

https://github.com/alloy-rs/core/blob/adcf7adfa1f35c56e6331bab85b8c56d32a465f1/crates/primitives/src/signature/sig.rs#L301-L305

leruaa commented 1 month ago

@mattsse fyi and for posterity, the branch in alloy applying this refactoring in alloy-core: https://github.com/alloy-rs/alloy/compare/main...leruaa:alloy:signature_refactoring