There are a few potential improvements mentioned by various reviewers of the chainman-deglobalizing work that are deserving of a tracking ticket, but are perhaps either not crucial to the main thrust of the work or the flaws already existed before the work. Let's discuss and collate them here so that we don't lose track of them!
m_chain methods should perhaps require holding cs_main
Ensure*() in src/rpc should be called at the top of functions to fail as early as possible
- IncrementExtraNonce does not need
cs_main, and the locking scope can be limited
CoinsDB.Cursor() passes ownership to caller and should return a std::unique_ptr
- Call to
GetNetworkHashPS in getnetworkhashps contains many operators, which may increase risk of precedence bugs
- Avoid repeated calls to
ActiveChain() where appropriate
- Review whether or not
m_blockman member of ChainstateManager needs cs_main to be accessed
- Review whether or not to support running bitcoin without a chainman
- Investigate how indexing interacts with
CChainState
Please feel free to comment here for extra ones I've missed!
There are a few potential improvements mentioned by various reviewers of the chainman-deglobalizing work that are deserving of a tracking ticket, but are perhaps either not crucial to the main thrust of the work or the flaws already existed before the work. Let's discuss and collate them here so that we don't lose track of them!
m_chainmethods should perhaps require holdingcs_mainEnsure*()in src/rpc should be called at the top of functions to fail as early as possiblecs_main, and the locking scope can be limitedCoinsDB.Cursor()passes ownership to caller and should return astd::unique_ptrGetNetworkHashPSingetnetworkhashpscontains many operators, which may increase risk of precedence bugsActiveChain()where appropriatem_blockmanmember ofChainstateManagerneedscs_mainto be accessedCChainStatePlease feel free to comment here for extra ones I've missed!