diff --git a/routing/errors.go b/routing/errors.go index b7421e6d..a5c7fd3c 100644 --- a/routing/errors.go +++ b/routing/errors.go @@ -24,6 +24,10 @@ const ( // 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/router.go b/routing/router.go index bbee6a65..8b91bbfd 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1244,6 +1244,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, @@ -1311,9 +1327,9 @@ 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. + // 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 @@ -1326,25 +1342,21 @@ func (r *ChannelRouter) processUpdate(msg interface{}, 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 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 fmt.Errorf("unable to mark spent "+ - "chan(id=%v) as a zombie: %w", msg.ChannelID, - zErr) + return zErr } + default: } return newErrf(ErrNoFundingTransaction, "unable to "+ - "fetch funding tx for chan_id=%v: %v", - msg.ChannelID, err) + "locate funding tx: %v", err) } // Recreate witness output to be sure that declared in channel @@ -1373,7 +1385,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,20 +1407,10 @@ 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, - ) + zErr := r.addZombieEdge(msg.ChannelID) if zErr != nil { - return fmt.Errorf("unable to mark spent "+ - "chan(id=%v) as a zombie: %w", msg.ChannelID, - zErr) + return zErr } } @@ -1555,9 +1564,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 b5de51b9..144ea13d 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -3149,6 +3149,10 @@ const ( // 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, @@ -3196,6 +3200,10 @@ func newChannelEdgeInfo(ctx *testCtx, fundingHeight uint32, }) } + if ecm == edgeCreationBadScript { + fundingTx.TxOut[0].PkScript[0] ^= 1 + } + return edge, nil } @@ -3247,4 +3255,10 @@ func TestChannelOnChainRejectionZombie(t *testing.T) { // 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) }