Multiple fixes/refactorings for ChainLocks #2765
Merged
Conversation
This file contains hidden or 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
|
Looks good 👍 Will re-review after rebase. |
|
Merged #2764 , pls rebase |
Calling EnforceBestChainLock might result in switching chains, which in turn might end up calling signals, so we get into a recursive call chain. Better to call EnforceBestChainLock from the scheduler.
As the name of this method implies, it's trying to sign something and not enforce/invalidate chains. Invalidating blocks is the job of EnforceBestChainLock.
This ensures that NotifyChainLock is not prematurely called before the block is fully connected.
It might happen that 2 threads enter ActivateBestChain at the same time start processing block by block, while randomly switching between threads so that sometimes one thread processed the block and then another one processes it. A mutex protects ActivateBestChain now against this race.
codablock
force-pushed
the
pr_llmq_chainlocks_enforce
branch
from
5aae990 to
9ba1b4b
Compare
Author
|
rebased and force-pushed |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
See individual commits.
The PR currently includes #2764 as well as it otherwise runs into deadlocks due to the fix in 5aae9907043da8811d5a0308b0d5fdc723413ff6. I'll remove these commits with a rebase after merge.The target of this PR is to remove all enforcement logic from signal handlers and move them into the scheduler instead. Otherwise we end up with recursive calls of signals and I don't even want to know what kind of issues might arise from this.
The PR also adds an additional step to enforcement, which is the resetting of failure flags on the CL signed chain. This lets the node retry connecting the correct chain until it eventually succeeds. This is especially important when nodes fork off due to missing superblock triggers.