routing: add new error for failed funding tx validation

In this commit we add a new error for when we fail to validate the
funding transaction (invalid script, etc) and mark it as a zombie like
the other failed validation cases.
This commit is contained in:
Olaoluwa Osuntokun 2021-04-22 18:41:15 -05:00
parent 92c47983cb
commit e0ce384f02
No known key found for this signature in database
GPG Key ID: 3BBD59E99B280306
3 changed files with 59 additions and 32 deletions

@ -24,6 +24,10 @@ const (
// ErrNoFundingTransaction is returned when we are unable to find the // ErrNoFundingTransaction is returned when we are unable to find the
// funding transaction described by the short channel ID on chain. // funding transaction described by the short channel ID on chain.
ErrNoFundingTransaction ErrNoFundingTransaction
// ErrInvalidFundingOutput is returned if the channle funding output
// fails validation.
ErrInvalidFundingOutput
) )
// routerError is a structure that represent the error inside the routing package, // routerError is a structure that represent the error inside the routing package,

@ -1244,6 +1244,22 @@ func (r *ChannelRouter) assertNodeAnnFreshness(node route.Vertex,
return nil 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 // processUpdate processes a new relate authenticated channel/edge, node or
// channel/edge update network update. If the update didn't affect the internal // 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, // 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) channelID := lnwire.NewShortChanIDFromInt(msg.ChannelID)
fundingTx, err := r.fetchFundingTx(&channelID) fundingTx, err := r.fetchFundingTx(&channelID)
if err != nil { if err != nil {
// In order to ensure we don't errnosuly mark a channel as // In order to ensure we don't erroneously mark a
// a zmobie due to an RPC failure, we'll attempt to string // channel as a zombie due to an RPC failure, we'll
// match for the relevant errors. // attempt to string match for the relevant errors.
// //
// * btcd: // * btcd:
// * https://github.com/btcsuite/btcd/blob/master/rpcserver.go#L1316 // * https://github.com/btcsuite/btcd/blob/master/rpcserver.go#L1316
@ -1326,25 +1342,21 @@ func (r *ChannelRouter) processUpdate(msg interface{},
fallthrough fallthrough
case strings.Contains(err.Error(), "out of range"): case strings.Contains(err.Error(), "out of range"):
// If the funding transaction isn't found at all, then // If the funding transaction isn't found at
// we'll mark the edge itself as a zombie so we don't // all, then we'll mark the edge itself as a
// continue to request it. We use the "zero key" for // zombie so we don't continue to request it.
// both node pubkeys so this edge can't be resurrected. // We use the "zero key" for both node pubkeys
var zeroKey [33]byte // so this edge can't be resurrected.
zErr := r.cfg.Graph.MarkEdgeZombie( zErr := r.addZombieEdge(msg.ChannelID)
msg.ChannelID, zeroKey, zeroKey,
)
if zErr != nil { if zErr != nil {
return fmt.Errorf("unable to mark spent "+ return zErr
"chan(id=%v) as a zombie: %w", msg.ChannelID,
zErr)
} }
default: default:
} }
return newErrf(ErrNoFundingTransaction, "unable to "+ return newErrf(ErrNoFundingTransaction, "unable to "+
"fetch funding tx for chan_id=%v: %v", "locate funding tx: %v", err)
msg.ChannelID, err)
} }
// Recreate witness output to be sure that declared in channel // Recreate witness output to be sure that declared in channel
@ -1373,9 +1385,16 @@ func (r *ChannelRouter) processUpdate(msg interface{},
FundingTx: fundingTx, FundingTx: fundingTx,
}) })
if err != nil { if err != nil {
// 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 err
} }
return newErrf(ErrInvalidFundingOutput, "output "+
"failed validation: %w", err)
}
// Now that we have the funding outpoint of the channel, ensure // Now that we have the funding outpoint of the channel, ensure
// that it hasn't yet been spent. If so, then this channel has // that it hasn't yet been spent. If so, then this channel has
// been closed so we'll ignore it. // been closed so we'll ignore it.
@ -1388,20 +1407,10 @@ func (r *ChannelRouter) processUpdate(msg interface{},
r.quit, r.quit,
) )
if err != nil { 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) { if errors.Is(err, btcwallet.ErrOutputSpent) {
var zeroKey [33]byte zErr := r.addZombieEdge(msg.ChannelID)
zErr := r.cfg.Graph.MarkEdgeZombie(
msg.ChannelID, zeroKey, zeroKey,
)
if zErr != nil { if zErr != nil {
return fmt.Errorf("unable to mark spent "+ return zErr
"chan(id=%v) as a zombie: %w", msg.ChannelID,
zErr)
} }
} }
@ -1555,9 +1564,9 @@ func (r *ChannelRouter) fetchFundingTx(
// block. // block.
numTxns := uint32(len(fundingBlock.Transactions)) numTxns := uint32(len(fundingBlock.Transactions))
if chanID.TxIndex > numTxns-1 { if chanID.TxIndex > numTxns-1 {
return nil, fmt.Errorf("tx_index=#%v is out of range "+ return nil, fmt.Errorf("tx_index=#%v "+
"(max_index=%v), network_chan_id=%v", chanID.TxIndex, "is out of range (max_index=%v), network_chan_id=%v",
numTxns-1, chanID) chanID.TxIndex, numTxns-1, chanID)
} }
return fundingBlock.Transactions[chanID.TxIndex], nil return fundingBlock.Transactions[chanID.TxIndex], nil

@ -3149,6 +3149,10 @@ const (
// edgeCreationNoUTXO is used to skip adding the UTXO of a channel to // edgeCreationNoUTXO is used to skip adding the UTXO of a channel to
// the UTXO set. // the UTXO set.
edgeCreationNoUTXO 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, // 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 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 // Instead now, we'll remove it from the set of UTXOs which should
// cause the spentness validation to fail. // cause the spentness validation to fail.
assertChanChainRejection(t, ctx, edge, ErrChannelSpent) 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)
} }