diff --git a/channeldb/graph.go b/channeldb/graph.go index fac890178..b3d1924c6 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -3397,6 +3397,38 @@ func (c *ChannelGraph) NewChannelEdgePolicy() *ChannelEdgePolicy { return &ChannelEdgePolicy{db: c.db} } +// MarkEdgeZombie attempts to mark a channel identified by its channel ID as a +// zombie. This method is used on an ad-hoc basis, when channels need to be +// marked as zombies outside the normal pruning cycle. +func (c *ChannelGraph) MarkEdgeZombie(chanID uint64, + pubKey1, pubKey2 [33]byte) error { + + c.cacheMu.Lock() + defer c.cacheMu.Unlock() + + err := kvdb.Batch(c.db, func(tx kvdb.RwTx) error { + edges := tx.ReadWriteBucket(edgeBucket) + if edges == nil { + return ErrGraphNoEdgesFound + } + zombieIndex, err := edges.CreateBucketIfNotExists(zombieBucket) + if err != nil { + return fmt.Errorf("unable to create zombie "+ + "bucket: %w", err) + } + + return markEdgeZombie(zombieIndex, chanID, pubKey1, pubKey2) + }) + if err != nil { + return err + } + + c.rejectCache.remove(chanID) + c.chanCache.remove(chanID) + + return nil +} + // markEdgeZombie marks an edge as a zombie within our zombie index. The public // keys should represent the node public keys of the two parties involved in the // edge. diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 0708c3ad1..b19ea82f4 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -3045,6 +3045,20 @@ func TestGraphZombieIndex(t *testing.T) { t.Fatal("expected edge to not be marked as zombie") } assertNumZombies(t, graph, 0) + + // If we mark the edge as a zombie manually, then it should show up as + // being a zombie once again. + err = graph.MarkEdgeZombie( + edge.ChannelID, node1.PubKeyBytes, node2.PubKeyBytes, + ) + if err != nil { + t.Fatalf("unable to mark edge as zombie: %v", err) + } + isZombie, _, _ = graph.IsZombieEdge(edge.ChannelID) + if !isZombie { + t.Fatal("expected edge to be marked as zombie") + } + assertNumZombies(t, graph, 1) } // compareNodes is used to compare two LightningNodes while excluding the diff --git a/routing/errors.go b/routing/errors.go index a8bd6fbaa..a5c7fd3ca 100644 --- a/routing/errors.go +++ b/routing/errors.go @@ -15,6 +15,19 @@ const ( // this update can't bring us something new, or because a node // announcement was given for node not found in any channel. ErrIgnored + + // ErrChannelSpent is returned when we go to validate a channel, but + // the purported funding output has actually already been spent on + // chain. + ErrChannelSpent + + // ErrNoFundingTransaction is returned when we are unable to find the + // funding transaction described by the short channel ID on chain. + ErrNoFundingTransaction + + // ErrInvalidFundingOutput is returned if the channle funding output + // fails validation. + ErrInvalidFundingOutput ) // routerError is a structure that represent the error inside the routing package, diff --git a/routing/notifications_test.go b/routing/notifications_test.go index 07e8e3a34..c5f9a6c43 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 6472e3441..76d1291d4 100644 --- a/routing/router.go +++ b/routing/router.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "runtime" + "strings" "sync" "sync/atomic" "time" @@ -25,6 +26,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" @@ -1276,6 +1278,22 @@ func (r *ChannelRouter) assertNodeAnnFreshness(node route.Vertex, return nil } +// addZombieEdge adds a channel that failed complete validation into the zombie +// index so we can avoid having to re-validate it in the future. +func (r *ChannelRouter) addZombieEdge(chanID uint64) error { + // If the edge fails validation 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 + err := r.cfg.Graph.MarkEdgeZombie(chanID, zeroKey, zeroKey) + if err != nil { + return fmt.Errorf("unable to mark spent chan(id=%v) as a "+ + "zombie: %w", chanID, err) + } + + return nil +} + // processUpdate processes a new relate authenticated channel/edge, node or // channel/edge update network update. If the update didn't affect the internal // state of the draft due to either being out of date, invalid, or redundant, @@ -1343,8 +1361,36 @@ func (r *ChannelRouter) processUpdate(msg interface{}, channelID := lnwire.NewShortChanIDFromInt(msg.ChannelID) fundingTx, err := r.fetchFundingTx(&channelID) if err != nil { - return errors.Errorf("unable to fetch funding tx for "+ - "chan_id=%v: %v", msg.ChannelID, err) + // In order to ensure we don't erroneously mark a + // channel as a zombie 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. + zErr := r.addZombieEdge(msg.ChannelID) + if zErr != nil { + return zErr + } + + default: + } + + return newErrf(ErrNoFundingTransaction, "unable to "+ + "locate funding tx: %v", err) } // Recreate witness output to be sure that declared in channel @@ -1373,7 +1419,14 @@ func (r *ChannelRouter) processUpdate(msg interface{}, FundingTx: fundingTx, }) if err != nil { - return err + // Mark the edge as a zombie so we won't try to + // re-validate it on start up. + if err := r.addZombieEdge(msg.ChannelID); err != nil { + return err + } + + return newErrf(ErrInvalidFundingOutput, "output "+ + "failed validation: %w", err) } // Now that we have the funding outpoint of the channel, ensure @@ -1388,7 +1441,14 @@ func (r *ChannelRouter) processUpdate(msg interface{}, r.quit, ) if err != nil { - return fmt.Errorf("unable to fetch utxo "+ + if errors.Is(err, btcwallet.ErrOutputSpent) { + zErr := r.addZombieEdge(msg.ChannelID) + if zErr != nil { + return zErr + } + } + + return newErrf(ErrChannelSpent, "unable to fetch utxo "+ "for chan_id=%v, chan_point=%v: %v", msg.ChannelID, fundingPoint, err) } @@ -1538,9 +1598,9 @@ func (r *ChannelRouter) fetchFundingTx( // block. numTxns := uint32(len(fundingBlock.Transactions)) if chanID.TxIndex > numTxns-1 { - return nil, fmt.Errorf("tx_index=#%v is out of range "+ - "(max_index=%v), network_chan_id=%v", chanID.TxIndex, - numTxns-1, chanID) + return nil, fmt.Errorf("tx_index=#%v "+ + "is out of range (max_index=%v), network_chan_id=%v", + chanID.TxIndex, numTxns-1, chanID) } return fundingBlock.Transactions[chanID.TxIndex], nil diff --git a/routing/router_test.go b/routing/router_test.go index 78ff6c687..b4739916b 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -3138,3 +3138,129 @@ 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 + + // edgeCreationBadScript is used to create the edge, but use the wrong + // scrip which should cause it to fail output validation. + edgeCreationBadScript +) + +// 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(), + }) + } + + if ecm == edgeCreationBadScript { + fundingTx.TxOut[0].PkScript[0] ^= 1 + } + + 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) + + // If we cause the funding transaction the chain to fail validation, we + // should see similar behavior. + edge, err = newChannelEdgeInfo(ctx, 3, edgeCreationBadScript) + require.Nil(t, err) + assertChanChainRejection(t, ctx, edge, ErrInvalidFundingOutput) +}