refactor: Avoid passing params where not needed #25168

pull MarcoFalke wants to merge 5 commits into bitcoin:master from MarcoFalke:2205-remove-bloat-🏮 changing 21 files +66 −92

MarcoFalke commented at 4:51 PM on May 18, 2022: member
It seems confusing to pass chain params, consensus params, or a time function around when it is not needed.

Fix this by:
- Inlining the passed time getter function. I don't see a use case why this should be mockable.
- Using chainman.GetConsensus() or chainman.GetParams(), where possible.
Do not pass time getter to Chainstate helpers fa4ee53dca
Do not pass Consensus::Params& to Chainstate helpers faf012b438
Do not pass CChainParams& to BlockAssembler constructor fafe5c0ca2
Do not pass CChainParams& to PeerManager::make fa30234be8
Do not call global Params() when chainman is in scope fa1b76aeb0
DrahtBot added the label Refactoring on May 18, 2022
vincenzopalazzo approved
vincenzopalazzo commented at 6:41 PM on May 18, 2022: none

ACK https://github.com/bitcoin/bitcoin/pull/25168/commits/fa1b76aeb064b315a3767a8f59836ca18aeb117e
fanquake commented at 8:58 AM on May 19, 2022: member

Concept ACK
DrahtBot commented at 3:16 PM on May 19, 2022: 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:
- #25073 (test: Cleanup miner_tests by MarcoFalke)
- #25064 ([kernel 2b/n] Add ChainstateManager::m_adjusted_time_callback by dongcarl)
- #24474 (Additional thread safety annotations for CNode/Peer by ajtowns)
- #24232 (assumeutxo: add init and completion logic by jamesob)
- #23443 (p2p: Erlay support signaling by naumenkogs)
- #21527 (net_processing: lock clean up by ajtowns)
- #21006 (rpc: reduce LOCK(cs_min) scope in rest_block: ~5 times as many requests per second by martinus)
- #18933 (rpc: Add submit option to generateblock by MarcoFalke)
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.
promag commented at 11:01 AM on May 20, 2022: member

Code review ACK fa1b76aeb064b315a3767a8f59836ca18aeb117e.
MarcoFalke merged this on May 20, 2022
MarcoFalke closed this on May 20, 2022
MarcoFalke deleted the branch on May 20, 2022
dongcarl commented at 10:38 PM on May 25, 2022: member

post-merge ACK. This synergizes well with some of the upcoming libbitcoinkernel changes. We might actually be able to remove all references to a global CChainParams :astonished: (no promises tho)
sidhujag referenced this in commit 3bb5318bd1 on May 28, 2022
DrahtBot locked this on May 25, 2023

Contributors

Labels

refactor: Avoid passing params where not needed #25168

Conflicts