From b117daaa3c93aeb3f137b85d0ae27824138ed5a5 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 30 Jan 2025 12:59:43 +0200 Subject: [PATCH 1/6] 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, From 8f37699db3feba7bc447bfcebf0d2ff29cf0494b Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 30 Jan 2025 13:12:15 +0200 Subject: [PATCH 2/6] discovery: prepare tests for shared chain state Convert a bunch of the helper functions to instead be methods on the testCtx type. This is in preparation for adding a mockChain to the testCtx that these helpers can then use to add blocks and utxos to. See `notifications_test.go` for an idea of what we are trying to emulate here. Once the funding tx code has moved to the gossiper, then the logic in `notifications_test.go` will be removed. --- discovery/gossiper_test.go | 101 ++++++++++++++++++++++--------------- 1 file changed, 60 insertions(+), 41 deletions(-) diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index c21d61fc3..c493d2124 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -468,15 +468,23 @@ type annBatch struct { remoteProofAnn *lnwire.AnnounceSignatures1 } -func createLocalAnnouncements(blockHeight uint32) (*annBatch, error) { - return createAnnouncements(blockHeight, selfKeyPriv, remoteKeyPriv1) +func (ctx *testCtx) createLocalAnnouncements(blockHeight uint32) (*annBatch, + error) { + + return ctx.createAnnouncements(blockHeight, selfKeyPriv, remoteKeyPriv1) } -func createRemoteAnnouncements(blockHeight uint32) (*annBatch, error) { - return createAnnouncements(blockHeight, remoteKeyPriv1, remoteKeyPriv2) +func (ctx *testCtx) createRemoteAnnouncements(blockHeight uint32) (*annBatch, + error) { + + return ctx.createAnnouncements( + blockHeight, remoteKeyPriv1, remoteKeyPriv2, + ) } -func createAnnouncements(blockHeight uint32, key1, key2 *btcec.PrivateKey) (*annBatch, error) { +func (ctx *testCtx) createAnnouncements(blockHeight uint32, key1, + key2 *btcec.PrivateKey) (*annBatch, error) { + var err error var batch annBatch timestamp := testTimestamp @@ -491,7 +499,9 @@ func createAnnouncements(blockHeight uint32, key1, key2 *btcec.PrivateKey) (*ann return nil, err } - batch.chanAnn, err = createChannelAnnouncement(blockHeight, key1, key2) + batch.chanAnn, err = ctx.createChannelAnnouncement( + blockHeight, key1, key2, + ) if err != nil { return nil, err } @@ -616,7 +626,7 @@ func signUpdate(nodeKey *btcec.PrivateKey, a *lnwire.ChannelUpdate1) error { return nil } -func createAnnouncementWithoutProof(blockHeight uint32, +func (ctx *testCtx) createAnnouncementWithoutProof(blockHeight uint32, key1, key2 *btcec.PublicKey, extraBytes ...[]byte) *lnwire.ChannelAnnouncement1 { @@ -639,16 +649,21 @@ func createAnnouncementWithoutProof(blockHeight uint32, return a } -func createRemoteChannelAnnouncement(blockHeight uint32, +func (ctx *testCtx) createRemoteChannelAnnouncement(blockHeight uint32, extraBytes ...[]byte) (*lnwire.ChannelAnnouncement1, error) { - return createChannelAnnouncement(blockHeight, remoteKeyPriv1, remoteKeyPriv2, extraBytes...) + return ctx.createChannelAnnouncement( + blockHeight, remoteKeyPriv1, remoteKeyPriv2, extraBytes..., + ) } -func createChannelAnnouncement(blockHeight uint32, key1, key2 *btcec.PrivateKey, +func (ctx *testCtx) createChannelAnnouncement(blockHeight uint32, key1, + key2 *btcec.PrivateKey, extraBytes ...[]byte) (*lnwire.ChannelAnnouncement1, error) { - a := createAnnouncementWithoutProof(blockHeight, key1.PubKey(), key2.PubKey(), extraBytes...) + a := ctx.createAnnouncementWithoutProof( + blockHeight, key1.PubKey(), key2.PubKey(), extraBytes..., + ) signer := mock.SingleSigner{Privkey: key1} sig, err := netann.SignAnnouncement(&signer, testKeyLoc, a) @@ -846,7 +861,7 @@ func TestProcessAnnouncement(t *testing.T) { // First, we'll craft a valid remote channel announcement and send it to // the gossiper so that it can be processed. - ca, err := createRemoteChannelAnnouncement(0) + ca, err := ctx.createRemoteChannelAnnouncement(0) require.NoError(t, err, "can't create channel announcement") select { @@ -958,7 +973,7 @@ func TestPrematureAnnouncement(t *testing.T) { // remote side, but block height of this announcement is greater than // highest know to us, for that reason it should be ignored and not // added to the router. - ca, err := createRemoteChannelAnnouncement(1) + ca, err := ctx.createRemoteChannelAnnouncement(1) require.NoError(t, err, "can't create channel announcement") select { @@ -996,7 +1011,7 @@ func TestSignatureAnnouncementLocalFirst(t *testing.T) { } } - batch, err := createLocalAnnouncements(0) + batch, err := ctx.createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") remoteKey, err := btcec.ParsePubKey(batch.nodeAnn2.NodeID[:]) @@ -1172,7 +1187,7 @@ func TestOrphanSignatureAnnouncement(t *testing.T) { } } - batch, err := createLocalAnnouncements(0) + batch, err := ctx.createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") remoteKey, err := btcec.ParsePubKey(batch.nodeAnn2.NodeID[:]) @@ -1343,7 +1358,7 @@ func TestSignatureAnnouncementRetryAtStartup(t *testing.T) { ctx, err := createTestCtx(t, proofMatureDelta, false) require.NoError(t, err, "can't create context") - batch, err := createLocalAnnouncements(0) + batch, err := ctx.createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") remoteKey, err := btcec.ParsePubKey(batch.nodeAnn2.NodeID[:]) @@ -1578,7 +1593,7 @@ func TestSignatureAnnouncementFullProofWhenRemoteProof(t *testing.T) { ctx, err := createTestCtx(t, proofMatureDelta, false) require.NoError(t, err, "can't create context") - batch, err := createLocalAnnouncements(0) + batch, err := ctx.createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") remoteKey, err := btcec.ParsePubKey(batch.nodeAnn2.NodeID[:]) @@ -1766,6 +1781,8 @@ func TestDeDuplicatedAnnouncements(t *testing.T) { timestamp := testTimestamp announcements := deDupedAnnouncements{} announcements.Reset() + ctx, err := createTestCtx(t, 0, false) + require.NoError(t, err) // Ensure that after new deDupedAnnouncements struct is created and // reset that storage of each announcement type is empty. @@ -1781,7 +1798,7 @@ func TestDeDuplicatedAnnouncements(t *testing.T) { // Ensure that remote channel announcements are properly stored // and de-duplicated. - ca, err := createRemoteChannelAnnouncement(0) + ca, err := ctx.createRemoteChannelAnnouncement(0) require.NoError(t, err, "can't create remote channel announcement") nodePeer := &mockPeer{bitcoinKeyPub2, nil, nil, atomic.Bool{}} @@ -1797,7 +1814,7 @@ func TestDeDuplicatedAnnouncements(t *testing.T) { // We'll create a second instance of the same announcement with the // same channel ID. Adding this shouldn't cause an increase in the // number of items as they should be de-duplicated. - ca2, err := createRemoteChannelAnnouncement(0) + ca2, err := ctx.createRemoteChannelAnnouncement(0) require.NoError(t, err, "can't create remote channel announcement") announcements.AddMsgs(networkMsg{ msg: ca2, @@ -2022,7 +2039,7 @@ func TestForwardPrivateNodeAnnouncement(t *testing.T) { // We'll start off by processing a channel announcement without a proof // (i.e., an unadvertised channel), followed by a node announcement for // this same channel announcement. - chanAnn := createAnnouncementWithoutProof( + chanAnn := ctx.createAnnouncementWithoutProof( startingHeight-2, selfKeyDesc.PubKey, remoteKeyPub1, ) pubKey := remoteKeyPriv1.PubKey() @@ -2068,7 +2085,9 @@ func TestForwardPrivateNodeAnnouncement(t *testing.T) { // by opening a public channel on the network. We'll create a // ChannelAnnouncement and hand it off to the gossiper in order to // process it. - remoteChanAnn, err := createRemoteChannelAnnouncement(startingHeight - 1) + remoteChanAnn, err := ctx.createRemoteChannelAnnouncement( + startingHeight - 1, + ) require.NoError(t, err, "unable to create remote channel announcement") peer := &mockPeer{pubKey, nil, nil, atomic.Bool{}} @@ -2118,7 +2137,7 @@ func TestRejectZombieEdge(t *testing.T) { ctx, err := createTestCtx(t, 0, false) require.NoError(t, err, "unable to create test context") - batch, err := createRemoteAnnouncements(0) + batch, err := ctx.createRemoteAnnouncements(0) require.NoError(t, err, "unable to create announcements") remotePeer := &mockPeer{pk: remoteKeyPriv2.PubKey()} @@ -2219,7 +2238,7 @@ func TestProcessZombieEdgeNowLive(t *testing.T) { ctx, err := createTestCtx(t, 0, false) require.NoError(t, err, "unable to create test context") - batch, err := createRemoteAnnouncements(0) + batch, err := ctx.createRemoteAnnouncements(0) require.NoError(t, err, "unable to create announcements") remotePeer := &mockPeer{pk: remoteKeyPriv1.PubKey()} @@ -2374,7 +2393,7 @@ func TestReceiveRemoteChannelUpdateFirst(t *testing.T) { ctx, err := createTestCtx(t, proofMatureDelta, false) require.NoError(t, err, "can't create context") - batch, err := createLocalAnnouncements(0) + batch, err := ctx.createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") remoteKey, err := btcec.ParsePubKey(batch.nodeAnn2.NodeID[:]) @@ -2581,7 +2600,7 @@ func TestExtraDataChannelAnnouncementValidation(t *testing.T) { // that we don't know of ourselves, but should still include in the // final signature check. extraBytes := []byte("gotta validate this still!") - ca, err := createRemoteChannelAnnouncement(0, extraBytes) + ca, err := ctx.createRemoteChannelAnnouncement(0, extraBytes) require.NoError(t, err, "can't create channel announcement") // We'll now send the announcement to the main gossiper. We should be @@ -2613,7 +2632,7 @@ func TestExtraDataChannelUpdateValidation(t *testing.T) { // In this scenario, we'll create two announcements, one regular // channel announcement, and another channel update announcement, that // has additional data that we won't be interpreting. - chanAnn, err := createRemoteChannelAnnouncement(0) + chanAnn, err := ctx.createRemoteChannelAnnouncement(0) require.NoError(t, err, "unable to create chan ann") chanUpdAnn1, err := createUpdateAnnouncement( 0, 0, remoteKeyPriv1, timestamp, @@ -2729,7 +2748,7 @@ func TestRetransmit(t *testing.T) { ctx, err := createTestCtx(t, proofMatureDelta, false) require.NoError(t, err, "can't create context") - batch, err := createLocalAnnouncements(0) + batch, err := ctx.createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") remoteKey, err := btcec.ParsePubKey(batch.nodeAnn2.NodeID[:]) @@ -2835,7 +2854,7 @@ func TestNodeAnnouncementNoChannels(t *testing.T) { ctx, err := createTestCtx(t, 0, false) require.NoError(t, err, "can't create context") - batch, err := createRemoteAnnouncements(0) + batch, err := ctx.createRemoteAnnouncements(0) require.NoError(t, err, "can't generate announcements") remoteKey, err := btcec.ParsePubKey(batch.nodeAnn2.NodeID[:]) @@ -2928,7 +2947,7 @@ func TestOptionalFieldsChannelUpdateValidation(t *testing.T) { // In this scenario, we'll test whether the message flags field in a // channel update is properly handled. - chanAnn, err := createRemoteChannelAnnouncement(chanUpdateHeight) + chanAnn, err := ctx.createRemoteChannelAnnouncement(chanUpdateHeight) require.NoError(t, err, "can't create channel announcement") select { @@ -3019,7 +3038,7 @@ func TestSendChannelUpdateReliably(t *testing.T) { ctx, err := createTestCtx(t, proofMatureDelta, false) require.NoError(t, err, "unable to create test context") - batch, err := createLocalAnnouncements(0) + batch, err := ctx.createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") // We'll also create two keys, one for ourselves and another for the @@ -3376,7 +3395,7 @@ func TestPropagateChanPolicyUpdate(t *testing.T) { const numChannels = 3 channelsToAnnounce := make([]*annBatch, 0, numChannels) for i := 0; i < numChannels; i++ { - newChan, err := createLocalAnnouncements(uint32(i + 1)) + newChan, err := ctx.createLocalAnnouncements(uint32(i + 1)) if err != nil { t.Fatalf("unable to make new channel ann: %v", err) } @@ -3553,10 +3572,10 @@ func TestProcessChannelAnnouncementOptionalMsgFields(t *testing.T) { ctx, err := createTestCtx(t, 0, false) require.NoError(t, err, "unable to create test context") - chanAnn1 := createAnnouncementWithoutProof( + chanAnn1 := ctx.createAnnouncementWithoutProof( 100, selfKeyDesc.PubKey, remoteKeyPub1, ) - chanAnn2 := createAnnouncementWithoutProof( + chanAnn2 := ctx.createAnnouncementWithoutProof( 101, selfKeyDesc.PubKey, remoteKeyPub1, ) @@ -3772,7 +3791,7 @@ func TestBroadcastAnnsAfterGraphSynced(t *testing.T) { // A remote channel announcement should not be broadcast since the graph // has not yet been synced. - chanAnn1, err := createRemoteChannelAnnouncement(0) + chanAnn1, err := ctx.createRemoteChannelAnnouncement(0) require.NoError(t, err, "unable to create channel announcement") assertBroadcast(chanAnn1, true, false) @@ -3786,7 +3805,7 @@ func TestBroadcastAnnsAfterGraphSynced(t *testing.T) { // should to be broadcast. ctx.gossiper.syncMgr.markGraphSynced() - chanAnn2, err := createRemoteChannelAnnouncement(1) + chanAnn2, err := ctx.createRemoteChannelAnnouncement(1) require.NoError(t, err, "unable to create channel announcement") assertBroadcast(chanAnn2, true, true) } @@ -3811,7 +3830,7 @@ func TestRateLimitChannelUpdates(t *testing.T) { // We'll create a batch of signed announcements, including updates for // both sides, for a channel and process them. They should all be // forwarded as this is our first time learning about the channel. - batch, err := createRemoteAnnouncements(blockHeight) + batch, err := ctx.createRemoteAnnouncements(blockHeight) require.NoError(t, err) nodePeer1 := &mockPeer{ @@ -3951,7 +3970,7 @@ func TestIgnoreOwnAnnouncement(t *testing.T) { ctx, err := createTestCtx(t, proofMatureDelta, false) require.NoError(t, err, "can't create context") - batch, err := createLocalAnnouncements(0) + batch, err := ctx.createLocalAnnouncements(0) require.NoError(t, err, "can't generate announcements") remoteKey, err := btcec.ParsePubKey(batch.nodeAnn2.NodeID[:]) @@ -4097,7 +4116,7 @@ func TestRejectCacheChannelAnn(t *testing.T) { // First, we create a channel announcement to send over to our test // peer. - batch, err := createRemoteAnnouncements(0) + batch, err := ctx.createRemoteAnnouncements(0) require.NoError(t, err, "can't generate announcements") remoteKey, err := btcec.ParsePubKey(batch.nodeAnn2.NodeID[:]) @@ -4189,7 +4208,7 @@ func TestChanAnnBanningNonChanPeer(t *testing.T) { // Craft a valid channel announcement for a channel we don't // have. We will ensure that it fails validation by modifying // the router. - ca, err := createRemoteChannelAnnouncement(uint32(i)) + ca, err := ctx.createRemoteChannelAnnouncement(uint32(i)) require.NoError(t, err, "can't create channel announcement") select { @@ -4209,7 +4228,7 @@ func TestChanAnnBanningNonChanPeer(t *testing.T) { // Assert that nodePeer has been disconnected. require.True(t, nodePeer1.disconnected.Load()) - ca, err := createRemoteChannelAnnouncement(101) + ca, err := ctx.createRemoteChannelAnnouncement(101) require.NoError(t, err, "can't create channel announcement") // Set the error to ErrChannelSpent so that we can test that the @@ -4269,7 +4288,7 @@ func TestChanAnnBanningChanPeer(t *testing.T) { // Craft a valid channel announcement for a channel we don't // have. We will ensure that it fails validation by modifying // the router. - ca, err := createRemoteChannelAnnouncement(uint32(i)) + ca, err := ctx.createRemoteChannelAnnouncement(uint32(i)) require.NoError(t, err, "can't create channel announcement") select { From c354fd8f703bf54f58c2273dae98b4bc00f4f0f9 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Fri, 7 Feb 2025 15:27:41 +0200 Subject: [PATCH 3/6] lnmock: let MockChain also implement lnwallet.BlockChainIO Note that a compile-time assertion was not added as this leads to an import cycle. --- lnmock/chain.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lnmock/chain.go b/lnmock/chain.go index c20a89373..1470513b6 100644 --- a/lnmock/chain.go +++ b/lnmock/chain.go @@ -74,6 +74,18 @@ func (m *MockChain) GetBlockHeader(hash *chainhash.Hash) ( return args.Get(0).(*wire.BlockHeader), args.Error(1) } +func (m *MockChain) GetUtxo(op *wire.OutPoint, pkScript []byte, + heightHint uint32, cancel <-chan struct{}) (*wire.TxOut, error) { + + args := m.Called(op, pkScript, heightHint, cancel) + + if args.Get(0) == nil { + return nil, args.Error(1) + } + + return args.Get(0).(*wire.TxOut), args.Error(1) +} + func (m *MockChain) IsCurrent() bool { args := m.Called() From b6210632f24bdb335f566a97cd2cfedd3a00f400 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Fri, 7 Feb 2025 15:33:39 +0200 Subject: [PATCH 4/6] discovery: prep testCtx with a mock Chain This is in preparation for moving the funding transaction validation code to the gossiper from the graph.Builder since then the gossiper will start making GetBlockHash/GetBlock and GetUtxo calls. --- discovery/gossiper_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index c493d2124..8d1ce32e1 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -28,6 +28,7 @@ import ( graphdb "github.com/lightningnetwork/lnd/graph/db" "github.com/lightningnetwork/lnd/graph/db/models" "github.com/lightningnetwork/lnd/keychain" + "github.com/lightningnetwork/lnd/lnmock" "github.com/lightningnetwork/lnd/lnpeer" "github.com/lightningnetwork/lnd/lntest/mock" "github.com/lightningnetwork/lnd/lntest/wait" @@ -715,10 +716,12 @@ func mockFindChannel(node *btcec.PublicKey, chanID lnwire.ChannelID) ( } type testCtx struct { + t *testing.T gossiper *AuthenticatedGossiper router *mockGraphSource notifier *mockNotifier broadcastedMessage chan msgWithSenders + chain *lnmock.MockChain } func createTestCtx(t *testing.T, startHeight uint32, isChanPeer bool) ( @@ -730,6 +733,10 @@ func createTestCtx(t *testing.T, startHeight uint32, isChanPeer bool) ( // broadcast functions won't be populated. notifier := newMockNotifier() router := newMockRouter(startHeight) + chain := &lnmock.MockChain{} + t.Cleanup(func() { + chain.AssertExpectations(t) + }) db := channeldb.OpenForTesting(t, t.TempDir()) @@ -761,6 +768,7 @@ func createTestCtx(t *testing.T, startHeight uint32, isChanPeer bool) ( } gossiper := New(Config{ + ChainIO: chain, Notifier: notifier, Broadcast: func(senders map[route.Vertex]struct{}, msgs ...lnwire.Message) error { @@ -832,10 +840,12 @@ func createTestCtx(t *testing.T, startHeight uint32, isChanPeer bool) ( }) return &testCtx{ + t: t, router: router, notifier: notifier, gossiper: gossiper, broadcastedMessage: broadcastedMessage, + chain: chain, }, nil } From 011d819315cffc7269f581515642ce3d3ed923e9 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 30 Jan 2025 13:31:45 +0200 Subject: [PATCH 5/6] discovery: update chanAnn creation methods to take modifier options In preparation for adding more modifiers. We want to later add a modifier that will tweak the errors returned by the mock chain once funding transaction validation has been moved to the gossiper. --- discovery/gossiper_test.go | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index 8d1ce32e1..afd07047a 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -627,9 +627,26 @@ func signUpdate(nodeKey *btcec.PrivateKey, a *lnwire.ChannelUpdate1) error { return nil } +type fundingTxOpts struct { + extraBytes []byte +} + +type fundingTxOption func(*fundingTxOpts) + +func withExtraBytes(extraBytes []byte) fundingTxOption { + return func(opts *fundingTxOpts) { + opts.extraBytes = extraBytes + } +} + func (ctx *testCtx) createAnnouncementWithoutProof(blockHeight uint32, key1, key2 *btcec.PublicKey, - extraBytes ...[]byte) *lnwire.ChannelAnnouncement1 { + options ...fundingTxOption) *lnwire.ChannelAnnouncement1 { + + var opts fundingTxOpts + for _, opt := range options { + opt(&opts) + } a := &lnwire.ChannelAnnouncement1{ ShortChannelID: lnwire.ShortChannelID{ @@ -643,27 +660,25 @@ func (ctx *testCtx) createAnnouncementWithoutProof(blockHeight uint32, copy(a.NodeID2[:], key2.SerializeCompressed()) copy(a.BitcoinKey1[:], bitcoinKeyPub1.SerializeCompressed()) copy(a.BitcoinKey2[:], bitcoinKeyPub2.SerializeCompressed()) - if len(extraBytes) == 1 { - a.ExtraOpaqueData = extraBytes[0] - } + a.ExtraOpaqueData = opts.extraBytes return a } func (ctx *testCtx) createRemoteChannelAnnouncement(blockHeight uint32, - extraBytes ...[]byte) (*lnwire.ChannelAnnouncement1, error) { + opts ...fundingTxOption) (*lnwire.ChannelAnnouncement1, error) { return ctx.createChannelAnnouncement( - blockHeight, remoteKeyPriv1, remoteKeyPriv2, extraBytes..., + blockHeight, remoteKeyPriv1, remoteKeyPriv2, opts..., ) } func (ctx *testCtx) createChannelAnnouncement(blockHeight uint32, key1, key2 *btcec.PrivateKey, - extraBytes ...[]byte) (*lnwire.ChannelAnnouncement1, error) { + opts ...fundingTxOption) (*lnwire.ChannelAnnouncement1, error) { a := ctx.createAnnouncementWithoutProof( - blockHeight, key1.PubKey(), key2.PubKey(), extraBytes..., + blockHeight, key1.PubKey(), key2.PubKey(), opts..., ) signer := mock.SingleSigner{Privkey: key1} @@ -2610,7 +2625,9 @@ func TestExtraDataChannelAnnouncementValidation(t *testing.T) { // that we don't know of ourselves, but should still include in the // final signature check. extraBytes := []byte("gotta validate this still!") - ca, err := ctx.createRemoteChannelAnnouncement(0, extraBytes) + ca, err := ctx.createRemoteChannelAnnouncement( + 0, withExtraBytes(extraBytes), + ) require.NoError(t, err, "can't create channel announcement") // We'll now send the announcement to the main gossiper. We should be From d0255871359e6c732e6f19398a8fe3079fffa0ce Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Fri, 7 Feb 2025 16:23:47 +0200 Subject: [PATCH 6/6] docs: update existing release notes --- docs/release-notes/release-notes-0.19.0.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index 44df0bd3c..316da07e1 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -243,8 +243,11 @@ The underlying functionality between those two options remain the same. * [Golang was updated to `v1.22.11`](https://github.com/lightningnetwork/lnd/pull/9462). -* Various refactors to simplify the - `graph.Builder` [1](https://github.com/lightningnetwork/lnd/pull/9476). +* Various refactors and preparations to simplify the + `graph.Builder` and to move the funding tx validation to the gossiper. + [1](https://github.com/lightningnetwork/lnd/pull/9476) + [2](https://github.com/lightningnetwork/lnd/pull/9477) + ## Breaking Changes ## Performance Improvements