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

chain: Revalidate blocks for new deployments. #2878

Merged
merged 6 commits into from
Mar 2, 2022

Conversation

rstaudt2
Copy link
Member

This adds functionality to automatically unmark blocks that were previously marked as failed when new consensus rules are detected. It is split into several commits to ease the review process. An overview of the approach to do this is as follows:

  • Track the current consensus deployment version in the database
    • The current deployment version is effectively the consensus ruleset version since new deployments are required to introduce new consensus rules
  • During initialization, if a deployment version is detected that is newer than the deployment version previously saved in the database, unmark blocks that were previously marked failed if their median time is after the start time of the newly detected deployments
    • The deployment start time is used for this purpose rather than the actual activation height to simplify the logic and context that would otherwise be required during initialization

Closes #2766.

Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

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

One thing I've noticed is that, compared to what I interpret the original issue to mean, this isn't tracking the per-block rejection ruleset, but rather the globally active ruleset.

In any case, this seems like a reasonable implementation.

blockchain/chainio.go Show resolved Hide resolved
@rstaudt2
Copy link
Member Author

One thing I've noticed is that, compared to what I interpret the original issue to mean, this isn't tracking the per-block rejection ruleset, but rather the globally active ruleset.

That is also what I was considering doing at first. However, when working on the implementation, it became apparent that the activation height of newer versions matters when determining which version to say a block was invalidated under.

For example, if we are running on software that only knows about an older version, and some blocks are invalidated both before and after a newer version is activated (that the older software doesn't know about), then all of the blocks would still be marked as invalidated under the older version. Then when the new software clears the invalidation flags, it would still need to know the activation height of the newer version if we don't want to blindly revalidate everything marked as invalid under the prior version.

For that reason, and also since it simplifies the solution quite a bit, I didn't see much added value in storing it per block.

@rstaudt2 rstaudt2 force-pushed the track-deployment-version branch from 020e7e3 to e0b251c Compare January 31, 2022 22:58
Copy link
Member

@dnldd dnldd left a comment

Choose a reason for hiding this comment

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

Nicely done, looks good overall.

@davecgh davecgh added this to the 1.8.0 milestone Feb 23, 2022
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Nice work on this overall. It lines up with what we discussed about making use of the defined deployments and such.

I identified a couple of nits inline, but the logic is sound and I've tested that it works as intended via simnet (along with some minor code modifications needed to define deployments).

blockchain/chainio_test.go Outdated Show resolved Hide resolved
blockchain/chainio.go Show resolved Hide resolved
blockchain/chainio.go Outdated Show resolved Hide resolved
@rstaudt2 rstaudt2 force-pushed the track-deployment-version branch from e0b251c to aefa164 Compare March 2, 2022 15:00
rstaudt2 added 6 commits March 2, 2022 09:04
This adds a currentDeploymentVersion helper function, which returns the
highest deployment version that is defined in the given network
parameters.
This adds a nextDeploymentVersion helper function, which returns the
lowest deployment version defined in the given network parameters that
is higher than the provided version.
This adds dbPutDeploymentVer and dbFetchDeploymentVer functions to put
and fetch the deployment version from the database.
This adds tracking of the current deployment version to the database.
During initialization, the deployment version is updated to the current
version as necessary.
This adds a newDeploymentsStartTime helper function, which returns the
earliest start time of newly detected deployment versions.
This adds functionality to unmark blocks that were previously marked
failed if their median time is after the earliest possible start time of
newly detected consensus rules.  This ensures clients that did not
update prior to new rules activating are able to automatically recover
under the new rules without having to download the entire chain again.
@rstaudt2 rstaudt2 force-pushed the track-deployment-version branch from aefa164 to c2a5990 Compare March 2, 2022 15:05
@davecgh davecgh merged commit 4a80402 into decred:master Mar 2, 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.

Track information regarding ruleset used to invalidate a block.
4 participants