forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
refactor: extract CActiveMasternodeManager from LLMQContext (3/n, DKG session isolation, ActiveContext consolidation)
#7065
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.
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
Draft
kwvg
wants to merge
12
commits into
dashpay:develop
Choose a base branch
from
kwvg:active_p4
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,897
−1,788
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 tasks
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
CActiveMasternodeManager from LLMQContext (3/3, DKG session isolation, ActiveContext consolidation)CActiveMasternodeManager from LLMQContext (3/n, DKG session isolation, ActiveContext consolidation)
5 tasks
|
This pull request has conflicts, please rebase. |
PastaPastaPasta
added a commit
that referenced
this pull request
Jan 3, 2026
…ontext` (2/n, `CQuorumManager` handler separation) d41e5bd chore: apply review suggestions (Kittywhiskers Van Gogh) 8a3ad09 refactor: abstract away parent implementation from handler (Kittywhiskers Van Gogh) 718ee50 refactor: streamline quorum cache logic (Kittywhiskers Van Gogh) 1360d9d refactor: drop unnecessary `CConnman` argument in handlers (Kittywhiskers Van Gogh) 2a57a44 refactor: move quorum manager to separate source file (Kittywhiskers Van Gogh) 50a5f26 refactor: pull handler initialization out of `CQuorumManager` (Kittywhiskers Van Gogh) 61da16d refactor: separate observer/common routines into dedicated class (Kittywhiskers Van Gogh) e8b771e move-only: move observer/common routines to `llmq/observer` (Kittywhiskers Van Gogh) 4078de0 refactor: disentangle watch-only and masternode mode conn checking (Kittywhiskers Van Gogh) a2d909b refactor: pull deletable quorums routine to dedicated function (Kittywhiskers Van Gogh) eaee1a8 refactor: disentangle watch-only and masternode mode thread triggers (Kittywhiskers Van Gogh) e6d8e69 refactor: pull data recovery lambda to a dedicated function (Kittywhiskers Van Gogh) 1fe85be refactor: separate observer/participant logic into dedicated class (Kittywhiskers Van Gogh) ba24b4e move-only: move observer/participant logic to `active/quorums.cpp` (Kittywhiskers Van Gogh) de0ce4e refactor: extract masternode-mode specific `SetSecretKeyShare()` (Kittywhiskers Van Gogh) 201ccfc refactor: extract `ENCRYPTED_CONTRIBUTIONS` processing case (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Depends on #7062 * Dependency for #7065 * To enforce the split between masternode mode (which can participate in quorums and seek quorum data) and watch-only mode (which can **only** seek quorum data), threading logic is split between `StartDataRecoveryThread()` and `StartVvecSyncThread()`, they both call the same underlying `DataRecoveryThread()` but one has access to masternode-specific parameters and the other does not. This becomes relevant as the entities are split out and the access to specific parameters are enforced by the relevant class members outright not existing. * It is recommended to use `TryStartVvecSyncThread()` as it will not start threads if no data is actually needed, calling `StartVvecSyncThread()` directly bypasses this check. * `CQuorumManager` exposes both `IsWatching()` and `IsMasternode()` to allow P2P code and interfaces to query the node's state (this is most relevant in `PeerManager` which can trivially detect masternode mode but not watch-only status). * Watch-only nodes cannot be masternodes but masternodes can **also** be watch-only (the term "observer" has been used in the codebase where possible as watch-only becomes a bit of a misnomer in this case but is the established term) nodes. * The `CQuorumManager` cache warmer was one of the tasks allocated to the common worker pool, as the rest of the activities are managed by the observer context, rather than keeping the worker pool in the quorum manager and then exposing it through the interface for the sake of one task, the worker pool has been moved to the observer context and we have a regular thread for the quorum manager instead. ## Breaking Changes None expected. ## 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 - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK d41e5bd Tree-SHA512: 9d4f0c67149dcec10d50a74dafe18b165590b8977203f82f3104645bf5a6e1e109fa901c1aaebf543ebc215aa910bd47b8b88dc993a66363655fb0c6ea57ed73
Required for upcoming commits.
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
We also need to move CCoinJoinServer creation back into ActiveContext
Also: - Drop stale TODO, remove headers in code that no longer refer to masternode capabilities
Now that `ActiveNotificationInterface` exclusively calls `ActiveContext` we can now merge them together. Also: - Drop `LLMQContext` from `ActiveContext`, only import what we use
Also: - Headers cleanup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Additional Information
Depends on refactor: extract
CActiveMasternodeManagerfromLLMQContext(2/n,CQuorumManagerhandler separation) #7063Dependency for refactor: extract
CActiveMasternodeManagerfromLLMQContext(4/n, followups) #7066Breaking Changes
None expected.
Checklist: