refactor: drop usage of chainstate globals in Dash-specific code, merge bitcoin#21866 (goodbye to a global chainstate)

[kwvg]

Thank you, I'll say goodbye soon
Though its the end of these globals, don't blame yourself now
And if its true, I will surround you and give life to a chainstate
That's our own

Additional Information

• In CDeterministicMN::ToJson(), collateralAddress is extracted by finding the scriptPubKey of a transaction output for a masternode, originally this used GetUTXOCoin but doesn't work for spent tranasction outputs (as they're not UTXOs), so in dash#5607, a fallback was introduced that looks through the general transaction set if going through the UTXO set yielded nothing.

  GetUTXOCoin accesses the active chainstate to get ahold of the UTXO set, this was done through globals. The removal of chainstate globals meant that whoever was calling GetUTXOCoin should have access to the chainstate handy. This is trivial in RPC code where ToJson() is used (source) through Ensure(Any)Chainman. Not the case in Qt code (source), which is supposed to be given restricted access to information by the interface.

  As the fallback seems to be capable of fetching UTXOs and spent outputs, we can remove the GetUTXOCoin method and make the fallback the only method.

• In develop, as of this writing, CChainState members FlushStateToDisk and {Enforce, Invalidate, MarkConflicting}Block were accessing their internals through the global, despite having direct access to them. As the globals they were calling are going to be bid farewell, they needed to be changed to access its members instead.

  The reason for going the roundabout way is unknown.

• CDSNotificationInterface takes in a ChainstateManager (instead of the CChainState it actually requires) as at the time of interface initialization (source), the active chainstate hasn't been loaded in yet as that happens further down (source).

  As CDSNotificationInterface::InitializeCurrentBlockTip() is called well after it is initialized, we can resolve to the active chainstate in there.

• As GetCreditPoolDiffForBlock requires access to ChainstateManager as GetCreditPoolDiffForBlock > ProcessLockUnlockTransaction > CheckAssetLockUnlockTx > CheckAssetUnlockTx > ChainstateManager::m_blockman.LookupBlockIndex() and BlockAssembler only has CChainState, it had to be reworked around ChainstateManager.

  CChainState is passed as a direct argument while ChainstateManager can be fetched from NodeContext. Unlike CTxMemPool, which can be passed custom instances (source, source), CChainState's argument value is taken from NodeContext::chainstate.ActiveChainstate() and since we're now accepting ChainstateManager wholesale, we can dispense with accepting CChainState as an argument.

  Changes to that effect have been made.

  AssumeUTXO introduces the need to be able to use different CChainStates, so this underlying assumption no longer holds true, the above described changes have been reverted. Asset locks code has been refactored to use BlockManager directly (which does come with the downside of needing to hold cs_main for longer than strictly necessary, this is why only asset locks uses BlockManager directly while other cases still benefit from having ChainstateManager as a whole).

• CMNHFManager::ConnectManagers will be taking in a ChainstateManager pointer due to the GetSignalsStage > GetForBlock > ProcessBlock > extractSignals > CheckMNHFTx > ChainstateManager::m_blockman.LookupBlockIndex() chain.

