From 45acdf30f35c6b9684ff81974df8cac19dc00421 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 15 Oct 2026 10:19:45 -0500 Subject: [PATCH] fix(net): announce sporks to new peers instead of requesting them during 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 #7891 Co-Authored-By: Claude Opus 5.5 --- src/net_processing.cpp | 9 +++++++++ src/node/sync_manager.cpp | 11 ----------- src/test/governance_vote_sync_tests.cpp | 4 ++-- test/functional/feature_sporks.py | 8 ++++---- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 77c4b2d2b572..7f8a668c0d0e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4239,6 +4239,15 @@ void PeerManagerImpl::ProcessMessage( m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); } + // Announce our sporks to every new peer so a node that lost or never had them + // picks them up without having to ask. Like spork relay, this includes masternode + // connections, which only become relay-capable after MNAUTH. + if (m_chainparams.IsTestChain()) { + for (const auto& spork : m_sporkman.ActiveSporks()) { + PushInv(*peer, CInv{MSG_SPORK, spork.GetHash()}); + } + } + if (!RejectIncomingTxs(pfrom)) { // Tell our peer that he should send us CoinJoin queue messages m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDDSQUEUE, true)); diff --git a/src/node/sync_manager.cpp b/src/node/sync_manager.cpp index b7539c34f6d1..5d170e79a2d9 100644 --- a/src/node/sync_manager.cpp +++ b/src/node/sync_manager.cpp @@ -201,17 +201,6 @@ void SyncManager::ProcessTick() continue; } - // SPORK : ASK FOR SPORKS AS WE SYNC, EXCEPT ON MAINNET WHERE THEY ARE NOT USED - - if (Params().IsTestChain() && !m_netfulfilledman.HasFulfilledRequest(pnode->addr, "spork-sync")) { - // always get sporks first, only request once from each peer - m_netfulfilledman.AddFulfilledRequest(pnode->addr, "spork-sync"); - // get current network sporks - m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::GETSPORKS)); - LogPrint(BCLog::MNSYNC, "Sync Tick -- nTick %d asset_id %d -- requesting sporks from peer=%d\n", nTick, - asset_id, pnode->GetId()); - } - if (asset_id == MASTERNODE_SYNC_BLOCKCHAIN) { int64_t nTimeSyncTimeout = snap.Nodes().size() > 3 ? MASTERNODE_SYNC_TICK_SECONDS : MASTERNODE_SYNC_TIMEOUT_SECONDS; diff --git a/src/test/governance_vote_sync_tests.cpp b/src/test/governance_vote_sync_tests.cpp index 1aae5188a30f..ac69392786f8 100644 --- a/src/test/governance_vote_sync_tests.cpp +++ b/src/test/governance_vote_sync_tests.cpp @@ -40,8 +40,8 @@ BOOST_FIXTURE_TEST_SUITE(governance_vote_sync_tests, BasicTestingSetup) // 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). +// outbound bookkeeping ("full-sync", "governance-sync", "mempool-sync") and the +// govsync quota is coupled to PeerMisbehaving(20). // Several honest nodes legitimately share one address (NAT, colocation, CI), and // collapsing them onto one key makes the second honest peer look like a repeat // offender -- the ban score escalates to discouragement, and mnsync breaks diff --git a/test/functional/feature_sporks.py b/test/functional/feature_sporks.py index 46160226b83a..57096e621c37 100755 --- a/test/functional/feature_sporks.py +++ b/test/functional/feature_sporks.py @@ -8,6 +8,7 @@ from test_framework.messages import ser_compact_size from test_framework.p2p import MESSAGEMAP, P2PInterface from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import force_finish_mnsync class msg_getsporks: @@ -111,10 +112,9 @@ def run_test(self): assert self.get_test_spork_state(self.nodes[0]) == spork_new_state assert self.get_test_spork_state(self.nodes[1]) == spork_new_state - # Generate one block to kick off masternode sync, which also starts sporks syncing for node2 - self.generate(self.nodes[1], 1, sync_fun=self.no_op) - - # connect new node and check spork propagation after restoring from cache + # A node that has already finished mnsync must still pick up sporks from a new peer, + # here one that connects to it inbound after restoring its sporks from cache + force_finish_mnsync(self.nodes[2]) self.connect_nodes(1, 2) self.wait_until(lambda: self.get_test_spork_state(self.nodes[2]), timeout=10)