Repository navigation
feat: run governance on pruned nodes #7860
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
Changes from all commits
4b5e94f
b3304f5
6490d96
8229bf6
f2ade38
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| Governance | ||
| ---------- | ||
|
|
||
| - Pruned nodes can now run governance. Proposal collateral is now looked up | ||
| in a new governance collateral index instead of `-txindex`. The index | ||
| stores only the transactions that could be a proposal's collateral, works | ||
| with `-prune`, and always runs; there is no option for it. Pruned nodes, and | ||
| nodes started with `-txindex=0`, now sync proposals and votes, check | ||
| superblock payments in full, and can create and submit proposals. | ||
|
|
||
| - Every node builds the index once, on the first start after upgrading. It | ||
| reads every block for that, and the node's governance sync waits until it | ||
| has finished. | ||
|
|
||
| - A node that pruned blocks before it had this index can't build it, because | ||
| the index has to start from the genesis block. It now starts with governance | ||
| turned off and a warning instead of failing; restarting it once with | ||
| `-reindex` downloads the blockchain again and enables governance, and the | ||
| node stays pruned. This replaces the warning such nodes showed before, that | ||
| governance is disabled because the node is pruned. | ||
|
|
||
| - Dash-Qt used to save `disablegovernance=1` in `settings.json` when pruning | ||
| was turned on in the options. That setting stays until it is removed, so | ||
| governance stays off on those nodes. | ||
|
|
||
| - `gobject prepare` and `gobject submit` now fail straight away on a node that | ||
| can't check or relay proposals. Before, `gobject prepare` burned the | ||
| proposal fee and only `gobject submit` failed afterwards, with the | ||
| misleading error `Invalid proposal collateral`. Collateral errors now also | ||
| say what is wrong with the collateral instead of only | ||
| `Invalid proposal collateral`. | ||
|
|
||
| - Pruned nodes now check superblock payments against the governance | ||
| triggers they know, like full nodes do, instead of only checking that a | ||
| superblock stays within the budget. | ||
|
|
||
| - `getindexinfo` reports the new index as `govcollateralindex`. | ||
|
|
||
| - Starting with `-prune` and `-disablegovernance=0` is no longer an error. | ||
| Governance validation no longer needs `-txindex`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // Copyright (c) 2026 The Dash Core developers | ||
| // Distributed under the MIT software license, see the accompanying | ||
| // file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
|
||
| #include <governance/collateral.h> | ||
|
|
||
| #include <primitives/transaction.h> | ||
| #include <script/script.h> | ||
| #include <span.h> | ||
|
|
||
| #include <algorithm> | ||
|
|
||
| namespace governance { | ||
| bool CollateralInfo::BurnsAtLeast(const uint256& hash, CAmount amount) const | ||
| { | ||
| return std::any_of(burns.begin(), burns.end(), | ||
| [&](const auto& burn) { return burn.first == hash && burn.second >= amount; }); | ||
| } | ||
|
|
||
| CollateralInfo GetCollateralInfo(const CTransaction& tx, const uint256& block_hash) | ||
| { | ||
| CollateralInfo info{block_hash, /*outputs_standard=*/true, /*burns=*/{}}; | ||
| for (const auto& output : tx.vout) { | ||
| const CScript& script{output.scriptPubKey}; | ||
| if (!script.IsPayToPublicKeyHash() && !script.IsUnspendable()) { | ||
| info.outputs_standard = false; | ||
| } | ||
| // Exactly the script `CScript() << OP_RETURN << ToByteVector(hash)` produces | ||
| if (script.size() == 2 + uint256::size() && script[0] == OP_RETURN && script[1] == uint256::size()) { | ||
| info.burns.emplace_back(uint256{Span{script}.subspan(2)}, output.nValue); | ||
| } | ||
| } | ||
| return info; | ||
| } | ||
| } // namespace governance |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| // Copyright (c) 2026 The Dash Core developers | ||
| // Distributed under the MIT software license, see the accompanying | ||
| // file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
|
||
| #ifndef BITCOIN_GOVERNANCE_COLLATERAL_H | ||
| #define BITCOIN_GOVERNANCE_COLLATERAL_H | ||
|
|
||
| #include <consensus/amount.h> | ||
| #include <serialize.h> | ||
| #include <uint256.h> | ||
|
|
||
| #include <optional> | ||
| #include <utility> | ||
| #include <vector> | ||
|
|
||
| class CTransaction; | ||
|
|
||
| static constexpr CAmount GOVERNANCE_PROPOSAL_FEE_TX = (1 * COIN); | ||
|
|
||
| namespace governance { | ||
| /** The parts of a mined transaction that decide whether it is valid proposal collateral. */ | ||
| struct CollateralInfo { | ||
| uint256 block_hash; | ||
| //! Whether every output pays to P2PKH or is unspendable. | ||
| bool outputs_standard{false}; | ||
| //! Value burned to each `OP_RETURN <32-byte hash>` output, together with that hash. | ||
| std::vector<std::pair<uint256, CAmount>> burns; | ||
|
|
||
| SERIALIZE_METHODS(CollateralInfo, obj) { READWRITE(obj.block_hash, obj.outputs_standard, obj.burns); } | ||
|
|
||
| bool BurnsAtLeast(const uint256& hash, CAmount amount) const; | ||
| }; | ||
|
|
||
| CollateralInfo GetCollateralInfo(const CTransaction& tx, const uint256& block_hash); | ||
|
|
||
| /** Where proposal collateral is looked up. Implemented by GovCollateralIndex. */ | ||
| class CollateralSource | ||
| { | ||
| public: | ||
| virtual ~CollateralSource() = default; | ||
|
|
||
| //! Whether every block up to the active tip has been read. Until then, a transaction that isn't | ||
| //! found may still be valid collateral. | ||
| virtual bool IsCollateralSynced() const = 0; | ||
|
|
||
| virtual std::optional<CollateralInfo> FindCollateral(const uint256& txid) const = 0; | ||
| }; | ||
| } // namespace governance | ||
|
|
||
| #endif // BITCOIN_GOVERNANCE_COLLATERAL_H |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -80,6 +80,8 @@ void NetGovernance::Schedule(CScheduler& scheduler) | |||||||||||||||||||||||||||||
| void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDataStream& vRecv) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| if (!m_gov_manager.IsValid()) return; | ||||||||||||||||||||||||||||||
| // Until proposal collateral can be looked up, valid proposals would look invalid and their senders get punished | ||||||||||||||||||||||||||||||
| if (m_gov_manager.IsCollateralSourceBehind()) return; | ||||||||||||||||||||||||||||||
| if (!m_node_sync.IsBlockchainSynced()) return; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // ANOTHER USER IS ASKING US TO HELP THEM SYNC GOVERNANCE OBJECT DATA | ||||||||||||||||||||||||||||||
|
|
@@ -275,12 +277,21 @@ bool NetGovernance::AlreadyHave(const CInv& inv) | |||||||||||||||||||||||||||||
| if (inv.type != MSG_GOVERNANCE_OBJECT && inv.type != MSG_GOVERNANCE_OBJECT_VOTE) { | ||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| // When governance isn't loaded (e.g. -disablegovernance), claim we already have | ||||||||||||||||||||||||||||||
| // the item so we don't fetch or track it in the net-layer request tracker. | ||||||||||||||||||||||||||||||
| if (!m_gov_manager.IsValid()) return true; | ||||||||||||||||||||||||||||||
| // When governance isn't loaded (e.g. -disablegovernance) or can't check collateral yet, claim we | ||||||||||||||||||||||||||||||
| // already have the item so we don't fetch or track it in the net-layer request tracker. | ||||||||||||||||||||||||||||||
| if (!m_gov_manager.IsValid() || m_gov_manager.IsCollateralSourceBehind()) return true; | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On an upgraded, already blockchain-synced node whose new index is doing its initial scan, this reports every governance object/vote INV as already held, so it is neither fetched nor queued for later. The sending peer has already recorded the hash in its per-connection known-inventory filter, and the later full AGENTS.md reference: AGENTS.md:L243-L243 Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 79df37e. Announcements received during the index scan are now held per peer, not dropped, and are requested from the announcing peer once the index has synced. The details are in the reply to the matching thepastaclaw thread on the same line. 🤖 Posted autonomously by Claude on behalf of pasta. |
||||||||||||||||||||||||||||||
| return !m_gov_manager.ConfirmInventoryRequest(inv); | ||||||||||||||||||||||||||||||
|
Comment on lines
+280
to
283
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🔴 Blocking: Retain governance inventory ignored during index catch-up On an upgraded, blockchain-synced node, this returns true for governance announcements throughout the new index's initial scan, so the request tracker never records or fetches them. The announcing peer nevertheless records each transmitted hash in its per-connection known-inventory filter ( source:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 79df37e, folded into "feat(governance): run governance on pruned nodes". You're right that the fix has to be on our side: most of the peers answering our sync requests run old versions, and their
The test now keeps the sender connected while the index finishes, and doesn't inject the INV a second time. It announces the object once while the index is behind, checks that 🤖 Posted autonomously by Claude on behalf of pasta.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still applies (re-reviewed at
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 6f28f94: when a peer's announcements overflow the held list, the peer is now disconnected and recovered through governance sync over a new connection. Details are in the reply on the new blocking thread at 🤖 Posted autonomously by Claude on behalf of pasta.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved (re-reviewed at |
||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| bool NetGovernance::DeferAnnouncement(const CInv& inv) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| if (inv.type != MSG_GOVERNANCE_OBJECT && inv.type != MSG_GOVERNANCE_OBJECT_VOTE) { | ||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| // Hold exactly the announcements ProcessMessage() would drop the objects of | ||||||||||||||||||||||||||||||
| return m_gov_manager.IsValid() && (m_gov_manager.IsCollateralSourceBehind() || !m_node_sync.IsBlockchainSynced()); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+286
to
+293
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🟡 Suggestion: Defer governance announcements until blockchain sync completes If the collateral index becomes ready while CMasternodeSync is still in its blockchain stage, this hook stops deferring and SendMessages drains the buffered inventory. However, ConfirmInventoryRequest returns false while IsBlockchainSynced() is false, causing AlreadyHave to report every entry as held; std::erase_if then removes those entries without scheduling requests. This can occur during startup or when mnsync is reset during the index scan. The sender's known-inventory filter subsequently suppresses those announcements on the same connection. Continue deferring until both the collateral source and blockchain-sync state permit requesting governance inventory, and exercise that readiness transition in the existing regression.
Suggested change
source:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved (re-reviewed at |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| bool NetGovernance::ProcessGetData(CNode& pfrom, const CInv& inv, const CNetMsgMaker& msgMaker) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| if (inv.type == MSG_GOVERNANCE_OBJECT) { | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a node whose index is ready sends
MNGOVERNANCESYNCto a peer that is still doing its initial index scan, this blanket return drops the sync request as well as object payloads.SyncManager::ProcessTickmarks that peer'sgovernance-syncrequest fulfilled before sending it and never retries it, so if the available peers are still scanning, the requester times out and advances to FINISHED without their cached objects or votes. The collateral-readiness gate should still allow sync requests to be served, or the requester needs a readiness response that causes a retry.AGENTS.md reference: AGENTS.md:L243-L243
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not changing this here. Moving the catch-up gate below the
MNGOVERNANCESYNCbranch wouldn't change anything: the next check in that branch,if (!m_node_sync.IsSynced()) return;, also drops the request, anddevelopalready has it. A node in the middle of mnsync doesn't serve governance sync today. What this PR changes is that an upgraded node stays in that state for the length of its first index build.A requester whose request was dropped still syncs from its other peers. Serving sync requests during catch-up would mean finishing mnsync before the index has synced. That is the alternative listed under "Still open" item 1 in the PR description (only hold back proposals while the index catches up). It also needs masternodes to stop voting on triggers while proposals are missing, so I'd rather decide it separately from this fix.
🤖 Posted autonomously by Claude on behalf of pasta.