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

fix: add special logic to handle ancestor errors[BNB-3] #14

Merged
merged 4 commits into from
Jan 5, 2024

Conversation

welkin22
Copy link
Contributor

Description

The function block_validator.go:ValidateBody() executes validateFuns concurrently using gopool . Under normal circumstances, ValidateBody() waits for all validateFuns to complete. However, if any of the validateFuns returns an error,
ValidateBody() promptly returns with that error. Consequently, if more than one validation function returns an error, it can lead to non-deterministic selection of which error is returned.
After examining the flow and call sources of ValidateBody() , we discovered that certain errors, specifically consensus.ErrUnknownAncestor and consensus.ErrPrunedAncestor , are returned before any errors related to transactions or withdrawals. This sequence could cause the client database to store invalid blocks. To clarify with an example: If a block contains invalid transactions or withdrawals (meaning the hashes in the header don't match the expected calculations) and it also references an unrecognized parent block, there's a risk. If the third goroutine is the first to respond, the main calling function might wrongly assume the block's transactions and withdrawals are correct because it didn't detect the anticipated errors.
These errors receive special handling in blockchain.go:insertChain()#L1721-1736 . For instance, the code attempts to insert the side-chain, recover the ancestor chain, or add it to a future block candidate. While most processes involve re-insertion of the block using the same function ( insertChain() ), a particularly concerning scenario occurs in the blockchain.go:insertSideChain() function, as it may call bc.writeBlockWithoutState() to write the invalid block data into the database.

Rationale

Prevent Race Condition Can Lead to Storing Invalid Blocks in DB

Example

none

Changes

  • Fix problematic code in ValidateBody function

@owen-reorg owen-reorg marked this pull request as draft November 23, 2023 09:05
@owen-reorg owen-reorg marked this pull request as ready for review November 23, 2023 09:05
@owen-reorg owen-reorg merged commit a6996ab into bnb-chain:develop Jan 5, 2024
1 check passed
sunny2022da added a commit to sunny2022da/op-geth that referenced this pull request Jul 29, 2024
The originStorage will miss some loading in txn execution,do merge
rather than simple copy

This fix also use stateObject specific lock for storage update, rather
than the one in stateDB.

Co-authored-by: Sunny <[email protected]>
sunny2022da added a commit to sunny2022da/op-geth that referenced this pull request Aug 7, 2024
The originStorage will miss some loading in txn execution,do merge
rather than simple copy

This fix also use stateObject specific lock for storage update, rather
than the one in stateDB.

Co-authored-by: Sunny <[email protected]>
sunny2022da added a commit to sunny2022da/op-geth that referenced this pull request Aug 13, 2024
The originStorage will miss some loading in txn execution,do merge
rather than simple copy

This fix also use stateObject specific lock for storage update, rather
than the one in stateDB.

Co-authored-by: Sunny <[email protected]>
sunny2022da added a commit to sunny2022da/op-geth that referenced this pull request Sep 25, 2024
The originStorage will miss some loading in txn execution,do merge
rather than simple copy

This fix also use stateObject specific lock for storage update, rather
than the one in stateDB.

Co-authored-by: Sunny <[email protected]>
sunny2022da added a commit to sunny2022da/op-geth that referenced this pull request Oct 11, 2024
The originStorage will miss some loading in txn execution,do merge
rather than simple copy

This fix also use stateObject specific lock for storage update, rather
than the one in stateDB.

Co-authored-by: Sunny <[email protected]>
welkin22 pushed a commit that referenced this pull request Oct 22, 2024
The originStorage will miss some loading in txn execution,do merge
rather than simple copy

This fix also use stateObject specific lock for storage update, rather
than the one in stateDB.

Co-authored-by: Sunny <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants