server: fix peer add/done race between peerHandler and syncManager

diff --git a/integration/sync_race_test.go b/integration/sync_race_test.go
new file mode 100644
index 0000000..910e637
--- /dev/null
+++ b/integration/sync_race_test.go
@@ -0,0 +1,228 @@
+//go:build rpctest
+// +build rpctest
+
+package integration
+
+import (
+	"math/rand"
+	"net"
+	"testing"
+	"time"
+
+	"github.com/btcsuite/btcd/chaincfg"
+	"github.com/btcsuite/btcd/integration/rpctest"
+	"github.com/btcsuite/btcd/wire"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	syncRaceIterations  = 1000
+	syncRaceConcurrency = 300
+	syncRaceRunDuration = 90 * time.Second
+	syncRaceProofBlocks = 5
+	syncRaceProofWait   = 8 * time.Second
+)
+
+// fakePeerConn connects to the node at nodeAddr, performs the minimum version/verack
+// handshake so the node registers a peer (NewPeer) and then disconnects so the node
+// runs DonePeer. This simulates attacker traffic: many connections that complete
+// handshake then drop, stressing the sync manager's ordering of NewPeer/DonePeer.
+func fakePeerConn(nodeAddr string) error {
+	conn, err := net.DialTimeout("tcp", nodeAddr, 5*time.Second)
+	if err != nil {
+		return err
+	}
+	defer conn.Close()
+
+	_ = conn.SetDeadline(time.Now().Add(15 * time.Second))
+
+	nodeTCP, err := net.ResolveTCPAddr("tcp", nodeAddr)
+	if err != nil {
+		return err
+	}
+	you := wire.NewNetAddress(
+		nodeTCP, wire.SFNodeNetwork|wire.SFNodeWitness,
+	)
+	me := wire.NewNetAddress(
+		&net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0},
+		wire.SFNodeNetwork|wire.SFNodeWitness,
+	)
+	you.Timestamp = time.Time{}
+	me.Timestamp = time.Time{}
+
+	nonce := uint64(rand.Int63())
+	msgVersion := wire.NewMsgVersion(me, you, nonce, 0)
+	msgVersion.Services = wire.SFNodeNetwork | wire.SFNodeWitness
+
+	if err := wire.WriteMessage(conn, msgVersion, wire.ProtocolVersion, wire.SimNet); err != nil {
+		return err
+	}
+
+	for {
+		msg, _, err := wire.ReadMessage(conn, wire.ProtocolVersion, wire.SimNet)
+		if err != nil {
+			return err
+		}
+		switch msg.(type) {
+		case *wire.MsgVersion:
+			// Node's version; send verack.
+			if err := wire.WriteMessage(conn, wire.NewMsgVerAck(), wire.ProtocolVersion, wire.SimNet); err != nil {
+				return err
+			}
+
+		case *wire.MsgSendAddrV2:
+			// Optional; keep reading.
+
+		case *wire.MsgVerAck:
+			// Handshake complete; close to trigger DonePeer.
+			return nil
+
+		default:
+			// Ignore other messages (e.g. wtxidrelay) and keep reading.
+		}
+	}
+}
+
+// TestSyncManagerRaceCorruption stresses a single simnet node with many inbound
+// connections that complete the version/verack handshake then disconnect. It then
+// proves corruption without a dedicated RPC: connect a fresh node that generates
+// blocks; if the stressed node does not sync, it was stuck with a dead sync peer
+// (getpeerinfo returns 0 peers when all disconnected; in the corrupted state the
+// sync manager still has a dead peer as sync peer, so it ignores the new live one).
+func TestSyncManagerRaceCorruption(t *testing.T) {
+	stressedHarness, err := rpctest.New(&chaincfg.SimNetParams, nil, nil, "")
+	require.NoError(t, err)
+	require.NoError(t, stressedHarness.SetUp(true, 0))
+	t.Cleanup(func() {
+		require.NoError(t, stressedHarness.TearDown())
+	})
+
+	nodeAddr := stressedHarness.P2PAddress()
+	deadline := time.Now().Add(syncRaceRunDuration)
+	iter := 0
+	var done int
+	doneCh := make(chan struct{}, syncRaceConcurrency*2)
+
+	for time.Now().Before(deadline) && iter < syncRaceIterations {
+		for i := 0; i < syncRaceConcurrency; i++ {
+			go func() {
+				_ = fakePeerConn(nodeAddr)
+				doneCh <- struct{}{}
+			}()
+		}
+		for i := 0; i < syncRaceConcurrency; i++ {
+			<-doneCh
+			done++
+		}
+		iter += syncRaceConcurrency
+	}
+
+	// Prove corruption: connect a live node and generate blocks.
+	// If the stressed node was corrupted (dead sync peer, 0 connected peers per getpeerinfo), it will not sync from the new one.
+	newHarness, err := rpctest.New(&chaincfg.SimNetParams, nil, nil, "")
+	require.NoError(t, err)
+	require.NoError(t, newHarness.SetUp(true, 0))
+	defer func() { _ = newHarness.TearDown() }()
+
+	require.NoError(t, rpctest.ConnectNode(stressedHarness, newHarness),
+		"stressed node must connect to the new node")
+
+	_, heightBefore, err := stressedHarness.Client.GetBestBlock()
+	require.NoError(t, err)
+
+	_, err = newHarness.Client.Generate(syncRaceProofBlocks)
+	require.NoError(t, err)
+
+	time.Sleep(syncRaceProofWait)
+
+	_, heightAfter, err := stressedHarness.Client.GetBestBlock()
+	require.NoError(t, err)
+
+	if heightAfter < heightBefore+int32(syncRaceProofBlocks) {
+		t.Fatalf("proved sync manager corruption after %d fake peer cycles: stressed node did not sync from new peer (height %d -> %d); it was stuck with a dead sync peer instead of the new live one",
+			done, heightBefore, heightAfter)
+	}
+
+	t.Logf("completed %d fake peer cycles; stressed node synced from new peer (height %d -> %d), no corruption observed", done, heightBefore, heightAfter)
+}
+
+// TestPreVerackDisconnect verifies that a peer disconnecting before completing
+// the version/verack handshake does not corrupt the sync manager state. In this
+// case only a peerDone event is produced (no peerAdd), since AddPeer is only
+// called from OnVerAck. The node must remain healthy and able to sync afterward.
+func TestPreVerackDisconnect(t *testing.T) {
+	harness, err := rpctest.New(&chaincfg.SimNetParams, nil, nil, "")
+	require.NoError(t, err)
+	require.NoError(t, harness.SetUp(true, 0))
+	t.Cleanup(func() { _ = harness.TearDown() })
+
+	nodeAddr := harness.P2PAddress()
+
+	// Connect and send version, then disconnect before receiving or
+	// sending verack. This produces a peerDone without a preceding
+	// peerAdd in the lifecycle channel.
+	for i := 0; i < 50; i++ {
+		conn, err := net.DialTimeout("tcp", nodeAddr, 5*time.Second)
+		if err != nil {
+			continue
+		}
+
+		_ = conn.SetDeadline(time.Now().Add(5 * time.Second))
+
+		nodeTCP, err := net.ResolveTCPAddr("tcp", nodeAddr)
+		if err != nil {
+			conn.Close()
+			continue
+		}
+
+		you := wire.NewNetAddress(
+			nodeTCP, wire.SFNodeNetwork|wire.SFNodeWitness,
+		)
+		me := wire.NewNetAddress(
+			&net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0},
+			wire.SFNodeNetwork|wire.SFNodeWitness,
+		)
+		you.Timestamp = time.Time{}
+		me.Timestamp = time.Time{}
+
+		nonce := uint64(rand.Int63())
+		msgVersion := wire.NewMsgVersion(me, you, nonce, 0)
+		msgVersion.Services = wire.SFNodeNetwork | wire.SFNodeWitness
+
+		_ = wire.WriteMessage(
+			conn, msgVersion, wire.ProtocolVersion, wire.SimNet,
+		)
+
+		// Close immediately without completing the handshake.
+		conn.Close()
+	}
+
+	// Allow the node time to process all the disconnects.
+	time.Sleep(2 * time.Second)
+
+	// Verify the node is still healthy: connect a real peer, generate
+	// blocks, and confirm the harness syncs them.
+	helper, err := rpctest.New(&chaincfg.SimNetParams, nil, nil, "")
+	require.NoError(t, err)
+	require.NoError(t, helper.SetUp(true, 0))
+	defer func() { _ = helper.TearDown() }()
+
+	require.NoError(t, rpctest.ConnectNode(harness, helper))
+
+	_, heightBefore, err := harness.Client.GetBestBlock()
+	require.NoError(t, err)
+
+	_, err = helper.Client.Generate(3)
+	require.NoError(t, err)
+
+	time.Sleep(5 * time.Second)
+
+	_, heightAfter, err := harness.Client.GetBestBlock()
+	require.NoError(t, err)
+
+	require.GreaterOrEqual(t, heightAfter, heightBefore+3,
+		"node failed to sync after pre-verack disconnects")
+
+	t.Logf("node healthy after 50 pre-verack disconnects (height %d -> %d)",
+		heightBefore, heightAfter)
+}
diff --git a/server.go b/server.go
index 40755c8..f5b6c0b 100644
--- a/server.go
+++ b/server.go
@@ -151,6 +151,23 @@ type updatePeerHeightsMsg struct {
 	originPeer *peer.Peer
 }
 
+// peerLifecycleAction describes the type of peer lifecycle event.
+type peerLifecycleAction uint8
+
+const (
+	peerAdd  peerLifecycleAction = iota
+	peerDone
+)
+
+// peerLifecycleEvent represents a peer connection or disconnection event.
+// Using a single channel for both event types guarantees FIFO ordering:
+// the add event from OnVerAck is always enqueued before the done event
+// from peerDoneHandler, so the receiver always sees add before done.
+type peerLifecycleEvent struct {
+	action peerLifecycleAction
+	sp     *serverPeer
+}
+
 // peerState maintains state of inbound, persistent, outbound peers as well
 // as banned peers and outbound groups.
 type peerState struct {
@@ -218,8 +235,7 @@ type server struct {
 	cpuMiner             *cpuminer.CPUMiner
 	modifyRebroadcastInv chan interface{}
 	p2pDowngrader        *peer.P2PDowngrader
-	newPeers             chan *serverPeer
-	donePeers            chan *serverPeer
+	peerLifecycle        chan peerLifecycleEvent
 	banPeers             chan *serverPeer
 	query                chan interface{}
 	relayInv             chan relayMsg
@@ -1907,7 +1923,22 @@ func (s *server) handleDonePeerMsg(state *peerState, sp *serverPeer) {
 		}
 		delete(list, sp.ID())
 		srvrLog.Debugf("Removed peer %s", sp)
-		return
+	}
+
+	// Notify the sync manager the peer is gone and evict any remaining
+	// orphans that were sent by the peer. This is done here rather than in
+	// peerDoneHandler so that the notification is serialized with NewPeer
+	// calls through the peerHandler goroutine, guaranteeing that the sync
+	// manager always sees NewPeer before DonePeer for a given peer.
+	if sp.VerAckReceived() {
+		s.syncManager.DonePeer(sp.Peer)
+
+		numEvicted := s.txMemPool.RemoveOrphansByTag(mempool.Tag(sp.ID()))
+		if numEvicted > 0 {
+			txmpLog.Debugf("Evicted %d %s from peer %v (id %d)",
+				numEvicted, pickNoun(numEvicted, "orphan",
+					"orphans"), sp, sp.ID())
+		}
 	}
 }
 
@@ -2291,21 +2322,7 @@ func (s *server) peerDoneHandler(sp *serverPeer) {
 		s.p2pDowngrader.MarkForDowngrade(sp.Addr())
 	}
 
-	// This is sent to a buffered channel, so it may not execute immediately.
-	s.donePeers <- sp
-
-	// Only tell sync manager we are gone if we ever told it we existed.
-	if sp.VerAckReceived() {
-		s.syncManager.DonePeer(sp.Peer)
-
-		// Evict any remaining orphans that were sent by the peer.
-		numEvicted := s.txMemPool.RemoveOrphansByTag(mempool.Tag(sp.ID()))
-		if numEvicted > 0 {
-			txmpLog.Debugf("Evicted %d %s from peer %v (id %d)",
-				numEvicted, pickNoun(numEvicted, "orphan",
-					"orphans"), sp, sp.ID())
-		}
-	}
+	s.peerLifecycle <- peerLifecycleEvent{action: peerDone, sp: sp}
 	close(sp.quit)
 }
 
@@ -2348,13 +2365,15 @@ func (s *server) peerHandler() {
 out:
 	for {
 		select {
-		// New peers connected to the server.
-		case p := <-s.newPeers:
-			s.handleAddPeerMsg(state, p)
-
-		// Disconnected peers.
-		case p := <-s.donePeers:
-			s.handleDonePeerMsg(state, p)
+		// Peer connected or disconnected.
+		case event := <-s.peerLifecycle:
+			switch event.action {
+			case peerAdd:
+				s.handleAddPeerMsg(state, event.sp)
+
+			case peerDone:
+				s.handleDonePeerMsg(state, event.sp)
+			}
 
 		// Block accepted in mainchain or orphan, update peer height.
 		case umsg := <-s.peerHeightsUpdate:
@@ -2395,8 +2414,7 @@ out:
 cleanup:
 	for {
 		select {
-		case <-s.newPeers:
-		case <-s.donePeers:
+		case <-s.peerLifecycle:
 		case <-s.peerHeightsUpdate:
 		case <-s.relayInv:
 		case <-s.broadcast:
@@ -2411,7 +2429,7 @@ cleanup:
 
 // AddPeer adds a new peer that has already been connected to the server.
 func (s *server) AddPeer(sp *serverPeer) {
-	s.newPeers <- sp
+	s.peerLifecycle <- peerLifecycleEvent{action: peerAdd, sp: sp}
 }
 
 // BanPeer bans a peer that has already been connected to the server by ip.
@@ -2847,8 +2865,7 @@ func newServer(listenAddrs, agentBlacklist, agentWhitelist []string,
 	s := server{
 		chainParams:          chainParams,
 		addrManager:          amgr,
-		newPeers:             make(chan *serverPeer, cfg.MaxPeers),
-		donePeers:            make(chan *serverPeer, cfg.MaxPeers),
+		peerLifecycle:        make(chan peerLifecycleEvent, cfg.MaxPeers*2),
 		banPeers:             make(chan *serverPeer, cfg.MaxPeers),
 		query:                make(chan interface{}),
 		relayInv:             make(chan relayMsg, cfg.MaxPeers),