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/Blockchain: receipt reorg logic #3146

Merged
merged 17 commits into from
Nov 13, 2023
Merged

Client/Blockchain: receipt reorg logic #3146

merged 17 commits into from
Nov 13, 2023

Conversation

jochem-brouwer
Copy link
Member

@jochem-brouwer jochem-brouwer commented Nov 6, 2023

This PR attempts to resolve hive failures in:

./hive --client ethereumjs --sim ethereum/engine --sim.limit="engine-cancun/Transaction Re-Org" and
./hive --client ethereumjs --sim ethereum/engine --sim.limit="engine-cancun/Invalid Missing Ancestor Syncing ReOrg"

This PR:

  • Adds events to blockchain: once blocks get reorged and deleted, deletedCanonicalBlocks event is emitted
  • Adds an extra guard on eth_getTransactionReceipt: this verifies that the receipt is in the canonical chain
  • Deletes receipts in client once a block gets reorged out
  • Ensures INVALID is not returned on newPayload / forkchoiceUpdated in case the block is lower than the currently known invalid blocks.

To test this, an extra diff has to be added. This hacks in a way that a peer is always returned, to ensure that the syncing process is completed. Diff for ./packages/client/src/sync/beaconsync.ts:

       const latest = await this.latest(peer)
       if (latest) {
         const { number } = latest
-        if ((!best && number >= this.chain.blocks.height) || (best && best[1] < number)) {
+        // if ((!best && number >= this.chain.blocks.height) || (best && best[1] < number))
+        {
           best = [peer, number]
         }
       }

When running all tests, this brings the failing tests down to just 3 (note, if more fail, re-run these individual tests, they might be flakey which sometimes happens)

Inconsistent Head in ForkchoiceState
Valid NewPayload->ForkchoiceUpdated on Syncing Client
Invalid PayloadAttributes, Missing BeaconRoot, Syncing=False

Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Merging #3146 (8384cd4) into master (3b257e9) will increase coverage by 0.52%.
The diff coverage is 70.41%.

❗ Current head 8384cd4 differs from pull request most recent head 3b2e972. Consider uploading reports for the commit 3b2e972 to get more accurate results

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 88.80% <ø> (ø)
blockchain 91.61% <63.93%> (-0.66%) ⬇️
client 85.36% <72.00%> (?)
common 98.19% <ø> (ø)
devp2p 86.32% <ø> (ø)
ethash ∅ <ø> (∅)
evm 71.93% <ø> (ø)
statemanager 90.13% <ø> (ø)
trie 89.76% <ø> (-0.25%) ⬇️
tx 96.36% <ø> (?)
util 88.91% <ø> (ø)
vm 81.61% <ø> (ø)

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

this._heads[name] = headHash
if (this.events.listenerCount('deletedCanonicalBlocks') > 0) {
const block = await this.getBlock(blockNumber)
this._deletedBlocks.push(block)
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this is only real change, rest is just formatting change because of try catch

Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm,

@acolytec3 acolytec3 merged commit 3fdd95e into master Nov 13, 2023
42 checks passed
@holgerd77 holgerd77 deleted the hive-reorg-logic branch July 11, 2024 08:16
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