Skip to content

Commit c965990

Browse files
Merge #7113: refactor: new chainlock::Chainlocks interface
0f1f8f1 fix: reset chainlock; make objects `chainlocks` and `clhandler` consistent (Konstantin Akimov) a418c9f fmt: clang-format for all changes in this PR (Konstantin Akimov) 5deb316 refactor: rename llmq::CChainLocksHandler to chainlock::ChainlockHandler (Konstantin Akimov) b57b68c fix: misusing chainlocks to determine expiring of cj's tx (Konstantin Akimov) a894ff7 refactor: drop clhandler and chainloks from dsnotificationinterface completely (Konstantin Akimov) 040a458 refactor: don't call m_chainlocks directly in dsnotificationinterface (Konstantin Akimov) ac22100 fix: drop dependency of CChainLocksHandler on qman; so CChainLocksHandler could be initialized before llmq_ctx (Konstantin Akimov) a0b6483 refactor: move out VerifyChainlock from CChainLocksHandler as a function (Konstantin Akimov) 5d97d57 refactor: move clhandler from llmq_ctx to NodeContext (Konstantin Akimov) fdfc092 fix: it should be used ChainManager inside chainlock's handler, not chainstate (Konstantin Akimov) 0d90bc1 refactor: break dependency of clhandler on chainlock/signing completely (Konstantin Akimov) b79e2a8 refactor: chainlocks's signer to have own scheduler for cleanup and signing (Konstantin Akimov) cf2e9b5 refactor: drop unused validation.h from spork (Konstantin Akimov) 34c05d3 refactor: move all spork usages for chainlock inside single module chainlock/chainlock (Konstantin Akimov) 3bb3433 refactor: split out lightweight chainlock::Chainlocks and widely used it interface (Konstantin Akimov) 57f661c refactor: code-move only CChainLockHandler from chainlock.h to handler.h (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Currently, there are several blockers that prevent pulling consensus code out from various handlers, schedulers, mempool, etc. One of them is `CChainLocksHandler` which does too many things. - `CChainLocksHandler` depends on the mempool and — because it is part of `LLMQContext` — it makes `LLMQContext` depend on the mempool (which is an unexpected dependency) - Chainlocks depends on Instant Send, and Instant Send depends on Chainlocks (this is logically expected), but it creates a circular dependency between `CChainLocksHandler` and the initialization of these objects. See: `CChainLocksHandler::Start(const llmq::CInstantSendManager& isman)` and `CInstantSendManager::CInstantSendManager(CChainLocksHandler& _clhandler, ...)` - `CChainLocksHandler` depends on masternode-only code (`chainlock::ChainLockSigner`). It is hidden behind the interface `chainlock::ChainLockSignerParent`, but this increases implementation complexity - `CChainLocksHandler` is used for blocks validation (`clhandler.HasConflictingChainLock`), but at the same time `CChainLocksHandler` can switch the chain tip itself via its background worker (see calls to `EnforceBestChainLock`). This creates another dependency loop that prevents independent initialization of `CChainLocksHandler` and the Chain State. ## What was done? This PR introduces a new lightweight interface `chainlock::Chainlocks` - `chainlock::Chainlocks` knows about the chain state and the best known chainlock, but it does not enforce chainlocks, has no dependency on mempool or transactions, and keeps no permanent on-disk state (no dependency on evodb). It does depend on `SporkManager` (to know whether chainlocks are enabled/disabled) and provides a clean interface over it. - `llmq::CChainLocksHandler` (`chainlock::ChainlocksHandler` in future commits) is the component that actually changes Chain State by enforcing the best tip, tracks transaction's times and runs a background thread. It is responsible for processing newly signed chainlocks and related network messages. However, it no longer handles any masternode-related logic or signing — that code was fully moved to `llmq::ChainLockSigner`. Other less important but noticeable changes: - `clhandler` now uses its own subscriptions to validation notifications (no longer goes through `dsnotificationinterface`) - `llmq::CChainLocksHandler` was renamed and moved to a different namespace (`chainlock::ChainlocksHandler`) since it no longer belongs to `llmq::` - `clhandler` was removed from `llmq-ctx` and its initialization can now be skipped completely [as required for kernel project]; only `ChainLockSigner` and `PeerManager` still strongly depend on it - `clhandler` no longer depends on `CQuorumManager` Noticeable, some circular dependencies over codebase has been untangled by this PR: - chainlock/chainlock <-> instantsend/instantsend - chainlock/chainlock <-> validation See linter's exception list for details. After this PR the next step to apply similar refactoring to `CInstantSendManager` which does too many things the similar to CChainLocksHandler: keeps track of instant-send locked transactions [as expected], but also knows about masternode's code [see `instantsend::InstantSendSigner`], removes conflicting transactions from mempool, invalidates blocks in Chain State (see `CInstantSendManager::ResolveBlockConflicts`). CInstantSendManager creates logical dependencies over masternode's signer; over chainstate; over mempool and finally there are the same issues with initialization of `isman` as `clhandler` had. ## How Has This Been Tested? Run unit & functional tests ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK 0f1f8f1 kwvg: utACK 0f1f8f1 Tree-SHA512: bee59b0e0e0d3530e562a8381303cf4bca721550c412ddc34edc77faee17ec02469e44f5d37fb3801fb789924b257e4dc12ac39d5ceb32087eb242c57a9bc1f3
2 parents 1ea35a4 + 0f1f8f1 commit c965990

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+985
-833
lines changed

src/Makefile.am

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ BITCOIN_CORE_H = \
174174
blockfilter.h \
175175
chain.h \
176176
chainlock/chainlock.h \
177+
chainlock/handler.h \
177178
chainlock/clsig.h \
178179
chainlock/signing.h \
179180
chainparams.h \
@@ -499,6 +500,7 @@ libbitcoin_node_a_SOURCES = \
499500
chain.cpp \
500501
chainlock/chainlock.cpp \
501502
chainlock/clsig.cpp \
503+
chainlock/handler.cpp \
502504
chainlock/signing.cpp \
503505
coinjoin/coinjoin.cpp \
504506
coinjoin/server.cpp \

src/active/context.cpp

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
#include <active/dkgsessionhandler.h>
88
#include <active/masternode.h>
99
#include <active/quorums.h>
10-
#include <chainlock/chainlock.h>
10+
#include <chainlock/handler.h>
1111
#include <chainlock/signing.h>
1212
#include <governance/governance.h>
1313
#include <governance/signing.h>
@@ -19,20 +19,19 @@
1919
#include <llmq/ehf_signals.h>
2020
#include <llmq/quorumsman.h>
2121
#include <llmq/signing_shares.h>
22-
2322
#include <util/check.h>
2423
#include <validation.h>
24+
#include <validationinterface.h>
2525

2626
ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman, CConnman& connman,
27-
CDeterministicMNManager& dmnman, CGovernanceManager& govman,
28-
CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, CSporkManager& sporkman,
29-
CTxMemPool& mempool, llmq::CChainLocksHandler& clhandler, llmq::CInstantSendManager& isman,
30-
llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,
31-
llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman,
32-
PeerManager& peerman, const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
33-
const llmq::QvvecSyncModeMap& sync_map, const util::DbWrapperParams& db_params,
34-
bool quorums_recovery, bool quorums_watch) :
35-
m_clhandler{clhandler},
27+
CDeterministicMNManager& dmnman, CGovernanceManager& govman, CMasternodeMetaMan& mn_metaman,
28+
CMNHFManager& mnhfman, CSporkManager& sporkman, const chainlock::Chainlocks& chainlocks,
29+
CTxMemPool& mempool, chainlock::ChainlockHandler& clhandler,
30+
llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman,
31+
llmq::CQuorumManager& qman, llmq::CQuorumSnapshotManager& qsnapman,
32+
llmq::CSigningManager& sigman, PeerManager& peerman, const CMasternodeSync& mn_sync,
33+
const CBLSSecretKey& operator_sk, const llmq::QvvecSyncModeMap& sync_map,
34+
const util::DbWrapperParams& db_params, bool quorums_recovery, bool quorums_watch) :
3635
m_isman{isman},
3736
m_qman{qman},
3837
nodeman{std::make_unique<CActiveMasternodeManager>(connman, dmnman, operator_sk)},
@@ -44,9 +43,9 @@ ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman
4443
ehf_sighandler{std::make_unique<llmq::CEHFSignalsHandler>(chainman, mnhfman, sigman, *shareman, qman)},
4544
qman_handler{std::make_unique<llmq::QuorumParticipant>(bls_worker, connman, dmnman, qman, qsnapman, *nodeman, chainman,
4645
mn_sync, sporkman, sync_map, quorums_recovery, quorums_watch)},
47-
cl_signer{std::make_unique<chainlock::ChainLockSigner>(chainman.ActiveChainstate(), clhandler, sigman, *shareman,
48-
sporkman, mn_sync)},
49-
is_signer{std::make_unique<instantsend::InstantSendSigner>(chainman.ActiveChainstate(), clhandler, isman, sigman,
46+
cl_signer{std::make_unique<chainlock::ChainLockSigner>(chainman.ActiveChainstate(), chainlocks, clhandler, isman,
47+
qman, sigman, *shareman, mn_sync)},
48+
is_signer{std::make_unique<instantsend::InstantSendSigner>(chainman.ActiveChainstate(), chainlocks, isman, sigman,
5049
*shareman, qman, sporkman, mempool, mn_sync)}
5150
{
5251
qdkgsman->InitializeHandlers([&](const Consensus::LLMQParams& llmq_params,
@@ -55,7 +54,6 @@ ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman
5554
qblockman, qsnapman, *nodeman, chainman, sporkman,
5655
llmq_params, quorums_watch, quorum_idx);
5756
});
58-
m_clhandler.ConnectSigner(cl_signer.get());
5957
m_isman.ConnectSigner(is_signer.get());
6058
m_qman.ConnectManagers(qman_handler.get(), qdkgsman.get());
6159
}
@@ -64,7 +62,6 @@ ActiveContext::~ActiveContext()
6462
{
6563
m_qman.DisconnectManagers();
6664
m_isman.DisconnectSigner();
67-
m_clhandler.DisconnectSigner();
6865
}
6966

7067
void ActiveContext::Interrupt()
@@ -77,16 +74,22 @@ void ActiveContext::Start(CConnman& connman, PeerManager& peerman)
7774
qman_handler->Start();
7875
qdkgsman->StartThreads(connman, peerman);
7976
shareman->Start();
77+
cl_signer->Start();
8078
cl_signer->RegisterRecoveryInterface();
8179
is_signer->RegisterRecoveryInterface();
8280
shareman->RegisterRecoveryInterface();
81+
82+
RegisterValidationInterface(cl_signer.get());
8383
}
8484

8585
void ActiveContext::Stop()
8686
{
87+
UnregisterValidationInterface(cl_signer.get());
88+
8789
shareman->UnregisterRecoveryInterface();
8890
is_signer->UnregisterRecoveryInterface();
8991
cl_signer->UnregisterRecoveryInterface();
92+
cl_signer->Stop();
9093
shareman->Stop();
9194
qdkgsman->StopThreads();
9295
qman_handler->Stop();

src/active/context.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@ class CTxMemPool;
2929
class GovernanceSigner;
3030
class PeerManager;
3131
namespace chainlock {
32+
class Chainlocks;
33+
class ChainlockHandler;
3234
class ChainLockSigner;
3335
} // namespace chainlock
3436
namespace instantsend {
3537
class InstantSendSigner;
3638
} // namespace instantsend
3739
namespace llmq {
38-
class CChainLocksHandler;
3940
class CDKGDebugManager;
4041
class CDKGSessionManager;
4142
class CEHFSignalsHandler;
@@ -53,7 +54,6 @@ struct DbWrapperParams;
5354

5455
struct ActiveContext final : public CValidationInterface {
5556
private:
56-
llmq::CChainLocksHandler& m_clhandler;
5757
llmq::CInstantSendManager& m_isman;
5858
llmq::CQuorumManager& m_qman;
5959

@@ -63,13 +63,13 @@ struct ActiveContext final : public CValidationInterface {
6363
ActiveContext& operator=(const ActiveContext&) = delete;
6464
explicit ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman, CConnman& connman,
6565
CDeterministicMNManager& dmnman, CGovernanceManager& govman, CMasternodeMetaMan& mn_metaman,
66-
CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool,
67-
llmq::CChainLocksHandler& clhandler, llmq::CInstantSendManager& isman,
68-
llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,
69-
llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman, PeerManager& peerman,
70-
const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
71-
const llmq::QvvecSyncModeMap& sync_map, const util::DbWrapperParams& db_params,
72-
bool quorums_recovery, bool quorums_watch);
66+
CMNHFManager& mnhfman, CSporkManager& sporkman, const chainlock::Chainlocks& chainlocks,
67+
CTxMemPool& mempool, chainlock::ChainlockHandler& clhandler,
68+
llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman,
69+
llmq::CQuorumManager& qman, llmq::CQuorumSnapshotManager& qsnapman,
70+
llmq::CSigningManager& sigman, PeerManager& peerman, const CMasternodeSync& mn_sync,
71+
const CBLSSecretKey& operator_sk, const llmq::QvvecSyncModeMap& sync_map,
72+
const util::DbWrapperParams& db_params, bool quorums_recovery, bool quorums_watch);
7373
~ActiveContext();
7474

7575
void Interrupt();

src/bench/rpc_blockchain.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ static void BlockToJsonVerbose(benchmark::Bench& bench)
4545
TestBlockAndIndex data;
4646
const LLMQContext& llmq_ctx = *data.testing_setup->m_node.llmq_ctx;
4747
bench.run([&] {
48-
auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, &data.blockindex, &data.blockindex, *llmq_ctx.clhandler, *llmq_ctx.isman, TxVerbosity::SHOW_DETAILS_AND_PREVOUT);
48+
auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, &data.blockindex, &data.blockindex, *data.testing_setup->m_node.chainlocks, *llmq_ctx.isman, TxVerbosity::SHOW_DETAILS_AND_PREVOUT);
4949
ankerl::nanobench::doNotOptimizeAway(univalue);
5050
});
5151
}
@@ -56,7 +56,7 @@ static void BlockToJsonVerboseWrite(benchmark::Bench& bench)
5656
{
5757
TestBlockAndIndex data;
5858
const LLMQContext& llmq_ctx = *data.testing_setup->m_node.llmq_ctx;
59-
auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, &data.blockindex, &data.blockindex, *llmq_ctx.clhandler, *llmq_ctx.isman, TxVerbosity::SHOW_DETAILS_AND_PREVOUT);
59+
auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, &data.blockindex, &data.blockindex, *data.testing_setup->m_node.chainlocks, *llmq_ctx.isman, TxVerbosity::SHOW_DETAILS_AND_PREVOUT);
6060
bench.run([&] {
6161
auto str = univalue.write();
6262
ankerl::nanobench::doNotOptimizeAway(str);

0 commit comments

Comments
 (0)