From b117daaa3c93aeb3f137b85d0ae27824138ed5a5 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 30 Jan 2025 12:59:43 +0200 Subject: [PATCH] discovery+graph: convert errors from codes to variables In preparation for moving funding transaction validiation from the Builder to the Gossiper in later commit, we first convert these graph Error Codes to normal error variables. This will help make the later commit a pure code move. --- discovery/gossiper.go | 9 +++---- discovery/gossiper_test.go | 51 +++++++++++++++----------------------- graph/builder.go | 10 +++----- graph/builder_test.go | 6 ++--- graph/errors.go | 32 ++++++++++++++---------- 5 files changed, 49 insertions(+), 59 deletions(-) diff --git a/discovery/gossiper.go b/discovery/gossiper.go index e5cb2ee65..9cff71c6e 100644 --- a/discovery/gossiper.go +++ b/discovery/gossiper.go @@ -2678,10 +2678,9 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg, return anns, true - case graph.IsError( - err, graph.ErrNoFundingTransaction, - graph.ErrInvalidFundingOutput, - ): + case errors.Is(err, graph.ErrNoFundingTransaction), + errors.Is(err, graph.ErrInvalidFundingOutput): + key := newRejectCacheKey( scid.ToUint64(), sourceToPub(nMsg.source), @@ -2695,7 +2694,7 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg, d.banman.incrementBanScore(nMsg.peer.PubKey()) } - case graph.IsError(err, graph.ErrChannelSpent): + case errors.Is(err, graph.ErrChannelSpent): key := newRejectCacheKey( scid.ToUint64(), sourceToPub(nMsg.source), diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index 5e2ebeac4..c21d61fc3 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -24,7 +24,6 @@ import ( "github.com/lightningnetwork/lnd/batch" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" - "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/graph" graphdb "github.com/lightningnetwork/lnd/graph/db" "github.com/lightningnetwork/lnd/graph/db/models" @@ -76,13 +75,13 @@ var ( type mockGraphSource struct { bestHeight uint32 - mu sync.Mutex - nodes []models.LightningNode - infos map[uint64]models.ChannelEdgeInfo - edges map[uint64][]models.ChannelEdgePolicy - zombies map[uint64][][33]byte - chansToReject map[uint64]struct{} - addEdgeErrCode fn.Option[graph.ErrorCode] + mu sync.Mutex + nodes []models.LightningNode + infos map[uint64]models.ChannelEdgeInfo + edges map[uint64][]models.ChannelEdgePolicy + zombies map[uint64][][33]byte + chansToReject map[uint64]struct{} + addEdgeErr error } func newMockRouter(height uint32) *mockGraphSource { @@ -113,10 +112,8 @@ func (r *mockGraphSource) AddEdge(info *models.ChannelEdgeInfo, r.mu.Lock() defer r.mu.Unlock() - if r.addEdgeErrCode.IsSome() { - return graph.NewErrf( - r.addEdgeErrCode.UnsafeFromSome(), "received error", - ) + if r.addEdgeErr != nil { + return r.addEdgeErr } if _, ok := r.infos[info.ChannelID]; ok { @@ -131,12 +128,12 @@ func (r *mockGraphSource) AddEdge(info *models.ChannelEdgeInfo, return nil } -func (r *mockGraphSource) resetAddEdgeErrCode() { - r.addEdgeErrCode = fn.None[graph.ErrorCode]() +func (r *mockGraphSource) resetAddEdgeErr() { + r.addEdgeErr = nil } -func (r *mockGraphSource) setAddEdgeErrCode(code graph.ErrorCode) { - r.addEdgeErrCode = fn.Some[graph.ErrorCode](code) +func (r *mockGraphSource) setAddEdgeErr(err error) { + r.addEdgeErr = err } func (r *mockGraphSource) queueValidationFail(chanID uint64) { @@ -4185,7 +4182,7 @@ func TestChanAnnBanningNonChanPeer(t *testing.T) { remoteKeyPriv2.PubKey(), nil, nil, atomic.Bool{}, } - ctx.router.setAddEdgeErrCode(graph.ErrInvalidFundingOutput) + ctx.router.setAddEdgeErr(graph.ErrInvalidFundingOutput) // Loop 100 times to get nodePeer banned. for i := 0; i < 100; i++ { @@ -4199,11 +4196,7 @@ func TestChanAnnBanningNonChanPeer(t *testing.T) { case err = <-ctx.gossiper.ProcessRemoteAnnouncement( ca, nodePeer1, ): - require.True( - t, graph.IsError( - err, graph.ErrInvalidFundingOutput, - ), - ) + require.ErrorIs(t, err, graph.ErrInvalidFundingOutput) case <-time.After(2 * time.Second): t.Fatalf("remote announcement not processed") @@ -4221,11 +4214,11 @@ func TestChanAnnBanningNonChanPeer(t *testing.T) { // Set the error to ErrChannelSpent so that we can test that the // gossiper ignores closed channels. - ctx.router.setAddEdgeErrCode(graph.ErrChannelSpent) + ctx.router.setAddEdgeErr(graph.ErrChannelSpent) select { case err = <-ctx.gossiper.ProcessRemoteAnnouncement(ca, nodePeer2): - require.True(t, graph.IsError(err, graph.ErrChannelSpent)) + require.ErrorIs(t, err, graph.ErrChannelSpent) case <-time.After(2 * time.Second): t.Fatalf("remote announcement not processed") @@ -4248,7 +4241,7 @@ func TestChanAnnBanningNonChanPeer(t *testing.T) { // Reset the AddEdge error and pass the same announcement again. An // error should be returned even though AddEdge won't fail. - ctx.router.resetAddEdgeErrCode() + ctx.router.resetAddEdgeErr() select { case err = <-ctx.gossiper.ProcessRemoteAnnouncement(ca, nodePeer2): @@ -4269,7 +4262,7 @@ func TestChanAnnBanningChanPeer(t *testing.T) { nodePeer := &mockPeer{remoteKeyPriv1.PubKey(), nil, nil, atomic.Bool{}} - ctx.router.setAddEdgeErrCode(graph.ErrInvalidFundingOutput) + ctx.router.setAddEdgeErr(graph.ErrInvalidFundingOutput) // Loop 100 times to get nodePeer banned. for i := 0; i < 100; i++ { @@ -4283,11 +4276,7 @@ func TestChanAnnBanningChanPeer(t *testing.T) { case err = <-ctx.gossiper.ProcessRemoteAnnouncement( ca, nodePeer, ): - require.True( - t, graph.IsError( - err, graph.ErrInvalidFundingOutput, - ), - ) + require.ErrorIs(t, err, graph.ErrInvalidFundingOutput) case <-time.After(2 * time.Second): t.Fatalf("remote announcement not processed") diff --git a/graph/builder.go b/graph/builder.go index 57f00c02a..3b18a30a3 100644 --- a/graph/builder.go +++ b/graph/builder.go @@ -1314,8 +1314,7 @@ func (b *Builder) addEdge(edge *models.ChannelEdgeInfo, default: } - return NewErrf(ErrNoFundingTransaction, "unable to "+ - "locate funding tx: %v", err) + return fmt.Errorf("%w: %w", ErrNoFundingTransaction, err) } // Recreate witness output to be sure that declared in channel edge @@ -1348,8 +1347,7 @@ func (b *Builder) addEdge(edge *models.ChannelEdgeInfo, return err } - return NewErrf(ErrInvalidFundingOutput, "output failed "+ - "validation: %w", err) + return fmt.Errorf("%w: %w", ErrInvalidFundingOutput, err) } // Now that we have the funding outpoint of the channel, ensure @@ -1366,8 +1364,8 @@ func (b *Builder) addEdge(edge *models.ChannelEdgeInfo, } } - return NewErrf(ErrChannelSpent, "unable to fetch utxo for "+ - "chan_id=%v, chan_point=%v: %v", edge.ChannelID, + return fmt.Errorf("%w: unable to fetch utxo for chan_id=%v, "+ + "chan_point=%v: %w", ErrChannelSpent, scid.ToUint64(), fundingPoint, err) } diff --git a/graph/builder_test.go b/graph/builder_test.go index 24aff2624..5b17da57b 100644 --- a/graph/builder_test.go +++ b/graph/builder_test.go @@ -1275,14 +1275,12 @@ func newChannelEdgeInfo(t *testing.T, ctx *testCtx, fundingHeight uint32, } func assertChanChainRejection(t *testing.T, ctx *testCtx, - edge *models.ChannelEdgeInfo, failCode ErrorCode) { + edge *models.ChannelEdgeInfo, expectedErr error) { t.Helper() err := ctx.builder.AddEdge(edge) - if !IsError(err, failCode) { - t.Fatalf("validation should have failed: %v", err) - } + require.ErrorIs(t, err, expectedErr) // This channel should now be present in the zombie channel index. _, _, _, isZombie, err := ctx.graph.HasChannelEdge( diff --git a/graph/errors.go b/graph/errors.go index 30996bb94..4192a26aa 100644 --- a/graph/errors.go +++ b/graph/errors.go @@ -2,6 +2,25 @@ package graph import "github.com/go-errors/errors" +var ( + // ErrNoFundingTransaction is returned when we are unable to find the + // funding transaction described by the short channel ID on chain. + ErrNoFundingTransaction = errors.New( + "unable to find the funding transaction", + ) + + // ErrInvalidFundingOutput is returned if the channel funding output + // fails validation. + ErrInvalidFundingOutput = errors.New( + "channel funding output validation failed", + ) + + // ErrChannelSpent is returned when we go to validate a channel, but + // the purported funding output has actually already been spent on + // chain. + ErrChannelSpent = errors.New("channel output has been spent") +) + // ErrorCode is used to represent the various errors that can occur within this // package. type ErrorCode uint8 @@ -15,19 +34,6 @@ 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 channel funding output - // fails validation. - ErrInvalidFundingOutput ) // Error is a structure that represent the error inside the graph package,