routing: add chans rejected due to failed chain validation to zombie index

In this commit, we start to add any channels that fail the normal chain
validation to the zombie index. With this change, we'll ensure that we
won't continue to re-process the same set of spent channels over and
over again.

Fixes #5191.

routing/notifications_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lnwallet"
+	"github.com/lightningnetwork/lnd/lnwallet/btcwallet"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/routing/chainview"
 	"github.com/lightningnetwork/lnd/routing/route"
@@ -170,8 +171,8 @@ func (m *mockChain) GetBlockHash(blockHeight int64) (*chainhash.Hash, error) {
 
 	hash, ok := m.blockIndex[uint32(blockHeight)]
 	if !ok {
-		return nil, fmt.Errorf("can't find block hash, for "+
+		return nil, fmt.Errorf("block number out of range: %v",
-			"height %v", blockHeight)
+			blockHeight)
 	}
 
 	return &hash, nil
@@ -182,6 +183,13 @@ func (m *mockChain) addUtxo(op wire.OutPoint, out *wire.TxOut) {
 	m.utxos[op] = *out
 	m.Unlock()
 }
+
+func (m *mockChain) delUtxo(op wire.OutPoint) {
+	m.Lock()
+	delete(m.utxos, op)
+	m.Unlock()
+}
+
 func (m *mockChain) GetUtxo(op *wire.OutPoint, _ []byte, _ uint32,
 	_ <-chan struct{}) (*wire.TxOut, error) {
 	m.RLock()
@@ -189,7 +197,7 @@ func (m *mockChain) GetUtxo(op *wire.OutPoint, _ []byte, _ uint32,
 
 	utxo, ok := m.utxos[*op]
 	if !ok {
-		return nil, fmt.Errorf("utxo not found")
+		return nil, btcwallet.ErrOutputSpent
 	}
 
 	return &utxo, nil

routing/router.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"runtime"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -23,6 +24,7 @@ import (
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwallet"
+	"github.com/lightningnetwork/lnd/lnwallet/btcwallet"
 	"github.com/lightningnetwork/lnd/lnwallet/chanvalidate"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/multimutex"
@@ -1309,6 +1311,37 @@ func (r *ChannelRouter) processUpdate(msg interface{},
 		channelID := lnwire.NewShortChanIDFromInt(msg.ChannelID)
 		fundingTx, err := r.fetchFundingTx(&channelID)
 		if err != nil {
+			// In order to ensure we don't errnosuly mark a channel as
+			// a zmobie due to an RPC failure, we'll attempt to string
+			// match for the relevant errors.
+			//
+			// * btcd:
+			//    * https://github.com/btcsuite/btcd/blob/master/rpcserver.go#L1316
+			//    * https://github.com/btcsuite/btcd/blob/master/rpcserver.go#L1086
+			// * bitcoind:
+			//    * https://github.com/bitcoin/bitcoin/blob/7fcf53f7b4524572d1d0c9a5fdc388e87eb02416/src/rpc/blockchain.cpp#L770
+			//     * https://github.com/bitcoin/bitcoin/blob/7fcf53f7b4524572d1d0c9a5fdc388e87eb02416/src/rpc/blockchain.cpp#L954
+			switch {
+			case strings.Contains(err.Error(), "not found"):
+				fallthrough
+
+			case strings.Contains(err.Error(), "out of range"):
+				// If the funding transaction isn't found at all, then
+				// we'll mark the edge itself as a zombie so we don't
+				// continue to request it. We use the "zero key" for
+				// both node pubkeys so this edge can't be resurrected.
+				var zeroKey [33]byte
+				zErr := r.cfg.Graph.MarkEdgeZombie(
+					msg.ChannelID, zeroKey, zeroKey,
+				)
+				if zErr != nil {
+					return fmt.Errorf("unable to mark spent "+
+						"chan(id=%v) as a zombie: %w", msg.ChannelID,
+						zErr)
+				}
+			default:
+			}
+
 			return newErrf(ErrNoFundingTransaction, "unable to "+
 				"fetch funding tx for chan_id=%v: %v",
 				msg.ChannelID, err)
@@ -1355,6 +1388,22 @@ func (r *ChannelRouter) processUpdate(msg interface{},
 			r.quit,
 		)
 		if err != nil {
+			// If we fail validation of the UTXO here, then we'll
+			// mark the channel as a zombie as otherwise, we may
+			// continue to continue to request it. We use the "zero
+			// key" for both node pubkeys so this edge can't be
+			// resurrected.
+			if errors.Is(err, btcwallet.ErrOutputSpent) {
+				var zeroKey [33]byte
+				zErr := r.cfg.Graph.MarkEdgeZombie(
+					msg.ChannelID, zeroKey, zeroKey,
+				)
+				if zErr != nil {
+					return fmt.Errorf("unable to mark spent "+
+						"chan(id=%v) as a zombie: %w", msg.ChannelID,
+						zErr)
+				}
+			}
 
 			return newErrf(ErrChannelSpent, "unable to fetch utxo "+
 				"for chan_id=%v, chan_point=%v: %v",

routing/router_test.go

@@ -3136,3 +3136,115 @@ func TestBuildRoute(t *testing.T) {
 		t.Fatalf("unexpected no channel error node")
 	}
 }
+
+// edgeCreationModifier is an enum-like type used to modify steps that are
+// skipped when creating a channel in the test context.
+type edgeCreationModifier uint8
+
+const (
+	// edgeCreationNoFundingTx is used to skip adding the funding
+	// transaction of an edge to the chain.
+	edgeCreationNoFundingTx edgeCreationModifier = iota
+
+	// edgeCreationNoUTXO is used to skip adding the UTXO of a channel to
+	// the UTXO set.
+	edgeCreationNoUTXO
+)
+
+// newChannelEdgeInfo is a helper function used to create a new channel edge,
+// possibly skipping adding it to parts of the chain/state as well.
+func newChannelEdgeInfo(ctx *testCtx, fundingHeight uint32,
+	ecm edgeCreationModifier) (*channeldb.ChannelEdgeInfo, error) {
+
+	node1, err := createTestNode()
+	if err != nil {
+		return nil, err
+	}
+	node2, err := createTestNode()
+	if err != nil {
+		return nil, err
+	}
+
+	fundingTx, _, chanID, err := createChannelEdge(
+		ctx, bitcoinKey1.SerializeCompressed(),
+		bitcoinKey2.SerializeCompressed(), 100, fundingHeight,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("unable to create edge: %w", err)
+	}
+
+	edge := &channeldb.ChannelEdgeInfo{
+		ChannelID:     chanID.ToUint64(),
+		NodeKey1Bytes: node1.PubKeyBytes,
+		NodeKey2Bytes: node2.PubKeyBytes,
+	}
+	copy(edge.BitcoinKey1Bytes[:], bitcoinKey1.SerializeCompressed())
+	copy(edge.BitcoinKey2Bytes[:], bitcoinKey2.SerializeCompressed())
+
+	if ecm == edgeCreationNoFundingTx {
+		return edge, nil
+	}
+
+	fundingBlock := &wire.MsgBlock{
+		Transactions: []*wire.MsgTx{fundingTx},
+	}
+	ctx.chain.addBlock(fundingBlock, chanID.BlockHeight, chanID.BlockHeight)
+
+	if ecm == edgeCreationNoUTXO {
+		ctx.chain.delUtxo(wire.OutPoint{
+			Hash: fundingTx.TxHash(),
+		})
+	}
+
+	return edge, nil
+}
+
+func assertChanChainRejection(t *testing.T, ctx *testCtx,
+	edge *channeldb.ChannelEdgeInfo, failCode errorCode) {
+
+	t.Helper()
+
+	err := ctx.router.AddEdge(edge)
+	if !IsError(err, failCode) {
+		t.Fatalf("validation should have failed: %v", err)
+	}
+
+	// This channel should now be present in the zombie channel index.
+	_, _, _, isZombie, err := ctx.graph.HasChannelEdge(
+		edge.ChannelID,
+	)
+	require.Nil(t, err)
+	require.True(t, isZombie, "edge should be marked as zombie")
+}
+
+// TestChannelOnChainRejectionZombie tests that if we fail validating a channel
+// due to some sort of on-chain rejection (no funding transaction, or invalid
+// UTXO), then we'll mark the channel as a zombie.
+func TestChannelOnChainRejectionZombie(t *testing.T) {
+	t.Parallel()
+
+	ctx, cleanup, err := createTestCtxSingleNode(0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanup()
+
+	// To start,  we'll make an edge for the channel, but we won't add the
+	// funding transaction to the mock blockchain, which should cause the
+	// validation to fail below.
+	edge, err := newChannelEdgeInfo(ctx, 1, edgeCreationNoFundingTx)
+	require.Nil(t, err)
+
+	// We expect this to fail as the transaction isn't present in the
+	// chain (nor the block).
+	assertChanChainRejection(t, ctx, edge, ErrNoFundingTransaction)
+
+	// Next, we'll make another channel edge, but actually add it to the
+	// graph this time.
+	edge, err = newChannelEdgeInfo(ctx, 2, edgeCreationNoUTXO)
+	require.Nil(t, err)
+
+	// Instead now, we'll remove it from the set of UTXOs which should
+	// cause the spentness validation to fail.
+	assertChanChainRejection(t, ctx, edge, ErrChannelSpent)
+}