-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat(engine): compare invalid block witness against a healthy node #10844
feat(engine): compare invalid block witness against a healthy node #10844
Conversation
83f3f65
to
f2d12f7
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.
mostly nits and style comments
let chain_id = futures::executor::block_on(async { | ||
EthApiClient::< | ||
reth_rpc_types::Transaction, | ||
reth_rpc_types::Block, | ||
reth_rpc_types::Receipt, | ||
>::chain_id(&client) |
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.
makes me wonder if we should just provide a blocking api in EthApiClient
somehow, but not something to address in this PR
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.
nice, lgtm
needs doctest renames |
let client = jsonrpsee::http_client::HttpClientBuilder::default().build(url)?; | ||
|
||
// Verify that the healthy node is running the same chain as the current node. | ||
let chain_id = futures::executor::block_on(async { |
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.
do we even need this check?
I mean this is a debug feature, so do we need to explicitly check this
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'd prefer to have it so we don't end up in a situation when generated witness diff is incorrect and we spend time figuring out why
64ac568
to
8b3969d
Compare
8b3969d
to
b980b8c
Compare
…hook-healthy-node-rpc-url
Closes #10549
Adds a
--debug.healthy-node-rpc-url
CLI argument that initializes an RPC client, checks if the chain ID matches, and passes the client to the witness invalid block hook.Later, if the witness hook is triggered, we query the healthy node for the witness of the same block, save the output in a separate file and compare it against the original one.