From 01c09ec811a3694525fd1005e64948a69ee485f5 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 17 Dec 2024 14:59:49 +0100 Subject: [PATCH 01/15] funding: refactor testcase This will make it simple in the next commits to test soley single messages rather than always the combination of announcement and channel update msg. --- funding/manager_test.go | 188 +++++++++++++++++++++++----------------- 1 file changed, 107 insertions(+), 81 deletions(-) diff --git a/funding/manager_test.go b/funding/manager_test.go index b6130176d16..ab6bd485413 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -1171,105 +1171,131 @@ func assertChannelAnnouncements(t *testing.T, alice, bob *testNode, t.Helper() - // The following checks are to make sure the parameters are used - // correctly, as we currently only support 2 values, one for each node. - aliceCfg := alice.fundingMgr.cfg - if len(customMinHtlc) > 0 { - require.Len(t, customMinHtlc, 2, "incorrect usage") - } - if len(customMaxHtlc) > 0 { - require.Len(t, customMaxHtlc, 2, "incorrect usage") - } - if len(baseFees) > 0 { - require.Len(t, baseFees, 2, "incorrect usage") - } - if len(feeRates) > 0 { - require.Len(t, feeRates, 2, "incorrect usage") - } + // Validate custom parameter arrays have expected length + validateCustomParams( + t, customMinHtlc, customMaxHtlc, baseFees, feeRates, + ) - // After the ChannelReady message is sent, Alice and Bob will each send - // the following messages to their gossiper: - // 1) ChannelAnnouncement - // 2) ChannelUpdate - // The ChannelAnnouncement is kept locally, while the ChannelUpdate is - // sent directly to the other peer, so the edge policies are known to - // both peers. nodes := []*testNode{alice, bob} - for j, node := range nodes { - announcements := make([]lnwire.Message, 2) - for i := 0; i < len(announcements); i++ { - select { - case announcements[i] = <-node.announceChan: - case <-time.After(time.Second * 5): - t.Fatalf("node didn't send announcement: %v", i) - } + for i, node := range nodes { + // Each node should send exactly 2 announcements + // ChannelAnnouncement and ChannelUpdate. + announcements, updates := collectGossipMsgs(t, node, 2) + verifyNoExtraMsgs(t, node) + + require.Len(t, announcements, 1, "expected 1 "+ + "ChannelAnnouncement from node %d", i) + require.Len(t, updates, 1, "expected 1 ChannelUpdate from "+ + "node %d", i) + for _, update := range updates { + verifyChannelUpdate( + t, update, i, nodes, + node.fundingMgr.cfg, capacity, customMinHtlc, + customMaxHtlc, baseFees, feeRates, + ) } + } +} - gotChannelAnnouncement := false - gotChannelUpdate := false - for _, msg := range announcements { +// validateCustomParams ensures custom parameter arrays have valid length. +func validateCustomParams(t *testing.T, params ...[]lnwire.MilliSatoshi) { + t.Helper() + + for _, param := range params { + if len(param) > 0 { + require.Len(t, param, 2, "custom parameters must "+ + "have length 2") + } + } +} + +// collectGossipMsgs gathers the expected number of gossip messages from a node. +func collectGossipMsgs(t *testing.T, node *testNode, + expectedNum int) ([]*lnwire.ChannelAnnouncement1, + []*lnwire.ChannelUpdate1) { + + t.Helper() + + var ( + announcements []*lnwire.ChannelAnnouncement1 + updates []*lnwire.ChannelUpdate1 + ) + + for i := 0; i < expectedNum; i++ { + select { + case msg := <-node.announceChan: switch m := msg.(type) { case *lnwire.ChannelAnnouncement1: - gotChannelAnnouncement = true + announcements = append(announcements, m) + case *lnwire.ChannelUpdate1: + updates = append(updates, m) - // The channel update sent by the node should - // advertise the MinHTLC value required by the - // _other_ node. - other := (j + 1) % 2 - otherCfg := nodes[other].fundingMgr.cfg + default: + t.Fatalf("unexpected message type: %T", msg) + } - minHtlc := otherCfg.DefaultMinHtlcIn - maxHtlc := aliceCfg.RequiredRemoteMaxValue( - capacity, - ) - baseFee := aliceCfg.DefaultRoutingPolicy.BaseFee - feeRate := aliceCfg.DefaultRoutingPolicy.FeeRate + case <-time.After(5 * time.Second): + t.Fatalf("node did not send gossip msg %d", i) + } + } - require.EqualValues(t, 1, m.MessageFlags) + return announcements, updates +} - // We might expect a custom MinHTLC value. - if len(customMinHtlc) > 0 { - minHtlc = customMinHtlc[j] - } - require.Equal(t, minHtlc, m.HtlcMinimumMsat) +// verifyNoExtraMsgs ensures no unexpected msgs are sent to the gossiper. +func verifyNoExtraMsgs(t *testing.T, node *testNode) { + t.Helper() - // We might expect a custom MaxHltc value. - if len(customMaxHtlc) > 0 { - maxHtlc = customMaxHtlc[j] - } - require.Equal(t, maxHtlc, m.HtlcMaximumMsat) + select { + case msg := <-node.announceChan: + t.Fatalf("received unexpected announcement: %v", msg) + case <-time.After(300 * time.Millisecond): + // Expected - no extra msg. + } +} - // We might expect a custom baseFee value. - if len(baseFees) > 0 { - baseFee = baseFees[j] - } - require.EqualValues(t, baseFee, m.BaseFee) +// verifyChannelUpdate checks that a ChannelUpdate has the expected parameters. +func verifyChannelUpdate(t *testing.T, update *lnwire.ChannelUpdate1, + nodeIndex int, nodes []*testNode, nodeCfg *Config, + capacity btcutil.Amount, customMinHtlc, customMaxHtlc, + baseFees, feeRates []lnwire.MilliSatoshi) { - // We might expect a custom feeRate value. - if len(feeRates) > 0 { - feeRate = feeRates[j] - } - require.EqualValues(t, feeRate, m.FeeRate) + t.Helper() - gotChannelUpdate = true - } - } + require.EqualValues(t, 1, update.MessageFlags) - require.Truef( - t, gotChannelAnnouncement, - "ChannelAnnouncement from %d", j, - ) - require.Truef(t, gotChannelUpdate, "ChannelUpdate from %d", j) + // Get parameters for the other node since each update advertises + // the remote node's requirements + otherIdx := (nodeIndex + 1) % 2 + otherCfg := nodes[otherIdx].fundingMgr.cfg - // Make sure no other message is sent. - select { - case <-node.announceChan: - t.Fatalf("received unexpected announcement") - case <-time.After(300 * time.Millisecond): - // Expected - } + // Set expected values, using customs if provided + minHtlc := otherCfg.DefaultMinHtlcIn + if len(customMinHtlc) > 0 { + minHtlc = customMinHtlc[nodeIndex] + } + + maxHtlc := nodeCfg.RequiredRemoteMaxValue(capacity) + if len(customMaxHtlc) > 0 { + maxHtlc = customMaxHtlc[nodeIndex] + } + + baseFee := nodeCfg.DefaultRoutingPolicy.BaseFee + if len(baseFees) > 0 { + baseFee = baseFees[nodeIndex] } + + feeRate := nodeCfg.DefaultRoutingPolicy.FeeRate + if len(feeRates) > 0 { + feeRate = feeRates[nodeIndex] + } + + // Verify all parameters match expected values + require.Equal(t, minHtlc, update.HtlcMinimumMsat) + require.Equal(t, maxHtlc, update.HtlcMaximumMsat) + require.EqualValues(t, baseFee, update.BaseFee) + require.EqualValues(t, feeRate, update.FeeRate) } func assertAnnouncementSignatures(t *testing.T, alice, bob *testNode) { From 0c76e297ed85c318749b954c69a148be816bf6e5 Mon Sep 17 00:00:00 2001 From: ziggie Date: Fri, 24 May 2024 11:57:06 +0100 Subject: [PATCH 02/15] multi: make reassignment of edge atomic When the option SCID is used we need to make sure we update the channel data in an atomic way so that we don't miss any updates by our peer and remove all edge info which still uses the alias as the channel id (e.g. ChannelUpdate msgs). Moreover add unit test for the new ReAddEdge method. --- funding/manager.go | 119 +++++++++++++++++++++++++++++++--------- funding/manager_test.go | 39 ++++++++++++- graph/db/graph.go | 74 +++++++++++++++++++++++++ graph/db/graph_test.go | 82 +++++++++++++++++++++++++++ server.go | 54 +++++++++++++++--- 5 files changed, 331 insertions(+), 37 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index 395cccb2a6b..9373657c5d7 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -530,9 +530,12 @@ type Config struct { // the initiator for channels of the anchor type. MaxAnchorsCommitFeeRate chainfee.SatPerKWeight - // DeleteAliasEdge allows the Manager to delete an alias channel edge - // from the graph. It also returns our local to-be-deleted policy. - DeleteAliasEdge func(scid lnwire.ShortChannelID) ( + // ReAssignSCID allows the Manager to assign a new SCID to an + // option-scid channel being part of the underlying graph. This is + // necessary because option-scid channels change their scid during their + // lifetime (public zeroconf channels for example) so we need to make + // sure to update the underlying graph. + ReAssignSCID func(aliasScID, newScID lnwire.ShortChannelID) ( *models.ChannelEdgePolicy, error) // AliasManager is an implementation of the aliasHandler interface that @@ -3719,19 +3722,29 @@ func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, "maps: %v", err) } - // We'll delete the edge and add it again via - // addToGraph. This is because the peer may have - // sent us a ChannelUpdate with an alias and we don't - // want to relay this. - ourPolicy, err := f.cfg.DeleteAliasEdge(baseScid) + // We reassign the same scid to the graph db. This will + // trigger a deletion of the current edge data and + // reinsert the channel with the same edge info and + // policy. This is done to guarantee that potential + // ChannelUpdates using the alias as the scid are + // removed and not relayed to the broader network + // because the alias is not a verifiable channel id. + ourPolicy, err := f.cfg.ReAssignSCID( + baseScid, baseScid, + ) if err != nil { - return fmt.Errorf("failed deleting real edge "+ - "for alias channel from graph: %v", - err) + return fmt.Errorf("unable to reassign alias "+ + "edge in graph: %w", err) } - err = f.addToGraph( - completeChan, &baseScid, nil, ourPolicy, + log.Infof("Successfully reassigned alias edge in "+ + "graph(non-zeroconf): %v(%d) -> %v(%d)", + baseScid, baseScid.ToUint64(), + baseScid, baseScid.ToUint64()) + + // We send the rassigned ChannelUpdate to the peer. + err = f.sendChanUpdate( + completeChan, &baseScid, ourPolicy, ) if err != nil { return fmt.Errorf("failed to re-add to "+ @@ -3808,23 +3821,33 @@ func (f *Manager) waitForZeroConfChannel(c *channeldb.OpenChannel) error { "six confirmations: %v", err) } - // TODO: Make this atomic! - ourPolicy, err := f.cfg.DeleteAliasEdge(c.ShortChanID()) + // The underlying graph entry for this channel id needs to be + // reassigned with the new confirmed scid. Moreover channel + // updates with the alias scid are removed so that we do not + // relay them to the broader network. + ourPolicy, err := f.cfg.ReAssignSCID( + c.ShortChanID(), confChan.shortChanID, + ) if err != nil { - return fmt.Errorf("unable to delete alias edge from "+ - "graph: %v", err) + return fmt.Errorf("unable to reassign alias edge in "+ + "graph: %w", err) } - // We'll need to update the graph with the new ShortChannelID - // via an addToGraph call. We don't pass in the peer's - // alias since we'll be using the confirmed SCID from now on - // regardless if it's public or not. - err = f.addToGraph( - c, &confChan.shortChanID, nil, ourPolicy, + aliasScid := c.ShortChanID() + confirmedScid := confChan.shortChanID + + log.Infof("Successfully reassigned alias edge in "+ + "graph(zeroconf): %v(%d) -> %v(%d)", + aliasScid, aliasScid.ToUint64(), + confirmedScid, confirmedScid.ToUint64()) + + // Send the ChannelUpdate with the confirmed scid to the peer. + err = f.sendChanUpdate( + c, &confChan.shortChanID, ourPolicy, ) if err != nil { - return fmt.Errorf("failed adding confirmed zero-conf "+ - "SCID to graph: %v", err) + return fmt.Errorf("failed to send ChannelUpdate to "+ + "gossiper: %v", err) } } @@ -4590,6 +4613,52 @@ func (f *Manager) announceChannel(localIDKey, remoteIDKey *btcec.PublicKey, return nil } +// sendChanUpdate sends a ChannelUpdate to the gossiper which is as a +// consequence sent to the peer. +// +// TODO(ziggie): Refactor the gossip msgs so that not always all msgs have +// to be created but only the ones which are needed. +func (f *Manager) sendChanUpdate(completeChan *channeldb.OpenChannel, + shortChanID *lnwire.ShortChannelID, + ourPolicy *models.ChannelEdgePolicy) error { + + chanID := lnwire.NewChanIDFromOutPoint(completeChan.FundingOutpoint) + + fwdMinHTLC, fwdMaxHTLC := f.extractAnnounceParams(completeChan) + + ann, err := f.newChanAnnouncement( + f.cfg.IDKey, completeChan.IdentityPub, + &completeChan.LocalChanCfg.MultiSigKey, + completeChan.RemoteChanCfg.MultiSigKey.PubKey, *shortChanID, + chanID, fwdMinHTLC, fwdMaxHTLC, ourPolicy, + completeChan.ChanType, + ) + if err != nil { + return fmt.Errorf("error generating channel "+ + "announcement: %v", err) + } + + errChan := f.cfg.SendAnnouncement(ann.chanUpdateAnn) + select { + case err := <-errChan: + if err != nil { + if graph.IsError(err, graph.ErrOutdated, + graph.ErrIgnored) { + + log.Debugf("Graph rejected "+ + "ChannelUpdate: %v", err) + } else { + return fmt.Errorf("error sending channel "+ + "update: %v", err) + } + } + case <-f.quit: + return ErrFundingManagerShuttingDown + } + + return nil +} + // InitFundingWorkflow sends a message to the funding manager instructing it // to initiate a single funder workflow with the source peer. func (f *Manager) InitFundingWorkflow(msg *InitFundingMsg) { diff --git a/funding/manager_test.go b/funding/manager_test.go index ab6bd485413..ea6326b4a83 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -550,7 +550,8 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, NotifyOpenChannelEvent: evt.NotifyOpenChannelEvent, OpenChannelPredicate: chainedAcceptor, NotifyPendingOpenChannelEvent: evt.NotifyPendingOpenChannelEvent, - DeleteAliasEdge: func(scid lnwire.ShortChannelID) ( + ReAssignSCID: func(aliasScID, + newScID lnwire.ShortChannelID) ( *models.ChannelEdgePolicy, error) { return nil, nil @@ -674,7 +675,7 @@ func recreateAliceFundingManager(t *testing.T, alice *testNode) { ZombieSweeperInterval: oldCfg.ZombieSweeperInterval, ReservationTimeout: oldCfg.ReservationTimeout, OpenChannelPredicate: chainedAcceptor, - DeleteAliasEdge: oldCfg.DeleteAliasEdge, + ReAssignSCID: oldCfg.ReAssignSCID, AliasManager: oldCfg.AliasManager, AuxLeafStore: oldCfg.AuxLeafStore, AuxSigner: oldCfg.AuxSigner, @@ -1197,6 +1198,38 @@ func assertChannelAnnouncements(t *testing.T, alice, bob *testNode, } } +// assertChannelUpdate checks that a ChannelUpdate has been sent by the node +// with the expected parameters. +func assertChannelUpdates(t *testing.T, alice, bob *testNode, + capacity btcutil.Amount, customMinHtlc, customMaxHtlc, + baseFees, feeRates []lnwire.MilliSatoshi) { + + t.Helper() + + // Validate custom parameter arrays have expected length + validateCustomParams( + t, customMinHtlc, customMaxHtlc, baseFees, feeRates, + ) + + nodes := []*testNode{alice, bob} + for i, node := range nodes { + // Each node should send exactly 2 announcements + // ChannelAnnouncement and ChannelUpdate. + _, updates := collectGossipMsgs(t, node, 1) + verifyNoExtraMsgs(t, node) + + require.Len(t, updates, 1, "expected 1 ChannelUpdate from "+ + "node %d", i) + for _, update := range updates { + verifyChannelUpdate( + t, update, i, nodes, + node.fundingMgr.cfg, capacity, customMinHtlc, + customMaxHtlc, baseFees, feeRates, + ) + } + } +} + // validateCustomParams ensures custom parameter arrays have valid length. func validateCustomParams(t *testing.T, params ...[]lnwire.MilliSatoshi) { t.Helper() @@ -4620,7 +4653,7 @@ func testZeroConf(t *testing.T, chanType *lnwire.ChannelType) { // For taproot channels, we don't expect them to be announced atm. if !isTaprootChanType(chanType) { - assertChannelAnnouncements( + assertChannelUpdates( t, alice, bob, fundingAmt, nil, nil, nil, nil, ) } diff --git a/graph/db/graph.go b/graph/db/graph.go index 2a91398c624..3e94acb4ccc 100644 --- a/graph/db/graph.go +++ b/graph/db/graph.go @@ -1178,6 +1178,80 @@ func (c *ChannelGraph) addChannelEdge(tx kvdb.RwTx, return chanIndex.Put(b.Bytes(), chanKey[:]) } +// ReAddChannelEdge removes the edge with the given channel ID from the +// database and adds the new edge to guarantee atomicity. +// This is important for option-scid-alias channels which may change its SCID +// over the course of its lifetime (e.g., public zero-conf channels). +func (c *ChannelGraph) ReAddChannelEdge( + chanID uint64, newEdge *models.ChannelEdgeInfo, + ourPolicy *models.ChannelEdgePolicy) error { + + c.cacheMu.Lock() + defer c.cacheMu.Unlock() + + err := kvdb.Update(c.db, func(tx kvdb.RwTx) error { + edges := tx.ReadWriteBucket(edgeBucket) + if edges == nil { + return ErrEdgeNotFound + } + edgeIndex := edges.NestedReadWriteBucket(edgeIndexBucket) + if edgeIndex == nil { + return ErrEdgeNotFound + } + chanIndex := edges.NestedReadWriteBucket(channelPointBucket) + if chanIndex == nil { + return ErrEdgeNotFound + } + nodes := tx.ReadWriteBucket(nodeBucket) + if nodes == nil { + return ErrGraphNodeNotFound + } + + var rawChanID [8]byte + byteOrder.PutUint64(rawChanID[:], chanID) + + // We don't mark this channel as zombie, because we are readding + // it immediately after deleting it below. This method also + // deletes the channel from the graph cache, however there is + // no entry in the cache because we only add it if we have + // received the proof(signatures). + err := c.delChannelEdgeUnsafe( + edges, edgeIndex, chanIndex, nil, + rawChanID[:], false, false, + ) + if err != nil { + return err + } + + // Now we add the channel with the new edge info. + err = c.addChannelEdge(tx, newEdge) + if err != nil { + return err + } + + // Also add the new channel update from our side. + _, err = updateEdgePolicy(tx, ourPolicy, c.graphCache) + + return err + }, func() {}) + if err != nil { + return err + } + + // Remove the Cache entries. + c.rejectCache.remove(chanID) + c.chanCache.remove(chanID) + + // We also make sure we clear the reject cache because we might have + // received a channel update msg with the new SCID from our peer which + // would have been put in the reject cache because the channel was not + // part of the graph. + c.rejectCache.remove(newEdge.ChannelID) + c.chanCache.remove(newEdge.ChannelID) + + return nil +} + // HasChannelEdge returns true if the database knows of a channel edge with the // passed channel ID, and false otherwise. If an edge with that ID is found // within the graph, then two time stamps representing the last time the edge diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index 8a02f24ff41..62e45df9238 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -4063,3 +4063,85 @@ func TestClosedScid(t *testing.T) { require.Nil(t, err) require.True(t, exists) } + +// TestUpdateAliasChannel tests that the underlying graph channel information +// for an alias channel is deleted and readded under a different short channel +// id. +func TestUpdateAliasChannel(t *testing.T) { + t.Parallel() + + graph, err := MakeTestGraph(t) + require.NoError(t, err, "unable to make test database") + + // Create first node and add it to the graph. + node1, err := createTestVertex(graph.db) + require.NoError(t, err, "unable to create test node") + err = graph.AddLightningNode(node1) + require.NoError(t, err) + + // Create second node and add it to the graph. + node2, err := createTestVertex(graph.db) + require.NoError(t, err, "unable to create test node") + err = graph.AddLightningNode(node2) + require.NoError(t, err) + + // Adding a new channel edge to the graph. + edgeInfo, edge1, edge2 := createChannelEdge( + graph.db, node1, node2, + ) + if err := graph.AddChannelEdge(edgeInfo); err != nil { + t.Fatalf("unable to create channel edge: %v", err) + } + + // Add both edge policies. + err = graph.UpdateEdgePolicy(edge1) + require.NoError(t, err) + + err = graph.UpdateEdgePolicy(edge2) + require.NoError(t, err) + + oldSCID := edgeInfo.ChannelID + + // Define the new channel id (under real conditions this is the new + // confirmed channel id. For non-zeroconf alias channels this is the + // same as the old SCID). but we for this test require a different SCID + // and therefore we add 1 to the old SCID. + newChanSCID := oldSCID + 1 + + // Readd the channel edgeInfo under a different scid. + newEdgeInfo := new(models.ChannelEdgeInfo) + *newEdgeInfo = *edgeInfo + newEdgeInfo.ChannelID = newChanSCID + + // Create a new edge policy with the new channel id. + newEdge1 := new(models.ChannelEdgePolicy) + *newEdge1 = *edge1 + newEdge1.ChannelID = newChanSCID + + // Re-add the channel edge to the graph which deletes the old data and + // inserts the new edge data (edge info and edge policy). + err = graph.ReAddChannelEdge(oldSCID, newEdgeInfo, newEdge1) + require.NoError(t, err) + + // The old channel data should not exist anymore. + _, _, _, err = graph.FetchChannelEdgesByID(oldSCID) + require.ErrorIs(t, err, ErrEdgeNotFound, "channel should not exist") + + // Fetch the new channel data. + newEdgeInfo, newEdge1, newEdge2, err := graph.FetchChannelEdgesByID( + newChanSCID, + ) + require.NoError(t, err, "unable to fetch channel by ID") + + // Edge 1 should be different and should have another channel id. + err = compareEdgePolicies(newEdge1, edge1) + require.ErrorContains(t, err, "ChannelID doesn't match") + + // Edge 2 should be nil, because we deleted the former peer data. + require.Nil(t, newEdge2) + + // Because we did not change the signatures the edge info should be + // the same as soon as we swap in the old channel id. + newEdgeInfo.ChannelID = oldSCID + assertEdgeInfoEqual(t, newEdgeInfo, edgeInfo) +} diff --git a/server.go b/server.go index 799bfde6b87..8961b4bd516 100644 --- a/server.go +++ b/server.go @@ -1380,13 +1380,13 @@ func newServer(cfg *Config, listenAddrs []net.Addr, return nil, err } - // Wrap the DeleteChannelEdges method so that the funding manager can + // Wrap the `ReAddChannelEdge` method so that the funding manager can // use it without depending on several layers of indirection. - deleteAliasEdge := func(scid lnwire.ShortChannelID) ( + reAssignSCID := func(aliasScID, newScID lnwire.ShortChannelID) ( *models.ChannelEdgePolicy, error) { info, e1, e2, err := s.graphDB.FetchChannelEdgesByID( - scid.ToUint64(), + aliasScID.ToUint64(), ) if errors.Is(err, graphdb.ErrEdgeNotFound) { // This is unlikely but there is a slim chance of this @@ -1398,7 +1398,13 @@ func newServer(cfg *Config, listenAddrs []net.Addr, return nil, err } - // Grab our key to find our policy. + // We create a new ChannelEdgeInfo with the new SCID. + newEdgeInfo := new(models.ChannelEdgeInfo) + *newEdgeInfo = *info + newEdgeInfo.ChannelID = newScID.ToUint64() + + // We also readd the channel policy from our side with the new + // short channel id so we grab our key to find our policy. var ourKey [33]byte copy(ourKey[:], nodeKeyDesc.PubKey.SerializeCompressed()) @@ -1410,13 +1416,43 @@ func newServer(cfg *Config, listenAddrs []net.Addr, } if ourPolicy == nil { - // Something is wrong, so return an error. - return nil, fmt.Errorf("we don't have an edge") + // We should always have our policy available. If that + // is not the case there might be an error in the + // ChannelUpdate msg logic so we return early. + return nil, fmt.Errorf("edge policy not found") } - err = s.graphDB.DeleteChannelEdges( - false, false, scid.ToUint64(), + // Update the policy data, this invalidates the signature + // therefore we need to resign the data. + ourPolicy.ChannelID = newEdgeInfo.ChannelID + chanUpdate := netann.UnsignedChannelUpdateFromEdge( + newEdgeInfo, ourPolicy, ) + + data, err := chanUpdate.DataToSign() + if err != nil { + return nil, err + } + + nodeSig, err := cc.MsgSigner.SignMessage( + nodeKeyDesc.KeyLocator, data, true, + ) + if err != nil { + return nil, err + } + + sig, err := lnwire.NewSigFromSignature(nodeSig) + if err != nil { + return nil, err + } + ourPolicy.SetSigBytes(sig.ToSignatureBytes()) + + // Delete the old edge information under the alias SCID and add + // the updated data with the new SCID. + err = s.graphDB.ReAddChannelEdge( + aliasScID.ToUint64(), newEdgeInfo, ourPolicy, + ) + return ourPolicy, err } @@ -1612,7 +1648,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr, EnableUpfrontShutdown: cfg.EnableUpfrontShutdown, MaxAnchorsCommitFeeRate: chainfee.SatPerKVByte( s.cfg.MaxCommitFeeRateAnchors * 1000).FeePerKWeight(), - DeleteAliasEdge: deleteAliasEdge, + ReAssignSCID: reAssignSCID, AliasManager: s.aliasMgr, IsSweeperOutpoint: s.sweeper.IsSweeperOutpoint, AuxFundingController: implCfg.AuxFundingController, From ce27b3450062bd32fcf5c99248130f1d3d98d213 Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 16 Dec 2024 15:07:07 +0100 Subject: [PATCH 03/15] lnwire: add string method to channel_ready type --- lnwire/channel_ready.go | 16 ++++++++++++++++ peer/brontide.go | 3 +-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lnwire/channel_ready.go b/lnwire/channel_ready.go index 912a068bded..813f1c1ed03 100644 --- a/lnwire/channel_ready.go +++ b/lnwire/channel_ready.go @@ -2,6 +2,7 @@ package lnwire import ( "bytes" + "fmt" "io" "github.com/btcsuite/btcd/btcec/v2" @@ -170,3 +171,18 @@ func (c *ChannelReady) Encode(w *bytes.Buffer, _ uint32) error { func (c *ChannelReady) MsgType() MessageType { return MsgChannelReady } + +// String returns a human-readable description of the ChannelReady msg. +func (c *ChannelReady) String() string { + // Handle the case where the AliasScid is nil. + aliasStr := "nil" + if c.AliasScid != nil { + aliasStr = fmt.Sprintf("%v(uint=%d)", + c.AliasScid, c.AliasScid.ToUint64()) + } + + return fmt.Sprintf("chan_id=%v, next_point=%x, aliasSCID=%s", + c.ChanID, c.NextPerCommitmentPoint.SerializeCompressed(), + aliasStr, + ) +} diff --git a/peer/brontide.go b/peer/brontide.go index f0399b4e8a8..c7fc4b6f465 100644 --- a/peer/brontide.go +++ b/peer/brontide.go @@ -2268,8 +2268,7 @@ func messageSummary(msg lnwire.Message) string { return fmt.Sprintf("chan_id=%v", msg.ChanID) case *lnwire.ChannelReady: - return fmt.Sprintf("chan_id=%v, next_point=%x", - msg.ChanID, msg.NextPerCommitmentPoint.SerializeCompressed()) + return msg.String() case *lnwire.Shutdown: return fmt.Sprintf("chan_id=%v, script=%x", msg.ChannelID, From 48f78427f62e17d348d67aca7aa98fb9a346463d Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 17 Dec 2024 15:05:31 +0100 Subject: [PATCH 04/15] graph: add channel point to utxo filter In case we assume valid channels or have alias channels we make sure we also add the channel point to the block filter so that we are notified if the channel point is spent in a block. --- graph/builder.go | 63 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 15 deletions(-) diff --git a/graph/builder.go b/graph/builder.go index d6984af7091..e47b03fef21 100644 --- a/graph/builder.go +++ b/graph/builder.go @@ -1206,6 +1206,16 @@ func (b *Builder) processUpdate(msg interface{}, "chan_id=%v", msg.ChannelID) } + // Look up the funding pk script so that we can register the + // channel output in the UTXO filter. + fundingPkScript, err := makeFundingScript( + msg.BitcoinKey1Bytes[:], msg.BitcoinKey2Bytes[:], + msg.Features, msg.TapscriptRoot, + ) + if err != nil { + return err + } + // If AssumeChannelValid is present, then we are unable to // perform any of the expensive checks below, so we'll // short-circuit our path straight to adding the edge to our @@ -1219,10 +1229,44 @@ func (b *Builder) processUpdate(msg interface{}, if err != nil { return fmt.Errorf("unable to add edge: %w", err) } - log.Tracef("New channel discovered! Link "+ - "connects %x and %x with ChannelID(%v)", - msg.NodeKey1Bytes, msg.NodeKey2Bytes, - msg.ChannelID) + + // Use different log levels based on channel type. + if b.cfg.IsAlias(scid) { + log.Debugf("New alias channel discovered! "+ + "Link connects %x and %x with "+ + "ChannelID(%v)", msg.NodeKey1Bytes, + msg.NodeKey2Bytes, + msg.ChannelID) + + // For alias channels, we make sure we add the + // channel to the UTXO filter so that we are + // notified if/when this channel is closed. + // This is safe because for zeroconf we trust + // the funding tx anyway. And for non-zeroconf + // alias channel we would only reach this point + // if the funding tx is confirmed. + // + //nolint:ll + filterUpdate := []graphdb.EdgePoint{ + { + FundingPkScript: fundingPkScript, + OutPoint: msg.ChannelPoint, + }, + } + err = b.cfg.ChainView.UpdateFilter( + filterUpdate, b.bestHeight.Load(), + ) + if err != nil { + return errors.Errorf("unable to "+ + "update chain view: %v", err) + } + } else { + log.Tracef("New channel discovered! Link "+ + "connects %x and %x with ChannelID(%v)", + msg.NodeKey1Bytes, msg.NodeKey2Bytes, + msg.ChannelID) + } + b.stats.incNumEdgesDiscovered() break @@ -1270,17 +1314,6 @@ func (b *Builder) processUpdate(msg interface{}, "locate funding tx: %v", err) } - // Recreate witness output to be sure that declared in channel - // edge bitcoin keys and channel value corresponds to the - // reality. - fundingPkScript, err := makeFundingScript( - msg.BitcoinKey1Bytes[:], msg.BitcoinKey2Bytes[:], - msg.Features, msg.TapscriptRoot, - ) - if err != nil { - return err - } - // Next we'll validate that this channel is actually well // formed. If this check fails, then this channel either // doesn't exist, or isn't the one that was meant to be created From 55a4ab676915df9737078a0bea6e77e291fdf595 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 17 Dec 2024 15:07:09 +0100 Subject: [PATCH 05/15] graph: fix log output --- graph/builder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graph/builder.go b/graph/builder.go index e47b03fef21..898812441d4 100644 --- a/graph/builder.go +++ b/graph/builder.go @@ -1724,7 +1724,7 @@ func (b *Builder) IsStaleNode(node route.Vertex, // then we know that this is actually a stale announcement. err := b.assertNodeAnnFreshness(node, timestamp) if err != nil { - log.Debugf("Checking stale node %x got %v", node, err) + log.Debugf("Checking stale node %v got %v", node, err) return true } From fc0a4772b393d899c8d6088d3891ceced0b81b97 Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 16 Dec 2024 15:09:37 +0100 Subject: [PATCH 06/15] docs: add release-notes --- docs/release-notes/release-notes-0.19.0.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index 43ed7ebbcdc..7f4b4927c82 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -63,6 +63,11 @@ * [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/9322) that caused estimateroutefee to ignore the default payment timeout. +* [Make reassignment of alias channel edges atomic]( + https://github.com/lightningnetwork/lnd/pull/8777). This fixes an edge case + where we might rate limit a peer because he already sent us a channel update + with the confirmed channel ID but the graph was still not updated. + # New Features * [Support](https://github.com/lightningnetwork/lnd/pull/8390) for From 115451cfc97c00fa18c90837888dc2f8e186f8b0 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 17 Dec 2024 23:40:19 +0100 Subject: [PATCH 07/15] channeldb: fix typo. --- channeldb/channel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/channeldb/channel.go b/channeldb/channel.go index f4e99a6f8cd..7bf54e51d6c 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -1804,7 +1804,7 @@ func (c *OpenChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) { currentCommitSecret[0] ^= 1 // If this is a tweakless channel, then we'll purposefully send - // a next local height taht's invalid to trigger a force close + // a next local height that's invalid to trigger a force close // on their end. We do this as tweakless channels don't require // that the commitment point is valid, only that it's present. if c.ChanType.IsTweakless() { From 681ab293173cfc271652903d3e9f7d961a5e7d33 Mon Sep 17 00:00:00 2001 From: ziggie Date: Wed, 18 Dec 2024 16:04:32 +0100 Subject: [PATCH 08/15] funding: refactor gossip msg code We almost never need to create all messages at the same time (ChanUpdate,ChanAnnouncement,Proof) so we split it up into own functions. --- funding/manager.go | 280 +++++++++++++++++++++------------------------ 1 file changed, 130 insertions(+), 150 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index 9373657c5d7..e21e4113c58 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -3542,20 +3542,13 @@ func (f *Manager) extractAnnounceParams(c *channeldb.OpenChannel) ( // ChannelUpdate they understand. ourPolicy may be set for various // option-scid-alias channels to re-use the same policy. func (f *Manager) addToGraph(completeChan *channeldb.OpenChannel, - shortChanID *lnwire.ShortChannelID, - peerAlias *lnwire.ShortChannelID, - ourPolicy *models.ChannelEdgePolicy) error { - - chanID := lnwire.NewChanIDFromOutPoint(completeChan.FundingOutpoint) + peerAlias *lnwire.ShortChannelID) error { - fwdMinHTLC, fwdMaxHTLC := f.extractAnnounceParams(completeChan) - - ann, err := f.newChanAnnouncement( + chanAnn, err := f.newChanAnnouncement( f.cfg.IDKey, completeChan.IdentityPub, &completeChan.LocalChanCfg.MultiSigKey, - completeChan.RemoteChanCfg.MultiSigKey.PubKey, *shortChanID, - chanID, fwdMinHTLC, fwdMaxHTLC, ourPolicy, - completeChan.ChanType, + completeChan.RemoteChanCfg.MultiSigKey.PubKey, + completeChan.ShortChanID(), completeChan.ChanType, ) if err != nil { return fmt.Errorf("error generating channel "+ @@ -3565,7 +3558,7 @@ func (f *Manager) addToGraph(completeChan *channeldb.OpenChannel, // Send ChannelAnnouncement and ChannelUpdate to the gossiper to add // to the Router's topology. errChan := f.cfg.SendAnnouncement( - ann.chanAnn, discovery.ChannelCapacity(completeChan.Capacity), + chanAnn, discovery.ChannelCapacity(completeChan.Capacity), discovery.ChannelPoint(completeChan.FundingOutpoint), discovery.TapscriptRoot(completeChan.TapscriptRoot), ) @@ -3582,13 +3575,20 @@ func (f *Manager) addToGraph(completeChan *channeldb.OpenChannel, "announcement: %v", err) } } + case <-f.quit: return ErrFundingManagerShuttingDown } + chanUpdateAnn, err := f.newChanUpdate(completeChan, nil) + if err != nil { + return fmt.Errorf("error generating channel update: %w", err) + } + errChan = f.cfg.SendAnnouncement( - ann.chanUpdateAnn, discovery.RemoteAlias(peerAlias), + chanUpdateAnn, discovery.RemoteAlias(peerAlias), ) + select { case err := <-errChan: if err != nil { @@ -3602,6 +3602,7 @@ func (f *Manager) addToGraph(completeChan *channeldb.OpenChannel, "update: %v", err) } } + case <-f.quit: return ErrFundingManagerShuttingDown } @@ -3616,7 +3617,7 @@ func (f *Manager) addToGraph(completeChan *channeldb.OpenChannel, // step in the channel opening process, and the opening state will be deleted // from the database if successful. func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, - shortChanID *lnwire.ShortChannelID) error { + confirmedScid *lnwire.ShortChannelID) error { // If this channel is not meant to be announced to the greater network, // we'll only send our NodeAnnouncement to our counterparty to ensure we @@ -3624,7 +3625,7 @@ func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, announceChan := completeChan.ChannelFlags&lnwire.FFAnnounceChannel != 0 if !announceChan { log.Debugf("Will not announce private channel %v.", - shortChanID.ToUint64()) + confirmedScid.ToUint64()) peer, err := f.waitForPeerOnline(completeChan.IdentityPub) if err != nil { @@ -3661,7 +3662,7 @@ func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, txid := completeChan.FundingOutpoint.Hash log.Debugf("Will announce channel %v after ChannelPoint"+ "(%v) has gotten %d confirmations", - shortChanID.ToUint64(), completeChan.FundingOutpoint, + confirmedScid.ToUint64(), completeChan.FundingOutpoint, numConfs) fundingScript, err := makeFundingScript(completeChan) @@ -3706,7 +3707,7 @@ func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, chanID := lnwire.NewChanIDFromOutPoint(fundingPoint) log.Infof("Announcing ChannelPoint(%v), short_chan_id=%v", - &fundingPoint, shortChanID) + &fundingPoint, confirmedScid) // If this is a non-zero-conf option-scid-alias channel, we'll // delete the mappings the gossiper uses so that ChannelUpdates @@ -3743,22 +3744,29 @@ func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, baseScid, baseScid.ToUint64()) // We send the rassigned ChannelUpdate to the peer. - err = f.sendChanUpdate( - completeChan, &baseScid, ourPolicy, - ) + err = f.sendChanUpdate(completeChan, ourPolicy) if err != nil { return fmt.Errorf("failed to re-add to "+ "graph: %v", err) } } - // Create and broadcast the proofs required to make this channel - // public and usable for other nodes for routing. - err = f.announceChannel( + chanAnn, err := f.newChanAnnouncement( f.cfg.IDKey, completeChan.IdentityPub, &completeChan.LocalChanCfg.MultiSigKey, completeChan.RemoteChanCfg.MultiSigKey.PubKey, - *shortChanID, chanID, completeChan.ChanType, + *confirmedScid, completeChan.ChanType, + ) + if err != nil { + return fmt.Errorf("error generating channel "+ + "announcement: %v", err) + } + + // Create and broadcast the proofs required so that nodes on + // the network can verify it and use this public channel for + // routing. + err = f.announceChannel( + completeChan, chanAnn, chanID, ) if err != nil { return fmt.Errorf("channel announcement failed: %w", @@ -3766,7 +3774,7 @@ func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, } log.Debugf("Channel with ChannelPoint(%v), short_chan_id=%v "+ - "sent to gossiper", &fundingPoint, shortChanID) + "sent to gossiper", &fundingPoint, confirmedScid) } return nil @@ -3786,6 +3794,9 @@ func (f *Manager) waitForZeroConfChannel(c *channeldb.OpenChannel) error { c.FundingOutpoint, err) } + aliasScid := c.ShortChanID() + confirmedScid := confChan.shortChanID + // We'll need to refresh the channel state so that things are properly // populated when validating the channel state. Otherwise, a panic may // occur due to inconsistency in the OpenChannel struct. @@ -3805,7 +3816,7 @@ func (f *Manager) waitForZeroConfChannel(c *channeldb.OpenChannel) error { // Once we know the confirmed ShortChannelID, we'll need to save it to // the database and refresh the OpenChannel struct with it. - err = c.MarkRealScid(confChan.shortChanID) + err = c.MarkRealScid(confirmedScid) if err != nil { return fmt.Errorf("unable to set confirmed SCID for zero "+ "channel: %v", err) @@ -3815,7 +3826,7 @@ func (f *Manager) waitForZeroConfChannel(c *channeldb.OpenChannel) error { // we'll delete some of the alias mappings the gossiper uses. isPublic := c.ChannelFlags&lnwire.FFAnnounceChannel != 0 if isPublic { - err = f.cfg.AliasManager.DeleteSixConfs(c.ShortChannelID) + err = f.cfg.AliasManager.DeleteSixConfs(aliasScid) if err != nil { return fmt.Errorf("unable to delete base alias after "+ "six confirmations: %v", err) @@ -3826,25 +3837,20 @@ func (f *Manager) waitForZeroConfChannel(c *channeldb.OpenChannel) error { // updates with the alias scid are removed so that we do not // relay them to the broader network. ourPolicy, err := f.cfg.ReAssignSCID( - c.ShortChanID(), confChan.shortChanID, + aliasScid, confirmedScid, ) if err != nil { return fmt.Errorf("unable to reassign alias edge in "+ "graph: %w", err) } - aliasScid := c.ShortChanID() - confirmedScid := confChan.shortChanID - log.Infof("Successfully reassigned alias edge in "+ "graph(zeroconf): %v(%d) -> %v(%d)", aliasScid, aliasScid.ToUint64(), confirmedScid, confirmedScid.ToUint64()) // Send the ChannelUpdate with the confirmed scid to the peer. - err = f.sendChanUpdate( - c, &confChan.shortChanID, ourPolicy, - ) + err = f.sendChanUpdate(c, ourPolicy) if err != nil { return fmt.Errorf("failed to send ChannelUpdate to "+ "gossiper: %v", err) @@ -4201,7 +4207,7 @@ func (f *Manager) handleChannelReadyReceived(channel *channeldb.OpenChannel, peerAlias = &foundAlias } - err := f.addToGraph(channel, scid, peerAlias, nil) + err := f.addToGraph(channel, peerAlias) if err != nil { return fmt.Errorf("failed adding to graph: %w", err) } @@ -4311,28 +4317,12 @@ func (f *Manager) ensureInitialForwardingPolicy(chanID lnwire.ChannelID, return nil } -// chanAnnouncement encapsulates the two authenticated announcements that we -// send out to the network after a new channel has been created locally. -type chanAnnouncement struct { - chanAnn *lnwire.ChannelAnnouncement1 - chanUpdateAnn *lnwire.ChannelUpdate1 - chanProof *lnwire.AnnounceSignatures1 -} - -// newChanAnnouncement creates the authenticated channel announcement messages -// required to broadcast a newly created channel to the network. The -// announcement is two part: the first part authenticates the existence of the -// channel and contains four signatures binding the funding pub keys and -// identity pub keys of both parties to the channel, and the second segment is -// authenticated only by us and contains our directional routing policy for the -// channel. ourPolicy may be set in order to re-use an existing, non-default -// policy. +// newChanAnnouncement creates the channel announcement message for the given +// channel without the signatures (proofs). func (f *Manager) newChanAnnouncement(localPubKey, remotePubKey *btcec.PublicKey, localFundingKey *keychain.KeyDescriptor, remoteFundingKey *btcec.PublicKey, shortChanID lnwire.ShortChannelID, - chanID lnwire.ChannelID, fwdMinHTLC, fwdMaxHTLC lnwire.MilliSatoshi, - ourPolicy *models.ChannelEdgePolicy, - chanType channeldb.ChannelType) (*chanAnnouncement, error) { + chanType channeldb.ChannelType) (*lnwire.ChannelAnnouncement1, error) { chainHash := *f.cfg.Wallet.Cfg.NetParams.GenesisHash @@ -4352,19 +4342,13 @@ func (f *Manager) newChanAnnouncement(localPubKey, // TODO(roasbeef): temp, remove after gossip 1.5 if chanType.IsTaproot() { log.Debugf("Applying taproot feature bit to "+ - "ChannelAnnouncement for %v", chanID) + "ChannelAnnouncement for %v", shortChanID) chanAnn.Features.Set( lnwire.SimpleTaprootChannelsRequiredStaging, ) } - // The chanFlags field indicates which directed edge of the channel is - // being updated within the ChannelUpdateAnnouncement announcement - // below. A value of zero means it's the edge of the "first" node and 1 - // being the other node. - var chanFlags lnwire.ChanUpdateChanFlags - // The lexicographical ordering of the two identity public keys of the // nodes indicates which of the nodes is "first". If our serialized // identity key is lower than theirs then we're the "first" node and @@ -4382,10 +4366,6 @@ func (f *Manager) newChanAnnouncement(localPubKey, chanAnn.BitcoinKey2[:], remoteFundingKey.SerializeCompressed(), ) - - // If we're the first node then update the chanFlags to - // indicate the "direction" of the update. - chanFlags = 0 } else { copy(chanAnn.NodeID1[:], remotePubKey.SerializeCompressed()) copy(chanAnn.NodeID2[:], localPubKey.SerializeCompressed()) @@ -4397,24 +4377,46 @@ func (f *Manager) newChanAnnouncement(localPubKey, chanAnn.BitcoinKey2[:], localFundingKey.PubKey.SerializeCompressed(), ) - - // If we're the second node then update the chanFlags to - // indicate the "direction" of the update. - chanFlags = 1 } + return chanAnn, nil +} + +// newChanUpdate creates a new channel update message for the given channel. +func (f *Manager) newChanUpdate(completeChan *channeldb.OpenChannel, + ourPolicy *models.ChannelEdgePolicy) (*lnwire.ChannelUpdate1, error) { + + chainHash := *f.cfg.Wallet.Cfg.NetParams.GenesisHash + // Our channel update message flags will signal that we support the // max_htlc field. msgFlags := lnwire.ChanUpdateRequiredMaxHtlc + fwdMinHTLC, fwdMaxHTLC := f.extractAnnounceParams(completeChan) + chanID := lnwire.NewChanIDFromOutPoint(completeChan.FundingOutpoint) + + chanUpdateChanFlags := lnwire.ChanUpdateChanFlags(0) + if !isLexicographicallyFirst(f.cfg.IDKey, + completeChan.IdentityPub) { + + chanUpdateChanFlags = 1 + } + + // If the channel is a zero-conf channel and confirmed, then we need to + // use the "real" scid. + shortChanID := completeChan.ShortChanID() + if completeChan.IsZeroConf() && completeChan.ZeroConfConfirmed() { + shortChanID = completeChan.ZeroConfRealScid() + } + // We announce the channel with the default values. Some of // these values can later be changed by crafting a new ChannelUpdate. - chanUpdateAnn := &lnwire.ChannelUpdate1{ + chanUpdate := &lnwire.ChannelUpdate1{ ShortChannelID: shortChanID, ChainHash: chainHash, Timestamp: uint32(time.Now().Unix()), MessageFlags: msgFlags, - ChannelFlags: chanFlags, + ChannelFlags: chanUpdateChanFlags, TimeLockDelta: uint16( f.cfg.DefaultRoutingPolicy.TimeLockDelta, ), @@ -4436,28 +4438,28 @@ func (f *Manager) newChanAnnouncement(localPubKey, case ourPolicy != nil: // If ourPolicy is non-nil, modify the default parameters of the // ChannelUpdate. - chanUpdateAnn.MessageFlags = ourPolicy.MessageFlags - chanUpdateAnn.ChannelFlags = ourPolicy.ChannelFlags - chanUpdateAnn.TimeLockDelta = ourPolicy.TimeLockDelta - chanUpdateAnn.HtlcMinimumMsat = ourPolicy.MinHTLC - chanUpdateAnn.HtlcMaximumMsat = ourPolicy.MaxHTLC - chanUpdateAnn.BaseFee = uint32(ourPolicy.FeeBaseMSat) - chanUpdateAnn.FeeRate = uint32( + chanUpdate.MessageFlags = ourPolicy.MessageFlags + chanUpdate.ChannelFlags = ourPolicy.ChannelFlags + chanUpdate.TimeLockDelta = ourPolicy.TimeLockDelta + chanUpdate.HtlcMinimumMsat = ourPolicy.MinHTLC + chanUpdate.HtlcMaximumMsat = ourPolicy.MaxHTLC + chanUpdate.BaseFee = uint32(ourPolicy.FeeBaseMSat) + chanUpdate.FeeRate = uint32( ourPolicy.FeeProportionalMillionths, ) case storedFwdingPolicy != nil: - chanUpdateAnn.BaseFee = uint32(storedFwdingPolicy.BaseFee) - chanUpdateAnn.FeeRate = uint32(storedFwdingPolicy.FeeRate) + chanUpdate.BaseFee = uint32(storedFwdingPolicy.BaseFee) + chanUpdate.FeeRate = uint32(storedFwdingPolicy.FeeRate) default: log.Infof("No channel forwarding policy specified for channel "+ "announcement of ChannelID(%v). "+ "Assuming default fee parameters.", chanID) - chanUpdateAnn.BaseFee = uint32( + chanUpdate.BaseFee = uint32( f.cfg.DefaultRoutingPolicy.BaseFee, ) - chanUpdateAnn.FeeRate = uint32( + chanUpdate.FeeRate = uint32( f.cfg.DefaultRoutingPolicy.FeeRate, ) } @@ -4466,7 +4468,7 @@ func (f *Manager) newChanAnnouncement(localPubKey, // signature that signs a double-sha digest of the announcement. // This'll serve to authenticate this announcement and any other future // updates we may send. - chanUpdateMsg, err := chanUpdateAnn.DataToSign() + chanUpdateMsg, err := chanUpdate.DataToSign() if err != nil { return nil, err } @@ -4475,12 +4477,21 @@ func (f *Manager) newChanAnnouncement(localPubKey, return nil, errors.Errorf("unable to generate channel "+ "update announcement signature: %v", err) } - chanUpdateAnn.Signature, err = lnwire.NewSigFromSignature(sig) + chanUpdate.Signature, err = lnwire.NewSigFromSignature(sig) if err != nil { return nil, errors.Errorf("unable to generate channel "+ "update announcement signature: %v", err) } + return chanUpdate, nil +} + +// newChanProof creates a new channel proof message for the given channel +// announcement message. +func (f *Manager) newChanProof(chanAnn *lnwire.ChannelAnnouncement1, + localFundingKey *keychain.KeyDescriptor, + chanID lnwire.ChannelID) (*lnwire.AnnounceSignatures1, error) { + // The channel existence proofs itself is currently announced in // distinct message. In order to properly authenticate this message, we // need two signatures: one under the identity public key used which @@ -4510,7 +4521,7 @@ func (f *Manager) newChanAnnouncement(localPubKey, // allow them to reconstruct the full channel announcement. proof := &lnwire.AnnounceSignatures1{ ChannelID: chanID, - ShortChannelID: shortChanID, + ShortChannelID: chanAnn.ShortChannelID, } proof.NodeSignature, err = lnwire.NewSigFromSignature(nodeSig) if err != nil { @@ -4521,11 +4532,16 @@ func (f *Manager) newChanAnnouncement(localPubKey, return nil, err } - return &chanAnnouncement{ - chanAnn: chanAnn, - chanUpdateAnn: chanUpdateAnn, - chanProof: proof, - }, nil + return proof, nil +} + +// isLexicographicallyFirst returns true if the node key of the first node is +// lexicographically less than the node key of the second node. +func isLexicographicallyFirst(nodeKey1 *btcec.PublicKey, + nodeKey2 *btcec.PublicKey) bool { + + return bytes.Compare(nodeKey1.SerializeCompressed(), + nodeKey2.SerializeCompressed()) == -1 } // announceChannel announces a newly created channel to the rest of the network @@ -4535,10 +4551,9 @@ func (f *Manager) newChanAnnouncement(localPubKey, // the network during its next trickle. // This method is synchronous and will return when all the network requests // finish, either successfully or with an error. -func (f *Manager) announceChannel(localIDKey, remoteIDKey *btcec.PublicKey, - localFundingKey *keychain.KeyDescriptor, - remoteFundingKey *btcec.PublicKey, shortChanID lnwire.ShortChannelID, - chanID lnwire.ChannelID, chanType channeldb.ChannelType) error { +func (f *Manager) announceChannel(completeChan *channeldb.OpenChannel, + chanAnn *lnwire.ChannelAnnouncement1, + chanID lnwire.ChannelID) error { // First, we'll create the batch of announcements to be sent upon // initial channel creation. This includes the channel announcement @@ -4547,9 +4562,8 @@ func (f *Manager) announceChannel(localIDKey, remoteIDKey *btcec.PublicKey, // // We can pass in zeroes for the min and max htlc policy, because we // only use the channel announcement message from the returned struct. - ann, err := f.newChanAnnouncement( - localIDKey, remoteIDKey, localFundingKey, remoteFundingKey, - shortChanID, chanID, 0, 0, nil, chanType, + chanProof, err := f.newChanProof( + chanAnn, &completeChan.LocalChanCfg.MultiSigKey, chanID, ) if err != nil { log.Errorf("can't generate channel announcement: %v", err) @@ -4560,20 +4574,15 @@ func (f *Manager) announceChannel(localIDKey, remoteIDKey *btcec.PublicKey, // because addToGraph previously sent the ChannelAnnouncement and // the ChannelUpdate announcement messages. The channel proof and node // announcements are broadcast to the greater network. - errChan := f.cfg.SendAnnouncement(ann.chanProof) + errChan := f.cfg.SendAnnouncement(chanProof) select { case err := <-errChan: - if err != nil { - if graph.IsError(err, graph.ErrOutdated, - graph.ErrIgnored) { + if graph.IsError(err, graph.ErrOutdated, graph.ErrIgnored) { + log.Debugf("Graph rejected AnnounceSignatures: %v", err) + } - log.Debugf("Graph rejected "+ - "AnnounceSignatures: %v", err) - } else { - log.Errorf("Unable to send channel "+ - "proof: %v", err) - return err - } + if err != nil { + return err } case <-f.quit: @@ -4593,70 +4602,41 @@ func (f *Manager) announceChannel(localIDKey, remoteIDKey *btcec.PublicKey, errChan = f.cfg.SendAnnouncement(&nodeAnn) select { case err := <-errChan: - if err != nil { - if graph.IsError(err, graph.ErrOutdated, - graph.ErrIgnored) { - - log.Debugf("Graph rejected "+ - "NodeAnnouncement: %v", err) - } else { - log.Errorf("Unable to send node "+ - "announcement: %v", err) - return err - } + if graph.IsError(err, graph.ErrOutdated, graph.ErrIgnored) { + log.Debugf("Graph rejected NodeAnnouncement: %v", err) } + return err + case <-f.quit: return ErrFundingManagerShuttingDown } - - return nil } // sendChanUpdate sends a ChannelUpdate to the gossiper which is as a // consequence sent to the peer. -// -// TODO(ziggie): Refactor the gossip msgs so that not always all msgs have -// to be created but only the ones which are needed. func (f *Manager) sendChanUpdate(completeChan *channeldb.OpenChannel, - shortChanID *lnwire.ShortChannelID, ourPolicy *models.ChannelEdgePolicy) error { - chanID := lnwire.NewChanIDFromOutPoint(completeChan.FundingOutpoint) - - fwdMinHTLC, fwdMaxHTLC := f.extractAnnounceParams(completeChan) - - ann, err := f.newChanAnnouncement( - f.cfg.IDKey, completeChan.IdentityPub, - &completeChan.LocalChanCfg.MultiSigKey, - completeChan.RemoteChanCfg.MultiSigKey.PubKey, *shortChanID, - chanID, fwdMinHTLC, fwdMaxHTLC, ourPolicy, - completeChan.ChanType, - ) + chanUpdate, err := f.newChanUpdate(completeChan, ourPolicy) if err != nil { - return fmt.Errorf("error generating channel "+ - "announcement: %v", err) + return fmt.Errorf("error generating channel update: %w", err) } - errChan := f.cfg.SendAnnouncement(ann.chanUpdateAnn) + errChan := f.cfg.SendAnnouncement(chanUpdate) select { case err := <-errChan: - if err != nil { - if graph.IsError(err, graph.ErrOutdated, - graph.ErrIgnored) { + if graph.IsError(err, graph.ErrOutdated, graph.ErrIgnored) { + log.Debugf("Graph rejected ChannelUpdate: %v", err) - log.Debugf("Graph rejected "+ - "ChannelUpdate: %v", err) - } else { - return fmt.Errorf("error sending channel "+ - "update: %v", err) - } + return nil } + + return err + case <-f.quit: return ErrFundingManagerShuttingDown } - - return nil } // InitFundingWorkflow sends a message to the funding manager instructing it From c0c3e9419400c57504c806330045f00148b932e5 Mon Sep 17 00:00:00 2001 From: ziggie Date: Thu, 19 Dec 2024 21:33:46 +0100 Subject: [PATCH 09/15] lnwire: Define new dont_forward bit in msg flags new msg flag in the channel update msg is defined. --- lnwire/channel_update.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lnwire/channel_update.go b/lnwire/channel_update.go index 33cc3bff038..e4ab6a26744 100644 --- a/lnwire/channel_update.go +++ b/lnwire/channel_update.go @@ -16,6 +16,10 @@ const ( // ChanUpdateRequiredMaxHtlc is a bit that indicates whether the // required htlc_maximum_msat field is present in this ChannelUpdate. ChanUpdateRequiredMaxHtlc ChanUpdateMsgFlags = 1 << iota + + // ChanUpdateDontForward is a bit that indicates whether the ChanUpdate + // msg should be forwarded to other peers when received. + ChanUpdateDontForward ) // String returns the bitfield flags as a string. @@ -29,6 +33,12 @@ func (c ChanUpdateMsgFlags) HasMaxHtlc() bool { return c&ChanUpdateRequiredMaxHtlc != 0 } +// HasDontForward returns true if the dont_forward option bit is set in the +// message flags. +func (c ChanUpdateMsgFlags) HasDontForward() bool { + return c&ChanUpdateDontForward != 0 +} + // ChanUpdateChanFlags is a bitfield that signals various options concerning a // particular channel edge. Each bit is to be examined in order to determine // how the ChannelUpdate message is to be interpreted. From 559e2b240bee0566cebd8e55f19b23a75fd854af Mon Sep 17 00:00:00 2001 From: ziggie Date: Thu, 19 Dec 2024 21:50:05 +0100 Subject: [PATCH 10/15] funding: set dont_forward bit in chan update msg Set the dont_forward bit in the chan update msg when a channel is not (yet) announced to the network. This is either the case for channels which signal this in the ChannelFlags of the OpenChannel msg but also for public channels which have still not reached the minimum depth height of 6 confirmations. --- funding/manager.go | 45 +++++++++++++++++++++++++++++++++++---------- server.go | 5 +++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/funding/manager.go b/funding/manager.go index e21e4113c58..ad32862a057 100644 --- a/funding/manager.go +++ b/funding/manager.go @@ -3542,7 +3542,7 @@ func (f *Manager) extractAnnounceParams(c *channeldb.OpenChannel) ( // ChannelUpdate they understand. ourPolicy may be set for various // option-scid-alias channels to re-use the same policy. func (f *Manager) addToGraph(completeChan *channeldb.OpenChannel, - peerAlias *lnwire.ShortChannelID) error { + peerAlias *lnwire.ShortChannelID, shouldAnnounce bool) error { chanAnn, err := f.newChanAnnouncement( f.cfg.IDKey, completeChan.IdentityPub, @@ -3580,7 +3580,7 @@ func (f *Manager) addToGraph(completeChan *channeldb.OpenChannel, return ErrFundingManagerShuttingDown } - chanUpdateAnn, err := f.newChanUpdate(completeChan, nil) + chanUpdateAnn, err := f.newChanUpdate(completeChan, nil, shouldAnnounce) if err != nil { return fmt.Errorf("error generating channel update: %w", err) } @@ -3713,9 +3713,11 @@ func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, // delete the mappings the gossiper uses so that ChannelUpdates // with aliases won't be accepted. This is done elsewhere for // zero-conf channels. - isScidFeature := completeChan.NegotiatedAliasFeature() + // We also do it for non option-scid-alias channels, because + // we need to update the msg flags of the channel policies so + // that the __dont_forward__ bit is unset. isZeroConf := completeChan.IsZeroConf() - if isScidFeature && !isZeroConf { + if !isZeroConf { baseScid := completeChan.ShortChanID() err := f.cfg.AliasManager.DeleteSixConfs(baseScid) if err != nil { @@ -3744,7 +3746,9 @@ func (f *Manager) annAfterSixConfs(completeChan *channeldb.OpenChannel, baseScid, baseScid.ToUint64()) // We send the rassigned ChannelUpdate to the peer. - err = f.sendChanUpdate(completeChan, ourPolicy) + err = f.sendChanUpdate( + completeChan, ourPolicy, true, + ) if err != nil { return fmt.Errorf("failed to re-add to "+ "graph: %v", err) @@ -3850,7 +3854,7 @@ func (f *Manager) waitForZeroConfChannel(c *channeldb.OpenChannel) error { confirmedScid, confirmedScid.ToUint64()) // Send the ChannelUpdate with the confirmed scid to the peer. - err = f.sendChanUpdate(c, ourPolicy) + err = f.sendChanUpdate(c, ourPolicy, true) if err != nil { return fmt.Errorf("failed to send ChannelUpdate to "+ "gossiper: %v", err) @@ -4189,6 +4193,17 @@ func (f *Manager) handleChannelReadyReceived(channel *channeldb.OpenChannel, delete(f.pendingMusigNonces, chanID) f.nonceMtx.Unlock() + // shouldAnnounce defines whether the __dont_forward__ bit in the + // ChanUpdate msg is set hence signaling to the peer whether it should + // forward this msg to the broader network. So whether this ChanUpdate + // should be announced depends on two factors. First the + // __announce_channel__ bit in the channel open msg has to be set. + // In case the required number of confirmations is met, but it is still + // less than the obligatory 6 confirmations we also signal to not relay + // the channel update to the broader network. + shouldAnnounce := channel.ChannelFlags&lnwire.FFAnnounceChannel == 1 && + channel.NumConfsRequired >= 6 + var peerAlias *lnwire.ShortChannelID if channel.IsZeroConf() { // We'll need to wait until channel_ready has been received and @@ -4204,10 +4219,13 @@ func (f *Manager) handleChannelReadyReceived(channel *channeldb.OpenChannel, return nil } + // In case of a zero-conf channel, we don't want to announce it + // before it has its obligatory 6 confirmations. + shouldAnnounce = false peerAlias = &foundAlias } - err := f.addToGraph(channel, peerAlias) + err := f.addToGraph(channel, peerAlias, shouldAnnounce) if err != nil { return fmt.Errorf("failed adding to graph: %w", err) } @@ -4384,7 +4402,8 @@ func (f *Manager) newChanAnnouncement(localPubKey, // newChanUpdate creates a new channel update message for the given channel. func (f *Manager) newChanUpdate(completeChan *channeldb.OpenChannel, - ourPolicy *models.ChannelEdgePolicy) (*lnwire.ChannelUpdate1, error) { + ourPolicy *models.ChannelEdgePolicy, + shouldAnnounce bool) (*lnwire.ChannelUpdate1, error) { chainHash := *f.cfg.Wallet.Cfg.NetParams.GenesisHash @@ -4392,6 +4411,10 @@ func (f *Manager) newChanUpdate(completeChan *channeldb.OpenChannel, // max_htlc field. msgFlags := lnwire.ChanUpdateRequiredMaxHtlc + if !shouldAnnounce { + msgFlags |= lnwire.ChanUpdateDontForward + } + fwdMinHTLC, fwdMaxHTLC := f.extractAnnounceParams(completeChan) chanID := lnwire.NewChanIDFromOutPoint(completeChan.FundingOutpoint) @@ -4616,9 +4639,11 @@ func (f *Manager) announceChannel(completeChan *channeldb.OpenChannel, // sendChanUpdate sends a ChannelUpdate to the gossiper which is as a // consequence sent to the peer. func (f *Manager) sendChanUpdate(completeChan *channeldb.OpenChannel, - ourPolicy *models.ChannelEdgePolicy) error { + ourPolicy *models.ChannelEdgePolicy, shouldAnnounce bool) error { - chanUpdate, err := f.newChanUpdate(completeChan, ourPolicy) + chanUpdate, err := f.newChanUpdate( + completeChan, ourPolicy, shouldAnnounce, + ) if err != nil { return fmt.Errorf("error generating channel update: %w", err) } diff --git a/server.go b/server.go index 8961b4bd516..c0a6d4706fb 100644 --- a/server.go +++ b/server.go @@ -1425,6 +1425,11 @@ func newServer(cfg *Config, listenAddrs []net.Addr, // Update the policy data, this invalidates the signature // therefore we need to resign the data. ourPolicy.ChannelID = newEdgeInfo.ChannelID + + // Make sure we reset the msgFlag to not set the + // __dont_forward__ bit. We only reassign the SCID if it is a + // public channel. + ourPolicy.MessageFlags = lnwire.ChanUpdateRequiredMaxHtlc chanUpdate := netann.UnsignedChannelUpdateFromEdge( newEdgeInfo, ourPolicy, ) From 0ebedcb53430d7ec61469b5faa48c917f0c36e74 Mon Sep 17 00:00:00 2001 From: ziggie Date: Thu, 19 Dec 2024 22:46:13 +0100 Subject: [PATCH 11/15] funding: adopt tests for the dont_forward bit Adopt tests to make sure the dont_forward bit is set when needed. --- funding/manager_test.go | 894 ++++++++++++++++++++++++++-------------- 1 file changed, 585 insertions(+), 309 deletions(-) diff --git a/funding/manager_test.go b/funding/manager_test.go index ea6326b4a83..03996ad1333 100644 --- a/funding/manager_test.go +++ b/funding/manager_test.go @@ -1158,131 +1158,14 @@ func assertAddedToGraph(t *testing.T, alice, bob *testNode, assertDatabaseState(t, bob, fundingOutPoint, addedToGraph) } -// assertChannelAnnouncements checks that alice and bob both sends the expected -// announcements (ChannelAnnouncement, ChannelUpdate) after the funding tx has -// confirmed. The last arguments can be set if we expect the nodes to advertise -// custom min_htlc values as part of their ChannelUpdate. We expect Alice to -// advertise the value required by Bob and vice versa. If they are not set the -// advertised value will be checked against the other node's default min_htlc, -// base fee and fee rate values. -func assertChannelAnnouncements(t *testing.T, alice, bob *testNode, - capacity btcutil.Amount, customMinHtlc []lnwire.MilliSatoshi, - customMaxHtlc []lnwire.MilliSatoshi, baseFees []lnwire.MilliSatoshi, - feeRates []lnwire.MilliSatoshi) { - - t.Helper() - - // Validate custom parameter arrays have expected length - validateCustomParams( - t, customMinHtlc, customMaxHtlc, baseFees, feeRates, - ) - - nodes := []*testNode{alice, bob} - for i, node := range nodes { - // Each node should send exactly 2 announcements - // ChannelAnnouncement and ChannelUpdate. - announcements, updates := collectGossipMsgs(t, node, 2) - verifyNoExtraMsgs(t, node) - - require.Len(t, announcements, 1, "expected 1 "+ - "ChannelAnnouncement from node %d", i) - require.Len(t, updates, 1, "expected 1 ChannelUpdate from "+ - "node %d", i) - for _, update := range updates { - verifyChannelUpdate( - t, update, i, nodes, - node.fundingMgr.cfg, capacity, customMinHtlc, - customMaxHtlc, baseFees, feeRates, - ) - } - } -} - -// assertChannelUpdate checks that a ChannelUpdate has been sent by the node -// with the expected parameters. -func assertChannelUpdates(t *testing.T, alice, bob *testNode, - capacity btcutil.Amount, customMinHtlc, customMaxHtlc, - baseFees, feeRates []lnwire.MilliSatoshi) { - - t.Helper() - - // Validate custom parameter arrays have expected length - validateCustomParams( - t, customMinHtlc, customMaxHtlc, baseFees, feeRates, - ) - - nodes := []*testNode{alice, bob} - for i, node := range nodes { - // Each node should send exactly 2 announcements - // ChannelAnnouncement and ChannelUpdate. - _, updates := collectGossipMsgs(t, node, 1) - verifyNoExtraMsgs(t, node) - - require.Len(t, updates, 1, "expected 1 ChannelUpdate from "+ - "node %d", i) - for _, update := range updates { - verifyChannelUpdate( - t, update, i, nodes, - node.fundingMgr.cfg, capacity, customMinHtlc, - customMaxHtlc, baseFees, feeRates, - ) - } - } -} - -// validateCustomParams ensures custom parameter arrays have valid length. -func validateCustomParams(t *testing.T, params ...[]lnwire.MilliSatoshi) { - t.Helper() - - for _, param := range params { - if len(param) > 0 { - require.Len(t, param, 2, "custom parameters must "+ - "have length 2") - } - } -} - -// collectGossipMsgs gathers the expected number of gossip messages from a node. -func collectGossipMsgs(t *testing.T, node *testNode, - expectedNum int) ([]*lnwire.ChannelAnnouncement1, - []*lnwire.ChannelUpdate1) { - - t.Helper() - - var ( - announcements []*lnwire.ChannelAnnouncement1 - updates []*lnwire.ChannelUpdate1 - ) - - for i := 0; i < expectedNum; i++ { - select { - case msg := <-node.announceChan: - switch m := msg.(type) { - case *lnwire.ChannelAnnouncement1: - announcements = append(announcements, m) - - case *lnwire.ChannelUpdate1: - updates = append(updates, m) - - default: - t.Fatalf("unexpected message type: %T", msg) - } - - case <-time.After(5 * time.Second): - t.Fatalf("node did not send gossip msg %d", i) - } - } - - return announcements, updates -} - // verifyNoExtraMsgs ensures no unexpected msgs are sent to the gossiper. func verifyNoExtraMsgs(t *testing.T, node *testNode) { t.Helper() select { case msg := <-node.announceChan: - t.Fatalf("received unexpected announcement: %v", msg) + t.Fatalf("received unexpected announcement(type: %T): %v", msg, + msg) case <-time.After(300 * time.Millisecond): // Expected - no extra msg. } @@ -1290,38 +1173,38 @@ func verifyNoExtraMsgs(t *testing.T, node *testNode) { // verifyChannelUpdate checks that a ChannelUpdate has the expected parameters. func verifyChannelUpdate(t *testing.T, update *lnwire.ChannelUpdate1, - nodeIndex int, nodes []*testNode, nodeCfg *Config, - capacity btcutil.Amount, customMinHtlc, customMaxHtlc, - baseFees, feeRates []lnwire.MilliSatoshi) { + node *testNode, capacity btcutil.Amount, customMinHtlc, customMaxHtlc, + baseFees, feeRates lnwire.MilliSatoshi, shouldAnnounce bool) { t.Helper() - require.EqualValues(t, 1, update.MessageFlags) + msgFlags := lnwire.ChanUpdateRequiredMaxHtlc + if !shouldAnnounce { + msgFlags |= lnwire.ChanUpdateDontForward + } + require.EqualValues(t, msgFlags, update.MessageFlags) - // Get parameters for the other node since each update advertises - // the remote node's requirements - otherIdx := (nodeIndex + 1) % 2 - otherCfg := nodes[otherIdx].fundingMgr.cfg + nodeCfg := node.fundingMgr.cfg // Set expected values, using customs if provided - minHtlc := otherCfg.DefaultMinHtlcIn - if len(customMinHtlc) > 0 { - minHtlc = customMinHtlc[nodeIndex] + minHtlc := nodeCfg.DefaultMinHtlcIn + if customMinHtlc > 0 { + minHtlc = customMinHtlc } maxHtlc := nodeCfg.RequiredRemoteMaxValue(capacity) - if len(customMaxHtlc) > 0 { - maxHtlc = customMaxHtlc[nodeIndex] + if customMaxHtlc > 0 { + maxHtlc = customMaxHtlc } baseFee := nodeCfg.DefaultRoutingPolicy.BaseFee - if len(baseFees) > 0 { - baseFee = baseFees[nodeIndex] + if baseFees > 0 { + baseFee = baseFees } feeRate := nodeCfg.DefaultRoutingPolicy.FeeRate - if len(feeRates) > 0 { - feeRate = feeRates[nodeIndex] + if feeRates > 0 { + feeRate = feeRates } // Verify all parameters match expected values @@ -1331,66 +1214,95 @@ func verifyChannelUpdate(t *testing.T, update *lnwire.ChannelUpdate1, require.EqualValues(t, feeRate, update.FeeRate) } -func assertAnnouncementSignatures(t *testing.T, alice, bob *testNode) { +// announcements holds the different types of announcement messages received. +type announcements struct { + channelAnn *lnwire.ChannelAnnouncement1 + nodeAnn *lnwire.NodeAnnouncement + channelUpd *lnwire.ChannelUpdate1 + channelSig *lnwire.AnnounceSignatures1 +} + +// hasChannelAnnouncement checks if the announcements struct contains a channel +// announcement message and fails the test if it's nil. +func (a *announcements) requireChannelAnnouncement(t *testing.T) { t.Helper() + require.NotNil(t, a.channelAnn, "expected channel announcement") +} - // After the ChannelReady message is sent and six confirmations have - // been reached, the channel will be announced to the greater network - // by having the nodes exchange announcement signatures. - // Two distinct messages will be sent: - // 1) AnnouncementSignatures - // 2) NodeAnnouncement - // These may arrive in no particular order. - // Note that sending the NodeAnnouncement at this point is an - // implementation detail, and not something required by the LN spec. - for j, node := range []*testNode{alice, bob} { - announcements := make([]lnwire.Message, 2) - for i := 0; i < len(announcements); i++ { - select { - case announcements[i] = <-node.announceChan: - case <-time.After(time.Second * 5): - t.Fatalf("node did not send announcement %v", i) - } - } +// hasNodeAnnouncement checks if the announcements struct contains a node +// announcement message and fails the test if it's nil. +func (a *announcements) requireNodeAnnouncement(t *testing.T) { + t.Helper() - gotAnnounceSignatures := false - gotNodeAnnouncement := false - for _, msg := range announcements { - switch msg.(type) { - case *lnwire.AnnounceSignatures1: - gotAnnounceSignatures = true - case *lnwire.NodeAnnouncement: - gotNodeAnnouncement = true - } - } + require.NotNil(t, a.nodeAnn, "expected node announcement") +} - if !gotAnnounceSignatures { - t.Fatalf("did not get AnnounceSignatures from node %d", - j) - } - if !gotNodeAnnouncement { - t.Fatalf("did not get NodeAnnouncement from node %d", j) - } - } +// hasChannelUpdate checks if the announcements struct contains a channel +// update message and fails the test if it's nil. +func (a *announcements) requireChannelUpdate(t *testing.T) { + t.Helper() + + require.NotNil(t, a.channelUpd, "expected channel update") } -func assertType[T any](t *testing.T, typ any) T { - value, ok := typ.(T) - require.True(t, ok) +// hasAnnounceSignatures checks if the announcements struct contains an +// announce signatures message and fails the test if it's nil. +func (a *announcements) requireAnnounceSignatures(t *testing.T) { + t.Helper() - return value + require.NotNil(t, a.channelSig, "expected announce signatures") } -func assertNodeAnnSent(t *testing.T, alice, bob *testNode) { +// collectGossipMsgs collects the expected different types of gossip messages +// from a node. Whether a particular msg type was received or not should be +// checked afterwards. +func collectGossipMsgs(t *testing.T, node *testNode, + expectedNum int) *announcements { + t.Helper() - for _, node := range []*testNode{alice, bob} { - nodeAnn, err := lnutils.RecvOrTimeout( - node.msgChan, time.Second*5, + msgs := make([]lnwire.Message, expectedNum) + for i := 0; i < len(msgs); i++ { + msg, err := lnutils.RecvOrTimeout( + node.announceChan, time.Second*5, ) require.NoError(t, err) - assertType[*lnwire.NodeAnnouncement](t, *nodeAnn) + msgs[i] = *msg + } + + ann := &announcements{} + for _, msg := range msgs { + switch msg := msg.(type) { + case *lnwire.AnnounceSignatures1: + ann.channelSig = msg + + case *lnwire.NodeAnnouncement: + ann.nodeAnn = msg + + case *lnwire.ChannelAnnouncement1: + ann.channelAnn = msg + + case *lnwire.ChannelUpdate1: + ann.channelUpd = msg + } } + + return ann +} + +// verifyDirectNodeAnnouncement verifies that a node announcement is sent +// directly to the peer when a private channel is opened. It is different for +// private channels because it is not funnelled through the gossip subsystem. +func verifyDirectNodeAnnouncement(t *testing.T, node *testNode) { + t.Helper() + + nodeAnn, err := lnutils.RecvOrTimeout( + node.msgChan, time.Second*5, + ) + require.NoError(t, err) + // Because we're using a pointer, we need to dereference the nodeAnn + // before checking the type. + require.IsType(t, &lnwire.NodeAnnouncement{}, *nodeAnn) } func waitForOpenUpdate(t *testing.T, updateChan chan *lnrpc.OpenStatusUpdate) { @@ -1532,8 +1444,9 @@ func testNormalWorkflow(t *testing.T, chanType *lnwire.ChannelType) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt + publicChannel := true fundingOutPoint, fundingTx := openChannel( - t, alice, bob, localAmt, pushAmt, 1, updateChan, true, + t, alice, bob, localAmt, pushAmt, 1, updateChan, publicChannel, chanType, ) @@ -1587,7 +1500,29 @@ func testNormalWorkflow(t *testing.T, chanType *lnwire.ChannelType) { // Make sure both fundingManagers send the expected channel // announcements. - assertChannelAnnouncements(t, alice, bob, capacity, nil, nil, nil, nil) + annAlice := collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob := collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireChannelAnnouncement(t) + annAlice.requireChannelUpdate(t) + + annBob.requireChannelAnnouncement(t) + annBob.requireChannelUpdate(t) + + // The channel is still not confirmed, so the channel update should set + // the __dont_forward__ flag. + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) // Check that the state machine is updated accordingly assertAddedToGraph(t, alice, bob, fundingOutPoint) @@ -1610,14 +1545,41 @@ func testNormalWorkflow(t *testing.T, chanType *lnwire.ChannelType) { // announcement message at this point. These channels aren't advertised // so we don't expect the other messages. case isTaprootChanType(chanType): - assertNodeAnnSent(t, alice, bob) + verifyDirectNodeAnnouncement(t, alice) + verifyDirectNodeAnnouncement(t, bob) // For regular channels, we'll make sure the fundingManagers exchange // announcement signatures. case chanType == nil: fallthrough + default: - assertAnnouncementSignatures(t, alice, bob) + annAlice = collectGossipMsgs(t, alice, 3) + verifyNoExtraMsgs(t, alice) + + annBob = collectGossipMsgs(t, bob, 3) + verifyNoExtraMsgs(t, bob) + + annAlice.requireAnnounceSignatures(t) + annAlice.requireChannelUpdate(t) + annAlice.requireNodeAnnouncement(t) + + annBob.requireAnnounceSignatures(t) + annBob.requireChannelUpdate(t) + annBob.requireNodeAnnouncement(t) + + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, + true, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, + true, + ) + } // The internal state-machine should now have deleted the channelStates @@ -1831,8 +1793,9 @@ func TestFundingManagerRestartBehavior(t *testing.T) { pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt updateChan := make(chan *lnrpc.OpenStatusUpdate) + publicChannel := true fundingOutPoint, fundingTx := openChannel( - t, alice, bob, localAmt, pushAmt, 1, updateChan, true, + t, alice, bob, localAmt, pushAmt, 1, updateChan, publicChannel, nil, ) @@ -1941,7 +1904,29 @@ func TestFundingManagerRestartBehavior(t *testing.T) { // Make sure both fundingManagers send the expected channel // announcements. - assertChannelAnnouncements(t, alice, bob, capacity, nil, nil, nil, nil) + annAlice := collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob := collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireChannelAnnouncement(t) + annAlice.requireChannelUpdate(t) + + annBob.requireChannelAnnouncement(t) + annBob.requireChannelUpdate(t) + + // The channel is still not confirmed, so the channel update should set + // the __dont_forward__ flag. + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) // Check that the state machine is updated accordingly assertAddedToGraph(t, alice, bob, fundingOutPoint) @@ -1961,8 +1946,30 @@ func TestFundingManagerRestartBehavior(t *testing.T) { Tx: fundingTx, } - // Make sure the fundingManagers exchange announcement signatures. - assertAnnouncementSignatures(t, alice, bob) + // Make sure the fundingManagers send the expected msgs. + annAlice = collectGossipMsgs(t, alice, 3) + verifyNoExtraMsgs(t, alice) + + annBob = collectGossipMsgs(t, bob, 3) + verifyNoExtraMsgs(t, bob) + + annAlice.requireAnnounceSignatures(t) + annAlice.requireChannelUpdate(t) + annAlice.requireNodeAnnouncement(t) + + annBob.requireAnnounceSignatures(t) + annBob.requireChannelUpdate(t) + annBob.requireNodeAnnouncement(t) + + otherNode = bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, true, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, true, + ) // The internal state-machine should now have deleted the channelStates // from the database, as the channel is announced. @@ -1990,8 +1997,9 @@ func TestFundingManagerOfflinePeer(t *testing.T) { pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt updateChan := make(chan *lnrpc.OpenStatusUpdate) + publicChannel := true fundingOutPoint, fundingTx := openChannel( - t, alice, bob, localAmt, pushAmt, 1, updateChan, true, + t, alice, bob, localAmt, pushAmt, 1, updateChan, publicChannel, nil, ) @@ -2106,7 +2114,29 @@ func TestFundingManagerOfflinePeer(t *testing.T) { // Make sure both fundingManagers send the expected channel // announcements. - assertChannelAnnouncements(t, alice, bob, capacity, nil, nil, nil, nil) + annAlice := collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob := collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireChannelAnnouncement(t) + annAlice.requireChannelUpdate(t) + + annBob.requireChannelAnnouncement(t) + annBob.requireChannelUpdate(t) + + // The channel is still not confirmed, so the channel update should set + // the __dont_forward__ flag. + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) // Check that the state machine is updated accordingly assertAddedToGraph(t, alice, bob, fundingOutPoint) @@ -2124,9 +2154,30 @@ func TestFundingManagerOfflinePeer(t *testing.T) { Tx: fundingTx, } - // Make sure both fundingManagers send the expected announcement - // signatures. - assertAnnouncementSignatures(t, alice, bob) + // Make sure both fundingManagers send the expected msgs. + annAlice = collectGossipMsgs(t, alice, 3) + verifyNoExtraMsgs(t, alice) + + annBob = collectGossipMsgs(t, bob, 3) + verifyNoExtraMsgs(t, bob) + + annAlice.requireAnnounceSignatures(t) + annAlice.requireChannelUpdate(t) + annAlice.requireNodeAnnouncement(t) + + annBob.requireAnnounceSignatures(t) + annBob.requireChannelUpdate(t) + annBob.requireNodeAnnouncement(t) + + otherNode = bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, true, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, true, + ) // The internal state-machine should now have deleted the channelStates // from the database, as the channel is announced. @@ -2502,8 +2553,10 @@ func TestFundingManagerReceiveChannelReadyTwice(t *testing.T) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt + publicChannel := true fundingOutPoint, fundingTx := openChannel( - t, alice, bob, localAmt, pushAmt, 1, updateChan, true, nil, + t, alice, bob, localAmt, pushAmt, 1, updateChan, publicChannel, + nil, ) // Notify that transaction was mined @@ -2565,7 +2618,29 @@ func TestFundingManagerReceiveChannelReadyTwice(t *testing.T) { // Make sure both fundingManagers send the expected channel // announcements. - assertChannelAnnouncements(t, alice, bob, capacity, nil, nil, nil, nil) + annAlice := collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob := collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireChannelAnnouncement(t) + annAlice.requireChannelUpdate(t) + + annBob.requireChannelAnnouncement(t) + annBob.requireChannelUpdate(t) + + // The channel is still not confirmed, so the channel update should set + // the __dont_forward__ flag. + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) // Check that the state machine is updated accordingly assertAddedToGraph(t, alice, bob, fundingOutPoint) @@ -2583,8 +2658,30 @@ func TestFundingManagerReceiveChannelReadyTwice(t *testing.T) { Tx: fundingTx, } - // Make sure the fundingManagers exchange announcement signatures. - assertAnnouncementSignatures(t, alice, bob) + // Make sure the fundingManagers sends the expected msgs. + annAlice = collectGossipMsgs(t, alice, 3) + verifyNoExtraMsgs(t, alice) + + annBob = collectGossipMsgs(t, bob, 3) + verifyNoExtraMsgs(t, bob) + + annAlice.requireAnnounceSignatures(t) + annAlice.requireChannelUpdate(t) + annAlice.requireNodeAnnouncement(t) + + annBob.requireAnnounceSignatures(t) + annBob.requireChannelUpdate(t) + annBob.requireNodeAnnouncement(t) + + otherNode = bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, true, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, true, + ) // The internal state-machine should now have deleted the channelStates // from the database, as the channel is announced. @@ -2615,8 +2712,10 @@ func TestFundingManagerRestartAfterChanAnn(t *testing.T) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt + publicChannel := true fundingOutPoint, fundingTx := openChannel( - t, alice, bob, localAmt, pushAmt, 1, updateChan, true, nil, + t, alice, bob, localAmt, pushAmt, 1, updateChan, publicChannel, + nil, ) // Notify that transaction was mined @@ -2658,7 +2757,29 @@ func TestFundingManagerRestartAfterChanAnn(t *testing.T) { // Make sure both fundingManagers send the expected channel // announcements. - assertChannelAnnouncements(t, alice, bob, capacity, nil, nil, nil, nil) + annAlice := collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob := collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireChannelAnnouncement(t) + annAlice.requireChannelUpdate(t) + + annBob.requireChannelAnnouncement(t) + annBob.requireChannelUpdate(t) + + // The channel is still not confirmed, so the channel update should set + // the __dont_forward__ flag. + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) // Check that the state machine is updated accordingly assertAddedToGraph(t, alice, bob, fundingOutPoint) @@ -2681,9 +2802,30 @@ func TestFundingManagerRestartAfterChanAnn(t *testing.T) { Tx: fundingTx, } - // Make sure both fundingManagers send the expected channel - // announcements. - assertAnnouncementSignatures(t, alice, bob) + // Make sure both fundingManagers send the expected msgs. + annAlice = collectGossipMsgs(t, alice, 3) + verifyNoExtraMsgs(t, alice) + + annBob = collectGossipMsgs(t, bob, 3) + verifyNoExtraMsgs(t, bob) + + annAlice.requireAnnounceSignatures(t) + annAlice.requireChannelUpdate(t) + annAlice.requireNodeAnnouncement(t) + + annBob.requireAnnounceSignatures(t) + annBob.requireChannelUpdate(t) + annBob.requireNodeAnnouncement(t) + + otherNode = bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, true, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, true, + ) // The internal state-machine should now have deleted the channelStates // from the database, as the channel is announced. @@ -2714,8 +2856,10 @@ func TestFundingManagerRestartAfterReceivingChannelReady(t *testing.T) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt + publicChannel := true fundingOutPoint, fundingTx := openChannel( - t, alice, bob, localAmt, pushAmt, 1, updateChan, true, nil, + t, alice, bob, localAmt, pushAmt, 1, updateChan, publicChannel, + nil, ) // Notify that transaction was mined @@ -2762,7 +2906,29 @@ func TestFundingManagerRestartAfterReceivingChannelReady(t *testing.T) { // Make sure both fundingManagers send the expected channel // announcements. - assertChannelAnnouncements(t, alice, bob, capacity, nil, nil, nil, nil) + annAlice := collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob := collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireChannelAnnouncement(t) + annAlice.requireChannelUpdate(t) + + annBob.requireChannelAnnouncement(t) + annBob.requireChannelUpdate(t) + + // The channel is still not confirmed, so the channel update should set + // the __dont_forward__ flag. + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) // Check that the state machine is updated accordingly assertAddedToGraph(t, alice, bob, fundingOutPoint) @@ -2776,9 +2942,30 @@ func TestFundingManagerRestartAfterReceivingChannelReady(t *testing.T) { Tx: fundingTx, } - // Make sure both fundingManagers send the expected channel - // announcements. - assertAnnouncementSignatures(t, alice, bob) + // Make sure both fundingManagers send the expected msgs. + annAlice = collectGossipMsgs(t, alice, 3) + verifyNoExtraMsgs(t, alice) + + annBob = collectGossipMsgs(t, bob, 3) + verifyNoExtraMsgs(t, bob) + + annAlice.requireAnnounceSignatures(t) + annAlice.requireChannelUpdate(t) + annAlice.requireNodeAnnouncement(t) + + annBob.requireAnnounceSignatures(t) + annBob.requireChannelUpdate(t) + annBob.requireNodeAnnouncement(t) + + otherNode = bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, true, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, true, + ) // The internal state-machine should now have deleted the channelStates // from the database, as the channel is announced. @@ -2809,8 +2996,10 @@ func TestFundingManagerPrivateChannel(t *testing.T) { localAmt := btcutil.Amount(500000) pushAmt := btcutil.Amount(0) capacity := localAmt + pushAmt + publicChannel := false fundingOutPoint, fundingTx := openChannel( - t, alice, bob, localAmt, pushAmt, 1, updateChan, false, nil, + t, alice, bob, localAmt, pushAmt, 1, updateChan, publicChannel, + nil, ) // Notify that transaction was mined @@ -2852,7 +3041,29 @@ func TestFundingManagerPrivateChannel(t *testing.T) { // Make sure both fundingManagers send the expected channel // announcements. - assertChannelAnnouncements(t, alice, bob, capacity, nil, nil, nil, nil) + annAlice := collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob := collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireChannelAnnouncement(t) + annAlice.requireChannelUpdate(t) + + annBob.requireChannelAnnouncement(t) + annBob.requireChannelUpdate(t) + + // The channel is a private channel, so the channel update should set + // the __dont_forward__ flag. + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) // The funding transaction is now confirmed, wait for the // OpenStatusUpdate_ChanOpen update @@ -2867,42 +3078,10 @@ func TestFundingManagerPrivateChannel(t *testing.T) { Tx: fundingTx, } - // Since this is a private channel, we shouldn't receive the - // announcement signatures. - select { - case ann := <-alice.announceChan: - t.Fatalf("unexpectedly got channel announcement message: %v", - ann) - case <-time.After(300 * time.Millisecond): - // Expected - } - - select { - case ann := <-bob.announceChan: - t.Fatalf("unexpectedly got channel announcement message: %v", - ann) - case <-time.After(300 * time.Millisecond): - // Expected - } - - // We should however receive each side's node announcement. - select { - case msg := <-alice.msgChan: - if _, ok := msg.(*lnwire.NodeAnnouncement); !ok { - t.Fatalf("expected to receive node announcement") - } - case <-time.After(time.Second): - t.Fatalf("expected to receive node announcement") - } - - select { - case msg := <-bob.msgChan: - if _, ok := msg.(*lnwire.NodeAnnouncement); !ok { - t.Fatalf("expected to receive node announcement") - } - case <-time.After(time.Second): - t.Fatalf("expected to receive node announcement") - } + // For private channels, we only expect the node announcements to be + // sent directly to the peer. + verifyDirectNodeAnnouncement(t, alice) + verifyDirectNodeAnnouncement(t, bob) // The internal state-machine should now have deleted the channelStates // from the database, as the channel is announced. @@ -2976,8 +3155,30 @@ func TestFundingManagerPrivateRestart(t *testing.T) { assertHandleChannelReady(t, alice, bob) // Make sure both fundingManagers send the expected channel - // announcements. - assertChannelAnnouncements(t, alice, bob, capacity, nil, nil, nil, nil) + // announcements.. + annAlice := collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob := collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireChannelAnnouncement(t) + annAlice.requireChannelUpdate(t) + + annBob.requireChannelAnnouncement(t) + annBob.requireChannelUpdate(t) + + // The channel is a private channel, so the channel update should set + // the __dont_forward__ flag. + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, 0, 0, 0, 0, false, + ) // Note: We don't check for the addedToGraph state because in // the private channel mode, the state is quickly changed from @@ -2997,40 +3198,10 @@ func TestFundingManagerPrivateRestart(t *testing.T) { Tx: fundingTx, } - // Since this is a private channel, we shouldn't receive the public - // channel announcement messages. - select { - case ann := <-alice.announceChan: - t.Fatalf("unexpectedly got channel announcement message: %v", - ann) - case <-time.After(300 * time.Millisecond): - } - - select { - case ann := <-bob.announceChan: - t.Fatalf("unexpectedly got channel announcement message: %v", - ann) - case <-time.After(300 * time.Millisecond): - } - - // We should however receive each side's node announcement. - select { - case msg := <-alice.msgChan: - if _, ok := msg.(*lnwire.NodeAnnouncement); !ok { - t.Fatalf("expected to receive node announcement") - } - case <-time.After(time.Second): - t.Fatalf("expected to receive node announcement") - } - - select { - case msg := <-bob.msgChan: - if _, ok := msg.(*lnwire.NodeAnnouncement); !ok { - t.Fatalf("expected to receive node announcement") - } - case <-time.After(time.Second): - t.Fatalf("expected to receive node announcement") - } + // For private channels, we only expect the node announcements to be + // sent directly to the peer. + verifyDirectNodeAnnouncement(t, alice) + verifyDirectNodeAnnouncement(t, bob) // Restart Alice's fundingManager so we can prove that the public // channel announcements are not sent upon restart and that the private @@ -3410,26 +3581,24 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { // Alice should advertise the default MinHTLC value of // 5, while bob should advertise the value minHtlc, since Alice // required him to use it. - minHtlcArr := []lnwire.MilliSatoshi{5, minHtlcIn} + minHtlcAlice := lnwire.MilliSatoshi(5) + minHtlcBob := lnwire.MilliSatoshi(minHtlcIn) // For maxHltc Alice should advertise the default MaxHtlc value of // maxValueAcceptChannel, while bob should advertise the value // maxValueInFlight since Alice required him to use it. - maxHtlcArr := []lnwire.MilliSatoshi{ - maxValueAcceptChannel, maxValueInFlight, - } + maxHtlcAlice := maxValueAcceptChannel + maxHtlcBob := lnwire.MilliSatoshi(maxValueInFlight) // Alice should have custom fees set whereas Bob should see his // configured default fees announced. defaultDelta := bob.fundingMgr.cfg.DefaultRoutingPolicy.TimeLockDelta defaultBaseFee := bob.fundingMgr.cfg.DefaultRoutingPolicy.BaseFee defaultFeeRate := bob.fundingMgr.cfg.DefaultRoutingPolicy.FeeRate - baseFees := []lnwire.MilliSatoshi{ - lnwire.MilliSatoshi(baseFee), defaultBaseFee, - } - feeRates := []lnwire.MilliSatoshi{ - lnwire.MilliSatoshi(feeRate), defaultFeeRate, - } + baseFeeAlice := lnwire.MilliSatoshi(baseFee) + baseFeeBob := defaultBaseFee + feeRateAlice := lnwire.MilliSatoshi(feeRate) + feeRateBob := defaultFeeRate // Check that they notify the breach arbiter and peer about the new // channel. @@ -3448,10 +3617,10 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { return reflect.DeepEqual( p, &models.ForwardingPolicy{ - MaxHTLC: maxHtlcArr[0], - MinHTLCOut: minHtlcArr[0], - BaseFee: baseFees[0], - FeeRate: feeRates[0], + MaxHTLC: maxHtlcAlice, + MinHTLCOut: minHtlcAlice, + BaseFee: baseFeeAlice, + FeeRate: feeRateAlice, TimeLockDelta: defaultDelta, }, ) @@ -3460,9 +3629,31 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { return true }, ) - assertChannelAnnouncements( - t, alice, bob, capacity, minHtlcArr, maxHtlcArr, baseFees, - feeRates, + + // The channel is confirmed, we expect the to receive the signatures + // and a new channel update msg. + annAlice := collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob := collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireChannelAnnouncement(t) + annAlice.requireChannelUpdate(t) + + annBob.requireChannelAnnouncement(t) + annBob.requireChannelUpdate(t) + + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, minHtlcAlice, + maxHtlcAlice, baseFeeAlice, feeRateAlice, false, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, minHtlcBob, + maxHtlcBob, baseFeeBob, feeRateBob, false, ) // The funding transaction is now confirmed, wait for the @@ -3478,7 +3669,32 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) { Tx: fundingTx, } - assertAnnouncementSignatures(t, alice, bob) + // Make sure both fundingManagers send the expected msgs. + annAlice = collectGossipMsgs(t, alice, 3) + verifyNoExtraMsgs(t, alice) + + annBob = collectGossipMsgs(t, bob, 3) + verifyNoExtraMsgs(t, bob) + + annAlice.requireAnnounceSignatures(t) + annAlice.requireChannelUpdate(t) + annAlice.requireNodeAnnouncement(t) + + annBob.requireAnnounceSignatures(t) + annBob.requireChannelUpdate(t) + annBob.requireNodeAnnouncement(t) + + otherNode = bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, capacity, minHtlcAlice, + maxHtlcAlice, baseFeeAlice, feeRateAlice, true, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, capacity, minHtlcBob, + maxHtlcBob, baseFeeBob, feeRateBob, true, + ) // After the announcement we expect Alice and Bob to have cleared // the fees for the channel from the database. @@ -4623,8 +4839,29 @@ func testZeroConf(t *testing.T, chanType *lnwire.ChannelType) { // We'll now assert that both sides send ChannelAnnouncement and // ChannelUpdate messages. - assertChannelAnnouncements( - t, alice, bob, fundingAmt, nil, nil, nil, nil, + annAlice := collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob := collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireChannelAnnouncement(t) + annAlice.requireChannelUpdate(t) + + annBob.requireChannelAnnouncement(t) + annBob.requireChannelUpdate(t) + + // The channel is still not confirmed, so the channel update should set + // the __dont_forward__ flag. + otherNode := bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, fundingAmt, 0, 0, 0, 0, + false, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, fundingAmt, 0, 0, 0, 0, false, ) // We'll now wait for the OpenStatusUpdate_ChanOpen update. @@ -4651,10 +4888,35 @@ func testZeroConf(t *testing.T, chanType *lnwire.ChannelType) { Tx: fundingTx, } - // For taproot channels, we don't expect them to be announced atm. - if !isTaprootChanType(chanType) { - assertChannelUpdates( - t, alice, bob, fundingAmt, nil, nil, nil, nil, + switch { + // For taproot channels, we expect nothing at this stage because they + // are private channels. + case isTaprootChanType(chanType): + + // For regular channels, we'll make sure the fundingManagers exchange + // announcement signatures. + case chanType == nil: + fallthrough + + default: + // Make sure both fundingManagers send the expected msgs. + // We only expect the channel update to be sent here. + annAlice = collectGossipMsgs(t, alice, 1) + verifyNoExtraMsgs(t, alice) + + annBob = collectGossipMsgs(t, bob, 1) + verifyNoExtraMsgs(t, bob) + + otherNode = bob + verifyChannelUpdate( + t, annAlice.channelUpd, otherNode, fundingAmt, 0, 0, 0, + 0, true, + ) + + otherNode = alice + verifyChannelUpdate( + t, annBob.channelUpd, otherNode, fundingAmt, 0, 0, 0, 0, + true, ) } @@ -4685,14 +4947,28 @@ func testZeroConf(t *testing.T, chanType *lnwire.ChannelType) { // announcement message at this point. These channels aren't advertised // so we don't expect the other messages. case isTaprootChanType(chanType): - assertNodeAnnSent(t, alice, bob) + verifyDirectNodeAnnouncement(t, alice) + verifyDirectNodeAnnouncement(t, bob) // For regular channels, we'll make sure the fundingManagers exchange // announcement signatures. case chanType == nil: fallthrough + default: - assertAnnouncementSignatures(t, alice, bob) + // Make sure both fundingManagers send the expected msgs. + annAlice = collectGossipMsgs(t, alice, 2) + verifyNoExtraMsgs(t, alice) + + annBob = collectGossipMsgs(t, bob, 2) + verifyNoExtraMsgs(t, bob) + + annAlice.requireAnnounceSignatures(t) + annAlice.requireNodeAnnouncement(t) + + annBob.requireAnnounceSignatures(t) + annBob.requireNodeAnnouncement(t) + } // Assert that the channel state is deleted from the fundingmanager's From d71a1c8b453a3e326f72444ce218eeac271f6984 Mon Sep 17 00:00:00 2001 From: ziggie Date: Thu, 19 Dec 2024 22:49:48 +0100 Subject: [PATCH 12/15] localchans: set dont_forward bit in chan update msg When a channel update is now executed the msg field will be migrated on the fly meaning that private channels or channels which still have no signatures exchanged will now signal the dont_forward bit. --- routing/localchans/manager.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/routing/localchans/manager.go b/routing/localchans/manager.go index cd9e58fcaa6..5a262fb0784 100644 --- a/routing/localchans/manager.go +++ b/routing/localchans/manager.go @@ -110,7 +110,7 @@ func (r *Manager) UpdatePolicy(newSchema routing.ChannelPolicy, } // Apply the new policy to the edge. - err := r.updateEdge(info.ChannelPoint, edge, newSchema) + err := r.updateEdge(info, edge, newSchema) if err != nil { failedUpdates = append(failedUpdates, makeFailureItem(info.ChannelPoint, @@ -255,7 +255,7 @@ func (r *Manager) createMissingEdge(channel *channeldb.OpenChannel, // Validate the newly created edge policy with the user defined new // schema before adding the edge to the database. - err = r.updateEdge(channel.FundingOutpoint, edge, newSchema) + err = r.updateEdge(info, edge, newSchema) if err != nil { return nil, nil, makeFailureItem( info.ChannelPoint, @@ -345,11 +345,11 @@ func (r *Manager) createEdge(channel *channeldb.OpenChannel, } // updateEdge updates the given edge with the new schema. -func (r *Manager) updateEdge(chanPoint wire.OutPoint, +func (r *Manager) updateEdge(edgeInfo *models.ChannelEdgeInfo, edge *models.ChannelEdgePolicy, newSchema routing.ChannelPolicy) error { - channel, err := r.FetchChannel(chanPoint) + channel, err := r.FetchChannel(edgeInfo.ChannelPoint) if err != nil { return err } @@ -407,6 +407,17 @@ func (r *Manager) updateEdge(chanPoint wire.OutPoint, // If the MaxHtlc flag wasn't already set, we can set it now. edge.MessageFlags |= lnwire.ChanUpdateRequiredMaxHtlc + // If the channel is still not (yet) announced to the broader network + // we signal this to our peer. The edge is later persisted to disk. This + // is not a problem for channels which have still not reached the min + // depth of six blocks because a new ChanUpdate is sent out when a + // public channel reaches six confirmations. + // + // NOTE: This is an on-the-fly migration. + if edgeInfo.AuthProof == nil { + edge.MessageFlags |= lnwire.ChanUpdateDontForward + } + // Validate htlc amount constraints. switch { case edge.MinHTLC < amtMin: From 828edf937ed1c23f18e63c023e6ff87cf063ab12 Mon Sep 17 00:00:00 2001 From: ziggie Date: Thu, 19 Dec 2024 23:10:43 +0100 Subject: [PATCH 13/15] discovery: dont_forward bit in msg flag. In case the __dont_forward__ is set in the ChanUpdate msg lnd will not forward this msg to the gossiper hence broadcasting it to all its peers. This makes sure we follow the network rules and dont forward ChanUpdates which do not signal it. --- discovery/gossiper.go | 150 +++++++++++++++++++++++++++++-------- discovery/gossiper_test.go | 21 +++++- 2 files changed, 136 insertions(+), 35 deletions(-) diff --git a/discovery/gossiper.go b/discovery/gossiper.go index 9c51734396a..2771dee1437 100644 --- a/discovery/gossiper.go +++ b/discovery/gossiper.go @@ -1462,6 +1462,10 @@ func (d *AuthenticatedGossiper) networkHandler() { sourceToPub(announcement.source), ) { + peer := sourceToPub(announcement.source) + log.Debugf("Throttling messages from peer=%v "+ + "(recently rejected)", peer) + announcement.err <- fmt.Errorf("recently " + "rejected") continue @@ -1927,17 +1931,24 @@ func (d *AuthenticatedGossiper) processRejectedEdge( msg: chanAnn, }) if e1Ann != nil { - announcements = append(announcements, networkMsg{ - source: d.selfKey, - msg: e1Ann, - }) + // Only add the ChanUpdate to the gossiper if the + // __dont_forward__ bit signals it. + if !e1Ann.MessageFlags.HasDontForward() { + announcements = append(announcements, networkMsg{ + source: d.selfKey, + msg: e1Ann, + }) + } } if e2Ann != nil { - announcements = append(announcements, networkMsg{ - source: d.selfKey, - msg: e2Ann, - }) - + // Only add the ChanUpdate to the gossiper if the + // __dont_forward__ bit signals it. + if !e2Ann.MessageFlags.HasDontForward() { + announcements = append(announcements, networkMsg{ + source: d.selfKey, + msg: e2Ann, + }) + } } return announcements, nil @@ -2347,6 +2358,18 @@ func IsKeepAliveUpdate(update *lnwire.ChannelUpdate1, if update.MessageFlags.HasMaxHtlc() && !prev.MessageFlags.HasMaxHtlc() { return false } + + // Every public channel starts with the dont_forward bit set before the + // mandatory six confirmations are reached. We will see 2 different + // ChanUpdates before a public channel is announced to the broader + // network. We need to make sure we don't consider the second one as a + // keepalive msg. + if !update.MessageFlags.HasDontForward() && + prev.MessageFlags.HasDontForward() { + + return false + } + if update.HtlcMaximumMsat != prev.MaxHTLC { return false } @@ -2459,6 +2482,10 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg, ) _, _ = d.recentRejects.Put(key, &cachedReject{}) + log.Debugf("Adding msg short_chan_id(%v) from "+ + "peer=%x to RejectCache", key.chanID, + key.pubkey) + nMsg.err <- err return nil, false } @@ -2476,6 +2503,10 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg, ) _, _ = d.recentRejects.Put(key, &cachedReject{}) + log.Debugf("Adding msg short_chan_id(%v) from "+ + "peer=%x to RejectCache", key.chanID, + key.pubkey) + nMsg.err <- err return nil, false } @@ -2560,6 +2591,10 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg, ) _, _ = d.recentRejects.Put(key, &cachedReject{}) + log.Debugf("Adding msg short_chan_id(%v) from "+ + "peer=%x to RejectCache", key.chanID, + key.pubkey) + log.Error(err) nMsg.err <- err return nil, false @@ -2673,6 +2708,10 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg, ) _, _ = d.recentRejects.Put(key, &cachedReject{}) + log.Debugf("Adding msg short_chan_id(%v) from "+ + "peer=%x to RejectCache", key.chanID, + key.pubkey) + // Increment the peer's ban score. We check isRemote // so we don't actually ban the peer in case of a local // bug. @@ -2687,6 +2726,10 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg, ) _, _ = d.recentRejects.Put(key, &cachedReject{}) + log.Debugf("Adding msg short_chan_id(%v) from "+ + "peer=%x to RejectCache", key.chanID, + key.pubkey) + // Since this channel has already been closed, we'll // add it to the graph's closed channel index such that // we won't attempt to do expensive validation checks @@ -2717,6 +2760,10 @@ func (d *AuthenticatedGossiper) handleChanAnnouncement(nMsg *networkMsg, sourceToPub(nMsg.source), ) _, _ = d.recentRejects.Put(key, &cachedReject{}) + + log.Debugf("Adding msg short_chan_id(%v) from "+ + "peer=%x to RejectCache", key.chanID, + key.pubkey) } if !nMsg.isRemote { @@ -2847,6 +2894,10 @@ func (d *AuthenticatedGossiper) handleChanUpdate(nMsg *networkMsg, ) _, _ = d.recentRejects.Put(key, &cachedReject{}) + log.Debugf("Adding msg short_chan_id(%v) from "+ + "peer=%x to RejectCache", key.chanID, + key.pubkey) + nMsg.err <- err return nil, false } @@ -2956,9 +3007,9 @@ func (d *AuthenticatedGossiper) handleChanUpdate(nMsg *networkMsg, // If the edge corresponding to this ChannelUpdate was not // found in the graph, this might be a channel in the process // of being opened, and we haven't processed our own - // ChannelAnnouncement yet, hence it is not not found in the - // graph. This usually gets resolved after the channel proofs - // are exchanged and the channel is broadcasted to the rest of + // ChannelAnnouncement yet, hence it is not found in the graph. + // This usually gets resolved after the channel proofs are + // exchanged and the channel is broadcasted to the rest of // the network, but in case this is a private channel this // won't ever happen. This can also happen in the case of a // zombie channel with a fresh update for which we don't have a @@ -3014,6 +3065,10 @@ func (d *AuthenticatedGossiper) handleChanUpdate(nMsg *networkMsg, ) _, _ = d.recentRejects.Put(key, &cachedReject{}) + log.Debugf("Adding msg short_chan_id(%v) from "+ + "peer=%x to RejectCache", key.chanID, + key.pubkey) + return nil, false } @@ -3143,6 +3198,10 @@ func (d *AuthenticatedGossiper) handleChanUpdate(nMsg *networkMsg, ) _, _ = d.recentRejects.Put(key, &cachedReject{}) + log.Debugf("Adding msg short_chan_id(%v) from "+ + "peer=%x to RejectCache", key.chanID, + key.pubkey) + log.Errorf("Update edge for short_chan_id(%v) got: %v", shortChanID, err) } @@ -3151,6 +3210,11 @@ func (d *AuthenticatedGossiper) handleChanUpdate(nMsg *networkMsg, return nil, false } + // Get our peer's public key. + remotePubKey := remotePubFromChanInfo( + chanInfo, upd.ChannelFlags, + ) + // If this is a local ChannelUpdate without an AuthProof, it means it // is an update to a channel that is not (yet) supposed to be announced // to the greater network. However, our channel counter party will need @@ -3186,11 +3250,6 @@ func (d *AuthenticatedGossiper) handleChanUpdate(nMsg *networkMsg, } } - // Get our peer's public key. - remotePubKey := remotePubFromChanInfo( - chanInfo, upd.ChannelFlags, - ) - log.Debugf("The message %v has no AuthProof, sending the "+ "update to remote peer %x", upd.MsgType(), remotePubKey) @@ -3214,12 +3273,29 @@ func (d *AuthenticatedGossiper) handleChanUpdate(nMsg *networkMsg, // contains an alias because the network would reject this. var announcements []networkMsg if chanInfo.AuthProof != nil && !d.cfg.IsAlias(upd.ShortChannelID) { - announcements = append(announcements, networkMsg{ - peer: nMsg.peer, - source: nMsg.source, - isRemote: nMsg.isRemote, - msg: upd, - }) + // We log the case where the dont_forward bit is set although + // the channel is already announced to the network because we + // already have a AuthProof for it. In addition we do not add + // this msg to the gossip msg queue to make sure we follow the + // network rules. + if upd.MessageFlags.HasDontForward() { + log.Warnf("Received %v with the dont_forward bit set "+ + "(msgflags=%s) for channel=%v from peer=%x "+ + "for an announced channel, not forwarding to "+ + "the broader network", + upd.MsgType().String(), + upd.MessageFlags.String(), + upd.ShortChannelID.ToUint64(), + remotePubKey, + ) + } else { + announcements = append(announcements, networkMsg{ + peer: nMsg.peer, + source: nMsg.source, + isRemote: nMsg.isRemote, + msg: upd, + }) + } } nMsg.err <- nil @@ -3495,18 +3571,26 @@ func (d *AuthenticatedGossiper) handleAnnSig(nMsg *networkMsg, msg: chanAnn, }) if src, err := chanInfo.NodeKey1(); err == nil && e1Ann != nil { - announcements = append(announcements, networkMsg{ - peer: nMsg.peer, - source: src, - msg: e1Ann, - }) + // Only add the ChanUpdate to the gossiper if the + // __dont_forward__ bit signals it. + if !e1Ann.MessageFlags.HasDontForward() { + announcements = append(announcements, networkMsg{ + peer: nMsg.peer, + source: src, + msg: e1Ann, + }) + } } if src, err := chanInfo.NodeKey2(); err == nil && e2Ann != nil { - announcements = append(announcements, networkMsg{ - peer: nMsg.peer, - source: src, - msg: e2Ann, - }) + // Only add the ChanUpdate to the gossiper if the + // __dont_forward__ bit signals it. + if !e2Ann.MessageFlags.HasDontForward() { + announcements = append(announcements, networkMsg{ + peer: nMsg.peer, + source: src, + msg: e2Ann, + }) + } } // We'll also send along the node announcements for each channel diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index b74f69bf0ad..ab45a8d5817 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -3214,15 +3214,32 @@ func TestSendChannelUpdateReliably(t *testing.T) { require.NoError(t, err, "unable to process remote channel proof") // Now that we've constructed our full proof, we can assert that the - // channel has been announced. + // channel has been announced. We anticipate the local ChanAnnouncement + // and the local ChanUpdate. + gotChannelAnnouncement := false + gotChannelUpdate := false for i := 0; i < 2; i++ { select { - case <-ctx.broadcastedMessage: + case gossip := <-ctx.broadcastedMessage: + switch msg := gossip.msg.(type) { + case *lnwire.ChannelUpdate1: + gotChannelUpdate = true + case *lnwire.ChannelAnnouncement1: + gotChannelAnnouncement = true + default: + t.Fatalf("send unexpected %v "+ + "message", msg.MsgType()) + } case <-time.After(2 * trickleDelay): t.Fatal("expected channel to be announced") } } + require.Truef( + t, gotChannelAnnouncement, "did not receive ChanAnnouncement", + ) + require.Truef(t, gotChannelUpdate, "did not receive ChanUpdate") + // With the channel announced, we'll generate a new channel update. This // one won't take the path of the reliable sender, as the channel has // already been announced. We'll keep track of the old message that is From 146bfb890e63791a0ea4443efa30e032955cee50 Mon Sep 17 00:00:00 2001 From: ziggie Date: Thu, 19 Dec 2024 23:11:46 +0100 Subject: [PATCH 14/15] routing: fix typo and log msg. and log msg. --- graph/notifications.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graph/notifications.go b/graph/notifications.go index 76eabdb02f0..8718877cabc 100644 --- a/graph/notifications.go +++ b/graph/notifications.go @@ -101,7 +101,7 @@ func (b *Builder) SubscribeTopology() (*TopologyClient, error) { // notification channel. type topologyClient struct { // ntfnChan is a send-only channel that's used to propagate - // notification s from the channel router to an instance of a + // notifications from the channel router to an instance of a // topologyClient client. ntfnChan chan<- *TopologyChange From 6bbd9a4997fb464955475006e4dad72aac9f173d Mon Sep 17 00:00:00 2001 From: ziggie Date: Thu, 19 Dec 2024 23:12:59 +0100 Subject: [PATCH 15/15] docs: update release-docs --- docs/release-notes/release-notes-0.19.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index 7f4b4927c82..fe5a3562d0e 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -99,6 +99,9 @@ are now [sorted](https://github.com/lightningnetwork/lnd/pull/9337) based on the `InvoiceHTLC.HtlcIndex`. +* Make sure to [set the dont_forward msg flag in the channel update msg + for unannounced channels](https://github.com/lightningnetwork/lnd/pull/8582). + ## lncli Additions * [A pre-generated macaroon root key can now be specified in `lncli create` and