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

check_known now takes total_difficulty into consideration #3298

Merged
merged 2 commits into from May 29, 2020

Conversation

antiochp
Copy link
Member

During block processing we check_known and halt processing if the block is "already known".
This avoids wasting resources reprocessing blocks from the network.

We treat a block as known if it matches the current chain head (reprocessing a recent block) or if the full block already exists in the local db (reprocessing a historical block).

With the work on "checkpoints" in #3266 there is another case to consider - one of "future blocks". If the chain head is reset or reverted to an earlier block (for any reason) the current block processing refuses to (re)process blocks that are "already known".

We can get into tricky situations where the chain head will refuse to progress because we incorrectly believe a block is "known" yet not actually on the current chain.

This PR makes check_known take total_difficulty into account when comparing blocks against current chain head. If the block would increase total difficulty then we want to treat it as "unknown" regardless of local db.

This makes is significantly easier to reset the chain head and still have block processing pipeline work correctly for reprocessing subsequent blocks.

Note: This only applies in situations where the chain head is explicitly reset. Under normal block processing the chain head only ever progresses in one direction, with total difficulty strictly increasing.

Added test coverage to demonstrate "forgetting" about an "already known" block, based on total difficulty relative to chain head.

chain/src/pipe.rs Outdated Show resolved Hide resolved
};

// attempt to reprocess latest block
{
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need a lexical scope here and below?

Copy link
Member Author

@antiochp antiochp Apr 30, 2020

Choose a reason for hiding this comment

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

We cannot call init_chain() repeatedly on windows from the same dir without dropping previous chain.
I believe it fails to release some file handles or something (but only on windows).

return Ok(());
// If we have already processed the full block for this header then done.
// Note: "already known" in this context is success so subsequent processing can continue.
{
Copy link
Contributor

Choose a reason for hiding this comment

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

for my education - why do we need a lexical scope here?

Copy link
Member Author

Choose a reason for hiding this comment

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

See above.

@antiochp antiochp requested a review from hashmap May 4, 2020 15:06
@antiochp antiochp added this to the 4.0.0 milestone May 4, 2020
@antiochp antiochp merged commit 5b825fb into mimblewimble:master May 29, 2020
@antiochp antiochp deleted the future_known branch May 29, 2020 09:21
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.

None yet

2 participants