Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove redundant SignedTransaction::Signature #12185

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

emhane
Copy link
Member

@emhane emhane commented Oct 30, 2024

Since we can remove the op feature gated signature logic from reth-primitives, the signature AT is redundant

Ref #12184

@emhane emhane added C-debt Refactor of code section that is hard to understand or maintain A-sdk Related to reth's use as a library labels Oct 30, 2024
@klkvr
Copy link
Collaborator

klkvr commented Oct 31, 2024

btw wondering which parts of the sdk actually need the fn signature() -> &Signature? wouldn't it be sufficient to just have something like trait Transaction { fn recover_signed(&self) -> Option<Address> }? that way the abstraction does not require each transaction to carry a concrete signature

@mattsse
Copy link
Collaborator

mattsse commented Nov 1, 2024

right, perhaps we can get around it like this, I think the only place we need to interact with the sig directly is win rpc, and this likely goes away and can be abstracted.

so we can proceed with this pr and should consider removing the sig fn if possible

@mattsse mattsse added this pull request to the merge queue Nov 1, 2024
Merged via the queue into main with commit f93dbf5 Nov 1, 2024
41 checks passed
@mattsse mattsse deleted the emhane/signed-tx-trait-sig branch November 1, 2024 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sdk Related to reth's use as a library C-debt Refactor of code section that is hard to understand or maintain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants