Bitcoin 0.17 locking PRs #5034
Open
Bitcoin 0.17 locking PRs #5034
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
LarryRuane
added
C-upstream-port
|
☔ The latest upstream changes (presumably #5009) made this pull request unmergeable. Please resolve the merge conflicts. |
LarryRuane
force-pushed
the
upstream-locking-0.17
branch
from
33243cd to
b03729d
Compare
|
Rebased on master, now that this PR is next in line. This is ready for review and possibly merging. |
str4d
requested changes
May 26, 2021
There was a problem hiding this comment.
One required change, and one mis-cherry-pick I'd like fixed (to make bitcoin/bitcoin@ecc3c4a a bit easier to review).
LarryRuane
force-pushed
the
upstream-locking-0.17
branch
from
b03729d to
b23bc94
Compare
|
Thanks, @str4d, I think I've resolved all your comments. Regarding #5034 (comment), I went ahead and added bitcoin/bitcoin@fb8fad1 to the set. |
zcash: cherry picked from commit bitcoin/bitcoin@c651df8 zcash: bitcoin/bitcoin#11041
When accessing mapBlockIndex cs_main must be held. zcash: cherry picked from commit bitcoin/bitcoin@f814a3e zcash: bitcoin/bitcoin#11041
zcash: cherry picked from commit bitcoin/bitcoin@43a32b7 zcash: bitcoin/bitcoin#11041
FlushStateToDisk(...) won't do anything besides check if we need to prune if FLUSH_STATE_NONE is given. We avoid reading the variable fCheckForPruning which is guarded by the mutex cs_LastBlockFile. zcash: cherry picked from commit bitcoin/bitcoin@2311c7c zcash: bitcoin/bitcoin#11617
zcash: cherry picked from commit bitcoin/bitcoin@0000d8f zcash: bitcoin/bitcoin#11617
Ensures that callbacks are invoked in the order in which the chain is updated Resolves #12978 (cherry picked from commit bitcoin/bitcoin@bitcoin/bitcoin@d86edd3)
zcash: cherry picked from commit bitcoin/bitcoin@d76962e zcash: bitcoin/bitcoin#12639
* reading variable 'mapTx' requires holding mutex 'cs' * reading variable 'mapNextTx' requires holding mutex 'cs' * reading variable 'nCheckFrequency' requires holding mutex 'cs' zcash: cherry picked from commit bitcoin/bitcoin@6bc5b71 zcash: bitcoin/bitcoin#11689
* writing variable 'nCheckFrequency' requires holding mutex 'cs' zcash: cherry picked from commit bitcoin/bitcoin@0e2dfa8 zcash: bitcoin/bitcoin#11689
zcash: cherry picked from commit bitcoin/bitcoin@47782b4 zcash: bitcoin/bitcoin#11689
Technically, some internal datastructures may be in an inconsistent state if we do this, though there are no known bugs there. Still, for future safety, its much better to only unlock cs_main if we've made progress (not just tried a reorg which may make progress). zcash: cherry picked from commit bitcoin/bitcoin@ecc3c4a zcash: bitcoin/bitcoin#13023 Zcash: Only includes part of the comment change to UpdatedBlockTip(), as we don't have BlockConnected() yet.
If multiple threads are invoking ActivateBestChain, it was possible to have them working towards different tips, and we could arrive at a less work tip than we should. Fix this by introducing a ChainState lock which must be held for the entire duration of ActivateBestChain to enforce exclusion in ABC. zcash: cherry picked from commit bitcoin/bitcoin@a3ae8e6 zcash: bitcoin/bitcoin#13023
zcash: cherry picked from commit bitcoin/bitcoin@4bcd5bb zcash: bitcoin/bitcoin#13114
zcash: cherry picked from commit bitcoin/bitcoin@01a06d6 zcash: bitcoin/bitcoin#11762
zcash: cherry picked from commit bitcoin/bitcoin@1e3bcd2 zcash: bitcoin/bitcoin#13423
str4d
force-pushed
the
upstream-locking-0.17
branch
from
1d6bebf to
b7b0e8e
Compare
|
Rebased on |
|
The assertion failure is being caused by the defensive assertion added to |
|
5cf6ebd is the problematic commit. This likely means that it is backported incorrectly (which is reasonable, it's a complex backport and our code isn't in the same state as the usptream code was). |
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.
These are locking changes from upstream (bitcoin core) release 0.17 (Feb 16, 2018 to Aug 13, 2018), oldest to newest (when merged to the master branch).
This PR is complete and ready for review.
Each commit also includes a reference both to the PR and the upstream commit.
PRs and when each was merged to master: