Skip to content

Commit 5fdaa45

Browse files
Merge dashpay#5966: refactor: rearrange database initialization, trim globals use in net and net processing, move CheckSpecialTx into CSpecialTxProcessor
191b3de refactor: move CheckSpecialTx into CSpecialTxProcessor (Kittywhiskers Van Gogh) 91f4588 refactor: const the pointer of type `const CActiveMasternodeManager` (Kittywhiskers Van Gogh) 1d9b7fa refactor: trim globals use in net processing functions (Kittywhiskers Van Gogh) 2a4fdbf refactor: trim globals use in net threads and functions (Kittywhiskers Van Gogh) ff825ac init: load databases of governance dependencies before loading its own (Kittywhiskers Van Gogh) cf940e8 init: decouple CMasternodeMetaMan construction from initialization (Kittywhiskers Van Gogh) fae5696 init: move CActiveMasternodeManager construction before PeerManager (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependency for dashpay#5980 * `CActiveMasternodeManager` construction is moved upwards (_before_ `PeerManager` construction) to avoid having to pass it as a `std::unique_ptr` const-ref. Shouldn't have any effect since _initialization_ is done in `ThreadImport`. * `CConnman::SetNetworkActive` will require a pointer to `CMasternodeSync` in order to call `CMasternodeSync::Reset()`. As it is no longer available as a global, it will need to be manually called through for the effect to happen as the `CConnman` constructor will simply pass a `nullptr` when calling `SetNetworkActive`, bypassing the `Reset()` call. `CMasternodeSync` is appropriately passed through in RPC (`setnetworkactive`) and interfaces. * `CheckSpecialTx` was moved into `CSpecialTxProcessor` to avoid having to expose `CDeterministicMNManager` to `MemPoolAccept` (though it has been exposed to `CChainstateHelper` through a helper, `CChainState::ChainHelper()` that was introduced in this PR). * As a drawback, some RPC functions that otherwise only needed access to `CDeterministicMNManager`, will also be accessing `CChainstateHelper`. ## Concerns * ~~Some tests seem to sporadically fail (fail as part of a suite but succeed when run individually, no changes in this PR should worsen resource contention) but attempting to reproduce them reliably hasn't succeeded so far.~~ It appears the sporadic failure is shown in `p2p_node_network_limited.py` during TSan runs (see [failed run](https://gitlab.com/dashpay/dash/-/jobs/6529052189) for commit f95ffa7 that then succeeded in a [second attempt](https://gitlab.com/dashpay/dash/-/jobs/6530402585) versus a similar [failed run](https://gitlab.com/dashpay/dash/-/jobs/6546952668) for e210cb7 that also succeeded on [second try](https://gitlab.com/dashpay/dash/-/jobs/6549470339)). Similar behaviour has not been observed on `develop` (as of this writing is 27c0813). ## Breaking changes `CMasternodeSync::Reset()` will not be called on every `CConnman` entity instantiated. Behaviour changes as a result of that are not substantiated. Protocol, networking or interface changes are not expected, changes are primarily refactoring. ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [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)_ ACKs for top commit: PastaPastaPasta: utACK 191b3de Tree-SHA512: 73a79818b0a79e3f26f079019c078ec8f81b2dae17638f695243b87fa76e9bed906a33ad7dd4e600f699100c5e994628cbb596e78a7802fa630c91f4d69cce4c
2 parents 0a62b9f + 191b3de commit 5fdaa45

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+272
-200
lines changed

src/coinjoin/client.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ PeerMsgRet CCoinJoinClientQueueManager::ProcessMessage(const CNode& peer, std::s
4242

4343
PeerMsgRet CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataStream& vRecv)
4444
{
45+
assert(::mmetaman->IsValid());
46+
4547
CCoinJoinQueue dsq;
4648
vRecv >> dsq;
4749

@@ -1117,6 +1119,8 @@ bool CCoinJoinClientSession::JoinExistingQueue(CAmount nBalanceNeedsAnonymized,
11171119

11181120
bool CCoinJoinClientSession::StartNewQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman)
11191121
{
1122+
assert(::mmetaman->IsValid());
1123+
11201124
if (!CCoinJoinClientOptions::IsEnabled()) return false;
11211125
if (nBalanceNeedsAnonymized <= 0) return false;
11221126

src/coinjoin/context.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
#include <coinjoin/server.h>
1515

1616
CJContext::CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CTxMemPool& mempool,
17-
const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes) :
17+
const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes) :
1818
dstxman{std::make_unique<CDSTXManager>()},
1919
#ifdef ENABLE_WALLET
2020
walletman{std::make_unique<CoinJoinWalletManager>(connman, dmnman, mempool, mn_sync, queueman)},

src/coinjoin/context.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ struct CJContext {
3030
CJContext() = delete;
3131
CJContext(const CJContext&) = delete;
3232
CJContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CTxMemPool& mempool,
33-
const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes);
33+
const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync, bool relay_txes);
3434
~CJContext();
3535

3636
const std::unique_ptr<CDSTXManager> dstxman;

