feat: add Signature trait #782
Conversation
| pub trait Signature<'a>: | ||
| TryFrom<&'a [u8], Error = SignatureError> + FromStr<Err = SignatureError> |
There was a problem hiding this comment.
| pub trait Signature<'a>: | |
| TryFrom<&'a [u8], Error = SignatureError> + FromStr<Err = SignatureError> | |
| pub trait Signature: | |
| for<'a> TryFrom<&'a [u8], Error = SignatureError> + FromStr<Err = SignatureError> |
| fn test_signature() -> Self | ||
| where | ||
| Self: Sized; |
There was a problem hiding this comment.
this shouldn't be in the trait imo. if it is, then with attribute #[cfg(test)] or #[cfg(feature = test-util], but that would change the logic here, but this is a refactor pr, so leave it out, just move it to the EcdsaSignature impl body.
| /// Instantiate a new signature from `r`, `s`, and `v` values. | ||
| #[allow(clippy::missing_const_for_fn)] | ||
| pub fn new(r: U256, s: U256, v: Parity) -> Self { | ||
| Self { r, s, v } | ||
| } | ||
|
|
||
| /// Returns the `r` component of this signature. | ||
| #[inline] | ||
| pub const fn r(&self) -> U256 { | ||
| self.r | ||
| } | ||
|
|
||
| /// Returns the `s` component of this signature. | ||
| #[inline] | ||
| pub const fn s(&self) -> U256 { | ||
| self.s | ||
| } | ||
|
|
||
| /// Returns the recovery ID as a `u8`. | ||
| #[inline] | ||
| pub const fn v(&self) -> Parity { | ||
| self.v | ||
| } |
There was a problem hiding this comment.
please add these as trait methods too, they make sense in reth integration
| #[cfg(feature = "rlp")] | ||
| fn decode_rlp_vrs(buf: &mut &[u8]) -> Result<Self, alloy_rlp::Error> | ||
| where | ||
| Self: Sized; |
There was a problem hiding this comment.
this for example can have a default impl, since the trait method Signature::from_rs_and_parity is accessible
grandima
force-pushed
the
signature-trait
branch
2 times, most recently
from
6d23d9f to
e9d3a60
Compare
| pub trait ArbitrarySuperSig: | ||
| arbitrary::Arbitrary | ||
| + proptest::arbitrary::Arbitrary<Parameters=(),Strategy=proptest::strategy::FilterMap< | ||
| <(U256, U256, Parity) as proptest::arbitrary::Arbitrary>::Strategy, | ||
| fn((U256, U256, Parity)) -> Option<Self>, | ||
| >> {} |
There was a problem hiding this comment.
should be enough to do
| pub trait ArbitrarySuperSig: | |
| arbitrary::Arbitrary | |
| + proptest::arbitrary::Arbitrary<Parameters=(),Strategy=proptest::strategy::FilterMap< | |
| <(U256, U256, Parity) as proptest::arbitrary::Arbitrary>::Strategy, | |
| fn((U256, U256, Parity)) -> Option<Self>, | |
| >> {} | |
| pub trait ArbitrarySuperSig: | |
| for <'a> arbitrary::Arbitrary<'a> {} |
| #[cfg(feature = "rlp")] | ||
| fn decode_rlp_vrs(buf: &mut &[u8]) -> Result<Self, alloy_rlp::Error> { | ||
| use alloy_rlp::Decodable; | ||
|
|
||
| let parity: Parity = Decodable::decode(buf)?; | ||
| let r = Decodable::decode(buf)?; | ||
| let s = Decodable::decode(buf)?; | ||
|
|
||
| Self::from_rs_and_parity(r, s, parity) | ||
| .map_err(|_| alloy_rlp::Error::Custom("attempted to decode invalid field element")) | ||
| } |
There was a problem hiding this comment.
on second thought, would be cool to move all the feature gated methods into their respective XSuperSig trait actually
There was a problem hiding this comment.
I thought that too! However, I took iterative approach and decided to implement it in the future commits to this PR. I just did not expect you to review it since I did not ping you. Anyway, thanks for valuable suggestions!
|
Hey @emhane! |
grandima
force-pushed
the
signature-trait
branch
2 times, most recently
from
a353cf6 to
2ac8cad
Compare
|
@emhane please review |
add ArbitrarySuperSig, K256SuperSig, RlpSuperSig, SerdeSuperSig feature traits to Signature
grandima
force-pushed
the
signature-trait
branch
from
b03f172 to
0e5c533
Compare
|
I don't know exactly who from code owners to tag so I'll just pick @mattsse Please review. |
|
Sorry for the late reply. Due to recent refactors and low demand for this feature this has gone stale. We may pick this up in the future. Thanks for your contribution nonetheless. |
Motivation
Create
trait Signatureand move all possible methods and traits fromstruct Signaturetotrait Signature. Resolve #766Solution
trait Signature.trait Signature.trait Signatureto all possible supertraits.struct Signatureintostruct EcdsaSignature.PR Checklist