Repository navigation
fix(net): announce sporks to new peers instead of requesting them during mnsync - #7893
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughAfter VERACK, the peer manager queues inventory announcements for active sporks to test-chain peers. The sync tick no longer requests sporks through its test-chain branch. The functional test now finishes masternode sync on node 2 before connecting it to node 1. Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e71323683b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const auto& spork : m_sporkman.ActiveSporks()) { | ||
| PushInv(*peer, CInv{MSG_SPORK, spork.GetHash()}); |
There was a problem hiding this comment.
Retain legacy spork fetching during rolling upgrades
When a patched testnet node starts with an empty cache and all of its peers still run the previous release, this loop is one-sided: those peers do not execute it and only send their cached ActiveSporks() in response to GETSPORKS, while this commit removes that request from SyncManager::ProcessTick. Consequently, the first upgraded node in a rolling deployment can still finish mnsync without SPORK_17/19 and remain in the same PoSe/ChainLock failure mode until a signer issues a new spork; retain a one-shot or version-gated GETSPORKS fallback for pre-announcement peers.
AGENTS.md reference: AGENTS.md:L15-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is intentional. The gap only exists on test networks, while a node is upgraded and every peer it can reach is still on the older release. Those peers still send their sporks whenever a spork is updated, and they still answer our announcements with getdata, so the network converges as soon as any upgraded node holds the sporks. We chose not to keep a version-gated GETSPORKS path just for that window; the trade-off is described in the PR body under "What was done?".
🤖 Posted autonomously by Claude on behalf of pasta.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
🕓 Review not started yet because the new head is waiting for the 30-minute push debounce.
Commit 45acdf3. Normal review starts when eligible; priority review starts as soon as a slot is available. |
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Verified the three-file PR diff at e713236 and traced handshake announcements through inventory queuing, requests, spork validation, serving, and relay; no actionable in-scope defects were found. The updated regression scenario exercises propagation to an already-synced node, and the legacy-only rolling-upgrade limitation is an explicitly documented trade-off. Verification was static only: no builds or tests were run, and the supplied CI snapshot has build checks queued and lint in progress.
Review provenance
Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — The small, contained diff moves test-network spork propagation from sync-time requests to connection-time inventory announcements, without changing message parsing, signature validation, or consensus rules. - Phase 1 reviewers:
gemini-3.8-flash-high— general (completed, effort high); agentphase1-reviewer - Phase 1 model:
gemini-3.8-flash-high— antigravity quota: weekly 100% left, 5h 100% left - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort medium); agentphase2-reviewer
| LogPrint(BCLog::MNSYNC, "Sync Tick -- nTick %d asset_id %d -- requesting sporks from peer=%d\n", nTick, | ||
| asset_id, pnode->GetId()); | ||
| } | ||
|
|
There was a problem hiding this comment.
need to fix this comment also: src/test/governance_vote_sync_tests.cpp
// CNetFulfilledRequestManager must stay keyed on the full CService, port included.
//
// It is tempting to strip the port: peer.addr carries the remote ephemeral source
// port for inbound peers, so the govsync quota is per-connection and an attacker
// resets it by reconnecting. But this map is shared with SyncManager's per-peer
// outbound bookkeeping ("full-sync", "governance-sync", "spork-sync",
// "mempool-sync") and the govsync quota is coupled to PeerMisbehaving(20).
//
There was a problem hiding this comment.
Thanks, fixed in ce24c40: dropped "spork-sync" from that list in governance_vote_sync_tests.cpp (amended into the commit).
🤖 Posted autonomously by Claude on behalf of pasta.
e713236 to
ce24c40
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce24c40452
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (m_chainparams.IsTestChain()) { | ||
| for (const auto& spork : m_sporkman.ActiveSporks()) { | ||
| PushInv(*peer, CInv{MSG_SPORK, spork.GetHash()}); |
There was a problem hiding this comment.
Announce sporks after masternode relay promotion
When an empty-cache masternode can obtain cached sporks only from a dedicated IQR peer, both patched endpoints skip this one-shot block: m_masternode_connection is already true at VERACK, while m_masternode_iqr_connection is set only after MNAUTH (src/evo/mnauth.cpp:165-172), so CanRelay() is false here. The link later becomes relayable without exchanging the active set; the removed sync tick used to reevaluate CanRelay() and issue GETSPORKS, leaving the node able to enter DKG/ChainLocks without SPORK_17/19. Queue the announcement on MNAUTH/quorum-relay promotion. This is distinct from the intentional old-peer upgrade gap because both endpoints run this patch.
AGENTS.md reference: AGENTS.md:L15-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid. Fixed in 45acdf3: the announcement no longer depends on CanRelay(). Like spork relay through RelayInv(inv), it now goes to every peer, masternode/IQR connections included. The inventory flush in SendMessages and the receiving INV handler already accept spork invs on those links. Re-ran feature_sporks.py, feature_llmq_signing.py --spork21, feature_llmq_chainlocks.py and p2p_instantsend.py locally; all pass.
🤖 Posted autonomously by Claude on behalf of pasta.
…ing mnsync On test networks a node only fetched sporks by sending GETSPORKS from the mnsync tick. That request was skipped once mnsync finished, skipped for any peer address with a still-valid "spork-sync" entry in netfulfilled.dat (which survives restarts), and skipped for inbound peers on an active masternode. A node that came up without sporks, e.g. after the sporks.dat format change in v24, could therefore finish mnsync without them and never ask again, leaving DKG and ChainLocks disabled until `mnsync reset`. Announce every active spork as an inv to each peer when the handshake completes, in both directions. Like spork relay, this includes masternode connections, which only become relay-capable after MNAUTH. Peers request the ones they lack with getdata, and a node that newly accepts a spork already relays it, so the spork set converges without any opt-in or sync state. The GETSPORKS handler is kept so v23 nodes can still ask us. Fixes dashpay#7891 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ce24c40 to
45acdf3
Compare
…ting them during mnsync ce24c40 fix(net): announce sporks to new peers instead of requesting them during mnsync (pasta) Pull request description: ## Issue being fixed or feature implemented Fixes #7891. On test networks (testnet, devnets, regtest), a node only fetched sporks by sending `GETSPORKS` from the mnsync tick. That request was skipped: - after mnsync finished, which on regtest and active masternodes is effectively permanent, because the 60-minute sync restart is disabled there; - for any peer address with a still-valid `spork-sync` entry in `netfulfilled.dat`, which survives restarts (it expires after 5 minutes on test chains); - for inbound peers on an active masternode, which in a small network are most of its peers. So a node that came up without sporks could finish mnsync without them and never ask again. One way that happens is v24 discarding v23's `sporks.dat` after its format change. Another is the peers it asked during sync having none themselves. Without SPORK_17/19 a masternode skips DKG, gets PoSe-banned, and ignores ChainLocks until someone runs `mnsync reset`. ## What was done? - In the `VERACK` handler, on test chains, a node now announces each active spork as an inv to every new peer that can relay. Both sides do this, so whichever side has the sporks hands them over regardless of connection direction or sync state. The peer requests only the sporks it lacks with `getdata`, and a node that accepts a new spork already relays it to its peers. The spork set therefore spreads through the network without any opt-in or saved state. - Removed the `spork-sync` / `GETSPORKS` request from `SyncManager::ProcessTick`. - Kept the `GETSPORKS` handler so older nodes can still ask us. New nodes no longer ask older peers. That only matters on test networks during the upgrade window, and an older node still sends its sporks to new nodes whenever it learns a new one. Overhead is about 36 bytes of inv per spork per new connection (at most 5 sporks). Signature checks happen only for sporks the node doesn't already have. Mainnet is unchanged. ## How Has This Been Tested? - Changed the existing scenario in `feature_sporks.py`: node 2 is forced to finish mnsync before node 1, which restored its sporks from cache, connects to it. Node 2 must then receive the spork. This replaces the `generate` the test previously needed to start mnsync. - Run against an unpatched `dashd` built from current develop, the test fails: it times out waiting for node 2's spork. - With this change it passes. - Also passing locally on macOS arm64 (debug build): `p2p_instantsend.py`, `feature_llmq_chainlocks.py`, `feature_asset_locks.py`, `feature_dip3_deterministicmns.py --descriptors`, `feature_llmq_signing.py`, `feature_llmq_signing.py --spork21`. - Every mininode connection in the functional tests (`P2PInterface`) now receives spork invs when sporks are set. I checked the tests that assert on `inv` messages or define their own `on_inv`: none of them set sporks, or they ignore spork invs. ## Breaking Changes None. Spork propagation on test networks moves from a request during mnsync to announcements at connect. The P2P messages involved are unchanged. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 7a88457a9a4c0bfb6ee87b2a6d94cc8df688f9b6a5fa56a5b9d927d40976aa58c188b56b7d8299f82045438c66091153eb3e32d5294c25f6a43b01c06a183046
…ing mnsync On test networks a node only fetched sporks by sending GETSPORKS from the mnsync tick. That request was skipped once mnsync finished, skipped for any peer address with a still-valid "spork-sync" entry in netfulfilled.dat (which survives restarts), and skipped for inbound peers on an active masternode. A node that came up without sporks, e.g. after the sporks.dat format change in v24, could therefore finish mnsync without them and never ask again, leaving DKG and ChainLocks disabled until `mnsync reset`. Announce every active spork as an inv to each peer when the handshake completes, in both directions. Like spork relay, this includes masternode connections, which only become relay-capable after MNAUTH. Peers request the ones they lack with getdata, and a node that newly accepts a spork already relays it, so the spork set converges without any opt-in or sync state. The GETSPORKS handler is kept so v23 nodes can still ask us. Fixes dashpay#7891 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 45acdf3) Backport notes: develop merged dashpay#7893 twice. The first merge (f99212b) carried an earlier revision of this commit, and the second merge (72a5997) carried this one, so develop now announces sporks in two places in the VERACK handler. Only the PR's final commit is picked here. Conflict in src/node/sync_manager.cpp: on this branch the mnsync tick still sent GETSPORKS on every network, because dashpay#7847 (which limited it to test chains) is not included. The request is removed entirely, as on develop. Mainnet spork values are hardcoded in CSporkManager::GetSporkValue, so the mainnet request had no effect.
6c02b2b ci: let DEPENDS_ARCH override the machine arch for multiprocess and tsan (pasta) ce5dbf3 Merge #7888: fix(consensus): canonical ExtNetInfo encoding and strict netInfo decoding (pasta) 54cba5c Merge #7537: fix: data races and check-then-act races in the CoinJoin server (pasta) 08d8bca Merge #7909: feat(mining): build the masternode list incrementally during block assembly (pasta) 5633761 Merge #7895: test: cover retroactive provider tx refusal, fix wording (pasta) cbb9500 Merge #7846: fix: treat pending owner keys and Platform node IDs as one conflict namespace (pasta) 8efdf5e Merge #7857: fix: mine a pending ProUpServTx before a key change of its masternode (pasta) e56e6ae Merge #7842: fix(wallet): reject mnemonic passphrases that seed derivation truncates (pasta) 79480ff test: follow-up for #7821: check if wallet is locked after failed upgrade instead creating brand-new scenario (Konstantin Akimov) a56ed47 fix(net): announce sporks to new peers instead of requesting them during mnsync (pasta) cdaea5f Merge #7894: fix: due to logical conflicts feature_pruning.py had been broken while (pasta) 98d92f6 Merge #7817: backport: partial bitcoin#34156 restore cleanup ownership (pasta) 35e6d18 Merge #7858: fix: do not InstantSend-lock provider transactions (pasta) 904d73d Merge #7881: fix: refuse to sign shared registrar updates that move the funder's coins (pasta) 95982f5 Merge #7782: fix(wallet): persist pending-observation coin locks once the record becomes readable (pasta) cdf65d0 Merge #7836: backport: partial bitcoin#25812, bitcoin#29855 (pasta) cc5716f Merge #7824: fix: retain initialized DKG sessions during inventory reads (pasta) 588e458 Merge #7827: backport: partial bitcoin#35984 (pasta) 795c6f1 Merge #7839: fix: release InstantSend tracking after mempool eviction (pasta) dc1a6b2 Merge #7875: docs(rpc): clarify getchainlockbyheight availability (pasta) e4798ff Merge #7876: fix(consensus): capture operator key scheme changes with unchanged bytes in state diffs (pasta) fe68ecb Merge #7871: backport!: Merge bitcoin#27217: wallet: Replace use of purpose strings with an enum (pasta) 8348d22 Merge #7867: fix: refuse masternode list RPCs for blocks outside the active chain (pasta) 861a579 Merge #7866: fix: stop `masternode payments` at the genesis block (pasta) efcef71 Merge #7521: fix: update masternode rate limit on failed governance trigger path (pasta) 0623182 Merge #7831: fix: bound governance object signatures and reset the governance cache (pasta) b230657 Merge #7841: fix: authenticate cached InstantSend proof bytes (pasta) a1f37ea Merge #7820: fix: hold chainstate lock during CoinJoin input validation (pasta) 50a5617 Merge #7829: fix: preserve wallet permissions when changing passphrases (pasta) 2ff0103 Merge #7818: fix: return REST errors for unavailable deployment data (pasta) cb39369 Merge #7834: fix: validate Electrum import input boundaries (pasta) e7e3daa Merge #7821: fix: relock encrypted wallets after HD upgrade failures (pasta) 2fea45f Merge #7792: fix: treat missing snapshots during evodb repair as repair errors (pasta) 3c5c039 Merge #7838: fix(wallet): authenticate unlock deadline extensions (pasta) 0bf80f8 Merge #7810: fix(rpc): handle unavailable masternode payment inputs (pasta) bcefee4 Merge #7822: fix: reject malformed detached quorum share batches (pasta) 86a7737 Merge #7828: fix: omit recovery words from invalid mnemonic errors (pasta) 886d5c4 Merge #7816: fix: check credit pool amount accumulation (pasta) 00fc3b9 Merge #7851: fix(rpc): correct masternode status help for top-level service field (pasta) 0e75c22 Merge #7814: fix(qt): restore HTML escaping of send confirmation recipients (pasta) Pull request description: ## Issue being fixed or feature implemented Backports for v24.0.0-rc.4, on top of `v24.0.x` at the v24.0.0-rc.3 tag (0364ed6, #7808). ## What was done? Cherry-picked (`git cherry-pick -m1 -x`) every develop merge since rc.3 that carries `backport-candidate-24.0.x`, plus eight unlabeled fixes picked after an audit of every other develop merge since rc.3 (marked † below). They are in develop merge order: - #7814 fix(qt): restore HTML escaping of send confirmation recipients † - #7851 fix(rpc): correct masternode status help for top-level service field - #7816 fix: check credit pool amount accumulation - #7828 fix: omit recovery words from invalid mnemonic errors † - #7822 fix: reject malformed detached quorum share batches † - #7810 fix(rpc): handle unavailable masternode payment inputs † - #7838 fix(wallet): authenticate unlock deadline extensions - #7792 fix: treat missing snapshots during evodb repair as repair errors - #7821 fix: relock encrypted wallets after HD upgrade failures - #7834 fix: validate Electrum import input boundaries - #7818 fix: return REST errors for unavailable deployment data - #7829 fix: preserve wallet permissions when changing passphrases - #7820 fix: hold chainstate lock during CoinJoin input validation - #7841 fix: authenticate cached InstantSend proof bytes - #7831 fix: bound governance object signatures and reset the governance cache - #7521 fix: update masternode rate limit on failed governance trigger path - #7866 fix: stop `masternode payments` at the genesis block - #7867 fix: refuse masternode list RPCs for blocks outside the active chain † - #7871 backport!: bitcoin#27217: wallet: Replace use of purpose strings with an enum - #7876 fix(consensus): capture operator key scheme changes with unchanged bytes in state diffs - #7875 docs(rpc): clarify getchainlockbyheight availability † - #7839 fix: release InstantSend tracking after mempool eviction - #7827 backport: partial bitcoin#35984 - #7824 fix: retain initialized DKG sessions during inventory reads - #7836 backport: partial bitcoin#25812, bitcoin#29855 - #7782 fix(wallet): persist pending-observation coin locks once the record becomes readable - #7881 fix: refuse to sign shared registrar updates that move the funder's coins † - #7858 fix: do not InstantSend-lock provider transactions - #7817 backport: partial bitcoin#34156 restore cleanup ownership - #7894 test: fix feature_pruning.py restart arguments † (needed because #7792 makes the evodb repair fatal, which the old restart arguments trip) - #7893 fix(net): announce sporks to new peers instead of requesting them during mnsync - 4a19742 (from #7859) test: follow-up for #7821: check if wallet is locked after failed upgrade instead creating brand-new scenario. Test-only, picked as its commit. Without it, #7821's `test_failed_upgrade()` expects RPC error -1 for a 257-byte mnemonic passphrase, which #7842's new precheck rejects with -8 first, so `wallet_upgradetohd.py --legacy-wallet` fails. On develop, #7859 replaced that scenario before #7842 merged. The rest of #7859 is test cleanup and docs and is left out. - #7842 fix(wallet): reject mnemonic passphrases that seed derivation truncates - #7857 fix: mine a pending ProUpServTx before a key change of its masternode - #7846 fix: treat pending owner keys and Platform node IDs as one conflict namespace - #7895 test: cover retroactive provider tx refusal, fix wording - #7909 feat(mining): build the masternode list incrementally during block assembly - #7537 fix: data races and check-then-act races in the CoinJoin server - #7888 fix(consensus): canonical ExtNetInfo encoding and strict netInfo decoding 34 of the 39 picks change exactly the same lines as their develop merges. #7888 and #7782 needed test-only changes (see testing), and three others needed adaptation, described in their commit messages: - **#7893** was merged to develop twice: f99212b with an earlier revision of its commit, then 72a5997 with the final one, so develop now announces sporks twice in the VERACK handler (a develop fix is open separately). Here the PR's final commit 45acdf3 is picked once instead of either merge. In `src/node/sync_manager.cpp`, this branch still sent `GETSPORKS` from the mnsync tick on every network, because #7847, which limited it to test chains, is not included. The request is removed entirely, as on develop. Mainnet spork values are hardcoded in `CSporkManager::GetSporkValue`, so the mainnet request had no effect. - **#7857** and **#7909**: develop folded `CChainstateHelper` into `Chainstate` (d5c1bd7), which is not on this branch, so the new code goes through `ChainHelper().special_tx` / `m_chain_helper.special_tx` instead of `m_special_tx`. The neighbouring develop-only `CSpecialTxProcessor::GetDeterministicMNListHash` is left out. #7888 tightens consensus rules for ExtAddr provider transactions behind V24, so I checked it against testnet. V24 is not active there yet (the tip's CbTx is still version 3), so the chain has no ExtAddr ProTxs the new rules could reject. rc.3's own serializer always writes canonical, sorted purposes, so every ProTx it builds passes the new rules. Mixed rc.3/rc.4 networks could only split on a hand-crafted non-canonical ExtAddr ProTx mined after V24 activates. So testnet miners and masternodes should move to rc.4 before V24 is signalled. Unlabeled develop fixes since rc.3 that were audited and left out: - Worth considering before the final release: #7869 (startup block replay after an unclean shutdown asserts on a quorum commitment, so recovery needs `-reindex-chainstate`; not a v24 regression), #7833 (gate signing intake before quorum retrieval, DoS hardening), #7682 (intermittent `relic_conf.h` parallel-build failure). - Left for 24.1: #7812, #7826, #7844, #7847, #7850, #7868, plus the routine upstream backports (#7780, #7806, #7830, #7855, #7856, #7862, #7863, #7865, #7883, #7884, #7898, #7906), refactors (#7805, #7878, #7880), and CI, docs and test-only changes. One CI-only commit comes last. It takes only the `ci/test` hunks of 6f71fa5 (#7901), which make `ci/test/00_setup_env_native_{multiprocess,tsan}.sh` honor `DEPENDS_ARCH`. Pull requests into `v24.0.x` run develop's workflow files (`pull_request_target`), and since #7901 those check the depends cache on x86_64 runners and pass the aarch64 build arch in `DEPENDS_ARCH`. Without the hunks, `linux64_multiprocess / Build depends` fails on any depends cache miss (`Build runner computes HOST=aarch64-linux-gnu but check-cache used x86_64-pc-linux-gnu`). Without `DEPENDS_ARCH` the scripts still use `uname -m`, so this branch's own push CI is unchanged. No version change is needed. `configure.ac` is already 24.0.0 with `CLIENT_VERSION_IS_RELEASE` false, so the build takes its version string from the `v24.0.0-rc.4` tag. ## How Has This Been Tested? Built and tested at 6c02b2b on macOS arm64 against depends, with `--enable-debug --enable-werror --disable-hardening`. - `make check`: all unit test suites pass, including `test_dash-qt`. - Functional tests, all passing: `feature_evo_shares_activation.py`, `feature_governance_objects.py`, `feature_llmq_signing.py` (plain and `--spork21`), `feature_masternode_shares.py`, `feature_sporks.py`, `interface_rest.py`, `p2p_instantsend.py`, `p2p_quorum_data.py`, `rpc_masternode.py`, `rpc_psbt.py`, `rpc_quorum.py`, `wallet_backup.py`, `wallet_dump.py`, `wallet_encryption.py`, `wallet_labels.py` and `wallet_upgradetohd.py`, each with `--descriptors` and `--legacy-wallet` where it supports them. - Each pick's changed lines were compared with its develop merge (or, for #7893, its PR commit). They are identical except for the #7893, #7857 and #7909 adaptations above, and the #7888 test adaptation below. - A local macOS arm64 debug build (`--enable-debug --enable-werror`) compiled most of the tree and stopped on a single error in `src/test/evo_netinfo_tests.cpp`. #7888's new test used `CNetAddr::V2()`, the serialization-parameter API from bitcoin#25284, which this branch lacks. The pick now writes that entry through an `OverrideStream` with `ADDRV2_FORMAT`, the way `NetInfoEntry::Serialize` does on this branch (see its commit message). - #7782's new unit test reloads the wallet with develop's three-argument `CWallet` constructor. On this branch the constructor still takes the CoinJoin loader and an `ArgsManager`, so the test passes them the way its fixture already does (see the commit message). - With both test fixes, a full local `make` (`dashd`, `dash-qt`, `test_dash`, `test_dash-qt`) completes with no errors. - `feature_pruning.py` is an extended test that CI does not run. #7894 is included because #7792 makes its restart path fail. ## Breaking Changes Carried over from the picked PRs: - #7876: rc.1 to rc.3 nodes that connected an affected block stored a wrong masternode list diff for it. Those nodes need `-reindex`; `-reindex-chainstate` is not enough for nodes that already marked the following block invalid. - #7888: stricter ExtAddr `netInfo` decoding and validation behind V24 (see above). The RFC1918 check now applies even with `-allowprivatenet`. - #7858: provider transactions no longer get InstantSend locks. Software that waits for an islock on a ProTx should wait for a ChainLock or confirmations instead. - #7857: a `ProUpServTx` spending an output of a pending operator key change, asset lock, asset unlock or MNHF signal is rejected from the mempool until that transaction confirms. - #7831: governance object signatures over 96 bytes are rejected, and the governance cache is discarded and re-synced on first start (`doc/release-notes-7831.md`). - #7871: address purposes are limited to `send`, `receive` and `refund`. Wallets with other purpose strings load with warnings, and `listlabels` errors on an unknown purpose (`doc/release-notes-27217.md`). - #7842: creating an HD wallet with a mnemonic passphrase over 248 bytes fails unless `-allow-deprecated-long-mnemonics` is set, and over 256 bytes always fails (`doc/release-notes-7842.md`). - #7792: a node whose evodb is missing a masternode list snapshot now stops at startup and asks for `-reindex`, and `evodb repair` reports it in `repairErrors`. - #7867: `protx info <proTxHash> <blockHash>`, `protx listdiff` and `masternode payments <blockHash>` return an error for blocks outside the active chain. - #7881: `protx shared_combine` on a `ProUpSharedRegTx` is refused unless its inputs are unlocked coins of a single address of the calling wallet, change returns to that address, and the fee is within `-maxtxfee`. - #7836: PSBTs referencing nonexistent previous outputs fail to deserialize (RPC error -22). - #7834: negative Electrum-import rescan indices are rejected. - #7893: on this branch, mainnet nodes no longer send `GETSPORKS` during mnsync (no effect, mainnet spork values are hardcoded). ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 7de9535ca51620f4ddf5a46df2c3341a5e5fdd2263606f83f7db6b576c31ff7b82e1b285bf66988ac4b2729a9060513c45710135f61d7fd5c147b670d5f405dc
Issue being fixed or feature implemented
Fixes #7891.
On test networks (testnet, devnets, regtest), a node only fetched sporks by sending
GETSPORKSfrom the mnsync tick. That request was skipped:spork-syncentry innetfulfilled.dat, which survives restarts (it expires after 5 minutes on test chains);So a node that came up without sporks could finish mnsync without them and never ask again. One way that happens is v24 discarding v23's
sporks.datafter its format change. Another is the peers it asked during sync having none themselves. Without SPORK_17/19 a masternode skips DKG, gets PoSe-banned, and ignores ChainLocks until someone runsmnsync reset.What was done?
VERACKhandler, on test chains, a node now announces each active spork as an inv to every new peer. Like spork relay (RelayInv), this includes masternode connections, which only become relay-capable after MNAUTH. Both sides do this, so whichever side has the sporks hands them over regardless of connection direction or sync state. The peer requests only the sporks it lacks withgetdata, and a node that accepts a new spork already relays it to its peers. The spork set therefore spreads through the network without any opt-in or saved state.spork-sync/GETSPORKSrequest fromSyncManager::ProcessTick.GETSPORKShandler so older nodes can still ask us. New nodes no longer ask older peers. That only matters on test networks during the upgrade window, and an older node still sends its sporks to new nodes whenever it learns a new one.Overhead is about 36 bytes of inv per spork per new connection (at most 5 sporks). Signature checks happen only for sporks the node doesn't already have. Mainnet is unchanged.
How Has This Been Tested?
feature_sporks.py: node 2 is forced to finish mnsync before node 1, which restored its sporks from cache, connects to it. Node 2 must then receive the spork. This replaces thegeneratethe test previously needed to start mnsync.dashdbuilt from current develop, the test fails: it times out waiting for node 2's spork.p2p_instantsend.py,feature_llmq_chainlocks.py,feature_asset_locks.py,feature_dip3_deterministicmns.py --descriptors,feature_llmq_signing.py,feature_llmq_signing.py --spork21.P2PInterface) now receives spork invs when sporks are set. I checked the tests that assert oninvmessages or define their ownon_inv: none of them set sporks, or they ignore spork invs.Breaking Changes
None. Spork propagation on test networks moves from a request during mnsync to announcements at connect. The P2P messages involved are unchanged.
Checklist:
🤖 Generated with Claude Code