By moving gArgs references out of chainparamsbase.cpp.
Note both chainparams.cpp, bitcoin-cli.cpp, and all callers of SetupChainParamsBaseOptions include util.h
By moving gArgs references out of chainparamsbase.cpp.
Note both chainparams.cpp, bitcoin-cli.cpp, and all callers of SetupChainParamsBaseOptions include util.h
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.
+1 for going after another dependency cycle :)
Extracting MakeUnique
seems like a good thing to do.
Does it make sense to refactor the dependency of util
on chainparamsbase
?
https://github.com/bitcoin/bitcoin/blob/9b638c7ce1b93c996b49f02c0bcefb25b6200d8e/src/util.cpp#L8
This relationship seems the crux to the circular dependency to me. Imoutil
should have as little dependencies as possible.
util
, but seems removing chainparamsbase
would be a bigger job, since ArgsManager
uses CBaseChainParams
, and ArgsManager
is the bulk of util
.
I don’t think moving the arguments for chain selection to util is the right solution here.
Why does util depend on knowing something about chain parameters anyway? It’s supposed to be a bunch of utility functions.
These two were being set together in all cases. Using just one removes the
internal reliance of chainparamsbase on gArgs in this case.
Instead expose GetRPCPort and GetDataDir methods, which can be called with
the m_network name stored by gArgs.
Ok I looked deeper into this and moved to a more comprehensive approach. The story is, there exist an m_network
attribute in ArgsManager
, and a globalChainBaseParams
variable that drives BaseParams()
. These are functionally redundant based on the fact that they are always set together. Given that the base params are only two: rpc port and data dir name, we can simply pass the m_network
value to a method that returns the port or dir name relative to that network.
This is functionally equivalent and simpler in implementation than the prior alternative. While chainparams
has a justification for doing its setup once (the voluminous setup), the mapping for chainparamsbase
is trivial.
What remains of CBaseChainParams is just a namespace for the network name
strings.
These options are intimately tied to ArgsManager::GetChainName, so their
presence here makes sense, and moving them removes the final cause for a
circular dependency between util and chainparamsbase.
Empact
DrahtBot
l2a5b1
sipa
MarcoFalke
Labels
Refactoring