-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: modular flashblocks #20122
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
base: main
Are you sure you want to change the base?
feat: modular flashblocks #20122
Conversation
progr simplify Update cache.rs Update cache.rs simplify trait simplify Update sequence.rs simplifies Update lib.rs Update service.rs Simplify simplify
fixes mods generic Update decoding.rs rpc fixes temp remove consensus
fix custom node tidy Update lib.rs Update worker.rs Update worker.rs
21a3276 to
536bebf
Compare
536bebf to
64b9548
Compare
mattsse
left a comment
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.
this looks very promising already
| where | ||
| F: serde::de::DeserializeOwned, |
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.
hmm, I think only the new fn needs this bound?
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.
all of this makes sense
| /// The base payload type containing block environment configuration. | ||
| type Base: FlashblockPayloadBase; | ||
| /// The diff type containing state changes. | ||
| type Diff: FlashblockDiff; | ||
| /// The signed transaction type for this chain. | ||
| type SignedTx: reth_primitives_traits::SignedTransaction; |
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 see, this assumes a general structure flashblocks have, which I think is okay
| /// Extension trait for OP-specific RPC types that includes flashblock configuration. | ||
| pub trait OpRpcTypes: RpcTypes { | ||
| /// The flashblock payload type for this chain. | ||
| type Flashblock: FlashblockPayload; | ||
| } | ||
|
|
||
| impl OpRpcTypes for Optimism { | ||
| type Flashblock = OpFlashblockPayload; | ||
| } |
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.
defining this on the rpctypes also makes sense to me
| flashblocks_sequence.subscribe(), | ||
| )?; | ||
| ctx.components.task_executor().spawn(Box::pin(flashblock_client.run())); | ||
| todo!("Modularize FlashBlockConsensusClient?") |
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.
yeah maybe a general purpose implementation would be great, but unsure if feasible
TODOs
closes #20014