diff --git a/routing/notifications_test.go b/routing/notifications_test.go index 07e8e3a3..c5f9a6c4 100644 --- a/routing/notifications_test.go +++ b/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 "+ - "height %v", blockHeight) + return nil, fmt.Errorf("block number out of range: %v", + 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 diff --git a/routing/router.go b/routing/router.go index 7cb3a526..bbee6a65 100644 --- a/routing/router.go +++ b/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", diff --git a/routing/router_test.go b/routing/router_test.go index c530daec..b5de51b9 100644 --- a/routing/router_test.go +++ b/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) +}