Skip to content

Commit 52f171d

Browse files
knstknst
committed
refactor: final removing of PeerManager from signing_shares
1 parent 6519b61 commit 52f171d

7 files changed

Lines changed: 47 additions & 57 deletions

File tree

‎src/active/context.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman
2929
CTxMemPool& mempool, llmq::CChainLocksHandler& clhandler, llmq::CInstantSendManager& isman,
3030
llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,
3131
llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman,
32-
PeerManager& peerman, const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
32+
const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
3333
const llmq::QvvecSyncModeMap& sync_map, const util::DbWrapperParams& db_params,
3434
bool quorums_recovery, bool quorums_watch) :
3535
m_clhandler{clhandler},
@@ -38,7 +38,7 @@ ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman
3838
nodeman{std::make_unique<CActiveMasternodeManager>(connman, dmnman, operator_sk)},
3939
dkgdbgman{std::make_unique<llmq::CDKGDebugManager>()},
4040
qdkgsman{std::make_unique<llmq::CDKGSessionManager>(dmnman, qsnapman, chainman, sporkman, db_params, quorums_watch)},
41-
shareman{std::make_unique<llmq::CSigSharesManager>(connman, chainman, sigman, peerman, *nodeman, qman, sporkman)},
41+
shareman{std::make_unique<llmq::CSigSharesManager>(connman, chainman, sigman, *nodeman, qman, sporkman)},
4242
gov_signer{std::make_unique<GovernanceSigner>(connman, dmnman, govman, *nodeman, chainman, mn_sync)},
4343
ehf_sighandler{std::make_unique<llmq::CEHFSignalsHandler>(chainman, mnhfman, sigman, *shareman, qman)},
4444
qman_handler{std::make_unique<llmq::QuorumParticipant>(bls_worker, connman, dmnman, qman, qsnapman, *nodeman, chainman,

‎src/active/context.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ struct ActiveContext final : public CValidationInterface {
6666
CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool,
6767
llmq::CChainLocksHandler& clhandler, llmq::CInstantSendManager& isman,
6868
llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,
69-
llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman, PeerManager& peerman,
69+
llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman,
7070
const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
7171
const llmq::QvvecSyncModeMap& sync_map, const util::DbWrapperParams& db_params,
7272
bool quorums_recovery, bool quorums_watch);

‎src/init.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2194,7 +2194,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
21942194
node.active_ctx = std::make_unique<ActiveContext>(*node.llmq_ctx->bls_worker, chainman, *node.connman, *node.dmnman, *node.govman, *node.mn_metaman,
21952195
*node.mnhf_manager, *node.sporkman, *node.mempool, *node.llmq_ctx->clhandler, *node.llmq_ctx->isman,
21962196
*node.llmq_ctx->quorum_block_processor, *node.llmq_ctx->qman, *node.llmq_ctx->qsnapman, *node.llmq_ctx->sigman,
2197-
*node.peerman, *node.mn_sync, operator_sk, sync_map, dash_db_params, quorums_recovery, quorums_watch);
2197+
*node.mn_sync, operator_sk, sync_map, dash_db_params, quorums_recovery, quorums_watch);
21982198
RegisterValidationInterface(node.active_ctx.get());
21992199
} else if (quorums_watch) {
22002200
node.observer_ctx = std::make_unique<llmq::ObserverContext>(*node.llmq_ctx->bls_worker, *node.connman, *node.dmnman, *node.mn_metaman, *node.mn_sync,

‎src/llmq/net_signing.cpp‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -156,20 +156,21 @@ void NetSigning::Stop()
156156
}
157157
}
158158

159-
void NetSigning::ProcessRecoveredSig(std::shared_ptr<const CRecoveredSig> recoveredSig, bool consider_proactive_relay)
159+
void NetSigning::ProcessRecoveredSig(std::shared_ptr<const CRecoveredSig> recovered_sig, bool consider_proactive_relay)
160160
{
161-
if (!m_sig_manager.ProcessRecoveredSig(recoveredSig)) return;
161+
if (recovered_sig == nullptr) return;
162+
if (!m_sig_manager.ProcessRecoveredSig(recovered_sig)) return;
162163

163164
auto listeners = m_sig_manager.GetListeners();
164165
for (auto& l : listeners) {
165-
m_peer_manager->PeerPostProcessMessage(l->HandleNewRecoveredSig(*recoveredSig));
166+
m_peer_manager->PeerPostProcessMessage(l->HandleNewRecoveredSig(*recovered_sig));
166167
}
167168

168169
// TODO refactor to use a better abstraction analogous to IsAllMembersConnectedEnabled
169-
auto proactive_relay = consider_proactive_relay && recoveredSig->getLlmqType() != Consensus::LLMQType::LLMQ_100_67 &&
170-
recoveredSig->getLlmqType() != Consensus::LLMQType::LLMQ_400_60 &&
171-
recoveredSig->getLlmqType() != Consensus::LLMQType::LLMQ_400_85;
172-
GetMainSignals().NotifyRecoveredSig(recoveredSig, recoveredSig->GetHash().ToString(), proactive_relay);
170+
auto proactive_relay = consider_proactive_relay && recovered_sig->getLlmqType() != Consensus::LLMQType::LLMQ_100_67 &&
171+
recovered_sig->getLlmqType() != Consensus::LLMQType::LLMQ_400_60 &&
172+
recovered_sig->getLlmqType() != Consensus::LLMQType::LLMQ_400_85;
173+
GetMainSignals().NotifyRecoveredSig(recovered_sig, recovered_sig->GetHash().ToString(), proactive_relay);
173174
}
174175

175176
bool NetSigning::ProcessPendingRecoveredSigs()
@@ -301,7 +302,8 @@ void NetSigning::WorkThreadDispatcher()
301302
if (workInterrupt) break;
302303

303304
worker_pool.push([this, work = std::move(work)](int) mutable {
304-
m_shares_manager->SignAndProcessSingleShare(std::move(work));
305+
auto rs = m_shares_manager->SignAndProcessSingleShare(std::move(work));
306+
ProcessRecoveredSig(rs, true);
305307
});
306308
}
307309
}
@@ -395,7 +397,10 @@ bool NetSigning::ProcessPendingSigShares()
395397
continue;
396398
}
397399

398-
m_shares_manager->ProcessPendingSigShares(v, quorums);
400+
auto rec_sigs = m_shares_manager->ProcessPendingSigShares(v, quorums);
401+
for (auto& rs : rec_sigs) {
402+
ProcessRecoveredSig(rs, true);
403+
}
399404
}
400405

401406
return more_work;

‎src/llmq/signing_shares.cpp‎

Lines changed: 19 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
#include <llmq/signhash.h>
1515
#include <llmq/signing.h>
1616
#include <msg_result.h>
17-
#include <net_processing.h>
1817
#include <netmessagemaker.h>
1918
#include <spork.h>
2019
#include <util/irange.h>
@@ -181,12 +180,11 @@ void CSigSharesNodeState::RemoveSession(const uint256& signHash)
181180
//////////////////////
182181

183182
CSigSharesManager::CSigSharesManager(CConnman& connman, const ChainstateManager& chainman, CSigningManager& _sigman,
184-
PeerManager& peerman, const CActiveMasternodeManager& mn_activeman,
185-
const CQuorumManager& _qman, const CSporkManager& sporkman) :
183+
const CActiveMasternodeManager& mn_activeman, const CQuorumManager& _qman,
184+
const CSporkManager& sporkman) :
186185
m_connman{connman},
187186
m_chainman{chainman},
188187
sigman{_sigman},
189-
m_peerman{peerman},
190188
m_mn_activeman{mn_activeman},
191189
qman{_qman},
192190
m_sporkman{sporkman}
@@ -521,26 +519,30 @@ bool CSigSharesManager::CollectPendingSigSharesToVerify(
521519
}
522520

523521
// It's ensured that no duplicates are passed to this method
524-
void CSigSharesManager::ProcessPendingSigShares(
522+
std::vector<std::shared_ptr<CRecoveredSig>> CSigSharesManager::ProcessPendingSigShares(
525523
const std::vector<CSigShare>& sigSharesToProcess,
526524
const std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& quorums)
527525
{
528526
cxxtimer::Timer t(true);
527+
std::vector<std::shared_ptr<CRecoveredSig>> recovered_sigs;
529528
for (const auto& sigShare : sigSharesToProcess) {
530529
auto quorumKey = std::make_pair(sigShare.getLlmqType(), sigShare.getQuorumHash());
531-
ProcessSigShare(sigShare, quorums.at(quorumKey));
530+
auto rs = ProcessSigShare(sigShare, quorums.at(quorumKey));
531+
if (rs != nullptr) {
532+
recovered_sigs.emplace_back(std::move(rs));
533+
}
532534
}
533535
t.stop();
534536

535537
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- processed sigShare batch. shares=%d, time=%ds\n", __func__,
536538
sigSharesToProcess.size(), t.count());
539+
return recovered_sigs;
537540
}
538541

539542
// sig shares are already verified when entering this method
540-
void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum)
543+
std::shared_ptr<CRecoveredSig> CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum)
541544
{
542545
auto llmqType = quorum->params.type;
543-
bool canTryRecovery = false;
544546

545547
const bool isAllMembersConnectedEnabled = IsAllMembersConnectedEnabled(llmqType, m_sporkman);
546548

@@ -552,14 +554,15 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorum
552554
}
553555

554556
if (sigman.HasRecoveredSigForId(llmqType, sigShare.getId())) {
555-
return;
557+
return nullptr;
556558
}
557559

560+
bool canTryRecovery = false;
558561
{
559562
LOCK(cs);
560563

561564
if (!sigShares.Add(sigShare.GetKey(), sigShare)) {
562-
return;
565+
return nullptr;
563566
}
564567
if (!isAllMembersConnectedEnabled) {
565568
sigSharesQueuedToAnnounce.Add(sigShare.GetKey(), true);
@@ -583,24 +586,9 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorum
583586
canTryRecovery = true;
584587
}
585588
}
589+
if (!canTryRecovery) return nullptr;
586590

587-
if (canTryRecovery) {
588-
auto rs = TryRecoverSig(*quorum, sigShare.getId(), sigShare.getMsgHash());
589-
if (rs != nullptr) {
590-
if (sigman.ProcessRecoveredSig(rs)) {
591-
// TODO: remove duplicated code with NetSigning
592-
auto listeners = sigman.GetListeners();
593-
for (auto& l : listeners) {
594-
m_peerman.PostProcessMessage(l->HandleNewRecoveredSig(*rs));
595-
}
596-
597-
bool proactive_relay = rs->getLlmqType() != Consensus::LLMQType::LLMQ_100_67 &&
598-
rs->getLlmqType() != Consensus::LLMQType::LLMQ_400_60 &&
599-
rs->getLlmqType() != Consensus::LLMQType::LLMQ_400_85;
600-
GetMainSignals().NotifyRecoveredSig(rs, rs->GetHash().ToString(), proactive_relay);
601-
}
602-
}
603-
}
591+
return TryRecoverSig(*quorum, sigShare.getId(), sigShare.getMsgHash());
604592
}
605593

606594
std::shared_ptr<CRecoveredSig> CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id,
@@ -1427,13 +1415,13 @@ bool CSigSharesManager::IsAnyPendingProcessing() const
14271415
[](const auto& entry) { return !entry.second.pendingIncomingSigShares.Empty(); });
14281416
}
14291417

1430-
void CSigSharesManager::SignAndProcessSingleShare(PendingSignatureData work)
1418+
std::shared_ptr<CRecoveredSig> CSigSharesManager::SignAndProcessSingleShare(PendingSignatureData work)
14311419
{
14321420
auto opt_sigShare = CreateSigShare(*work.quorum, work.id, work.msgHash);
14331421

14341422
if (opt_sigShare.has_value() && opt_sigShare->sigShare.Get().IsValid()) {
14351423
auto& sigShare = *opt_sigShare;
1436-
ProcessSigShare(sigShare, work.quorum);
1424+
auto rs = ProcessSigShare(sigShare, work.quorum);
14371425

14381426
if (IsAllMembersConnectedEnabled(work.quorum->params.type, m_sporkman)) {
14391427
LOCK(cs);
@@ -1443,7 +1431,9 @@ void CSigSharesManager::SignAndProcessSingleShare(PendingSignatureData work)
14431431
session.nextAttemptTime = 0;
14441432
session.attempt = 0;
14451433
}
1434+
return rs;
14461435
}
1436+
return nullptr;
14471437
}
14481438

14491439
void CSigSharesManager::AsyncSign(CQuorumCPtr quorum, const uint256& id, const uint256& msgHash)

‎src/llmq/signing_shares.h‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ class CConnman;
3333
class CDeterministicMN;
3434
class CSporkManager;
3535
struct MessageProcessingResult;
36-
class PeerManager;
3736

3837
namespace llmq
3938
{
@@ -411,7 +410,6 @@ class CSigSharesManager : public llmq::CRecoveredSigsListener
411410
CConnman& m_connman;
412411
const ChainstateManager& m_chainman;
413412
CSigningManager& sigman;
414-
PeerManager& m_peerman;
415413
const CActiveMasternodeManager& m_mn_activeman;
416414
const CQuorumManager& qman;
417415
const CSporkManager& m_sporkman;
@@ -424,8 +422,8 @@ class CSigSharesManager : public llmq::CRecoveredSigsListener
424422
CSigSharesManager(const CSigSharesManager&) = delete;
425423
CSigSharesManager& operator=(const CSigSharesManager&) = delete;
426424
explicit CSigSharesManager(CConnman& connman, const ChainstateManager& chainman, CSigningManager& _sigman,
427-
PeerManager& peerman, const CActiveMasternodeManager& mn_activeman,
428-
const CQuorumManager& _qman, const CSporkManager& sporkman);
425+
const CActiveMasternodeManager& mn_activeman, const CQuorumManager& _qman,
426+
const CSporkManager& sporkman);
429427
~CSigSharesManager() override;
430428

431429
void RegisterRecoveryInterface() EXCLUSIVE_LOCKS_REQUIRED(!cs);
@@ -466,7 +464,7 @@ class CSigSharesManager : public llmq::CRecoveredSigsListener
466464
std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& retQuorums)
467465
EXCLUSIVE_LOCKS_REQUIRED(!cs);
468466

