kernel: pass params to BlockManager rather than using a global #25264

pull theuni wants to merge 1 commits into bitcoin:master from theuni:no-kernel-global-params changing 3 files +4 −4

theuni commented at 7:45 PM on June 1, 2022: member

In a discussion today, dongcarl and I realized that is the only usage of the global Params() left in the kernel code.

We can use the readily available reference in ChainstateManager instead.

Note: There are still some uses of BaseParams in the kernel, so it doesn't make sense to rearrange the definitions quite yet. Once those are gone we can split the globals into new files.
fanquake commented at 7:53 PM on June 1, 2022: member

Looks like this is somewhat of a followup to #25168.
dongcarl commented at 7:57 PM on June 1, 2022: member

Code Review ACK fee666b3521d8a1561fd5fce10e2ac12d01f84ff
DrahtBot added the label Block storage on Jun 1, 2022
DrahtBot added the label Validation on Jun 1, 2022
theuni commented at 9:48 PM on June 1, 2022: member

Looks like this is somewhat of a followup to #25168.

Aha, yep, thanks for doing the hard part @MarcoFalke.

in src/validation.cpp:4161 in fee666b352 outdated

4157 | @@ -4158,7 +4158,7 @@ bool ChainstateManager::LoadBlockIndex()
4158 |      // Load block index from databases
4159 |      bool needs_init = fReindex;
4160 |      if (!fReindex) {
4161 | -        bool ret = m_blockman.LoadBlockIndexDB();
4162 | +        bool ret = m_blockman.LoadBlockIndexDB(m_chainparams.GetConsensus());

ajtowns commented at 5:23 AM on June 2, 2022:

Could just be m_blockman.LoadBlockIndexDB(GetConsensus()); here.

ajtowns commented at 5:25 AM on June 2, 2022: member

ACK fee666b3521d8a1561fd5fce10e2ac12d01f84ff - code review only

in src/node/blockstorage.h:156 in fee666b352 outdated

 152 | @@ -153,7 +153,7 @@ class BlockManager
 153 |      std::unique_ptr<CBlockTreeDB> m_block_tree_db GUARDED_BY(::cs_main);
 154 |  
 155 |      bool WriteBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 156 | -    bool LoadBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 157 | +    bool LoadBlockIndexDB(const Consensus::Params& consensusParams) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);

MarcoFalke commented at 6:39 AM on June 2, 2022:

    bool LoadBlockIndexDB(const Consensus::Params& consensus_params) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);

style nit (if you address the other nit)

MarcoFalke approved
MarcoFalke commented at 6:39 AM on June 2, 2022: member

lgtm
laanwj commented at 2:03 PM on June 2, 2022: member

Concept ACK, agree with the current suggestions.
kernel: pass params to BlockManager rather than using a global a4741bd8d4
theuni force-pushed on Jun 2, 2022
theuni commented at 3:21 PM on June 2, 2022: member

Pushed a4741bd8d4b90b451d982d98eabd0dc5572baa29 with suggestions from @MarcoFalke and @ajtowns.
MarcoFalke commented at 3:22 PM on June 2, 2022: member

cr ACK a4741bd8d4b90b451d982d98eabd0dc5572baa29
MarcoFalke removed the label Validation on Jun 2, 2022
MarcoFalke removed the label Block storage on Jun 2, 2022
MarcoFalke added the label Refactoring on Jun 2, 2022
laanwj commented at 5:04 PM on June 2, 2022: member

Code review ACK a4741bd8d4b90b451d982d98eabd0dc5572baa29
laanwj merged this on Jun 2, 2022
laanwj closed this on Jun 2, 2022
sidhujag referenced this in commit 6b9ad3fcb1 on Jun 3, 2022
DrahtBot locked this on Jun 2, 2023

Contributors

Labels