Move BlockManager to node/blockstorage #23581

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2111-blockstorage changing 4 files +455 −453

MarcoFalke commented at 7:55 PM on November 23, 2021: member

BlockManager is responsible for reading and writing block(headers). So move it to the existing blockstorage module in node. Also, move validation code unrelated to block-storage out from BlockManager.
MarcoFalke added the label Refactoring on Nov 23, 2021
MarcoFalke force-pushed on Nov 23, 2021
MarcoFalke force-pushed on Nov 24, 2021
DrahtBot commented at 2:17 PM on November 24, 2021: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #23810 (refactor: destroy all C-style casts; use modern C++ casts, enforce via -Wold-style-cast by PastaPastaPasta)
- #22932 (Guard CBlockIndex::nStatus by cs_main, require GetBlockPos/GetUndoPos to hold cs_main by jonatack)
- #21726 (Improve Indices on pruned nodes via prune blockers by fjahr)
- #20827 (During IBD, prune as much as possible until we get close to where we will eventually keep blocks by luke-jr)
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.

in src/validation.cpp:721 in fa646a5ae9 outdated

 717 | @@ -718,7 +718,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
 718 |      if (!CheckSequenceLocks(m_active_chainstate.m_chain.Tip(), m_view, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp))
 719 |          return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-BIP68-final");
 720 |  
 721 | -    if (!Consensus::CheckTxInputs(tx, state, m_view, m_active_chainstate.m_blockman.GetSpendHeight(m_view), ws.m_base_fees)) {
 722 | +    assert(m_active_chainstate.m_blockman.LookupBlockIndex(m_view.GetBestBlock()) == m_active_chainstate.m_chain.Tip());

promag commented at 9:36 AM on November 25, 2021:

fa646a5ae9b39b7987d5701aec55a54ae4ec80ed

nit, move after L706.

MarcoFalke commented at 11:35 AM on November 25, 2021:

thx, done

in src/validation.h:431 in faabaa8654 outdated

 427 | @@ -427,6 +428,7 @@ class BlockManager
 428 |       *                                     which we've downloaded all transactions.
 429 |       */
 430 |      bool LoadBlockIndex(
 431 | +        CBlockIndex*& best_invalid,

promag commented at 9:46 AM on November 25, 2021:

faabaa8654c9446846e79ac5ae2edf70e0ae6d98

Update doc above? Make it the 2nd arg?

MarcoFalke commented at 11:36 AM on November 25, 2021:

thx, done

promag commented at 9:48 AM on November 25, 2021: member

Concept ACK.
MarcoFalke force-pushed on Nov 25, 2021
DrahtBot added the label Needs rebase on Dec 3, 2021
MarcoFalke force-pushed on Dec 3, 2021
DrahtBot removed the label Needs rebase on Dec 3, 2021
DrahtBot added the label Needs rebase on Dec 15, 2021
MarcoFalke force-pushed on Jan 2, 2022
Move BlockManager to node/blockstorage
Can be reviewed with --color-moved=dimmed-zebra
fade2a44f4

Fixup style of moved code

Can be reviewed with --word-diff-regex=. -U0 --ignore-all-space

fa7efc915b

MarcoFalke force-pushed on Jan 2, 2022
MarcoFalke commented at 5:00 PM on January 2, 2022: member

Rebased
DrahtBot removed the label Needs rebase on Jan 2, 2022
ryanofsky approved
ryanofsky commented at 8:29 PM on January 3, 2022: member

Code review obvious ACK fa7efc915b87ec56ca1cc0bad7d8f79591bfa099
fanquake merged this on Jan 4, 2022
fanquake closed this on Jan 4, 2022
MarcoFalke deleted the branch on Jan 4, 2022
sidhujag referenced this in commit 81d78ac374 on Jan 4, 2022
Fabcien referenced this in commit 65330d5b88 on Nov 16, 2022
DrahtBot locked this on Jan 4, 2023

Contributors

Labels

Move BlockManager to node/blockstorage #23581

Conflicts