Skip to content

feat: proactively push recovered signatures; fully connect instantsend quorums for recovered sigs #6967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 7 commits into from
Dec 2, 2025
Merged

feat: proactively push recovered signatures; fully connect instantsend quorums for recovered sigs #6967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7be3d4a
feat: proactively relay recovered signatures when we are the member w…
PastaPastaPasta Nov 13, 2025
4e449d0
feat: enhance recovered signature propagation by leveraging full-mesh…
PastaPastaPasta Nov 13, 2025
9f27cfb
fix: proper include syntax
PastaPastaPasta Nov 13, 2025
a57a811
fix: avoid improper dual way connection attempts
PastaPastaPasta Nov 19, 2025
08a3a18
refactor: avoid passing unused `proactive_relay` to `CZMQAbstractNoti…
UdjinM6 Nov 24, 2025
f18a12e
fix: use llmq/params.h
PastaPastaPasta Nov 25, 2025
a58f1bb
trivial: add a few comments and fix reference location
PastaPastaPasta Nov 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/llmq/signing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <llmq/signing.h>

#include <llmq/commitment.h>
#include <llmq/params.h>
#include <llmq/quorums.h>
#include <llmq/signhash.h>
#include <llmq/signing_shares.h>
Expand Down Expand Up @@ -517,7 +518,7 @@ void CSigningManager::ProcessPendingReconstructedRecoveredSigs(PeerManager& peer
WITH_LOCK(cs_pending, swap(m, pendingReconstructedRecoveredSigs));

for (const auto& p : m) {
ProcessRecoveredSig(p.second, peerman);
ProcessRecoveredSig(p.second, peerman, /*from=*/-1);
}
}

Expand Down Expand Up @@ -579,15 +580,15 @@ bool CSigningManager::ProcessPendingRecoveredSigs(PeerManager& peerman)
continue;
}

ProcessRecoveredSig(recSig, peerman);
ProcessRecoveredSig(recSig, peerman, nodeId);
}
}

return more_work;
}

// signature must be verified already
void CSigningManager::ProcessRecoveredSig(const std::shared_ptr<const CRecoveredSig>& recoveredSig, PeerManager& peerman)
void CSigningManager::ProcessRecoveredSig(const std::shared_ptr<const CRecoveredSig>& recoveredSig, PeerManager& peerman, NodeId from)
{
auto llmqType = recoveredSig->getLlmqType();

Expand Down Expand Up @@ -630,7 +631,12 @@ void CSigningManager::ProcessRecoveredSig(const std::shared_ptr<const CRecovered
peerman.PostProcessMessage(l->HandleNewRecoveredSig(*recoveredSig));
}

GetMainSignals().NotifyRecoveredSig(recoveredSig, recoveredSig->GetHash().ToString());
// TODO refactor to use a better abstraction analogous to IsAllMembersConnectedEnabled
auto proactive_relay = from == -1 &&
llmqType != Consensus::LLMQType::LLMQ_100_67 &&
Copy link
Collaborator

@kwvg kwvg Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we document why these LLMQs are excluded from proactive relay? Either by placing them in a dedicated function or by comment? Similar exclusions are present in llmq/options.cpp (see below) but aren't immediately clear why.

dash/src/llmq/options.cpp

Lines 28 to 30 in 199ac79

if (spork_value == 1 && llmqType != Consensus::LLMQType::LLMQ_100_67 && llmqType != Consensus::LLMQType::LLMQ_400_60 && llmqType != Consensus::LLMQType::LLMQ_400_85) {
return true;
}

llmqType != Consensus::LLMQType::LLMQ_400_60 &&
llmqType != Consensus::LLMQType::LLMQ_400_85;
GetMainSignals().NotifyRecoveredSig(recoveredSig, recoveredSig->GetHash().ToString(), proactive_relay);
}

void CSigningManager::PushReconstructedRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& recoveredSig)
Expand Down
2 changes: 1 addition & 1 deletion src/llmq/signing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ class CSigningManager

// Used by CSigSharesManager
CRecoveredSigsDb& GetDb() { return db; }
void ProcessRecoveredSig(const std::shared_ptr<const CRecoveredSig>& recoveredSig, PeerManager& peerman)
void ProcessRecoveredSig(const std::shared_ptr<const CRecoveredSig>& recoveredSig, PeerManager& peerman, NodeId from)
EXCLUSIVE_LOCKS_REQUIRED(!cs_pending, !cs_listeners);

// Needed for access to GetDb() and ProcessRecoveredSig()
Expand Down
8 changes: 4 additions & 4 deletions src/llmq/signing_shares.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id,

// Handle single-member quorum case after releasing the lock
if (singleMemberRecoveredSig) {
sigman.ProcessRecoveredSig(singleMemberRecoveredSig, m_peerman);
sigman.ProcessRecoveredSig(singleMemberRecoveredSig, m_peerman, /*from=*/-1);
return; // end of single-quorum processing
}

Expand Down Expand Up @@ -876,7 +876,7 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id,
}
}

sigman.ProcessRecoveredSig(rs, m_peerman);
sigman.ProcessRecoveredSig(rs, m_peerman, /*from=*/-1);
}

CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorum& quorum, const uint256 &id, int attempt)
Expand Down Expand Up @@ -974,9 +974,9 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigning
return true;
}

void CSigSharesManager::NotifyRecoveredSig(const std::shared_ptr<const CRecoveredSig>& sig) const
void CSigSharesManager::NotifyRecoveredSig(const std::shared_ptr<const CRecoveredSig>& sig, bool proactive_relay) const
{
m_peerman.RelayRecoveredSig(Assert(sig)->GetHash());
m_peerman.RelayRecoveredSig(*Assert(sig), proactive_relay);
}

void CSigSharesManager::CollectSigSharesToRequest(std::unordered_map<NodeId, Uint256HashMap<CSigSharesInv>>& sigSharesToRequest)
Expand Down
2 changes: 1 addition & 1 deletion src/llmq/signing_shares.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ class CSigSharesManager : public CRecoveredSigsListener
const uint256& msgHash, const uint256& quorumHash = uint256(), bool allowReSign = false,
bool allowDiffMsgHashSigning = false) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);

void NotifyRecoveredSig(const std::shared_ptr<const CRecoveredSig>& sig) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
void NotifyRecoveredSig(const std::shared_ptr<const CRecoveredSig>& sig, bool proactive_relay) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
Copy link

@coderabbitai coderabbitai bot Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Recovered‑sig notification wiring is correct; fix clang‑format to unbreak CI

The new NotifyRecoveredSig(const std::shared_ptr<const CRecoveredSig>& sig, bool proactive_relay) const EXCLUSIVE_LOCKS_REQUIRED(!cs) overload is consistent with its implementation in signing_shares.cpp and with the locking strategy (no cs held while calling PeerManager::RelayRecoveredSig). Semantics look good.

CI is currently failing on src/llmq/signing_shares.h due to a clang‑format diff, though. Please run the project’s clang‑format tooling on this file (or at least around this declaration) and commit the resulting formatting changes so the pipeline passes.

🤖 Prompt for AI Agents 
In src/llmq/signing_shares.h around line 452, the new overload declaration is
correctly implemented but its formatting doesn't match the project's
clang-format rules and is causing CI failures; run the project's clang-format
tool (or apply the repo's clang-format style) to reformat this file or at least
the vicinity of this declaration, update the header with the formatted result,
and commit that change so the CI clang-format check passes.


private:
std::optional<CSigShare> CreateSigShareForSingleMember(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const;
Expand Down
7 changes: 1 addition & 6 deletions src/llmq/utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,17 +858,14 @@ bool EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, CConnman&
__func__, isMember, pQuorumBaseBlockIndex->GetBlockHash().ToString());

Uint256HashSet connections;
Uint256HashSet relayMembers;
if (isMember) {
connections = GetQuorumConnections(llmqParams, dmnman, qsnapman, sporkman, pQuorumBaseBlockIndex, myProTxHash,
true);
relayMembers = GetQuorumRelayMembers(llmqParams, dmnman, qsnapman, pQuorumBaseBlockIndex, myProTxHash, true);
} else {
auto cindexes = CalcDeterministicWatchConnections(llmqParams.type, pQuorumBaseBlockIndex, members.size(), 1);
for (auto idx : cindexes) {
connections.emplace(members[idx]->proTxHash);
}
relayMembers = connections;
}
if (!connections.empty()) {
if (!connman.HasMasternodeQuorumNodes(llmqParams.type, pQuorumBaseBlockIndex->GetBlockHash()) &&
Expand All @@ -886,9 +883,7 @@ bool EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, CConnman&
LogPrint(BCLog::NET_NETCONN, debugMsg.c_str()); /* Continued */
}
connman.SetMasternodeQuorumNodes(llmqParams.type, pQuorumBaseBlockIndex->GetBlockHash(), connections);
}
if (!relayMembers.empty()) {
connman.SetMasternodeQuorumRelayMembers(llmqParams.type, pQuorumBaseBlockIndex->GetBlockHash(), relayMembers);
connman.SetMasternodeQuorumRelayMembers(llmqParams.type, pQuorumBaseBlockIndex->GetBlockHash(), connections);
}
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions src/masternode/active/notificationinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ void ActiveNotificationInterface::UpdatedBlockTip(const CBlockIndex* pindexNew,
m_active_ctx.gov_signer->UpdatedBlockTip(pindexNew);
}

void ActiveNotificationInterface::NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig)
void ActiveNotificationInterface::NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, bool proactive_relay)
{
m_active_ctx.shareman->NotifyRecoveredSig(sig);
m_active_ctx.shareman->NotifyRecoveredSig(sig, proactive_relay);
Comment on lines +32 to +34
Copy link

@coderabbitai coderabbitai bot Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix clang-format issues.

The CI pipeline reports formatting differences in this file. Please run clang-format to resolve the formatting issues before merging.

Run the following command to fix formatting:

#!/bin/bash
# Format the file according to project style
clang-format -i src/masternode/active/notificationinterface.cpp
🤖 Prompt for AI Agents 
In src/masternode/active/notificationinterface.cpp around lines 32 to 34, the
file fails CI due to clang-format style differences; run clang-format -i on that
file to reformat it (or apply your project's clang-format settings) so the
NotifyRecoveredSig function and surrounding whitespace/indentation match project
style, then re-run clang-format or the CI linter and commit the updated file.

}

std::unique_ptr<ActiveNotificationInterface> g_active_notification_interface;
2 changes: 1 addition & 1 deletion src/masternode/active/notificationinterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ActiveNotificationInterface final : public CValidationInterface

protected:
// CValidationInterface
void NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig) override;
void NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, bool proactive_relay) override;
void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override;

private:
Expand Down
19 changes: 15 additions & 4 deletions src/net_processing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ class PeerManagerImpl final : public PeerManager
void RelayInv(const CInv& inv) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void RelayInv(const CInv& inv, const int minProtoVersion) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void RelayTransaction(const uint256& txid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void RelayRecoveredSig(const uint256& sigHash) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void RelayRecoveredSig(const llmq::CRecoveredSig& sig, bool proactive_relay) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void RelayDSQ(const CCoinJoinQueue& queue) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
void SetBestHeight(int height) override { m_best_height = height; };
void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message = "") override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
Expand Down Expand Up @@ -2536,9 +2536,20 @@ void PeerManagerImpl::_RelayTransaction(const uint256& txid)
};
}

void PeerManagerImpl::RelayRecoveredSig(const uint256& sigHash)
{
const CInv inv{MSG_QUORUM_RECOVERED_SIG, sigHash};
void PeerManagerImpl::RelayRecoveredSig(const llmq::CRecoveredSig& sig, bool proactive_relay) {
if (proactive_relay) {
// We were the peer that recovered this; avoid a bunch of `inv` -> `GetData` spam by proactively sending
m_connman.ForEachNode([this, &sig](CNode* pnode) -> bool {
// Skip nodes that don't want recovered signatures
PeerRef peer = GetPeerRef(pnode->GetId());
if (peer == nullptr || !peer->m_wants_recsigs) return true;
CNetMsgMaker msgMaker(pnode->GetCommonVersion());
m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGREC, sig));
return true;
});
return;
}
const CInv inv{MSG_QUORUM_RECOVERED_SIG, sig.GetHash()};
READ_LOCK(m_peer_mutex);
for (const auto& [_, peer] : m_peer_map) {
if (peer->m_wants_recsigs) {
Expand Down
2 changes: 1 addition & 1 deletion src/net_processing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class PeerManager : public CValidationInterface, public NetEventsInterface, publ
virtual void RelayTransaction(const uint256& txid) = 0;

/** Relay recovered sigs to all interested peers */
virtual void RelayRecoveredSig(const uint256& sigHash) = 0;
virtual void RelayRecoveredSig(const llmq::CRecoveredSig& sig, bool proactive_relay) = 0;

/** Set the best height */
virtual void SetBestHeight(int height) = 0;
Expand Down
6 changes: 3 additions & 3 deletions src/validationinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,9 @@ void CMainSignals::NotifyInstantSendDoubleSpendAttempt(const CTransactionRef& cu
previousTx->GetHash().ToString());
}

void CMainSignals::NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, const std::string& id) {
auto event = [sig, this] {
m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyRecoveredSig(sig); });
void CMainSignals::NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, const std::string& id, bool proactive_relay) {
auto event = [sig, proactive_relay, this] {
m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.NotifyRecoveredSig(sig, proactive_relay); });
};
ENQUEUE_AND_LOG_EVENT(event, "%s: notify recoveredsig=%s", __func__,
id);
Expand Down
4 changes: 2 additions & 2 deletions src/validationinterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class CValidationInterface {
virtual void NotifyGovernanceVote(const std::shared_ptr<CDeterministicMNList>& tip_mn_list, const std::shared_ptr<const CGovernanceVote>& vote) {}
virtual void NotifyGovernanceObject(const std::shared_ptr<const Governance::Object>& object) {}
virtual void NotifyInstantSendDoubleSpendAttempt(const CTransactionRef& currentTx, const CTransactionRef& previousTx) {}
virtual void NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig) {}
virtual void NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, bool proactive_relay) {}
virtual void NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff) {}
/**
* Notifies listeners of the new active block chain on-disk.
Expand Down Expand Up @@ -236,7 +236,7 @@ class CMainSignals {
void NotifyGovernanceVote(const std::shared_ptr<CDeterministicMNList>& tip_mn_list, const std::shared_ptr<const CGovernanceVote>& vote, const std::string& id);
void NotifyGovernanceObject(const std::shared_ptr<const Governance::Object>& object, const std::string& id);
void NotifyInstantSendDoubleSpendAttempt(const CTransactionRef &currentTx, const CTransactionRef &previousTx);
void NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig> &sig, const std::string& id);
void NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig> &sig, const std::string& id, bool proactive_relay);
void NotifyMasternodeListChanged(bool undo, const CDeterministicMNList& oldMNList, const CDeterministicMNListDiff& diff);
void ChainStateFlushed(const CBlockLocator &);
void BlockChecked(const CBlock&, const BlockValidationState&);
Expand Down
2 changes: 1 addition & 1 deletion src/zmq/zmqnotificationinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ void CZMQNotificationInterface::NotifyInstantSendDoubleSpendAttempt(const CTrans
});
}

void CZMQNotificationInterface::NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig)
void CZMQNotificationInterface::NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, bool /*proactive_relay*/)
Copy link
Collaborator

@kwvg kwvg Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void CZMQNotificationInterface::NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, bool /*proactive_relay*/)
void CZMQNotificationInterface::NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, [[maybe_unused]] bool proactive_relay)

{
TryForEachAndRemoveFailed(notifiers, [&sig](CZMQAbstractNotifier* notifier) {
return notifier->NotifyRecoveredSig(sig);
Expand Down
2 changes: 1 addition & 1 deletion src/zmq/zmqnotificationinterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class CZMQNotificationInterface final : public CValidationInterface
void NotifyGovernanceVote(const std::shared_ptr<CDeterministicMNList>& tip_mn_list, const std::shared_ptr<const CGovernanceVote>& vote) override;
void NotifyGovernanceObject(const std::shared_ptr<const Governance::Object>& object) override;
void NotifyInstantSendDoubleSpendAttempt(const CTransactionRef& currentTx, const CTransactionRef& previousTx) override;
void NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig) override;
void NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, bool /*proactive_relay*/) override;

private:
CZMQNotificationInterface();
Expand Down
Loading