Skip to content

fix(net): announce sporks to new peers instead of requesting them during mnsync #7893

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.

Sign up for GitHub

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

Merged
PastaPastaPasta merged 1 commit into from
Oct 15, 2026
Merged

fix(net): announce sporks to new peers instead of requesting them during mnsync #7893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/net_processing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
11 changes: 0 additions & 11 deletions src/node/sync_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

@knst knst Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).
//

@PastaPastaPasta PastaPastaPasta Oct 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (asset_id == MASTERNODE_SYNC_BLOCKCHAIN) {
int64_t nTimeSyncTimeout = snap.Nodes().size() > 3 ? MASTERNODE_SYNC_TICK_SECONDS
: MASTERNODE_SYNC_TIMEOUT_SECONDS;
Expand Down
4 changes: 2 additions & 2 deletions src/test/governance_vote_sync_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions test/functional/feature_sporks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)

Expand Down
Loading