diff --git a/channeldb/channel.go b/channeldb/channel.go index f4e99a6f8c..7bf54e51d6 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() { diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index 43ed7ebbcd..7f4b4927c8 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 diff --git a/funding/manager.go b/funding/manager.go index 395cccb2a6..9373657c5d 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 b6130176d1..ea6326b4a8 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, @@ -1171,105 +1172,163 @@ 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, + ) + + 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, + ) - // 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. + _, 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, + ) } + } +} - 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) { @@ -4594,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/builder.go b/graph/builder.go index d6984af709..898812441d 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 @@ -1691,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 } diff --git a/graph/db/graph.go b/graph/db/graph.go index 2a91398c62..3e94acb4cc 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 8a02f24ff4..62e45df923 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/lnwire/channel_ready.go b/lnwire/channel_ready.go index 912a068bde..813f1c1ed0 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 f0399b4e8a..c7fc4b6f46 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, diff --git a/server.go b/server.go index 799bfde6b8..8961b4bd51 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,