Skip to content

Commit eeb84cb

Browse files
committed
refactor: replace usages of MessageProcessingResult in HandleNewRecoveredSig to variant
It could return only CInv or CTransaction; MessageProcessingResult is misused there
1 parent 65366a4 commit eeb84cb

10 files changed

Lines changed: 40 additions & 33 deletions

File tree

src/chainlock/signing.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,10 @@ ChainLockSigner::BlockTxs::mapped_type ChainLockSigner::GetBlockTxs(const uint25
260260
return ret;
261261
}
262262

263-
MessageProcessingResult ChainLockSigner::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
263+
llmq::RecoveredSigResult ChainLockSigner::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
264264
{
265265
if (!m_chainlocks.IsEnabled()) {
266-
return {};
266+
return std::monostate{};
267267
}
268268

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

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

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

287293
void ChainLockSigner::Cleanup()

src/chainlock/signing.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
class CScheduler;
1515
class CMasternodeSync;
16-
struct MessageProcessingResult;
1716
namespace llmq {
1817
class CInstantSendManager;
1918
class CRecoveredSig;
@@ -80,7 +79,7 @@ class ChainLockSigner final : public llmq::CRecoveredSigsListener, public CValid
8079
void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override
8180
EXCLUSIVE_LOCKS_REQUIRED(!cs_try_sign, !cs_signer);
8281

83-
[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) override
82+
[[nodiscard]] llmq::RecoveredSigResult HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) override
8483
EXCLUSIVE_LOCKS_REQUIRED(!cs_signer);
8584

8685
void Cleanup() EXCLUSIVE_LOCKS_REQUIRED(!cs_signer);

src/instantsend/signing.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,14 @@ void InstantSendSigner::ClearLockFromQueue(const InstantSendLockPtr& islock)
7373
txToCreatingInstantSendLocks.erase(islock->txid);
7474
}
7575

76-
MessageProcessingResult InstantSendSigner::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
76+
llmq::RecoveredSigResult InstantSendSigner::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
7777
{
7878
if (!m_isman.IsInstantSendEnabled()) {
79-
return {};
79+
return std::monostate{};
8080
}
8181

8282
if (Params().GetConsensus().llmqTypeDIP0024InstantSend == Consensus::LLMQType::LLMQ_NONE) {
83-
return {};
83+
return std::monostate{};
8484
}
8585

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

9898
bool InstantSendSigner::IsInstantSendMempoolSigningEnabled() const

src/instantsend/signing.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
class CMasternodeSync;
1515
class CSporkManager;
1616
class CTxMemPool;
17-
struct MessageProcessingResult;
1817

1918
namespace Consensus {
2019
struct Params;
@@ -93,7 +92,7 @@ class InstantSendSigner final : public llmq::CRecoveredSigsListener
9392
void ClearLockFromQueue(const InstantSendLockPtr& islock)
9493
EXCLUSIVE_LOCKS_REQUIRED(!cs_creating);
9594

96-
[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) override
95+
[[nodiscard]] llmq::RecoveredSigResult HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) override
9796
EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_input_requests);
9897

9998
void ProcessPendingRetryLockTxs(const std::vector<CTransactionRef>& retryTxs)

src/llmq/ehf_signals.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,15 @@ void CEHFSignalsHandler::trySignEHFSignal(int bit, const CBlockIndex* const pind
8181
shareman.AsyncSignIfMember(llmqType, sigman, requestId, msgHash, quorum->qc->quorumHash, false, true);
8282
}
8383

84-
MessageProcessingResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig)
84+
RecoveredSigResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig)
8585
{
8686
if (g_txindex) {
8787
g_txindex->BlockUntilSyncedToCurrentChain();
8888
}
8989

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

9595
const auto ehfSignals = m_chainman.ActiveChainstate().ChainHelper().ehf_manager->GetSignalsStage(
@@ -118,14 +118,12 @@ MessageProcessingResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecover
118118
LOCK(::cs_main);
119119
const MempoolAcceptResult result = m_chainman.ProcessTransaction(tx_to_sent);
120120
if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
121-
MessageProcessingResult ret;
122-
ret.m_transactions.push_back(tx_to_sent->GetHash());
123-
return ret;
121+
return tx_to_sent;
124122
}
125123
LogPrintf("CEHFSignalsHandler::HandleNewRecoveredSig -- AcceptToMemoryPool failed: %s\n",
126124
result.m_state.ToString());
127-
return {};
125+
return std::monostate{};
128126
}
129-
return {};
127+
return std::monostate{};
130128
}
131129
} // namespace llmq

src/llmq/ehf_signals.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
#define BITCOIN_LLMQ_EHF_SIGNALS_H
77

88
#include <llmq/signing.h>
9-
#include <msg_result.h>
109
#include <saltedhasher.h>
1110

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

47-
[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
46+
[[nodiscard]] RecoveredSigResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
4847
EXCLUSIVE_LOCKS_REQUIRED(!cs);
4948

5049
private:

src/llmq/net_signing.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,12 @@ void NetSigning::ProcessRecoveredSig(std::shared_ptr<const CRecoveredSig> recove
166166

167167
auto listeners = m_sig_manager.GetListeners();
168168
for (auto& l : listeners) {
169-
m_peer_manager->PeerPostProcessMessage(l->HandleNewRecoveredSig(*recovered_sig));
169+
auto result = l->HandleNewRecoveredSig(*recovered_sig);
170+
if (const auto* inv = std::get_if<CInv>(&result)) {
171+
m_peer_manager->PeerRelayInv(*inv);
172+
} else if (const auto* tx_ref = std::get_if<CTransactionRef>(&result)) {
173+
m_peer_manager->PeerRelayTransaction((*tx_ref)->GetHash());
174+
}
170175
}
171176

172177
// TODO refactor to use a better abstraction analogous to IsAllMembersConnectedEnabled

src/llmq/signing.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include <bls/bls.h>
99
#include <llmq/params.h>
1010
#include <llmq/types.h>
11-
#include <msg_result.h>
1211
#include <net_types.h>
1312
#include <random.h>
1413
#include <saltedhasher.h>
@@ -18,20 +17,26 @@
1817
#include <memory>
1918
#include <string_view>
2019
#include <unordered_map>
20+
#include <variant>
2121

2222
class CChainState;
2323
class CDataStream;
2424
class CDBBatch;
2525
class CDBWrapper;
2626
class CInv;
27+
class CTransaction;
2728
struct RPCResult;
2829
namespace util {
2930
struct DbWrapperParams;
3031
} // namespace util
3132

3233
class UniValue;
34+
using CTransactionRef = std::shared_ptr<const CTransaction>;
3335

3436
namespace llmq {
37+
38+
using RecoveredSigResult = std::variant<std::monostate, CInv, CTransactionRef>;
39+
3540
class CQuorumManager;
3641
class CSigSharesManager;
3742
class SignHash;
@@ -151,8 +156,7 @@ class CRecoveredSigsListener
151156
public:
152157
virtual ~CRecoveredSigsListener() = default;
153158

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

158162
class CSigningManager

src/llmq/signing_shares.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,9 @@
1313
#include <llmq/quorumsman.h>
1414
#include <llmq/signhash.h>
1515
#include <llmq/signing.h>
16-
#include <msg_result.h>
16+
#include <netmessagemaker.h>
1717
#include <util/helpers.h>
1818
#include <util/std23.h>
19-
20-
#include <netmessagemaker.h>
2119
#include <util/thread.h>
2220
#include <util/time.h>
2321
#include <validation.h>
@@ -1535,11 +1533,11 @@ void CSigSharesManager::ForceReAnnouncement(const CQuorum& quorum, Consensus::LL
15351533
}
15361534
}
15371535

1538-
MessageProcessingResult CSigSharesManager::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
1536+
RecoveredSigResult CSigSharesManager::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig)
15391537
{
15401538
auto signHash = recoveredSig.buildSignHash().Get();
15411539
LOCK(cs);
15421540
RemoveSigSharesForSession(signHash);
1543-
return {};
1541+
return std::monostate{};
15441542
}
15451543
} // namespace llmq

src/llmq/signing_shares.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ class ChainstateManager;
3333
class CNode;
3434
class CConnman;
3535
class CSporkManager;
36-
struct MessageProcessingResult;
3736

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

437-
[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
436+
[[nodiscard]] RecoveredSigResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
438437
EXCLUSIVE_LOCKS_REQUIRED(!cs);
439438

440439
static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorum& quorum, const uint256& id, int attempt);

0 commit comments

Comments
 (0)