When validation is waiting for parent chain daemon, "stall". #1022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gwillen
force-pushed
the
feature-debug-revalidation-v18
branch
2 times, most recently
from
8fb9283 to
841fd14
Compare
gwillen
force-pushed
the
feature-debug-revalidation-v18
branch
from
469948b to
9c5ea27
Compare
apoelstra
reviewed
Sep 1, 2021
apoelstra
reviewed
Sep 1, 2021
| UniValue error = reply["error"]; | ||
| if (!error.isNull()) { | ||
| // On the first call, it's possible to node is still in | ||
| // warmup; in that case, just wait and retry. |
There was a problem hiding this comment.
Should drop On the first call from this comment since you removed the init flag
apoelstra
approved these changes
Sep 1, 2021
|
rescinding ACK |
apoelstra
reviewed
Sep 2, 2021
|
I rescind my rescindation. My previous ACK is still good. |
|
I rescind my ACK for real this time. We can't combine the schedulers, cf #647 |
|
To expand on the need for two schedulers: this code puts |
Currently, if -validatepegin is given, and block validation can't proceed because the parent chain is not synced, we mark the block invalid and put it in a queue to be "revalidated" later. Unfortunately, marking a block invalid has downstream consequences, in particular causing descendant blocks to be marked invalid, which are not currently fixed by the queue. Instead, we'll use a different strategy: if the mainchain daemon isn't sufficiently synced to validate a block, we will "stall" connecting that block to the chain, and have ActivateBestChain simply keep the tip at the previous block until we're ready. We can still download and validate (partly) blocks past this point while we're waiting. They will be connected once the parent chain daemon catches up.
- Finish removing all references to 'recheckpeginblockinterval', including documentation and tests. - Remove periodic calls to MainchainRPCCheck; use it only at startup (and refactor accordingly to simplify logic.) - Move MainchainRPCCheck from validation.h/cpp (public) to an internal helper function of init.cpp. - Comment out definition of 'revalidation queue' type in txdb, to suppress "unused variable" warning. (Leave it visible to avoid future reuse.)
Add a regression test for ElementsProject#891 . This checks that we can sync successfully when making a bunch of new blocks just as we have transient loss of parent daemon connectivity. This reliably fails without the fix, and reliably succeeds with it. (It stands in for the situation, more common in production, where we sync faster than the parent daemon can keep up after a long outage.)
apoelstra
force-pushed
the
feature-debug-revalidation-v18
branch
from
9c5ea27 to
e5ba4e8
Compare
|
cc @gwillen I squashed a couple commits and un-removed the second scheduling thread |
|
@apoelstra Thanks, looks great. If that fixes your tests then I'm 100% happy with it. |
apoelstra
added a commit
that referenced
this pull request
Sep 14, 2021
…in daemon, "stall" b2e1cc3 Regression test for pegin validation issues during sync (Glenn Willen) d5042b4 Finish removing 'recheckpeginblockinterval'; move MainchainRPCCheck (Glenn Willen) 313f73d When validation is waiting for parent chain daemon, "stall". (Andrew Poelstra) Pull request description: Forward-port of #1022 ACKs for top commit: gwillen: utACK b2e1cc3, verified that it contains only the requested changes from da11d7b. Tree-SHA512: 971b6a137efdc54f84995b17595346bf3d3ebd5a04eef1676225c664923e0b52393550bc51ea366c4d3010aa35a9de16da0643bb5bfb40c7eb788a180c99b1a0
gwillen
added a commit
to gwillen/elements
that referenced
this pull request
Apr 21, 2022
When PR 1022 was merged, an & got dropped from the declarations of the two "fStall" out-parameters, rendering them unused (and triggering a compiler warning.) Restore them, restoring the functionality of ElementsProject#1022.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, if -validatepegin is given, and block validation can't proceed
because the parent chain is not synced, we mark the block invalid and put
it in a queue to be "revalidated" later. Unfortunately, marking a block
invalid has downstream consequences, in particular causing descendant blocks
to be marked invalid, which are not currently fixed by the queue.
Instead, we'll use a different strategy: if the mainchain daemon isn't
sufficiently synced to validate a block, we will "stall" connecting that
block to the chain, and have ActivateBestChain simply keep the tip at the
previous block until we're ready.
We can still download and validate (partly) blocks past this point while
we're waiting. They will be connected once the parent chain daemon catches
up.