-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Fixing block hang on syncing empty blocks #266
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* External Imports */ | ||
| import { getLogger, logError } from '@eth-optimism/core-utils' | ||
| import { getLogger, logError, sleep } from '@eth-optimism/core-utils' | ||
| import { Block, Provider, TransactionReceipt } from 'ethers/providers' | ||
|
|
||
| /* Internal Imports */ | ||
|
|
@@ -112,11 +112,28 @@ export class EthereumBlockProcessor { | |
| `Waiting for ${this.confirmsUntilFinal} confirms before disseminating block ${blockNumber}` | ||
| ) | ||
| try { | ||
| const receipt: TransactionReceipt = await provider.waitForTransaction( | ||
| (block.transactions[0] as any).hash, | ||
| this.confirmsUntilFinal | ||
| ) | ||
| if (receipt.blockHash !== block.hash) { | ||
| let refetchedHash: string | ||
| if (block.transactions.length > 0) { | ||
| const receipt: TransactionReceipt = await provider.waitForTransaction( | ||
| (block.transactions[0] as any).hash, | ||
| this.confirmsUntilFinal | ||
| ) | ||
| refetchedHash = receipt.blockHash | ||
| } else { | ||
| while ( | ||
| (await provider.getBlockNumber()) < | ||
| blockNumber + this.confirmsUntilFinal | ||
| ) { | ||
| log.debug( | ||
| `Waiting for empty block ${blockNumber} to be final. Sleeping...` | ||
| ) | ||
| await sleep(15_000) | ||
| } | ||
|
Comment on lines
+123
to
+131
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need both cases or can we just keep this one?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, don't need to. Was thinking maaaaaybe ethers does something better than just polling, so that's better than this? Happy to consolidate if you think though. Eventually we'll go with the "this block finalizes the block X blocks ago" approach vs the "I got a block, now wait a bunch of blocks for this block to be final" approach.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Going to ship for now but happy to change it post-merge if you'd like! |
||
| const refetched = await provider.getBlock(blockNumber) | ||
| refetchedHash = refetched.hash | ||
| } | ||
|
|
||
| if (refetchedHash !== block.hash) { | ||
| log.info( | ||
| `Re-org processing block number ${blockNumber}. Re-fetching block.` | ||
| ) | ||
|
|
||
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.
👍👍