feat: add support for debug_getBadblock #20144
Conversation
0xKarl98
requested review from
Rjected,
fgimenez,
klkvr and
mattsse
as code owners
| /// Returns an array of recent bad blocks that the client has seen on the network. | ||
| #[method(name = "getBadBlocks")] | ||
| async fn bad_blocks(&self) -> RpcResult<Vec<Block>>; | ||
| async fn bad_blocks(&self) -> RpcResult<Vec<BadBlock>>; |
There was a problem hiding this comment.
I think we can actually make this just serde_json::Value, seems easier or we use the actual rpc type see also
reth/crates/rpc/rpc-eth-api/src/core.rs
Lines 90 to 91 in 1902970
There was a problem hiding this comment.
Yeah think we need to unify the block def in rpc here
| eth_api: Eth, | ||
| // restrict the number of concurrent calls to blocking calls | ||
| blocking_task_guard: BlockingTaskGuard, | ||
| bad_block_store: BadBlockStore<ProviderBlock<Eth::Provider>>, |
There was a problem hiding this comment.
I see we need the types for this
I guess an alternative here would be to just use EthApiTypes here
| // drop existing entry with same hash to keep most recent order | ||
| guard.retain(|b| b.hash() != hash); | ||
| guard.push_back(Arc::new(block)); |
There was a problem hiding this comment.
instead of removing, I think we should just check if we already have it
because it's common that we see bad blocks a few times, and this would then mess up the ordering
There was a problem hiding this comment.
Yeah def should do that , skip the duplicated bad block hash
| /// A bounded, deduplicating store of recently observed bad blocks. | ||
| #[derive(Clone, Debug)] | ||
| pub struct BadBlockStore<B: BlockTrait> { |
There was a problem hiding this comment.
this type def is out of order, we should move this to after all the impl blocks
There was a problem hiding this comment.
Yeah will look more ordered
| // keep track of invalid blocks for `debug_getBadBlocks` | ||
| let bad_block_store = registry.bad_block_store().clone(); | ||
| let mut engine_events_stream = engine_events.new_listener(); | ||
| node.task_executor().spawn_critical( |
There was a problem hiding this comment.
this is kinda redundant if we dont have the debug_ endpoint installed because then the blockstore wont be useable, but this doesnt really hurt anyway
There was a problem hiding this comment.
Have set a flag to enable blockstore only when debug_ endpoint installed
| // keep track of invalid blocks for `debug_getBadBlocks` | ||
| let bad_block_store = registry.bad_block_store().clone(); | ||
| let mut engine_events_stream = engine_events.new_listener(); | ||
| node.task_executor().spawn_critical( |
There was a problem hiding this comment.
we can spawn this as regular task imo
mattsse
added
M-changelog
|
Just closes this and open another with more proper name debug_getBadBlocks |
2 participants
This targets on issue #20123 , as below :
cc @mattsse @klkvr @Rjected