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

[R4R] fix: remove diffhash patch introduced from separate node #1020

Merged
merged 1 commit into from Jul 27, 2022

Conversation

j75689
Copy link
Contributor

@j75689 j75689 commented Jul 27, 2022

Description

BSC has merged ethereum/go-ethereum#23628 to avoid hitting outdated snapshots, so we can remove #926 code that was introduced at the time to fix diffHash mismatch.

Rationale

image

In previous versions, the snapshot tree executed parent.flatten() without first locking the current difflayer when cap, but first executed the parent.flatten() to change it to be stale state and relinks the current difflayer.parent (removes the stale's difflayer from the recursive structure).

image

And this has caused a problem. There is a small chance that the stale difflayer will be obtained first when the difflayer is accessed externally, and then the difflayer.parent will be locked and re-linked, resulting in the snapshot being judged when acquiring the Account or Storage data. Returns null for Stale.

image

Therefore, in the implementation of #926, a new retry mechanism is added to retry when a null value is obtained, but this approach is not particularly good and may cause over-stack problems.

After the Ethereum version(ethereum/go-ethereum#23628) was merged, it was changed to lock the current difflayer first and then change the parent to avoid the above problems, so we can remove the patch fix introduced at that time.

Example

N/A

Changes

Notable changes:

  • remove the patch from separate-node

@j75689 j75689 force-pushed the fix/stale_difflayer_issue branch from 4151071 to 373c1de Compare July 27, 2022 02:28
@j75689 j75689 changed the title [R4R] fix: remove diffhash patch introduced from separate node [WIP] fix: remove diffhash patch introduced from separate node Jul 27, 2022
Copy link
Collaborator

@brilliant-lx brilliant-lx left a comment

Choose a reason for hiding this comment

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

LGTM.
It has been protected by diffLayer.lock right now.

@unclezoro unclezoro changed the title [WIP] fix: remove diffhash patch introduced from separate node [R4R] fix: remove diffhash patch introduced from separate node Jul 27, 2022
@unclezoro unclezoro merged commit 06bc2a0 into bnb-chain:develop Jul 27, 2022
This was referenced Jul 28, 2022
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.

None yet

4 participants