-
Notifications
You must be signed in to change notification settings - Fork 440
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
Add custom Eth module that exposes additional OP related fields #7096
Conversation
db82fc5
to
b0fd3ab
Compare
b0fd3ab
to
bd6a882
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks a bit overengeneered, couldn't we have OptimismEthRpcModule : IEthRpcModule
and then return results with derived classes that can have different json serialization rules?
src/Nethermind/Nethermind.Serialization.Rlp/ReceiptArrayStorageDecoder.cs
Outdated
Show resolved
Hide resolved
src/Nethermind/Nethermind.Optimism/Rpc/IOptimismEthRpcModule.cs
Outdated
Show resolved
Hide resolved
@LukaszRozmej the latest commit a bit more complicated. But in addition to deduplication, I wanted to bring this PoC of abstracted key data structures. Maybe we can do the same with transactions, blocks, etc later |
f999a2b
to
4322d4f
Compare
4322d4f
to
183daf2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kind of would prefer composition for the module, but it is fine.
@LukaszRozmej I don't mind, but why? |
Composition is always more flexible |
Changes
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
Optional. Remove if not applicable.
Documentation
Requires documentation update