refactor: remove Signature generic
#719
Conversation
| type Error = k256::ecdsa::Error; | ||
|
|
||
| fn try_from(value: Signature) -> Result<Self, Self::Error> { | ||
| Self::from_scalars(value.r.to_be_bytes(), value.s.to_be_bytes()) |
There was a problem hiding this comment.
recovery becomes more expensive because this is not a free conversion
There was a problem hiding this comment.
worth looking into easier ways to construct the scalars without going through big endian bytes
There was a problem hiding this comment.
I don't think k256 Signature allows instantiation without going through bytes
Given that signature fields are not public, should we consider adding this to it:
#[cfg(feature = "k256")]
signature: Option<k256::ecdsa::Signature>,which will be set to None if invalid signature is instantiated, and recovery will just fail if its None
There was a problem hiding this comment.
I don't think k256 Signature allows instantiation without going through bytes
it doesn't
I think we should be okay with this overhead. this only affects the alloy sig -> k256 signature conversion and most common is decoding this from rlp
and getting rid of the additional footprint is more impactful, at least from reth pov
Motivation
Currently it is impossible to construct an invalid signature, because
k256feature requires it to contain a validk256::Signature. This makes it impossible to decode and store invalid signatures.For example, we require this in reth to keep deposit transaction signatures: https://github.com/paradigmxyz/reth/blob/42dc5eea1685385b835edd03029640609f4c4ab4/crates/primitives/src/transaction/signature.rs#L200
Solution
Remove generic from
Signature. Instead, we now convert signature intok256::Signatureduring recovery.PR Checklist