Only load BlockMan in BlockMan member functions #24515
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
|
(Ditto what @MarcoFalke said) |
|
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 11, 2022
|
The main thing this PR seems to be doing is moving |
af44e85 to
9cb1165
Compare
...since the height information in already in CBlockIndex* and we can use an easy custom sorter.
This commit effectively splits the "load block index itself" logic from
"derive Chainstate variables from loaded block index" logic.
This means that BlockManager::LoadBlockIndex{,DB} will only load what's
relevant to the BlockManager.
I strongly recommend reviewing with the following git-diff flags:
--color-moved=dimmed_zebra --color-moved-ws=allow-indentation-change
...it's declared in blockstorage.h
...also use std::sort for clarity
9cb1165 to
f865cf8
Compare
maflcko
approved these changes
Mar 16, 2022
There was a problem hiding this comment.
review ACK f865cf8 🗂
Show signature
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
review ACK f865cf8ded2b2fbc82a6fbc41226d991909a6880 🗂
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUg0DAv/VZj7G6Rp0qriCmTSDHrk21I938vrPa39hBPwQ9GU3DpaKPoIth5mj2Vl
jMREh59ZzqKKorCPa0RdcVaRyiVPTRup0EOSi0b9qwIQREr1pumfJb1Mir9HJtCY
WbLA/sj1YGuXpIUvWAEUkKE56vpLVahWnSx5jd9QFO0DBWbfQ3iHWj/Soc5EBxEV
5pquK8ceeWyZxN7NKHWwKZegfjMmuPvmrr1HVfFB9qwjzYIsJk7Hv83PXbTHNzMa
OBBxlGy6o/anN1w+iS6tflGCzmTgxXXWKROlm2pCDAgOJZi4FyM2vj/V/9XM4E4F
RghIzqLltFmVJwvRV7x/o0Ahf3QHsez5TIl8JvFxVjBAwPP6WW2cbUQnXJRGrpho
oIUVnBf/r5iZCImOGzCkslm5oBDmWHCGi2jscPcdIQAcQNrusIN2jy4OrWxVAYFw
fjORBHiWF1TnQElKrfBurviwt+LO8SZFcjtTm7ZN4BGFFknCGgUBD9hqP9GB6mzt
Tu78aGDd
=BBgB
-----END PGP SIGNATURE-----
|
ACK f865cf8 ; code review only |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Mar 18, 2022
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 23, 2023
Summary: This is a partial backport of [[bitcoin/bitcoin#24515 | core#24515]] bitcoin/bitcoin@5be9ee3 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D13020
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 23, 2023
Summary: ...since the height information in already in CBlockIndex* and we can use an easy custom sorter. This is a partial backport of [[bitcoin/bitcoin#24515 | core#24515]] bitcoin/bitcoin@42e56d9 Also includes a comment removal from bitcoin/bitcoin@3bbb6fe The rest of this commit is already done in Bitcoin ABC (review of D12993 and D12994) Depends on D13020 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D13021
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 23, 2023
Summary:
This commit effectively splits the "load block index itself" logic from
"derive Chainstate variables from loaded block index" logic.
This means that BlockManager::LoadBlockIndex{,DB} will only load what's
relevant to the BlockManager.
I strongly recommend reviewing with the following git-diff flags:
--color-moved=dimmed_zebra --color-moved-ws=allow-indentation-change
This is a partial backport of [[bitcoin/bitcoin#24515 | core#24515]]
bitcoin/bitcoin@c600ee3
Depends on D13021
The sorting code is deduplicated in D13023
Test Plan: `ninja all check-all`
Reviewers: #bitcoin_abc, Fabien
Reviewed By: #bitcoin_abc, Fabien
Subscribers: Fabien
Differential Revision: https://reviews.bitcoinabc.org/D13022
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 23, 2023
Summary: ...also use std::sort for clarity Add also a simple unit test for coverage. This is a partial backport of [[bitcoin/bitcoin#24515 | core#24515]] bitcoin/bitcoin@28ba031 Depends on D13022 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D13023
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 23, 2023
Summary: This concludes backport of [[bitcoin/bitcoin#24515 | core#24515]] bitcoin/bitcoin@f865cf8 Depends on D13023 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D13024
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.
The only important commit is "Only load BlockMan in BlockMan member functions", everything else is all just small style changes.
Here's the commit message, reproduced: