Skip to content

refactor: replace usages of MessageProcessingResult in HandleNewRecoveredSig to variant #7250

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 1 commit into from
Apr 5, 2026
Merged

refactor: replace usages of MessageProcessingResult in HandleNewRecoveredSig to variant #7250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 11 additions & 5 deletions src/chainlock/signing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,10 @@ ChainLockSigner::BlockTxs::mapped_type ChainLockSigner::GetBlockTxs(const uint25
return ret;
}

MessageProcessingResult ChainLockSigner::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
llmq::RecoveredSigResult ChainLockSigner::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
{
if (!m_chainlocks.IsEnabled()) {
return {};
return std::monostate{};
}

ChainLockSig clsig;
Expand All @@ -272,16 +272,22 @@ MessageProcessingResult ChainLockSigner::HandleNewRecoveredSig(const llmq::CReco

if (recoveredSig.getId() != lastSignedRequestId || recoveredSig.getMsgHash() != lastSignedMsgHash) {
// this is not what we signed, so lets not create a CLSIG for it
return {};
return std::monostate{};
}
if (m_chainlocks.GetBestChainLockHeight() >= lastSignedHeight) {
// already got the same or a better CLSIG through the CLSIG message
return {};
return std::monostate{};
}

clsig = ChainLockSig(lastSignedHeight, lastSignedMsgHash, recoveredSig.sig.Get());
}
return m_clhandler.ProcessNewChainLock(-1, clsig, m_qman, ::SerializeHash(clsig));
// TODO: split ProcessNewChainLock into network and non-network variants; when no peer
// is specified (node == -1), only m_inventory is ever populated
auto clresult = m_clhandler.ProcessNewChainLock(-1, clsig, m_qman, ::SerializeHash(clsig));
if (!clresult.m_inventory.empty()) {
return clresult.m_inventory.front();
}
return std::monostate{};
}

void ChainLockSigner::Cleanup()
Expand Down
3 changes: 1 addition & 2 deletions src/chainlock/signing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

class CScheduler;
class CMasternodeSync;
struct MessageProcessingResult;
namespace llmq {
class CInstantSendManager;
class CRecoveredSig;
Expand Down Expand Up @@ -80,7 +79,7 @@ class ChainLockSigner final : public llmq::CRecoveredSigsListener, public CValid
void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override
EXCLUSIVE_LOCKS_REQUIRED(!cs_try_sign, !cs_signer);

[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) override
[[nodiscard]] llmq::RecoveredSigResult HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) override
EXCLUSIVE_LOCKS_REQUIRED(!cs_signer);

void Cleanup() EXCLUSIVE_LOCKS_REQUIRED(!cs_signer);
Expand Down
8 changes: 4 additions & 4 deletions src/instantsend/signing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@ void InstantSendSigner::ClearLockFromQueue(const InstantSendLockPtr& islock)
txToCreatingInstantSendLocks.erase(islock->txid);
}

MessageProcessingResult InstantSendSigner::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
llmq::RecoveredSigResult InstantSendSigner::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
{
if (!m_isman.IsInstantSendEnabled()) {
return {};
return std::monostate{};
}

if (Params().GetConsensus().llmqTypeDIP0024InstantSend == Consensus::LLMQType::LLMQ_NONE) {
return {};
return std::monostate{};
}

uint256 txid;
Expand All @@ -92,7 +92,7 @@ MessageProcessingResult InstantSendSigner::HandleNewRecoveredSig(const llmq::CRe
} else if (/*isInstantSendLock=*/WITH_LOCK(cs_creating, return creatingInstantSendLocks.count(recoveredSig.getId()))) {
HandleNewInstantSendLockRecoveredSig(recoveredSig);
}
return {};
return std::monostate{};
}

bool InstantSendSigner::IsInstantSendMempoolSigningEnabled() const
Expand Down
3 changes: 1 addition & 2 deletions src/instantsend/signing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
class CMasternodeSync;
class CSporkManager;
class CTxMemPool;
struct MessageProcessingResult;

namespace Consensus {
struct Params;
Expand Down Expand Up @@ -93,7 +92,7 @@ class InstantSendSigner final : public llmq::CRecoveredSigsListener
void ClearLockFromQueue(const InstantSendLockPtr& islock)
EXCLUSIVE_LOCKS_REQUIRED(!cs_creating);

[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) override
[[nodiscard]] llmq::RecoveredSigResult HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) override
EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_input_requests);

void ProcessPendingRetryLockTxs(const std::vector<CTransactionRef>& retryTxs)
Expand Down
12 changes: 5 additions & 7 deletions src/llmq/ehf_signals.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ void CEHFSignalsHandler::trySignEHFSignal(int bit, const CBlockIndex* const pind
shareman.AsyncSignIfMember(llmqType, sigman, requestId, msgHash, quorum->qc->quorumHash, false, true);
}

MessageProcessingResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig)
RecoveredSigResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig)
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 24, 2026

Choose a reason for hiding this comment

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

P2 Badge Return monostate when EHF handler ignores a recovered sig

Changing HandleNewRecoveredSig to return RecoveredSigResult means return {} now default-constructs the first variant alternative (CInv) instead of an empty result. In the non-matching-ID path this causes NetSigning::ProcessRecoveredSig to treat the result as inventory and call PeerRelayInv with a zeroed/unknown inv, so unrelated recovered signatures generate bogus relays (at least once per peer). This should return std::monostate{} for the ignore case to preserve prior behavior.

Useful? React with 👍 / 👎.

{
if (g_txindex) {
g_txindex->BlockUntilSyncedToCurrentChain();
}

if (WITH_LOCK(cs, return ids.find(recoveredSig.getId()) == ids.end())) {
// Do nothing, it's not for this handler
return {};
return std::monostate{};
}

const auto ehfSignals = m_chainman.ActiveChainstate().ChainHelper().ehf_manager->GetSignalsStage(
Expand Down Expand Up @@ -118,14 +118,12 @@ MessageProcessingResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecover
LOCK(::cs_main);
const MempoolAcceptResult result = m_chainman.ProcessTransaction(tx_to_sent);
if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
MessageProcessingResult ret;
ret.m_transactions.push_back(tx_to_sent->GetHash());
return ret;
return tx_to_sent;
}
LogPrintf("CEHFSignalsHandler::HandleNewRecoveredSig -- AcceptToMemoryPool failed: %s\n",
result.m_state.ToString());
return {};
return std::monostate{};
}
return {};
return std::monostate{};
}
} // namespace llmq
3 changes: 1 addition & 2 deletions src/llmq/ehf_signals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#define BITCOIN_LLMQ_EHF_SIGNALS_H

#include <llmq/signing.h>
#include <msg_result.h>
#include <saltedhasher.h>

#include <set>
Expand Down Expand Up @@ -44,7 +43,7 @@ class CEHFSignalsHandler : public CRecoveredSigsListener
*/
void UpdatedBlockTip(const CBlockIndex* const pindexNew) EXCLUSIVE_LOCKS_REQUIRED(!cs);

[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
[[nodiscard]] RecoveredSigResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
EXCLUSIVE_LOCKS_REQUIRED(!cs);

private:
Expand Down
7 changes: 6 additions & 1 deletion src/llmq/net_signing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,12 @@ void NetSigning::ProcessRecoveredSig(std::shared_ptr<const CRecoveredSig> recove

auto listeners = m_sig_manager.GetListeners();
for (auto& l : listeners) {
m_peer_manager->PeerPostProcessMessage(l->HandleNewRecoveredSig(*recovered_sig));
auto result = l->HandleNewRecoveredSig(*recovered_sig);
if (const auto* inv = std::get_if<CInv>(&result)) {
m_peer_manager->PeerRelayInv(*inv);
} else if (const auto* tx_ref = std::get_if<CTransactionRef>(&result)) {
m_peer_manager->PeerRelayTransaction((*tx_ref)->GetHash());
}
}

// TODO refactor to use a better abstraction analogous to IsAllMembersConnectedEnabled
Expand Down
10 changes: 7 additions & 3 deletions src/llmq/signing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <bls/bls.h>
#include <llmq/params.h>
#include <llmq/types.h>
#include <msg_result.h>
#include <net_types.h>
#include <random.h>
#include <saltedhasher.h>
Expand All @@ -18,20 +17,26 @@
#include <memory>
#include <string_view>
#include <unordered_map>
#include <variant>

class CChainState;
class CDataStream;
class CDBBatch;
class CDBWrapper;
class CInv;
class CTransaction;
struct RPCResult;
namespace util {
struct DbWrapperParams;
} // namespace util

class UniValue;
using CTransactionRef = std::shared_ptr<const CTransaction>;

namespace llmq {

using RecoveredSigResult = std::variant<std::monostate, CInv, CTransactionRef>;

class CQuorumManager;
class CSigSharesManager;
class SignHash;
Expand Down Expand Up @@ -151,8 +156,7 @@ class CRecoveredSigsListener
public:
virtual ~CRecoveredSigsListener() = default;

// TODO: simplify returned type to std::variant<CInv, CTransaction, std::monostate>
[[nodiscard]] virtual MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) = 0;
[[nodiscard]] virtual RecoveredSigResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) = 0;
};

class CSigningManager
Expand Down
8 changes: 3 additions & 5 deletions src/llmq/signing_shares.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,9 @@
#include <llmq/quorumsman.h>
#include <llmq/signhash.h>
#include <llmq/signing.h>
#include <msg_result.h>
#include <netmessagemaker.h>
#include <util/helpers.h>
#include <util/std23.h>

#include <netmessagemaker.h>
#include <util/thread.h>
#include <util/time.h>
#include <validation.h>
Expand Down Expand Up @@ -1535,11 +1533,11 @@ void CSigSharesManager::ForceReAnnouncement(const CQuorum& quorum, Consensus::LL
}
}

MessageProcessingResult CSigSharesManager::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
RecoveredSigResult CSigSharesManager::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
{
auto signHash = recoveredSig.buildSignHash().Get();
LOCK(cs);
RemoveSigSharesForSession(signHash);
return {};
return std::monostate{};
}
} // namespace llmq
3 changes: 1 addition & 2 deletions src/llmq/signing_shares.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ class ChainstateManager;
class CNode;
class CConnman;
class CSporkManager;
struct MessageProcessingResult;

namespace llmq
{
Expand Down Expand Up @@ -434,7 +433,7 @@ class CSigSharesManager : public llmq::CRecoveredSigsListener
void ForceReAnnouncement(const CQuorum& quorum, Consensus::LLMQType llmqType, const uint256& id,
const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs);

[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
[[nodiscard]] RecoveredSigResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
EXCLUSIVE_LOCKS_REQUIRED(!cs);

static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorum& quorum, const uint256& id, int attempt);
Expand Down
Loading