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

Client: Add executeBlocks debug option #1538

Merged
merged 2 commits into from
Oct 21, 2021
Merged

Conversation

holgerd77
Copy link
Member

This PR adds a new option --executeBlocks to the client. The option can be used to re-execute blocks in the client which are already synced for debugging purposes without changing the state of the client DB in any way.

Option can be used as follows:

Run a single block:

npm run client:start -- --executeBlocks=46370

grafik

Run a range of blocks:

npm run client:start -- --executeBlocks=46370-46380

grafik

Run selected txs from a block:

npm run client:start -- --executeBlocks=46370[0xba4b5fd92a26badad3cad22eb6f7c7e745053739b5f5d1e8a3afb00f8fb2a280]

grafik

Option can very well be combined with a DEBUG=vm:* VM debug logger activated. This should hopefully help to debug on things like the Shanghai dDoS blocks as well as various other scenarios.

@codecov
Copy link

codecov bot commented Oct 21, 2021

Codecov Report

Merging #1538 (feb9de1) into master (1ce6713) will decrease coverage by 0.38%.
The diff coverage is 29.72%.

Impacted file tree graph

Flag Coverage Δ
block 86.28% <ø> (+0.13%) ⬆️
blockchain 82.53% <ø> (ø)
client 77.78% <29.72%> (-1.28%) ⬇️
common 94.08% <ø> (ø)
devp2p 82.62% <ø> (-0.37%) ⬇️
ethash 90.76% <ø> (ø)
tx 87.34% <ø> (-0.13%) ⬇️
vm 79.70% <ø> (+0.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@emersonmacro emersonmacro left a comment

Choose a reason for hiding this comment

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

One small comment/question, but overall looks good to me 👍

last = blockRange.length === 2 ? (blockRange[1][0] as number) : first
txHashes = blockRange[0][1] as string[]

if ((blockRange[0][1] as string[]).length > 0 && blockRange.length === 2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For readability's sake, could this just be if (txHashes.length > 0 && ...?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you are totally right, didn't see the forest for all the trees. 😛 Won't rerun CI for this, I'll try to remember to update on some follow-up PR and have otherwise added to the tiny-fixes-issue.

@holgerd77 holgerd77 merged commit 2dfa7ee into master Oct 21, 2021
@holgerd77 holgerd77 deleted the client-execute-blocks-flag branch October 21, 2021 20:21
@jochem-brouwer
Copy link
Member

This is cool. What is the client behavior in case we run into a bad block? I think it deletes the block from database by default right? It would be useful to keep this bad block in the database instead, such that we can figure out the consensus bug (these bad blocks are bad blocks reported by our client; assuming that the canonical chain follows through it is thus a false positive by our client and it would thus be useful to keep the block, such that we can use this flag to re-run and debug the block).

@holgerd77
Copy link
Member Author

This should be completely side-effects free respectively not modify any state (in the broader sense) within the client, that's why I settled this pretty high in the call hierachy and also didn't use the existing VM execution logic but wrote an own version with some inspiration from the engine API block execution. This is running on a copy of the VM and also not goes in any fail logic or accidentally emits events which could have side effects. One nice thing is that one also can adopt the log messages for the use case and do e.g. more detailed execution logic than one would normally do.

@holgerd77
Copy link
Member Author

(so this is a completely separate "client mode" without running the start() methods, I actually also made this a bit cleaner along the way so that now - when no start() is called - really nothing starts at all (before the bootstrap retriggering was still actively running triggered in the PeerPool. This might also have the side effect that some tests might be easier to set up, this might have been a cause for the thing we had earlier that tests were not terminating)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants