[release-v2.1] spv: Validate merkle root for all fetched blocks

Blocks are now always fetched from SPV peers through a single
blocksFromPeer helper, which checks the transaction trees against the
merkle root commitments in the header and disconnects the peer when they
do not match.

This not only removes duplication but also adds the merkle root check to
two call sites which were previously missing it (cacheMissingCommitments
in discovery.go and handleBlockInvs in sync.go).

Backport of 2c37f24.

Author: jholdstock

spv/backend.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2018-2025 The Decred developers
+// Copyright (c) 2018-2026 The Decred developers
 // Use of this source code is governed by an ISC
 // license that can be found in the LICENSE file.
 
@@ -60,6 +60,37 @@ func pickForGetCfilters(lastHeaderHeight int32) func(rp *p2p.RemotePeer) bool {
 	}
 }
 
+// blocksFromPeer requests blocks from rp and verifies the transaction trees in
+// the block match what is promised by the merkle commitment in the block
+// header.  The remote peer is disconnected if it returns a block that fails
+// this verification.
+func blocksFromPeer(ctx context.Context, rp *p2p.RemotePeer, blockHashes []*chainhash.Hash) ([]*wire.MsgBlock, error) {
+	blocks, err := rp.Blocks(ctx, blockHashes)
+	if err != nil {
+		return nil, err
+	}
+	for _, b := range blocks {
+		if b == nil {
+			continue
+		}
+
+		// A block hash only commits to the header, so a peer is free to return
+		// any transactions it likes for a requested hash and still satisfy the
+		// request. Every block obtained from a remote peer must therefore have
+		// its transaction trees checked against the merkle root commitments of
+		// the header before the transactions are used for anything.
+		err := validate.MerkleRoots(b)
+		if err != nil {
+			err = validate.DCP0005MerkleRoot(b)
+		}
+		if err != nil {
+			rp.Disconnect(err)
+			return nil, err
+		}
+	}
+	return blocks, nil
+}
+
 // Blocks implements the Blocks method of the wallet.Peer interface.
 func (s *Syncer) Blocks(ctx context.Context, blockHashes []*chainhash.Hash) ([]*wire.MsgBlock, error) {
 	for {
@@ -70,7 +101,7 @@ func (s *Syncer) Blocks(ctx context.Context, blockHashes []*chainhash.Hash) ([]*
 		if err != nil {
 			return nil, err
 		}
-		blocks, err := rp.Blocks(ctx, blockHashes)
+		blocks, err := blocksFromPeer(ctx, rp, blockHashes)
 		if err != nil {
 			log.Debugf("Unable to fetch blocks from %v: %v", rp, err)
 			continue
@@ -556,32 +587,12 @@ func (s *Syncer) Rescan(ctx context.Context, blockHashes []chainhash.Hash, save
 				return err
 			}
 
-			blocks, err := rp.Blocks(ctx, fmatches)
+			blocks, err := blocksFromPeer(ctx, rp, fmatches)
 			if err != nil {
 				continue PickPeer
 			}
 
 			for j, b := range blocks {
-				// Validate fetched blocks before rescanning transactions.  PoW
-				// and PoS difficulties have already been validated since the
-				// header is saved by the wallet, and modifications to these in
-				// the downloaded block would result in a different block hash
-				// and failure to fetch the block.
-				//
-				// Block filters were also validated
-				// against the header (assuming dcp0005
-				// was activated).
-				err = validate.MerkleRoots(b)
-				if err != nil {
-					err = validate.DCP0005MerkleRoot(b)
-				}
-				if err != nil {
-					err := errors.E(op, err)
-					rp.Disconnect(err)
-					rp = nil
-					continue PickPeer
-				}
-
 				i := fmatchidx[j]
 				blockMatches[i] = b
 			}

spv/sync.go

@@ -1069,11 +1069,12 @@ func (s *Syncer) handleBlockInvs(ctx context.Context, rp *p2p.RemotePeer, hashes
 		return nil
 	}
 
-	blocks, err := rp.Blocks(ctx, hashes)
+	blocks, err := blocksFromPeer(ctx, rp, hashes)
 	if err != nil {
 		op := errors.Opf(opf, rp)
 		return errors.E(op, err)
 	}
+
 	headers := make([]*wire.BlockHeader, len(blocks))
 	bmap := make(map[chainhash.Hash]*wire.MsgBlock)
 	for i, block := range blocks {
@@ -1348,24 +1349,12 @@ func (s *Syncer) scanChain(ctx context.Context, rp *p2p.RemotePeer, chain []*wal
 	wg.Wait()
 
 	if len(fmatches) != 0 {
-		blocks, err := rp.Blocks(ctx, fmatches)
+		blocks, err := blocksFromPeer(ctx, rp, fmatches)
 		if err != nil {
 			return nil, err
 		}
 		for j, b := range blocks {
 			i := fmatchidx[j]
-
-			// Perform context-free validation on the block.
-			// Disconnect peer when invalid.
-			err := validate.MerkleRoots(b)
-			if err != nil {
-				err = validate.DCP0005MerkleRoot(b)
-			}
-			if err != nil {
-				rp.Disconnect(err)
-				return nil, err
-			}
-
 			fetched[i] = b
 		}
 	}