• The use of a bespoke NodeContext in coinselector_tests breaks tests if any interface call relies on a chainstate as testNode doesn't initialize one. For the most part, this was masked by WalletTestingSetup populating the chainstate globals from its own NodeContext even if the tests themselves preferred to use their own stripped down testNode.

  Though, removing the chainstate globals meant that they can no longer rely on WalletTestingSetup's NodeContext to mask the barebones testNode global being used in the test (specifically, addCoins > listMNCollaterials > ChainActive() worked because ChainActive() accessed WalletTestingSetup's NodeContext but when ChainActive() was gone and replaced with NodeContext::chainman.ActiveChain(), it uses testNode's ChainstateManager, which doesn't exist, which causes it to crash).

  To remedy this, a5595b13 and 5e54aa9b from bitcoin#23288 were adapted for the limited purpose of eliminating testNode and using WalletTestingSetup's NodeContext instead. This comes with the unfortunate effect of skipping a lot of the refactoring, cleanups and optimizations done before and adapting the ones after them non-trivial.

  It is therefore best recommended that the commit be reverted and changes implemented step-by-step in a pull request at some point in the future. For now, it's kept around here for the sake of this pull request, which, if merged, should prevent more chainstate globals use from leaking into the codebase.

  Pre-fix crash stacktrace:

  ``` dash@71aecd6afb45:/src/dash$ lldb-16 ./src/test/test_dash (lldb) target create "./src/test/test_dash" Current executable set to '/src/dash/src/test/test_dash' (x86_64). (lldb) r -t coinselector_tests Process 395006 launched: '/src/dash/src/test/test_dash' (x86_64) Running 4 test cases... node/interfaces.cpp:711 chainman: Assertion `m_node.chainman' failed. Process 395006 stopped * thread #1, name = 'd-test', stop reason = signal SIGABRT frame #0: 0x00007ffff7a7300b libc.so.6`__GI_raise(sig=) at raise.c:51:1 (lldb) bt * thread #1, name = 'd-test', stop reason = signal SIGABRT * frame #0: 0x00007ffff7a7300b libc.so.6`__GI_raise(sig=) at raise.c:51:1 frame #1: 0x00007ffff7a52859 libc.so.6`__GI_abort at abort.c:79:7 frame #2: 0x00005555563cba33 test_dash`assertion_fail(file="node/interfaces.cpp", line=711, func="chainman", assertion="m_node.chainman") at check.cpp:13:5 frame #3: 0x0000555555fb47aa test_dash`node::(anonymous namespace)::ChainImpl::listMNCollaterials(std::vector const&, unsigned int>, std::allocator const&, unsigned int>>> const&) [inlined] std::unique_ptr>& inline_assertion_check>&>(val=nullptr, file=, line=711, func=, assertion=) at check.h:62:13 frame #4: 0x0000555555fb4781 test_dash`node::(anonymous namespace)::ChainImpl::listMNCollaterials(std::vector const&, unsigned int>, std::allocator const&, unsigned int>>> const&) [inlined] node::(anonymous namespace)::ChainImpl::chainman(this=0x000055555723e830)at interfaces.cpp:711:45 frame #5: 0x0000555555fb477d test_dash`node::(anonymous namespace)::ChainImpl::listMNCollaterials(std::vector const&, unsigned int>, std::allocator const&, unsigned int>>> const&) [inlined] node::(anonymous namespace)::ChainImpl::listMNCollaterials(this=)::'lambda'()::operator()() const at interfaces.cpp:788:34 frame #6: 0x0000555555fb474f test_dash`node::(anonymous namespace)::ChainImpl::listMNCollaterials(this=0x000055555723e830, outputs=size=0) at interfaces.cpp:788:34 frame #7: 0x00005555565bcd07 test_dash`CWallet::AddToWallet(this=0x00005555571701e0, tx=, confirm=, update_wtx=, fFlushOnClose=) at wallet.cpp:886:46 frame #8: 0x0000555555bed3ef test_dash`coinselector_tests::add_coin(wallet=0x00005555571701e0, nValue=0x00007fffffffc7c0, nAge=144, fIsFromMe=false, nInput=0, spendable=) at coinselector_tests.cpp:77:29 frame #9: 0x0000555555bead3e test_dash`coinselector_tests::bnb_search_test::test_method() [inlined] coinselector_tests::add_coin(nValue=0x00007fffffffc7c0, nAge=144, fIsFromMe=false, nInput=0, spendable=false) at coinselector_tests.cpp:88:5 frame #10: 0x0000555555bead20 test_dash`coinselector_tests::bnb_search_test::test_method(this=0x00007fffffffcad0) at coinselector_tests.cpp:278:5 frame #11: 0x0000555555be6607 test_dash`coinselector_tests::bnb_search_test_invoker() at coinselector_tests.cpp:138:1 ```

Breaking Changes

• Backporting coinselector_tests changes are now much more annoying.

• The following RPCs, protx list, protx listdiff, protx info will no longer report collateralAddress if the transaction index has been disabled (txindex=0).

Checklist:

• [x] I have performed a self-review of my own code
• [x] I have commented my code, particularly in hard-to-understand areas (note: N/A)
• [x] I have added or updated relevant unit/integration/functional/e2e tests
• [x] I have made corresponding changes to the documentation (note: N/A)
• [x] I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[github-actions[bot]]

This pull request has conflicts, please rebase.

[kwvg]

The latest diff of changes got buried due to a force push to fix a typo, the diff is available here.