deploymentstatus: move g_versionbitscache global to ChainstateManager #24595
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
|
Concept ACK! Also wanted to note that in one of my branches for |
3b93ef1 to
1724029
Compare
1724029 to
88b14a6
Compare
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
This was referenced Mar 19, 2022
88b14a6 to
9e0b983
Compare
d490648 to
0f40460
Compare
|
Rebased past #24538 EDIT: also tweaked validation.h to not directly include consensus/params.h as per #24595 (comment) |
…eCommitment into ChainstateManager
0f40460 to
bb5c24b
Compare
|
reACK bb5c24b |
maflcko
approved these changes
May 13, 2022
There was a problem hiding this comment.
review ACK bb5c24b 📙
Show signature
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
review ACK bb5c24b120a3ac7df367a1c5d9b075ca564efb5f 📙
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUi6+Qv8Dw67I3SBVnRjV/M10aNNkHAduz39CLI+zz2TwT/0IyAbgOEB9QAwzWS1
RFEQdpklN4Vxu+4qwkpEUWtl0cpJV8buVab33eUgDI1WuN/jo8nzF0GWpFwkHUJp
4VEYDP+9zWssKUGgtsV0LPadXFsk74f9KW0q1FN+UljnEBBksPtRfGok1TQSzesH
m0JlsdZcOBMFjNA+k7cQ3xyZi4b7tERRe8NZ1dewvHMBGpeh5J/n/TwHVO/4NO4Z
S7qAENfXCP8PWblW5LgvUSlxpyKbwma5ygHe7M1/RnmzMQO1BRkcq0x78ktyxWwU
Nq1hh4/ETvJgy3j3FIIm2AZeRRo+I1HhhJ1DuqKsqs3KNsM/+jToHrkel7Qi4WtT
nzyK66L2Tb/MKGsXq2lyU9UC2U4REW3FTi0btmEnRl35RCUyhWbNppl2eZDaheHk
NKnJV/wgZvgJUVra4Gt6pverD2dPL9/PA6V/M27tF1R9/buUMeEyQYVcBPBw5zSB
ySIzD/fI
=er/p
-----END PGP SIGNATURE-----
| @@ -1457,7 +1457,7 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx | |||
| assert(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;})); | |||
|
|
|||
| std::vector<COutPoint> coins_to_uncache; | |||
| const CChainParams& chainparams = Params(); | |||
| const CChainParams& chainparams = active_chainstate.m_params; | |||
There was a problem hiding this comment.
5c67e84 nit: .m_chainman.GetParams()
Seems odd to add a TODO and violate it in the same commit ( /* TODO: replace with m_chainman.GetParams() */)
| @@ -429,7 +429,7 @@ BOOST_AUTO_TEST_CASE(versionbits_computeblockversion) | |||
| const uint32_t dep_mask{g_versionbitscache.Mask(chainParams->GetConsensus(), dep)}; | |||
| BOOST_CHECK(!(chain_all_vbits & dep_mask)); | |||
| chain_all_vbits |= dep_mask; | |||
| check_computeblockversion(chainParams->GetConsensus(), dep); | |||
| check_computeblockversion(g_versionbitscache, chainParams->GetConsensus(), dep); | |||
There was a problem hiding this comment.
eca22c7 nit: Later diff could be smaller by a few lines if you added a VersionBitsCache& vbcache{g_versionbitscache}; in this commit already.
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
May 13, 2022
maflcko
reviewed
May 16, 2022
| } | ||
|
|
||
| BOOST_AUTO_TEST_CASE(versionbits_computeblockversion) | ||
| { | ||
| VersionBitsCache vbcache; // don't use chainman versionbitscache since we want custom chain params |
There was a problem hiding this comment.
Removed chainman completely from this test in #25125
aureleoules
added a commit
to aureleoules/bitcoin
that referenced
this pull request
Aug 12, 2022
Fixes a TODO introduced in bitcoin#24595.
aureleoules
added a commit
to aureleoules/bitcoin
that referenced
this pull request
Sep 16, 2022
Fixes a TODO introduced in bitcoin#24595.
aureleoules
added a commit
to aureleoules/bitcoin
that referenced
this pull request
Oct 10, 2022
Fixes a TODO introduced in bitcoin#24595.
maflcko
pushed a commit
that referenced
this pull request
Oct 19, 2022
5d3f98d refactor: Replace m_params with chainman.GetParams() (Aurèle Oulès) Pull request description: Fixes a TODO introduced in #24595. Removes `m_params` from `CChainState` class and replaces it with `m_chainman.GetParams()`. ACKs for top commit: MarcoFalke: review ACK 5d3f98d 🌎 Tree-SHA512: de0fe31450d281cc7307c0d820495e86c93c7998e77a148db2c703da66cff1059e6560c041f1864913c42075aa24d259c2623d45e929ca0a8056ed330a9f9978
adam2k
pushed a commit
to adam2k/bitcoin
that referenced
this pull request
Oct 19, 2022
Fixes a TODO introduced in bitcoin#24595.
satsie
pushed a commit
to satsie/bitcoin
that referenced
this pull request
Oct 31, 2022
Fixes a TODO introduced in bitcoin#24595.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Gives
ChainstateManagera reference to theCChainParamsits working on, and simplifies some of the functions that would otherwise take that as a parameter. Removes theg_versionbitscacheglobal by moving it intoChainstateManager.