forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
refactor: update lock annotations for CGovernanceObject
and GovernanceStore
, move away from RecursiveMutex
es
#6849
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
14
commits into
dashpay:develop
Choose a base branch
from
kwvg:gov_mutex
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.
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
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
5 tasks
This pull request has conflicts, please rebase. |
This pull request has conflicts, please rebase. |
This pull request has conflicts, please rebase. |
PastaPastaPasta
added a commit
that referenced
this pull request
Oct 20, 2025
…`GovernanceSigner`, drop `Relay()`s and use periodic relay instead, minor cleanup b51cd1d lint: apply most `clang-format` suggestions, update circular allowlist (Kittywhiskers Van Gogh) 700e069 refactor: cleanup headers and forward decls (Kittywhiskers Van Gogh) c516fd3 refactor: apply review suggestions (Kittywhiskers Van Gogh) d19e1f3 fix: add `nullptr` check before using `FindGovernanceObject()` retval (Kittywhiskers Van Gogh) 1777919 refactor: use `std::chrono` for governance time constants (Kittywhiskers Van Gogh) 4d96f5f refactor: move `DoMaintenance()` inside `Schedule()` (Kittywhiskers Van Gogh) e224991 governance: add lock annotations for `cs_relay` (Kittywhiskers Van Gogh) df589c7 governance: introduce task for relaying governance objects (Kittywhiskers Van Gogh) 3811924 governance: drop `Relay()` from `CGovernance{Object,Vote}` (Kittywhiskers Van Gogh) 63448ff refactor: abstract away parent implementation from signer (Kittywhiskers Van Gogh) 757ded3 refactor: remove need for access to private members (Kittywhiskers Van Gogh) bd6af83 refactor: separate masternode mode logic into dedicated signer class (Kittywhiskers Van Gogh) 2b74e15 refactor: move masternode mode logic to `governance/signing.cpp` (Kittywhiskers Van Gogh) fefbe27 refactor: replace `Sign` functions with direct activeman call (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Depends on #6828 * Depends on #6842 * Depends on #6877 * Dependency for #6849 * To reduce the proliferation of `PeerManager` in governance logic, relaying has been centralized to `CGovernanceManager` and now done through a scheduled task that processes a queue of requests. This reduces the amount of code that needs to fetch a reference to `PeerManager` in order to queue a message for relay and brings us closer to dropping it as a member in other objects. * On test chains, the queue of requests is processed every second but on mainnet, it is processed every 5 seconds. This is because functional tests assume instantaneous relay and fail if it takes too long to relay messages (as cascading delays cause test timeout). * Likewise, to reduce the references to `CActiveMasternodeManager`, signing logic is consolidated to `SignBasic()` with the objects using simple getter/setters (or direct assignment) where applicable. ## 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 b51cd1d knst: utACK b51cd1d Tree-SHA512: cd4bba1e7314695a215a8a83e2d5319be8d2eb2e88eb42f641f6c88ca6d74ee68ced9e95475c6b1d93f5b061993efa6a195d23f61a8775f920867d9836a0dcf8
`cmapVoteToObject` isn't (de)ser'ed so we move it to the manager and don't consider it for the mutex
Review with `git log -p -n1 --color-moved=dimmed_zebra`.
This will simplify our work in subsequent commits. Review with `git log -p -n1 --color-moved=dimmed_zebra`.
Needed to make the next commit easier to follow.
Currently, the LOCKs aren't guarding anything. Variables protected were based on existing LOCK'ing patterns (i.e. (de)ser'ed variables except for m_obj).
GovernanceStore
, CGovernanceManager
and CGovernanceStore
, move away from RecursiveMutex
esGovernanceStore
and CGovernanceManager
, move away from RecursiveMutex
es
GovernanceStore
and CGovernanceManager
, move away from RecursiveMutex
esGovernanceStore
and CGovernanceObject
, move away from RecursiveMutex
es
GovernanceStore
and CGovernanceObject
, move away from RecursiveMutex
esCGovernanceObject
and GovernanceStore
, move away from RecursiveMutex
es
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
GovernanceSigner
, dropRelay()
s and use periodic relay instead, minor cleanup #6838Breaking Changes
None expected.
Checklist