Skip to content

Commit f608e16

Browse files
knstknst
committed
refactor: RAII initialization for members of llmq::context and related changes (see dashpay#5150)
1 parent 3c65626 commit f608e16

23 files changed

+147
-188
lines changed

‎src/evo/specialtxman.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
#include <primitives/block.h>
1818
#include <validation.h>
1919

20-
bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs)
20+
bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, bool check_sigs, TxValidationState& state)
2121
{
2222
AssertLockHeld(cs_main);
2323

@@ -100,7 +100,7 @@ bool UndoSpecialTx(const CTransaction& tx, const CBlockIndex* pindex)
100100
}
101101

102102
bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, const llmq::CChainLocksHandler& chainlock_handler,
103-
BlockValidationState& state, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots)
103+
const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots, BlockValidationState& state)
104104
{
105105
AssertLockHeld(cs_main);
106106

@@ -117,7 +117,7 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll
117117
TxValidationState tx_state;
118118
// At this moment CheckSpecialTx() and ProcessSpecialTx() may fail by 2 possible ways:
119119
// consensus failures and "TX_BAD_SPECIAL"
120-
if (!CheckSpecialTx(*ptr_tx, pindex->pprev, tx_state, view, fCheckCbTxMerleRoots)) {
120+
if (!CheckSpecialTx(*ptr_tx, pindex->pprev, view, fCheckCbTxMerleRoots, tx_state)) {
121121
assert(tx_state.GetResult() == TxValidationResult::TX_CONSENSUS || tx_state.GetResult() == TxValidationResult::TX_BAD_SPECIAL);
122122
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(),
123123
strprintf("Special Transaction check failed (tx hash %s) %s", ptr_tx->GetHash().ToString(), tx_state.GetDebugMessage()));

‎src/evo/specialtxman.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ class CChainLocksHandler;
2121

2222
extern RecursiveMutex cs_main;
2323

24-
bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
24+
bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view, bool check_sigs, TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
2525
bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, const llmq::CChainLocksHandler& chainlock_handler,
26-
BlockValidationState& state, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
26+
const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
2727
bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
2828

2929
#endif // BITCOIN_EVO_SPECIALTXMAN_H

‎src/init.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2017,7 +2017,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
20172017
node.evodb = std::make_unique<CEvoDB>(nEvoDbCache, false, fReset || fReindexChainState);
20182018

20192019
chainman.Reset();
2020-
chainman.InitializeChainstate(Assert(node.mempool.get()), *node.evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor);
2020+
chainman.InitializeChainstate(Assert(node.mempool.get()), *node.evodb, node.llmq_ctx);
20212021
chainman.m_total_coinstip_cache = nCoinCacheUsage;
20222022
chainman.m_total_coinsdb_cache = nCoinDBCache;
20232023

‎src/llmq/context.cpp‎

Lines changed: 55 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -20,108 +20,81 @@
2020
#include <masternode/sync.h>
2121

2222
LLMQContext::LLMQContext(CEvoDB& evo_db, CTxMemPool& mempool, CConnman& connman, CSporkManager& sporkman,
23-
const std::unique_ptr<PeerManager>& peerman, bool unit_tests, bool wipe) {
24-
Create(evo_db, mempool, connman, sporkman, peerman, unit_tests, wipe);
25-
26-
/* Context aliases to globals used by the LLMQ system */
27-
quorum_block_processor = llmq::quorumBlockProcessor.get();
28-
qman = llmq::quorumManager.get();
29-
clhandler = llmq::chainLocksHandler.get();
30-
isman = llmq::quorumInstantSendManager.get();
31-
}
32-
33-
LLMQContext::~LLMQContext() {
34-
isman = nullptr;
35-
clhandler = nullptr;
36-
qman = nullptr;
37-
quorum_block_processor = nullptr;
38-
39-
Destroy();
40-
}
41-
42-
void LLMQContext::Create(CEvoDB& evo_db, CTxMemPool& mempool, CConnman& connman, CSporkManager& sporkman,
43-
const std::unique_ptr<PeerManager>& peerman, bool unit_tests, bool wipe) {
44-
bls_worker = std::make_shared<CBLSWorker>();
45-
46-
dkg_debugman = std::make_unique<llmq::CDKGDebugManager>();
47-
llmq::quorumBlockProcessor = std::make_unique<llmq::CQuorumBlockProcessor>(evo_db, connman, peerman);
48-
qdkgsman = std::make_unique<llmq::CDKGSessionManager>(connman, *bls_worker, *dkg_debugman, *llmq::quorumBlockProcessor, sporkman, peerman, unit_tests, wipe);
49-
llmq::quorumManager = std::make_unique<llmq::CQuorumManager>(evo_db, connman, *bls_worker, *llmq::quorumBlockProcessor, *qdkgsman, ::masternodeSync, peerman);
50-
sigman = std::make_unique<llmq::CSigningManager>(connman, *llmq::quorumManager, peerman, unit_tests, wipe);
51-
shareman = std::make_unique<llmq::CSigSharesManager>(connman, *llmq::quorumManager, *sigman, peerman);
52-
llmq::chainLocksHandler = std::make_unique<llmq::CChainLocksHandler>(mempool, connman, sporkman, *sigman, *shareman, *llmq::quorumManager, *::masternodeSync, peerman);
53-
llmq::quorumInstantSendManager = std::make_unique<llmq::CInstantSendManager>(mempool, connman, sporkman, *llmq::quorumManager, *sigman, *shareman, *llmq::chainLocksHandler, *::masternodeSync, peerman, unit_tests, wipe);
54-
23+
const std::unique_ptr<PeerManager>& peerman, bool unit_tests, bool wipe) :
24+
bls_worker{std::make_shared<CBLSWorker>()},
25+
dkg_debugman{std::make_unique<llmq::CDKGDebugManager>()},
26+
quorum_block_processor{[&]() -> llmq::CQuorumBlockProcessor* const {
27+
llmq::quorumBlockProcessor = std::make_unique<llmq::CQuorumBlockProcessor>(evo_db, connman, peerman);
28+
return llmq::quorumBlockProcessor.get();
29+
}()},
30+
qdkgsman{std::make_unique<llmq::CDKGSessionManager>(connman, *bls_worker, *dkg_debugman, *quorum_block_processor, sporkman, peerman, unit_tests, wipe)},
31+
qman{[&]() -> llmq::CQuorumManager* const {
32+
llmq::quorumManager = std::make_unique<llmq::CQuorumManager>(evo_db, connman, *bls_worker, *quorum_block_processor, *qdkgsman, ::masternodeSync, peerman);
33+
return llmq::quorumManager.get();
34+
}()},
35+
sigman{std::make_unique<llmq::CSigningManager>(connman, *llmq::quorumManager, peerman, unit_tests, wipe)},
36+
shareman{std::make_unique<llmq::CSigSharesManager>(connman, *llmq::quorumManager, *sigman, peerman)},
37+
clhandler{[&]() -> llmq::CChainLocksHandler* const {
38+
llmq::chainLocksHandler = std::make_unique<llmq::CChainLocksHandler>(mempool, connman, sporkman, *sigman, *shareman, *llmq::quorumManager, *::masternodeSync, peerman);
39+
return llmq::chainLocksHandler.get();
40+
}()},
41+
isman{[&]() -> llmq::CInstantSendManager* const {
42+
llmq::quorumInstantSendManager = std::make_unique<llmq::CInstantSendManager>(mempool, connman, sporkman, *llmq::quorumManager, *sigman, *shareman, *llmq::chainLocksHandler, *::masternodeSync, peerman, unit_tests, wipe);
43+
return llmq::quorumInstantSendManager.get();
44+
}()}
45+
{
5546
// NOTE: we use this only to wipe the old db, do NOT use it for anything else
5647
// TODO: remove it in some future version
5748
auto llmqDbTmp = std::make_unique<CDBWrapper>(unit_tests ? "" : (GetDataDir() / "llmq"), 1 << 20, unit_tests, true);
5849
}
5950

60-
void LLMQContext::Destroy() {
51+
LLMQContext::~LLMQContext() {
52+
// LLMQContext doesn't own these objects, but still need to care of them for consistancy:
6153
llmq::quorumInstantSendManager.reset();
6254
llmq::chainLocksHandler.reset();
63-
shareman.reset();
64-
sigman.reset();
6555
llmq::quorumManager.reset();
66-
qdkgsman.reset();
6756
llmq::quorumBlockProcessor.reset();
68-
dkg_debugman.reset();
69-
bls_worker.reset();
7057
{
7158
LOCK(llmq::cs_llmq_vbc);
7259
llmq::llmq_versionbitscache.Clear();
7360
}
7461
}
7562

7663
void LLMQContext::Interrupt() {
77-
if (shareman != nullptr) {
78-
shareman->InterruptWorkerThread();
79-
}
80-
if (llmq::quorumInstantSendManager != nullptr) {
81-
llmq::quorumInstantSendManager->InterruptWorkerThread();
82-
}
64+
shareman->InterruptWorkerThread();
65+
66+
assert(isman == llmq::quorumInstantSendManager.get());
67+
llmq::quorumInstantSendManager->InterruptWorkerThread();
8368
}
8469

8570
void LLMQContext::Start() {
86-
if (bls_worker != nullptr) {
87-
bls_worker->Start();
88-
}
89-
if (qdkgsman != nullptr) {
90-
qdkgsman->StartThreads();
91-
}
92-
if (llmq::quorumManager != nullptr) {
93-
llmq::quorumManager->Start();
94-
}
95-
if (shareman != nullptr) {
96-
shareman->RegisterAsRecoveredSigsListener();
97-
shareman->StartWorkerThread();
98-
}
99-
if (llmq::chainLocksHandler != nullptr) {
100-
llmq::chainLocksHandler->Start();
101-
}
102-
if (llmq::quorumInstantSendManager != nullptr) {
103-
llmq::quorumInstantSendManager->Start();
104-
}
71+
assert(quorum_block_processor == llmq::quorumBlockProcessor.get());
72+
assert(qman == llmq::quorumManager.get());
73+
assert(clhandler == llmq::chainLocksHandler.get());
74+
assert(isman == llmq::quorumInstantSendManager.get());
75+
76+
bls_worker->Start();
77+
qdkgsman->StartThreads();
78+
qman->Start();
79+
shareman->RegisterAsRecoveredSigsListener();
80+
shareman->StartWorkerThread();
81+
82+
llmq::chainLocksHandler->Start();
83+
llmq::quorumInstantSendManager->Start();
10584
}
10685

10786
void LLMQContext::Stop() {
108-
if (llmq::quorumInstantSendManager != nullptr) {
109-
llmq::quorumInstantSendManager->Stop();
110-
}
111-
if (llmq::chainLocksHandler != nullptr) {
112-
llmq::chainLocksHandler->Stop();
113-
}
114-
if (shareman != nullptr) {
115-
shareman->StopWorkerThread();
116-
shareman->UnregisterAsRecoveredSigsListener();
117-
}
118-
if (llmq::quorumManager != nullptr) {
119-
llmq::quorumManager->Stop();
120-
}
121-
if (qdkgsman != nullptr) {
122-
qdkgsman->StopThreads();
123-
}
124-
if (bls_worker != nullptr) {
125-
bls_worker->Stop();
126-
}
87+
assert(quorum_block_processor == llmq::quorumBlockProcessor.get());
88+
assert(qman == llmq::quorumManager.get());
89+
assert(clhandler == llmq::chainLocksHandler.get());
90+
assert(isman == llmq::quorumInstantSendManager.get());
91+
92+
llmq::quorumInstantSendManager->Stop();
93+
llmq::chainLocksHandler->Stop();
94+
95+
shareman->StopWorkerThread();
96+
shareman->UnregisterAsRecoveredSigsListener();
97+
qman->Stop();
98+
qdkgsman->StopThreads();
99+
bls_worker->Stop();
127100
}

‎src/llmq/context.h‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,27 +27,35 @@ class CInstantSendManager;
2727
}
2828

2929
struct LLMQContext {
30+
LLMQContext() = delete;
31+
LLMQContext(const LLMQContext&) = delete;
3032
LLMQContext(CEvoDB& evo_db, CTxMemPool& mempool, CConnman& connman, CSporkManager& sporkman,
3133
const std::unique_ptr<PeerManager>& peerman, bool unit_tests, bool wipe);
3234
~LLMQContext();
3335

34-
void Create(CEvoDB& evo_db, CTxMemPool& mempool, CConnman& connman, CSporkManager& sporkman,
35-
const std::unique_ptr<PeerManager>& peerman, bool unit_tests, bool wipe);
36-
void Destroy();
3736
void Interrupt();
3837
void Start();
3938
void Stop();
4039

41-
std::shared_ptr<CBLSWorker> bls_worker;
42-
std::unique_ptr<llmq::CDKGDebugManager> dkg_debugman;
43-
std::unique_ptr<llmq::CDKGSessionManager> qdkgsman;
44-
std::unique_ptr<llmq::CSigSharesManager> shareman;
45-
std::unique_ptr<llmq::CSigningManager> sigman;
46-
47-
llmq::CQuorumBlockProcessor* quorum_block_processor{nullptr};
48-
llmq::CQuorumManager* qman{nullptr};
49-
llmq::CChainLocksHandler* clhandler{nullptr};
50-
llmq::CInstantSendManager* isman{nullptr};
40+
/** Guaranteed if LLMQContext is initialized then all members are valid too
41+
*
42+
* Please note, that members here should not be re-ordered, because initialization
43+
* some of them requires other member initialized.
44+
* For example, constructor `quorumManager` requires `bls_worker`.
45+
*
46+
* Some objects are still global variables and their de-globalization is not trivial
47+
* at this point. LLMQContext keeps just a pointer to them and doesn't own these objects,
48+
* but it still guarantees that objects are created and valid
49+
*/
50+
const std::shared_ptr<CBLSWorker> bls_worker;
51+
const std::unique_ptr<llmq::CDKGDebugManager> dkg_debugman;
52+
llmq::CQuorumBlockProcessor* const quorum_block_processor;
53+
const std::unique_ptr<llmq::CDKGSessionManager> qdkgsman;
54+
llmq::CQuorumManager* const qman;
55+
const std::unique_ptr<llmq::CSigningManager> sigman;
56+
const std::unique_ptr<llmq::CSigSharesManager> shareman;
57+
llmq::CChainLocksHandler* const clhandler;
58+
llmq::CInstantSendManager* const isman;
5159
};
5260

5361
#endif // BITCOIN_LLMQ_CONTEXT_H

‎src/miner.cpp‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <governance/governance.h>
2828
#include <llmq/blockprocessor.h>
2929
#include <llmq/chainlocks.h>
30+
#include <llmq/context.h>
3031
#include <llmq/instantsend.h>
3132
#include <llmq/utils.h>
3233
#include <masternode/payments.h>
@@ -56,16 +57,15 @@ BlockAssembler::Options::Options() {
5657
}
5758

5859
BlockAssembler::BlockAssembler(const CSporkManager& sporkManager, CGovernanceManager& governanceManager,
59-
const llmq::CQuorumBlockProcessor& quorumBlockProcessor, llmq::CChainLocksHandler& clhandler,
60-
llmq::CInstantSendManager& isman, CEvoDB& evoDb, CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params, const Options& options) :
60+
LLMQContext& llmq_ctx, CEvoDB& evoDb, CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params, const Options& options) :
6161
chainparams(params),
6262
m_mempool(mempool),
6363
m_chainstate(chainstate),
6464
spork_manager(sporkManager),
6565
governance_manager(governanceManager),
66-
quorum_block_processor(quorumBlockProcessor),
67-
m_clhandler(clhandler),
68-
m_isman(isman),
66+
quorum_block_processor(*llmq_ctx.quorum_block_processor),
67+
m_clhandler(*llmq_ctx.clhandler),
68+
m_isman(*llmq_ctx.isman),
6969
m_evoDb(evoDb)
7070
{
7171
blockMinFeeRate = options.blockMinFeeRate;
@@ -91,9 +91,8 @@ static BlockAssembler::Options DefaultOptions()
9191
}
9292

9393
BlockAssembler::BlockAssembler(const CSporkManager& sporkManager, CGovernanceManager& governanceManager,
94-
const llmq::CQuorumBlockProcessor& quorumBlockProcessor, llmq::CChainLocksHandler& clhandler,
95-
llmq::CInstantSendManager& isman, CEvoDB& evoDb, CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params)
96-
: BlockAssembler(sporkManager, governanceManager, quorumBlockProcessor, clhandler, isman, evoDb, chainstate, mempool, params, DefaultOptions()) {}
94+
LLMQContext& llmq_ctx, CEvoDB& evoDb, CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params)
95+
: BlockAssembler(sporkManager, governanceManager, llmq_ctx, evoDb, chainstate, mempool, params, DefaultOptions()) {}
9796

9897
void BlockAssembler::resetBlock()
9998
{

‎src/miner.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class CConnman;
2323
class CGovernanceManager;
2424
class CScript;
2525
class CSporkManager;
26+
struct LLMQContext;
2627

2728
namespace Consensus { struct Params; };
2829
namespace llmq {
@@ -171,11 +172,9 @@ class BlockAssembler
171172
};
172173

173174
explicit BlockAssembler(const CSporkManager& sporkManager, CGovernanceManager& governanceManager,
174-
const llmq::CQuorumBlockProcessor& quorumBlockProcessor, llmq::CChainLocksHandler& clhandler,
175-
llmq::CInstantSendManager& isman, CEvoDB& evoDb, CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params);
175+
LLMQContext& llmq_ctx, CEvoDB& evoDb, CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params);
176176
explicit BlockAssembler(const CSporkManager& sporkManager, CGovernanceManager& governanceManager,
177-
const llmq::CQuorumBlockProcessor& quorumBlockProcessor, llmq::CChainLocksHandler& clhandler,
178-
llmq::CInstantSendManager& isman, CEvoDB& evoDb, CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params, const Options& options);
177+
LLMQContext& llmq_ctx, CEvoDB& evoDb, CChainState& chainstate, const CTxMemPool& mempool, const CChainParams& params, const Options& options);
179178

