Skip to content

Commit d52795c

Browse files
kwvgkwvg
committed
refactor: clean up CCoinJoinServer, remove redundant checks
Now that CCoinJoinServer is conditionally initialized, we can remove checks that assumed its unconditional existence.
1 parent afec9ae commit d52795c

File tree

5 files changed

+32
-59
lines changed

5 files changed

+32
-59
lines changed

‎src/coinjoin/server.cpp‎

Lines changed: 18 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525

2626
MessageProcessingResult CCoinJoinServer::ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv)
2727
{
28-
if (!m_mn_activeman) return {};
2928
if (!m_mn_sync.IsBlockchainSynced()) return {};
3029

3130
if (msg_type == NetMsgType::DSACCEPT) {
@@ -42,7 +41,6 @@ MessageProcessingResult CCoinJoinServer::ProcessMessage(CNode& peer, std::string
4241

4342
void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
4443
{
45-
assert(m_mn_activeman);
4644
assert(m_mn_metaman.IsValid());
4745

4846
if (IsSessionReady()) {
@@ -58,7 +56,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
5856
LogPrint(BCLog::COINJOIN, "DSACCEPT -- nDenom %d (%s) txCollateral %s", dsa.nDenom, CoinJoin::DenominationToString(dsa.nDenom), dsa.txCollateral.ToString()); /* Continued */
5957

6058
auto mnList = m_dmnman.GetListAtChainTip();
61-
auto dmn = mnList.GetValidMNByCollateral(m_mn_activeman->GetOutPoint());
59+
auto dmn = mnList.GetValidMNByCollateral(m_mn_activeman.GetOutPoint());
6260
if (!dmn) {
6361
PushStatus(peer, STATUS_REJECTED, ERR_MN_LIST);
6462
return;
@@ -69,7 +67,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
6967
TRY_LOCK(cs_vecqueue, lockRecv);
7068
if (!lockRecv) return;
7169

72-
auto mnOutpoint = m_mn_activeman->GetOutPoint();
70+
auto mnOutpoint = m_mn_activeman.GetOutPoint();
7371

7472
if (ranges::any_of(vecCoinJoinQueue,
7573
[&mnOutpoint](const auto& q){return q.masternodeOutpoint == mnOutpoint;})) {
@@ -183,7 +181,7 @@ MessageProcessingResult CCoinJoinServer::ProcessDSQUEUE(NodeId from, CDataStream
183181
TRY_LOCK(cs_vecqueue, lockRecv);
184182
if (!lockRecv) return ret;
185183
vecCoinJoinQueue.push_back(dsq);
186-
m_peerman->RelayDSQ(dsq);
184+
m_peerman.RelayDSQ(dsq);
187185
}
188186
return ret;
189187
}
@@ -254,8 +252,6 @@ void CCoinJoinServer::SetNull()
254252
//
255253
void CCoinJoinServer::CheckPool()
256254
{
257-
if (!m_mn_activeman) return;
258-
259255
if (int entries = GetEntriesCount(); entries != 0) LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- entries count %lu\n", entries);
260256

261257
// If we have an entry for each collateral, then create final tx
@@ -317,7 +313,6 @@ void CCoinJoinServer::CreateFinalTransaction()
317313
void CCoinJoinServer::CommitFinalTransaction()
318314
{
319315
AssertLockNotHeld(cs_coinjoin);
320-
if (!m_mn_activeman) return; // check and relay final tx only on masternode
321316

322317
CTransactionRef finalTransaction = WITH_LOCK(cs_coinjoin, return MakeTransactionRef(finalMutableTransaction));
323318
uint256 hashTx = finalTransaction->GetHash();
@@ -341,18 +336,16 @@ void CCoinJoinServer::CommitFinalTransaction()
341336

342337
// create and sign masternode dstx transaction
343338
if (!m_dstxman.GetDSTX(hashTx)) {
344-
CCoinJoinBroadcastTx dstxNew(finalTransaction,
345-
m_mn_activeman->GetOutPoint(),
346-
m_mn_activeman->GetProTxHash(),
347-
GetAdjustedTime());
348-
dstxNew.Sign(*m_mn_activeman);
339+
CCoinJoinBroadcastTx dstxNew(finalTransaction, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(),
340+
GetAdjustedTime());
341+
dstxNew.Sign(m_mn_activeman);
349342
m_dstxman.AddDSTX(dstxNew);
350343
}
351344

352345
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- TRANSMITTING DSTX\n");
353346

354347
CInv inv(MSG_DSTX, hashTx);
355-
Assert(m_peerman)->RelayInv(inv);
348+
m_peerman.RelayInv(inv);
356349

357350
// Tell the clients it was successful
358351
RelayCompletedTransaction(MSG_SUCCESS);
@@ -380,7 +373,6 @@ void CCoinJoinServer::CommitFinalTransaction()
380373
void CCoinJoinServer::ChargeFees() const
381374
{
382375
AssertLockNotHeld(cs_coinjoin);
383-
if (!m_mn_activeman) return;
384376

385377
//we don't need to charge collateral for every offence.
386378
if (GetRand<int>(/*nMax=*/100) > 33) return;
@@ -448,8 +440,6 @@ void CCoinJoinServer::ChargeFees() const
448440
*/
449441
void CCoinJoinServer::ChargeRandomFees() const
450442
{
451-
if (!m_mn_activeman) return;
452-
453443
for (const auto& txCollateral : vecSessionCollaterals) {
454444
if (GetRand<int>(/*nMax=*/100) > 10) return;
455445
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::ChargeRandomFees -- charging random fees, txCollateral=%s", txCollateral->ToString()); /* Continued */
@@ -463,15 +453,13 @@ void CCoinJoinServer::ConsumeCollateral(const CTransactionRef& txref) const
463453
if (!ATMPIfSaneFee(m_chainman, txref)) {
464454
LogPrint(BCLog::COINJOIN, "%s -- ATMPIfSaneFee failed\n", __func__);
465455
} else {
466-
Assert(m_peerman)->RelayTransaction(txref->GetHash());
456+
m_peerman.RelayTransaction(txref->GetHash());
467457
LogPrint(BCLog::COINJOIN, "%s -- Collateral was consumed\n", __func__);
468458
}
469459
}
470460

471461
bool CCoinJoinServer::HasTimedOut() const
472462
{
473-
if (!m_mn_activeman) return false;
474-
475463
if (nState == POOL_STATE_IDLE) return false;
476464

477465
int nTimeout = (nState == POOL_STATE_SIGNING) ? COINJOIN_SIGNING_TIMEOUT : COINJOIN_QUEUE_TIMEOUT;
@@ -484,8 +472,6 @@ bool CCoinJoinServer::HasTimedOut() const
484472
//
485473
void CCoinJoinServer::CheckTimeout()
486474
{
487-
if (!m_mn_activeman) return;
488-
489475
CheckQueue();
490476

491477
// Too early to do anything
@@ -504,19 +490,15 @@ void CCoinJoinServer::CheckTimeout()
504490
*/
505491
void CCoinJoinServer::CheckForCompleteQueue()
506492
{
507-
if (!m_mn_activeman) return;
508-
509493
if (nState == POOL_STATE_QUEUE && IsSessionReady()) {
510494
SetState(POOL_STATE_ACCEPTING_ENTRIES);
511495

512-
CCoinJoinQueue dsq(nSessionDenom,
513-
m_mn_activeman->GetOutPoint(),
514-
m_mn_activeman->GetProTxHash(),
515-
GetAdjustedTime(), true);
496+
CCoinJoinQueue dsq(nSessionDenom, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(),
497+
GetAdjustedTime(), true);
516498
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckForCompleteQueue -- queue is ready, signing and relaying (%s) " /* Continued */
517499
"with %d participants\n", dsq.ToString(), vecSessionCollaterals.size());
518-
dsq.Sign(*m_mn_activeman);
519-
m_peerman->RelayDSQ(dsq);
500+
dsq.Sign(m_mn_activeman);
501+
m_peerman.RelayDSQ(dsq);
520502
WITH_LOCK(cs_vecqueue, vecCoinJoinQueue.push_back(dsq));
521503
}
522504
}
@@ -572,7 +554,6 @@ bool CCoinJoinServer::IsInputScriptSigValid(const CTxIn& txin) const
572554
bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessageIDRet)
573555
{
574556
AssertLockNotHeld(cs_coinjoin);
575-
if (!m_mn_activeman) return false;
576557

577558
if (size_t(GetEntriesCount()) >= vecSessionCollaterals.size()) {
578559
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: entries is full!\n", __func__);
@@ -682,8 +663,6 @@ bool CCoinJoinServer::IsSignaturesComplete() const
682663

683664
bool CCoinJoinServer::IsAcceptableDSA(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet) const
684665
{
685-
if (!m_mn_activeman) return false;
686-
687666
// is denom even something legit?
688667
if (!CoinJoin::IsValidDenomination(dsa.nDenom)) {
689668
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- denom not valid!\n", __func__);
@@ -703,7 +682,7 @@ bool CCoinJoinServer::IsAcceptableDSA(const CCoinJoinAccept& dsa, PoolMessage& n
703682

704683
bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet)
705684
{
706-
if (!m_mn_activeman || nSessionID != 0) return false;
685+
if (nSessionID != 0) return false;
707686

708687
// new session can only be started in idle mode
709688
if (nState != POOL_STATE_IDLE) {
@@ -725,13 +704,11 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
725704

726705
if (!fUnitTest) {
727706
//broadcast that I'm accepting entries, only if it's the first entry through
728-
CCoinJoinQueue dsq(nSessionDenom,
729-
m_mn_activeman->GetOutPoint(),
730-
m_mn_activeman->GetProTxHash(),
731-
GetAdjustedTime(), false);
707+
CCoinJoinQueue dsq(nSessionDenom, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(),
708+
GetAdjustedTime(), false);
732709
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString());
733-
dsq.Sign(*m_mn_activeman);
734-
m_peerman->RelayDSQ(dsq);
710+
dsq.Sign(m_mn_activeman);
711+
m_peerman.RelayDSQ(dsq);
735712
LOCK(cs_vecqueue);
736713
vecCoinJoinQueue.push_back(dsq);
737714
}
@@ -745,7 +722,7 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
745722

746723
bool CCoinJoinServer::AddUserToExistingSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet)
747724
{
748-
if (!m_mn_activeman || nSessionID == 0 || IsSessionReady()) return false;
725+
if (nSessionID == 0 || IsSessionReady()) return false;
749726

750727
if (!IsAcceptableDSA(dsa, nMessageIDRet)) {
751728
return false;
@@ -881,8 +858,6 @@ void CCoinJoinServer::RelayCompletedTransaction(PoolMessage nMessageID)
881858

882859
void CCoinJoinServer::SetState(PoolState nStateNew)
883860
{
884-
if (!m_mn_activeman) return;
885-
886861
if (nStateNew == POOL_STATE_ERROR) {
887862
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::SetState -- Can't set state to ERROR as a Masternode. \n");
888863
return;
@@ -895,7 +870,6 @@ void CCoinJoinServer::SetState(PoolState nStateNew)
895870

896871
void CCoinJoinServer::DoMaintenance()
897872
{
898-
if (!m_mn_activeman) return; // only run on masternodes
899873
if (!m_mn_sync.IsBlockchainSynced()) return;
900874
if (ShutdownRequested()) return;
901875

‎src/coinjoin/server.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
3535
CDSTXManager& m_dstxman;
3636
CMasternodeMetaMan& m_mn_metaman;
3737
CTxMemPool& mempool;
38-
const CActiveMasternodeManager* const m_mn_activeman;
38+
PeerManager& m_peerman;
39+
const CActiveMasternodeManager& m_mn_activeman;
3940
const CMasternodeSync& m_mn_sync;
4041
const llmq::CInstantSendManager& m_isman;
41-
std::unique_ptr<PeerManager>& m_peerman;
4242

4343
// Mixing uses collateral transactions to trust parties entering the pool
4444
// to behave honestly. If they don't it takes their money.
@@ -95,18 +95,18 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
9595
public:
9696
explicit CCoinJoinServer(ChainstateManager& chainman, CConnman& _connman, CDeterministicMNManager& dmnman,
9797
CDSTXManager& dstxman, CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool,
98-
const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
99-
const llmq::CInstantSendManager& isman, std::unique_ptr<PeerManager>& peerman) :
98+
PeerManager& peerman, const CActiveMasternodeManager& mn_activeman,
99+
const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman) :
100100
m_chainman(chainman),
101101
connman(_connman),
102102
m_dmnman(dmnman),
103103
m_dstxman(dstxman),
104104
m_mn_metaman(mn_metaman),
105105
mempool(mempool),
106+
m_peerman(peerman),
106107
m_mn_activeman(mn_activeman),
107108
m_mn_sync(mn_sync),
108109
m_isman{isman},
109-
m_peerman(peerman),
110110
vecSessionCollaterals(),
111111
fUnitTest(false)
112112
{}

‎src/init.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2165,8 +2165,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
21652165
assert(!node.active_ctx);
21662166
if (node.mn_activeman) {
21672167
node.active_ctx = std::make_unique<ActiveContext>(chainman, *node.connman, *node.dmnman, *node.cj_ctx->dstxman, *node.mn_metaman,
2168-
*node.llmq_ctx, *node.sporkman, *node.mempool, *node.mn_activeman, *node.mn_sync,
2169-
node.peerman);
2168+
*node.llmq_ctx, *node.sporkman, *node.mempool, *node.peerman, *node.mn_activeman,
2169+
*node.mn_sync);
21702170
}
21712171

21722172
// ********************************************************* Step 7e: Setup other Dash services

‎src/masternode/active/context.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,16 @@
1414

1515
ActiveContext::ActiveContext(ChainstateManager& chainman, CConnman& connman, CDeterministicMNManager& dmnman,
1616
CDSTXManager& dstxman, CMasternodeMetaMan& mn_metaman, LLMQContext& llmq_ctx,
17-
CSporkManager& sporkman, CTxMemPool& mempool, const CActiveMasternodeManager& mn_activeman,
18-
const CMasternodeSync& mn_sync, std::unique_ptr<PeerManager>& peerman) :
17+
CSporkManager& sporkman, CTxMemPool& mempool, PeerManager& peerman,
18+
const CActiveMasternodeManager& mn_activeman, const CMasternodeSync& mn_sync) :
1919
m_llmq_ctx{llmq_ctx},
2020
cl_signer{std::make_unique<chainlock::ChainLockSigner>(chainman.ActiveChainstate(), *llmq_ctx.clhandler,
2121
*llmq_ctx.sigman, *llmq_ctx.shareman, sporkman, mn_sync)},
2222
is_signer{std::make_unique<instantsend::InstantSendSigner>(chainman.ActiveChainstate(), *llmq_ctx.clhandler,
2323
*llmq_ctx.isman, *llmq_ctx.sigman, *llmq_ctx.shareman,
2424
*llmq_ctx.qman, sporkman, mempool, mn_sync)},
25-
cj_server{std::make_unique<CCoinJoinServer>(chainman, connman, dmnman, dstxman, mn_metaman, mempool, &mn_activeman,
26-
mn_sync, *llmq_ctx.isman, peerman)}
25+
cj_server{std::make_unique<CCoinJoinServer>(chainman, connman, dmnman, dstxman, mn_metaman, mempool, peerman,
26+
mn_activeman, mn_sync, *llmq_ctx.isman)}
2727
{
2828
m_llmq_ctx.clhandler->ConnectSigner(cl_signer.get());
2929
m_llmq_ctx.isman->ConnectSigner(is_signer.get());

‎src/masternode/active/context.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,9 @@ struct ActiveContext {
4141
public:
4242
ActiveContext() = delete;
4343
ActiveContext(const ActiveContext&) = delete;
44-
ActiveContext(ChainstateManager& chainman, CConnman& connman, CDeterministicMNManager& dmnman,
45-
CDSTXManager& dstxman, CMasternodeMetaMan& mn_metaman, LLMQContext& llmq_ctx, CSporkManager& sporkman,
46-
CTxMemPool& mempool, const CActiveMasternodeManager& mn_activeman, const CMasternodeSync& mn_sync,
47-
std::unique_ptr<PeerManager>& peerman);
44+
ActiveContext(ChainstateManager& chainman, CConnman& connman, CDeterministicMNManager& dmnman, CDSTXManager& dstxman,
45+
CMasternodeMetaMan& mn_metaman, LLMQContext& llmq_ctx, CSporkManager& sporkman, CTxMemPool& mempool,
46+
PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, const CMasternodeSync& mn_sync);
4847
~ActiveContext();
4948

5049
/*

0 commit comments

Comments
 (0)