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

chore(sdk): Add blanket impls for refs to prim traits #12613

Merged
merged 1 commit into from
Nov 17, 2024

Conversation

emhane
Copy link
Member

@emhane emhane commented Nov 17, 2024

A lot of places in the code take references to the primitive data types. For components to use these as generics, blanket impls of the data primitive traits for reference types, are needed.

@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 Nov 17, 2024
@@ -32,7 +33,7 @@ pub trait SignedTransaction:
+ alloy_rlp::Decodable
+ Encodable2718
+ Decodable2718
+ TransactionExt
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since trait TransactionExt shrunk to basically only this method which isn't called on TransactionSigned, its redundant as super trait

Comment on lines +34 to +35
/// Extension if [`Receipt`] used in block execution.
pub trait ReceiptExt: Receipt {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, splitting this makes sense we definitely want this because op can't have this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah true, optimism needs the chain spec too! in general, I'm not convinced that this is the right place for the method...think this approach made the most sense #11314

@mattsse mattsse added this pull request to the merge queue Nov 17, 2024
Merged via the queue into main with commit 7ae8ce1 Nov 17, 2024
41 checks passed
@mattsse mattsse deleted the emhane/auto-impl-prims branch November 17, 2024 17:04
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.

2 participants