469-
void ProcessPendingSigShares(
467+
std::vector<std::shared_ptr<CRecoveredSig>> ProcessPendingSigShares(
470468
const std::vector<CSigShare>& sigSharesToProcess,
471469
const std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& quorums)
472470
EXCLUSIVE_LOCKS_REQUIRED(!cs);
@@ -478,9 +476,10 @@ class CSigSharesManager : public llmq::CRecoveredSigsListener
478476
const CSigSharesNodeState::SessionInfo& session,
479477
const CBatchedSigShares& batchedSigShares, bool& retBan);
480478

481-
void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum) EXCLUSIVE_LOCKS_REQUIRED(!cs);
482-
std::shared_ptr<CRecoveredSig> TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash)
479+
[[nodiscard]] std::shared_ptr<CRecoveredSig> ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum)
483480
EXCLUSIVE_LOCKS_REQUIRED(!cs);
481+
[[nodiscard]] std::shared_ptr<CRecoveredSig> TryRecoverSig(const CQuorum& quorum, const uint256& id,
482+
const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs);
484483

485484
bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo)
486485
EXCLUSIVE_LOCKS_REQUIRED(!cs);
@@ -511,7 +510,8 @@ class CSigSharesManager : public llmq::CRecoveredSigsListener
511510
std::vector<PendingSignatureData> DispatchPendingSigns() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
512511
// Worker pool task functions
513512
bool IsAnyPendingProcessing() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
514-
void SignAndProcessSingleShare(PendingSignatureData work) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);
513+
[[nodiscard]] std::shared_ptr<CRecoveredSig> SignAndProcessSingleShare(PendingSignatureData work)
514+
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);
515515
};
516516
} // namespace llmq
517517

‎test/lint/lint-circular-dependencies.py‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,15 @@
2121
"wallet/wallet -> wallet/walletdb -> wallet/wallet",
2222
"kernel/coinstats -> validation -> kernel/coinstats",
2323
# Dash
24-
"active/context -> llmq/signing_shares -> net_processing -> active/context",
24+
"active/context -> active/dkgsessionhandler -> llmq/dkgsessionhandler -> net_processing -> active/context",
2525
"banman -> common/bloom -> evo/assetlocktx -> llmq/quorumsman -> net -> banman",
2626
"chainlock/chainlock -> instantsend/instantsend -> chainlock/chainlock",
27-
"chainlock/chainlock -> chainlock/signing -> llmq/signing_shares -> net_processing -> chainlock/chainlock",
28-
"chainlock/chainlock -> chainlock/signing -> llmq/signing_shares -> net_processing -> llmq/context -> chainlock/chainlock",
2927
"chainlock/chainlock -> instantsend/instantsend -> instantsend/signing -> chainlock/chainlock",
3028
"chainlock/chainlock -> llmq/quorumsman -> msg_result -> coinjoin/coinjoin -> chainlock/chainlock",
3129
"chainlock/chainlock -> validation -> chainlock/chainlock",
3230
"chainlock/chainlock -> validation -> evo/chainhelper -> chainlock/chainlock",
3331
"coinjoin/coinjoin -> instantsend/instantsend -> spork -> msg_result -> coinjoin/coinjoin",
34-
"coinjoin/coinjoin -> instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> coinjoin/coinjoin",
35-
"coinjoin/client -> coinjoin/coinjoin -> instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> coinjoin/walletman -> coinjoin/client",
32+
"coinjoin/client -> coinjoin/util -> wallet/wallet -> psbt -> node/transaction -> net_processing -> coinjoin/walletman -> coinjoin/client",
3633
"common/bloom -> evo/assetlocktx -> llmq/commitment -> evo/deterministicmns -> evo/simplifiedmns -> merkleblock -> common/bloom",
3734
"common/bloom -> evo/assetlocktx -> llmq/quorumsman -> net -> common/bloom",
3835
"consensus/tx_verify -> evo/assetlocktx -> llmq/commitment -> validation -> consensus/tx_verify",
@@ -48,8 +45,6 @@
4845
"evo/specialtxman -> validation -> evo/specialtxman",
4946
"governance/classes -> governance/object -> governance/governance -> governance/classes",
5047
"governance/governance -> governance/signing -> governance/object -> governance/governance",
51-
"instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> instantsend/instantsend",
52-
"instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> llmq/context -> instantsend/instantsend",
5348
"instantsend/instantsend -> txmempool -> instantsend/instantsend",
5449
"llmq/blockprocessor -> llmq/utils -> llmq/snapshot -> llmq/blockprocessor",
5550
"llmq/commitment -> llmq/utils -> llmq/snapshot -> llmq/commitment",

0 commit comments

Comments
 (0)