180179
/** Construct a new block template with coinbase to scriptPubKeyIn */
181180
std::unique_ptr<CBlockTemplate> CreateNewBlock(const CScript& scriptPubKeyIn);

‎src/rpc/evo.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ static std::string SignAndSendSpecialTx(const JSONRPCRequest& request, const CMu
329329
LOCK(cs_main);
330330

331331
TxValidationState state;
332-
if (!CheckSpecialTx(CTransaction(tx), ::ChainActive().Tip(), state, ::ChainstateActive().CoinsTip(), true)) {
332+
if (!CheckSpecialTx(CTransaction(tx), ::ChainActive().Tip(), ::ChainstateActive().CoinsTip(), true, state)) {
333333
throw std::runtime_error(state.ToString());
334334
}
335335
} // cs_main
@@ -1467,7 +1467,7 @@ static UniValue protx_diff(const JSONRPCRequest& request)
14671467

14681468
CSimplifiedMNListDiff mnListDiff;
14691469
std::string strError;
1470-
LLMQContext& llmq_ctx = EnsureLLMQContext(request.context);
1470+
const LLMQContext& llmq_ctx = EnsureLLMQContext(request.context);
14711471

14721472
if (!BuildSimplifiedMNListDiff(baseBlockHash, blockHash, mnListDiff, *llmq_ctx.quorum_block_processor, strError, extended)) {
14731473
throw std::runtime_error(strError);

0 commit comments

Comments
 (0)