src/coinjoin/server.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ PeerMsgRet CCoinJoinServer::ProcessMessage(CNode& peer, std::string_view msg_typ
4444
void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
4545
{
4646
assert(m_mn_activeman);
47+
assert(::mmetaman->IsValid());
4748

4849
if (IsSessionReady()) {
4950
// too many users in this session already, reject new ones
@@ -110,6 +111,8 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
110111

111112
PeerMsgRet CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv)
112113
{
114+
assert(::mmetaman->IsValid());
115+
113116
CCoinJoinQueue dsq;
114117
vRecv >> dsq;
115118

src/coinjoin/server.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
3030
CDeterministicMNManager& m_dmnman;
3131
CDSTXManager& m_dstxman;
3232
CTxMemPool& mempool;
33-
const CActiveMasternodeManager* m_mn_activeman;
33+
const CActiveMasternodeManager* const m_mn_activeman;
3434
const CMasternodeSync& m_mn_sync;
3535

3636
// Mixing uses collateral transactions to trust parties entering the pool
@@ -87,7 +87,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
8787

8888
public:
8989
explicit CCoinJoinServer(CChainState& chainstate, CConnman& _connman, CDeterministicMNManager& dmnman, CDSTXManager& dstxman,
90-
CTxMemPool& mempool, const CActiveMasternodeManager* mn_activeman, const CMasternodeSync& mn_sync) :
90+
CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync) :
9191
m_chainstate(chainstate),
9292
connman(_connman),
9393
m_dmnman(dmnman),

src/dsnotificationinterface.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,9 @@ void CDSNotificationInterface::BlockDisconnected(const std::shared_ptr<const CBl
134134
void CDSNotificationInterface::NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff)
135135
{
136136
CMNAuth::NotifyMasternodeListChanged(undo, oldMNList, diff, m_connman);
137-
m_govman.UpdateCachesAndClean();
137+
if (m_govman.IsValid()) {
138+
m_govman.CheckAndRemove();
139+
}
138140
}
139141

140142
void CDSNotificationInterface::NotifyChainLock(const CBlockIndex* pindex, const std::shared_ptr<const llmq::CChainLockSig>& clsig)

src/evo/mnauth.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ void CMNAuth::PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip)
6363

6464
PeerMsgRet CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, const CDeterministicMNList& tip_mn_list, std::string_view msg_type, CDataStream& vRecv)
6565
{
66+
assert(::mmetaman->IsValid());
67+
6668
if (msg_type != NetMsgType::MNAUTH || !::masternodeSync->IsBlockchainSynced()) {
6769
// we can't verify MNAUTH messages when we don't have the latest MN list
6870
return {};

src/evo/mnauth.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ class CMNAuth
4949
}
5050

5151
static void PushMNAUTH(CNode& peer, CConnman& connman, const CBlockIndex* tip);
52+
53+
/**
54+
* @pre CMasternodeMetaMan's database must be successfully loaded before
55+
* attempting to call this function regardless of sync state
56+
*/
5257
static PeerMsgRet ProcessMessage(CNode& peer, CConnman& connman, const CDeterministicMNList& tip_mn_list, std::string_view msg_type, CDataStream& vRecv);
5358
static void NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff, CConnman& connman);
5459
};

src/evo/specialtxman.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ static bool CheckSpecialTxInner(CDeterministicMNManager& dmnman, const CTransact
7272
return state.Invalid(TxValidationResult::TX_BAD_SPECIAL, "bad-tx-type-check");
7373
}
7474

75-
bool CheckSpecialTx(CDeterministicMNManager& dmnman, const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, bool check_sigs, TxValidationState& state)
75+
bool CSpecialTxProcessor::CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, bool check_sigs, TxValidationState& state)
7676
{
7777
AssertLockHeld(cs_main);
78-
return CheckSpecialTxInner(dmnman, tx, pindexPrev, view, std::nullopt, check_sigs, state);
78+
return CheckSpecialTxInner(m_dmnman, tx, pindexPrev, view, std::nullopt, check_sigs, state);
7979
}
8080

8181
[[nodiscard]] bool CSpecialTxProcessor::ProcessSpecialTx(const CTransaction& tx, const CBlockIndex* pindex, TxValidationState& state)

src/evo/specialtxman.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,6 @@ class CChainLocksHandler;
2929

3030
extern RecursiveMutex cs_main;
3131

32-
bool CheckSpecialTx(CDeterministicMNManager& dmnman, const CTransaction& tx, const CBlockIndex* pindexPrev,
33-
const CCoinsViewCache& view, bool check_sigs, TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
34-
3532
class CSpecialTxProcessor
3633
{
3734
private:
@@ -51,6 +48,8 @@ class CSpecialTxProcessor
5148
const Consensus::Params& consensus_params, const llmq::CChainLocksHandler& clhandler) :
5249
m_cpoolman(cpoolman), m_dmnman{dmnman}, m_mnhfman{mnhfman}, m_qblockman{qblockman}, m_consensus_params{consensus_params}, m_clhandler{clhandler} {}
5350

51+
bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, bool check_sigs, TxValidationState& state)
52+
EXCLUSIVE_LOCKS_REQUIRED(cs_main);
5453
bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, const CCoinsViewCache& view, bool fJustCheck,
5554
bool fCheckCbTxMerkleRoots, BlockValidationState& state, std::optional<MNListUpdates>& updatesRet)
5655
EXCLUSIVE_LOCKS_REQUIRED(cs_main);

0 commit comments

Comments
 (0)