diff --git a/chan_series.go b/chan_series.go index 83b05b028..7f592503a 100644 --- a/chan_series.go +++ b/chan_series.go @@ -89,6 +89,22 @@ func (c *chanSeries) UpdatesInHorizon(chain chainhash.Hash, return nil, err } for _, nodeAnn := range nodeAnnsInHorizon { + // Ensure we only forward nodes that are publicly advertised to + // prevent leaking information about nodes. + isNodePublic, err := c.graph.IsPublicNode(nodeAnn.PubKeyBytes) + if err != nil { + srvrLog.Errorf("Unable to determine if node %x is "+ + "advertised: %v", nodeAnn.PubKeyBytes, err) + continue + } + + if !isNodePublic { + srvrLog.Tracef("Skipping forwarding announcement for "+ + "node %x due to being unadvertised", + nodeAnn.PubKeyBytes) + continue + } + nodeUpdate, err := nodeAnn.NodeAnnouncement(true) if err != nil { return nil, err diff --git a/channeldb/graph.go b/channeldb/graph.go index eb681395e..0fd9e0c8a 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/sha256" "encoding/binary" + "errors" "fmt" "image/color" "io" @@ -1804,6 +1805,47 @@ func (l *LightningNode) NodeAnnouncement(signed bool) (*lnwire.NodeAnnouncement, return nodeAnn, nil } +// isPublic determines whether the node is seen as public within the graph from +// the source node's point of view. An existing database transaction can also be +// specified. +func (l *LightningNode) isPublic(tx *bolt.Tx, sourcePubKey []byte) (bool, error) { + // In order to determine whether this node is publicly advertised within + // the graph, we'll need to look at all of its edges and check whether + // they extend to any other node than the source node. errDone will be + // used to terminate the check early. + nodeIsPublic := false + errDone := errors.New("done") + err := l.ForEachChannel(tx, func(_ *bolt.Tx, info *ChannelEdgeInfo, + _, _ *ChannelEdgePolicy) error { + + // If this edge doesn't extend to the source node, we'll + // terminate our search as we can now conclude that the node is + // publicly advertised within the graph due to the local node + // knowing of the current edge. + if !bytes.Equal(info.NodeKey1Bytes[:], sourcePubKey) && + !bytes.Equal(info.NodeKey2Bytes[:], sourcePubKey) { + + nodeIsPublic = true + return errDone + } + + // Since the edge _does_ extend to the source node, we'll also + // need to ensure that this is a public edge. + if info.AuthProof != nil { + nodeIsPublic = true + return errDone + } + + // Otherwise, we'll continue our search. + return nil + }) + if err != nil && err != errDone { + return false, err + } + + return nodeIsPublic, nil +} + // FetchLightningNode attempts to look up a target node by its identity public // key. If the node isn't found in the database, then ErrGraphNodeNotFound is // returned. @@ -2566,6 +2608,35 @@ func (c *ChannelGraph) FetchChannelEdgesByID(chanID uint64) (*ChannelEdgeInfo, * return edgeInfo, policy1, policy2, nil } +// IsPublicNode is a helper method that determines whether the node with the +// given public key is seen as a public node in the graph from the graph's +// source node's point of view. +func (c *ChannelGraph) IsPublicNode(pubKey [33]byte) (bool, error) { + var nodeIsPublic bool + err := c.db.View(func(tx *bolt.Tx) error { + nodes := tx.Bucket(nodeBucket) + if nodes == nil { + return ErrGraphNodesNotFound + } + ourPubKey := nodes.Get(sourceKey) + if ourPubKey == nil { + return ErrSourceNodeNotSet + } + node, err := fetchLightningNode(nodes, pubKey[:]) + if err != nil { + return err + } + + nodeIsPublic, err = node.isPublic(tx, ourPubKey) + return err + }) + if err != nil { + return false, err + } + + return nodeIsPublic, nil +} + // genMultiSigP2WSH generates the p2wsh'd multisig script for 2 of 2 pubkeys. func genMultiSigP2WSH(aPub, bPub []byte) ([]byte, error) { if len(aPub) != 33 || len(bPub) != 33 { diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 92fcd4f1d..369bdc6f7 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -2438,6 +2438,162 @@ func TestNodePruningUpdateIndexDeletion(t *testing.T) { } } +// TestNodeIsPublic ensures that we properly detect nodes that are seen as +// public within the network graph. +func TestNodeIsPublic(t *testing.T) { + t.Parallel() + + // We'll start off the test by creating a small network of 3 + // participants with the following graph: + // + // Alice <-> Bob <-> Carol + // + // We'll need to create a separate database and channel graph for each + // participant to replicate real-world scenarios (private edges being in + // some graphs but not others, etc.). + aliceDB, cleanUp, err := makeTestDB() + defer cleanUp() + if err != nil { + t.Fatalf("unable to make test database: %v", err) + } + aliceNode, err := createTestVertex(aliceDB) + if err != nil { + t.Fatalf("unable to create test node: %v", err) + } + aliceGraph := aliceDB.ChannelGraph() + if err := aliceGraph.SetSourceNode(aliceNode); err != nil { + t.Fatalf("unable to set source node: %v", err) + } + + bobDB, cleanUp, err := makeTestDB() + defer cleanUp() + if err != nil { + t.Fatalf("unable to make test database: %v", err) + } + bobNode, err := createTestVertex(bobDB) + if err != nil { + t.Fatalf("unable to create test node: %v", err) + } + bobGraph := bobDB.ChannelGraph() + if err := bobGraph.SetSourceNode(bobNode); err != nil { + t.Fatalf("unable to set source node: %v", err) + } + + carolDB, cleanUp, err := makeTestDB() + defer cleanUp() + if err != nil { + t.Fatalf("unable to make test database: %v", err) + } + carolNode, err := createTestVertex(carolDB) + if err != nil { + t.Fatalf("unable to create test node: %v", err) + } + carolGraph := carolDB.ChannelGraph() + if err := carolGraph.SetSourceNode(carolNode); err != nil { + t.Fatalf("unable to set source node: %v", err) + } + + aliceBobEdge, _ := createEdge(10, 0, 0, 0, aliceNode, bobNode) + bobCarolEdge, _ := createEdge(10, 1, 0, 1, bobNode, carolNode) + + // After creating all of our nodes and edges, we'll add them to each + // participant's graph. + nodes := []*LightningNode{aliceNode, bobNode, carolNode} + edges := []*ChannelEdgeInfo{&aliceBobEdge, &bobCarolEdge} + dbs := []*DB{aliceDB, bobDB, carolDB} + graphs := []*ChannelGraph{aliceGraph, bobGraph, carolGraph} + for i, graph := range graphs { + for _, node := range nodes { + node.db = dbs[i] + if err := graph.AddLightningNode(node); err != nil { + t.Fatalf("unable to add node: %v", err) + } + } + for _, edge := range edges { + edge.db = dbs[i] + if err := graph.AddChannelEdge(edge); err != nil { + t.Fatalf("unable to add edge: %v", err) + } + } + } + + // checkNodes is a helper closure that will be used to assert that the + // given nodes are seen as public/private within the given graphs. + checkNodes := func(nodes []*LightningNode, graphs []*ChannelGraph, + public bool) { + + t.Helper() + + for _, node := range nodes { + for _, graph := range graphs { + isPublic, err := graph.IsPublicNode(node.PubKeyBytes) + if err != nil { + t.Fatalf("unable to determine if pivot "+ + "is public: %v", err) + } + + switch { + case isPublic && !public: + t.Fatalf("expected %x to be private", + node.PubKeyBytes) + case !isPublic && public: + t.Fatalf("expected %x to be public", + node.PubKeyBytes) + } + } + } + } + + // Due to the way the edges were set up above, we'll make sure each node + // can correctly determine that every other node is public. + checkNodes(nodes, graphs, true) + + // Now, we'll remove the edge between Alice and Bob from everyone's + // graph. This will make Alice be seen as a private node as it no longer + // has any advertised edges. + for _, graph := range graphs { + err := graph.DeleteChannelEdge(&aliceBobEdge.ChannelPoint) + if err != nil { + t.Fatalf("unable to remove edge: %v", err) + } + } + checkNodes( + []*LightningNode{aliceNode}, + []*ChannelGraph{bobGraph, carolGraph}, + false, + ) + + // We'll also make the edge between Bob and Carol private. Within Bob's + // and Carol's graph, the edge will exist, but it will not have a proof + // that allows it to be advertised. Within Alice's graph, we'll + // completely remove the edge as it is not possible for her to know of + // it without it being advertised. + for i, graph := range graphs { + err := graph.DeleteChannelEdge(&bobCarolEdge.ChannelPoint) + if err != nil { + t.Fatalf("unable to remove edge: %v", err) + } + + if graph == aliceGraph { + continue + } + + bobCarolEdge.AuthProof = nil + bobCarolEdge.db = dbs[i] + if err := graph.AddChannelEdge(&bobCarolEdge); err != nil { + t.Fatalf("unable to add edge: %v", err) + } + } + + // With the modifications above, Bob should now be seen as a private + // node from both Alice's and Carol's perspective. + checkNodes( + []*LightningNode{bobNode}, + []*ChannelGraph{aliceGraph, carolGraph}, + false, + ) +} + // compareNodes is used to compare two LightningNodes while excluding the // Features struct, which cannot be compared as the semantics for reserializing // the featuresMap have not been defined. diff --git a/discovery/gossiper.go b/discovery/gossiper.go index 641f4c9ed..88fd401aa 100644 --- a/discovery/gossiper.go +++ b/discovery/gossiper.go @@ -1613,13 +1613,26 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement( return nil } - // Node announcement was successfully proceeded and know it - // might be broadcast to other connected nodes. - announcements = append(announcements, networkMsg{ - peer: nMsg.peer, - source: nMsg.source, - msg: msg, - }) + // In order to ensure we don't leak unadvertised nodes, we'll + // make a quick check to ensure this node intends to publicly + // advertise itself to the network. + isPublic, err := d.cfg.Router.IsPublicNode(node.PubKeyBytes) + if err != nil { + log.Errorf("Unable to determine if node %x is "+ + "advertised: %v", node.PubKeyBytes, err) + nMsg.err <- err + return nil + } + + // If it does, we'll add their announcement to our batch so that + // it can be broadcast to the rest of our peers. + if isPublic { + announcements = append(announcements, networkMsg{ + peer: nMsg.peer, + source: nMsg.source, + msg: msg, + }) + } nMsg.err <- nil // TODO(roasbeef): get rid of the above diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index 560125259..d9499a4a3 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -1,23 +1,19 @@ package discovery import ( + "bytes" "encoding/hex" "fmt" + "io/ioutil" + "math/big" + prand "math/rand" "net" + "os" "reflect" "sync" - - prand "math/rand" - "testing" - - "math/big" - "time" - "io/ioutil" - "os" - "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" @@ -213,6 +209,22 @@ func (r *mockGraphSource) IsStaleNode(nodePub routing.Vertex, timestamp time.Tim return false } +// IsPublicNode determines whether the given vertex is seen as a public node in +// the graph from the graph's source node's point of view. +func (r *mockGraphSource) IsPublicNode(node routing.Vertex) (bool, error) { + for _, info := range r.infos { + if !bytes.Equal(node[:], info.NodeKey1Bytes[:]) && + !bytes.Equal(node[:], info.NodeKey2Bytes[:]) { + continue + } + + if info.AuthProof != nil { + return true, nil + } + } + return false, nil +} + // IsKnownEdge returns true if the graph source already knows of the passed // channel ID. func (r *mockGraphSource) IsKnownEdge(chanID lnwire.ShortChannelID) bool { @@ -441,10 +453,9 @@ func createUpdateAnnouncement(blockHeight uint32, flags lnwire.ChanUpdateFlag, return a, nil } -func createRemoteChannelAnnouncement(blockHeight uint32, - extraBytes ...[]byte) (*lnwire.ChannelAnnouncement, error) { +func createAnnouncementWithoutProof(blockHeight uint32, + extraBytes ...[]byte) *lnwire.ChannelAnnouncement { - var err error a := &lnwire.ChannelAnnouncement{ ShortChannelID: lnwire.ShortChannelID{ BlockHeight: blockHeight, @@ -461,6 +472,14 @@ func createRemoteChannelAnnouncement(blockHeight uint32, a.ExtraOpaqueData = extraBytes[0] } + return a +} + +func createRemoteChannelAnnouncement(blockHeight uint32, + extraBytes ...[]byte) (*lnwire.ChannelAnnouncement, error) { + + a := createAnnouncementWithoutProof(blockHeight, extraBytes...) + pub := nodeKeyPriv1.PubKey() signer := mockSigner{nodeKeyPriv1} sig, err := SignAnnouncement(&signer, pub, a) @@ -597,40 +616,10 @@ func TestProcessAnnouncement(t *testing.T) { } } - // Create node valid, signed announcement, process it with - // gossiper service, check that valid announcement have been - // propagated farther into the lightning network, and check that we - // added new node into router. - na, err := createNodeAnnouncement(nodeKeyPriv1, timestamp) - if err != nil { - t.Fatalf("can't create node announcement: %v", err) - } - nodePeer := &mockPeer{nodeKeyPriv1.PubKey(), nil, nil} - select { - case err = <-ctx.gossiper.ProcessRemoteAnnouncement(na, nodePeer): - case <-time.After(2 * time.Second): - t.Fatal("remote announcement not processed") - } - if err != nil { - t.Fatalf("can't process remote announcement: %v", err) - } - - select { - case msg := <-ctx.broadcastedMessage: - assertSenderExistence(nodePeer.IdentityKey(), msg) - case <-time.After(2 * trickleDelay): - t.Fatal("announcement wasn't proceeded") - } - - if len(ctx.router.nodes) != 1 { - t.Fatalf("node wasn't added to router: %v", err) - } - - // Pretending that we receive the valid channel announcement from - // remote side, and check that we broadcasted it to the our network, - // and added channel info in the router. + // First, we'll craft a valid remote channel announcement and send it to + // the gossiper so that it can be processed. ca, err := createRemoteChannelAnnouncement(0) if err != nil { t.Fatalf("can't create channel announcement: %v", err) @@ -645,6 +634,8 @@ func TestProcessAnnouncement(t *testing.T) { t.Fatalf("can't process remote announcement: %v", err) } + // The announcement should be broadcast and included in our local view + // of the graph. select { case msg := <-ctx.broadcastedMessage: assertSenderExistence(nodePeer.IdentityKey(), msg) @@ -656,9 +647,8 @@ func TestProcessAnnouncement(t *testing.T) { t.Fatalf("edge wasn't added to router: %v", err) } - // Pretending that we received valid channel policy update from remote - // side, and check that we broadcasted it to the other network, and - // added updates to the router. + // We'll then craft the channel policy of the remote party and also send + // it to the gossiper. ua, err := createUpdateAnnouncement(0, 0, nodeKeyPriv1, timestamp) if err != nil { t.Fatalf("can't create update announcement: %v", err) @@ -673,6 +663,7 @@ func TestProcessAnnouncement(t *testing.T) { t.Fatalf("can't process remote announcement: %v", err) } + // The channel policy should be broadcast to the rest of the network. select { case msg := <-ctx.broadcastedMessage: assertSenderExistence(nodePeer.IdentityKey(), msg) @@ -683,6 +674,34 @@ func TestProcessAnnouncement(t *testing.T) { if len(ctx.router.edges) != 1 { t.Fatalf("edge update wasn't added to router: %v", err) } + + // Finally, we'll craft the remote party's node announcement. + na, err := createNodeAnnouncement(nodeKeyPriv1, timestamp) + if err != nil { + t.Fatalf("can't create node announcement: %v", err) + } + + select { + case err = <-ctx.gossiper.ProcessRemoteAnnouncement(na, nodePeer): + case <-time.After(2 * time.Second): + t.Fatal("remote announcement not processed") + } + if err != nil { + t.Fatalf("can't process remote announcement: %v", err) + } + + // It should also be broadcast to the network and included in our local + // view of the graph. + select { + case msg := <-ctx.broadcastedMessage: + assertSenderExistence(nodePeer.IdentityKey(), msg) + case <-time.After(2 * trickleDelay): + t.Fatal("announcement wasn't proceeded") + } + + if len(ctx.router.nodes) != 1 { + t.Fatalf("node wasn't added to router: %v", err) + } } // TestPrematureAnnouncement checks that premature announcements are @@ -1966,6 +1985,115 @@ func TestDeDuplicatedAnnouncements(t *testing.T) { } } +// TestForwardPrivateNodeAnnouncement ensures that we do not forward node +// announcements for nodes who do not intend to publicly advertise themselves. +func TestForwardPrivateNodeAnnouncement(t *testing.T) { + t.Parallel() + + const ( + startingHeight = 100 + timestamp = 123456 + ) + + ctx, cleanup, err := createTestCtx(startingHeight) + if err != nil { + t.Fatalf("can't create context: %v", err) + } + defer cleanup() + + // We'll start off by processing a channel announcement without a proof + // (i.e., an unadvertised channel), followed by a node announcement for + // this same channel announcement. + chanAnn := createAnnouncementWithoutProof(startingHeight - 2) + pubKey := nodeKeyPriv1.PubKey() + + select { + case err := <-ctx.gossiper.ProcessLocalAnnouncement(chanAnn, pubKey): + if err != nil { + t.Fatalf("unable to process local announcement: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatalf("local announcement not processed") + } + + // The gossiper should not broadcast the announcement due to it not + // having its announcement signatures. + select { + case <-ctx.broadcastedMessage: + t.Fatal("gossiper should not have broadcast channel announcement") + case <-time.After(2 * trickleDelay): + } + + nodeAnn, err := createNodeAnnouncement(nodeKeyPriv1, timestamp) + if err != nil { + t.Fatalf("unable to create node announcement: %v", err) + } + + select { + case err := <-ctx.gossiper.ProcessLocalAnnouncement(nodeAnn, pubKey): + if err != nil { + t.Fatalf("unable to process remote announcement: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("remote announcement not processed") + } + + // The gossiper should also not broadcast the node announcement due to + // it not being part of any advertised channels. + select { + case <-ctx.broadcastedMessage: + t.Fatal("gossiper should not have broadcast node announcement") + case <-time.After(2 * trickleDelay): + } + + // Now, we'll attempt to forward the NodeAnnouncement for the same node + // by opening a public channel on the network. We'll create a + // ChannelAnnouncement and hand it off to the gossiper in order to + // process it. + remoteChanAnn, err := createRemoteChannelAnnouncement(startingHeight - 1) + if err != nil { + t.Fatalf("unable to create remote channel announcement: %v", err) + } + peer := &mockPeer{pubKey, nil, nil} + + select { + case err := <-ctx.gossiper.ProcessRemoteAnnouncement(remoteChanAnn, peer): + if err != nil { + t.Fatalf("unable to process remote announcement: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("remote announcement not processed") + } + + select { + case <-ctx.broadcastedMessage: + case <-time.After(2 * trickleDelay): + t.Fatal("gossiper should have broadcast the channel announcement") + } + + // We'll recreate the NodeAnnouncement with an updated timestamp to + // prevent a stale update. The NodeAnnouncement should now be forwarded. + nodeAnn, err = createNodeAnnouncement(nodeKeyPriv1, timestamp+1) + if err != nil { + t.Fatalf("unable to create node announcement: %v", err) + } + + select { + case err := <-ctx.gossiper.ProcessRemoteAnnouncement(nodeAnn, peer): + if err != nil { + t.Fatalf("unable to process remote announcement: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("remote announcement not processed") + } + + select { + case <-ctx.broadcastedMessage: + case <-time.After(2 * trickleDelay): + t.Fatal("gossiper should have broadcast the node announcement") + } +} + // TestReceiveRemoteChannelUpdateFirst tests that if we receive a // ChannelUpdate from the remote before we have processed our // own ChannelAnnouncement, it will be reprocessed later, after diff --git a/fundingmanager.go b/fundingmanager.go index 0acfadcfb..abd7df3fd 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -15,9 +15,6 @@ import ( "github.com/coreos/bbolt" "github.com/davecgh/go-spew/spew" "github.com/go-errors/errors" - "golang.org/x/crypto/salsa20" - "google.golang.org/grpc" - "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/htlcswitch" @@ -27,6 +24,8 @@ import ( "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing" + "golang.org/x/crypto/salsa20" + "google.golang.org/grpc" ) const ( @@ -2149,16 +2148,44 @@ func (f *fundingManager) addToRouterGraph(completeChan *channeldb.OpenChannel, func (f *fundingManager) annAfterSixConfs(completeChan *channeldb.OpenChannel, shortChanID *lnwire.ShortChannelID) error { - // If this channel is meant to be announced to the greater network, - // wait until the funding tx has reached 6 confirmations before - // announcing it. + // 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 + // don't leak any of our information. announceChan := completeChan.ChannelFlags&lnwire.FFAnnounceChannel != 0 if !announceChan { fndgLog.Debugf("Will not announce private channel %v.", shortChanID.ToUint64()) + + peerChan := make(chan lnpeer.Peer, 1) + f.cfg.NotifyWhenOnline(completeChan.IdentityPub, peerChan) + + var peer lnpeer.Peer + select { + case peer = <-peerChan: + case <-f.quit: + return ErrFundingManagerShuttingDown + } + + nodeAnn, err := f.cfg.CurrentNodeAnnouncement() + if err != nil { + return fmt.Errorf("unable to retrieve current node "+ + "announcement: %v", err) + } + + chanID := lnwire.NewChanIDFromOutPoint( + &completeChan.FundingOutpoint, + ) + pubKey := peer.PubKey() + fndgLog.Debugf("Sending our NodeAnnouncement for "+ + "ChannelID(%v) to %x", chanID, pubKey) + + if err := peer.SendMessage(true, &nodeAnn); err != nil { + return fmt.Errorf("unable to send node announcement "+ + "to peer %x: %v", pubKey, err) + } } else { - // Register with the ChainNotifier for a notification once the - // funding transaction reaches at least 6 confirmations. + // Otherwise, we'll wait until the funding transaction has + // reached 6 confirmations before announcing it. numConfs := uint32(completeChan.NumConfsRequired) if numConfs < 6 { numConfs = 6 @@ -2176,8 +2203,11 @@ func (f *fundingManager) annAfterSixConfs(completeChan *channeldb.OpenChannel, completeChan.FundingOutpoint, err) } + // Register with the ChainNotifier for a notification once the + // funding transaction reaches at least 6 confirmations. confNtfn, err := f.cfg.Notifier.RegisterConfirmationsNtfn( - &txid, fundingScript, numConfs, completeChan.FundingBroadcastHeight, + &txid, fundingScript, numConfs, + completeChan.FundingBroadcastHeight, ) if err != nil { return fmt.Errorf("Unable to register for "+ diff --git a/fundingmanager_test.go b/fundingmanager_test.go index 2cfd80972..969e4efe4 100644 --- a/fundingmanager_test.go +++ b/fundingmanager_test.go @@ -1933,7 +1933,7 @@ func TestFundingManagerPrivateChannel(t *testing.T) { bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} // Since this is a private channel, we shouldn't receive the - // announcement signatures or node announcement messages. + // announcement signatures. select { case ann := <-alice.announceChan: t.Fatalf("unexpectedly got channel announcement message: %v", ann) @@ -1948,6 +1948,25 @@ func TestFundingManagerPrivateChannel(t *testing.T) { // 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") + } + // The internal state-machine should now have deleted the channelStates // from the database, as the channel is announced. assertNoChannelState(t, alice, bob, fundingOutPoint) @@ -2013,19 +2032,48 @@ func TestFundingManagerPrivateRestart(t *testing.T) { // channel. assertHandleFundingLocked(t, alice, bob) - // Restart Alice's fundingManager so we can prove that the public - // channel announcements are not sent upon restart and that the private - // setting persists upon restart. - recreateAliceFundingManager(t, alice) - time.Sleep(300 * time.Millisecond) - // Notify that six confirmations has been reached on funding transaction. alice.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} bob.mockNotifier.sixConfChannel <- &chainntnfs.TxConfirmation{} // Since this is a private channel, we shouldn't receive the public - // channel announcement messages announcement signatures or - // node announcement. + // 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") + } + + // Restart Alice's fundingManager so we can prove that the public + // channel announcements are not sent upon restart and that the private + // setting persists upon restart. + recreateAliceFundingManager(t, alice) + select { case ann := <-alice.announceChan: t.Fatalf("unexpectedly got channel announcement message: %v", ann) diff --git a/lnd_test.go b/lnd_test.go index 9a72e994c..00506f8b1 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -4661,6 +4661,23 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { }, ) + // We'll also create a public channel between Bob and Carol to ensure + // that Bob gets selected as the only routing hint. We do this as + // we should only include routing hints for nodes that are publicly + // advertised, otherwise we'd end up leaking information about nodes + // that wish to stay unadvertised. + if err := net.ConnectNodes(ctxb, net.Bob, carol); err != nil { + t.Fatalf("unable to connect alice to carol: %v", err) + } + ctxt, _ = context.WithTimeout(ctxb, timeout) + chanPointBobCarol := openChannelAndAssert( + ctxt, t, net, net.Bob, carol, + lntest.OpenChannelParams{ + Amt: chanAmt, + PushAmt: chanAmt / 2, + }, + ) + // Then, we'll create Dave's node and open a private channel between him // and Alice. We will not include a push amount in order to not consider // this channel as a routing hint as it will not have enough remote @@ -4707,7 +4724,8 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { // Make sure all the channels have been opened. nodeNames := []string{"bob", "carol", "dave", "eve"} aliceChans := []*lnrpc.ChannelPoint{ - chanPointBob, chanPointCarol, chanPointDave, chanPointEve, + chanPointBob, chanPointCarol, chanPointBobCarol, chanPointDave, + chanPointEve, } for i, chanPoint := range aliceChans { ctxt, _ := context.WithTimeout(ctxb, timeout) @@ -4799,6 +4817,8 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { ctxt, _ = context.WithTimeout(ctxb, timeout) closeChannelAndAssert(ctxt, t, net, net.Alice, chanPointCarol, false) ctxt, _ = context.WithTimeout(ctxb, timeout) + closeChannelAndAssert(ctxt, t, net, net.Bob, chanPointBobCarol, false) + ctxt, _ = context.WithTimeout(ctxb, timeout) closeChannelAndAssert(ctxt, t, net, net.Alice, chanPointDave, false) // The channel between Alice and Eve should be force closed since Eve @@ -7657,20 +7677,16 @@ func testGraphTopologyNotifications(net *lntest.NetworkHarness, t *harnessTest) // The channel opening above should have triggered a few notifications // sent to the notification client. We'll expect two channel updates, // and two node announcements. - const numExpectedUpdates = 4 - for i := 0; i < numExpectedUpdates; i++ { + var numChannelUpds int + var numNodeAnns int + for numChannelUpds < 2 && numNodeAnns < 2 { select { // Ensure that a new update for both created edges is properly // dispatched to our registered client. case graphUpdate := <-graphSub.updateChan: - - if len(graphUpdate.ChannelUpdates) > 0 { - chanUpdate := graphUpdate.ChannelUpdates[0] - if chanUpdate.Capacity != int64(chanAmt) { - t.Fatalf("channel capacities mismatch:"+ - " expected %v, got %v", chanAmt, - btcutil.Amount(chanUpdate.Capacity)) - } + // Process all channel updates prsented in this update + // message. + for _, chanUpdate := range graphUpdate.ChannelUpdates { switch chanUpdate.AdvertisingNode { case net.Alice.PubKeyStr: case net.Bob.PubKeyStr: @@ -7685,10 +7701,16 @@ func testGraphTopologyNotifications(net *lntest.NetworkHarness, t *harnessTest) t.Fatalf("unknown connecting node: %v", chanUpdate.ConnectingNode) } + + if chanUpdate.Capacity != int64(chanAmt) { + t.Fatalf("channel capacities mismatch:"+ + " expected %v, got %v", chanAmt, + btcutil.Amount(chanUpdate.Capacity)) + } + numChannelUpds++ } - if len(graphUpdate.NodeUpdates) > 0 { - nodeUpdate := graphUpdate.NodeUpdates[0] + for _, nodeUpdate := range graphUpdate.NodeUpdates { switch nodeUpdate.IdentityKey { case net.Alice.PubKeyStr: case net.Bob.PubKeyStr: @@ -7696,11 +7718,14 @@ func testGraphTopologyNotifications(net *lntest.NetworkHarness, t *harnessTest) t.Fatalf("unknown node: %v", nodeUpdate.IdentityKey) } + numNodeAnns++ } case err := <-graphSub.errChan: t.Fatalf("unable to recv graph update: %v", err) case <-time.After(time.Second * 10): - t.Fatalf("timeout waiting for graph notification %v", i) + t.Fatalf("timeout waiting for graph notifications, "+ + "only received %d/2 chanupds and %d/2 nodeanns", + numChannelUpds, numNodeAnns) } } @@ -7799,11 +7824,12 @@ out: // We should receive an update advertising the newly connected node, // Bob's new node announcement, and the channel between Bob and Carol. - for i := 0; i < 3; i++ { + numNodeAnns = 0 + numChannelUpds = 0 + for numChannelUpds < 2 && numNodeAnns < 1 { select { case graphUpdate := <-graphSub.updateChan: - if len(graphUpdate.NodeUpdates) > 0 { - nodeUpdate := graphUpdate.NodeUpdates[0] + for _, nodeUpdate := range graphUpdate.NodeUpdates { switch nodeUpdate.IdentityKey { case carol.PubKeyStr: case net.Bob.PubKeyStr: @@ -7811,15 +7837,10 @@ out: t.Fatalf("unknown node update pubey: %v", nodeUpdate.IdentityKey) } + numNodeAnns++ } - if len(graphUpdate.ChannelUpdates) > 0 { - chanUpdate := graphUpdate.ChannelUpdates[0] - if chanUpdate.Capacity != int64(chanAmt) { - t.Fatalf("channel capacities mismatch:"+ - " expected %v, got %v", chanAmt, - btcutil.Amount(chanUpdate.Capacity)) - } + for _, chanUpdate := range graphUpdate.ChannelUpdates { switch chanUpdate.AdvertisingNode { case carol.PubKeyStr: case net.Bob.PubKeyStr: @@ -7834,11 +7855,20 @@ out: t.Fatalf("unknown connecting node: %v", chanUpdate.ConnectingNode) } + + if chanUpdate.Capacity != int64(chanAmt) { + t.Fatalf("channel capacities mismatch:"+ + " expected %v, got %v", chanAmt, + btcutil.Amount(chanUpdate.Capacity)) + } + numChannelUpds++ } case err := <-graphSub.errChan: t.Fatalf("unable to recv graph update: %v", err) case <-time.After(time.Second * 10): - t.Fatalf("timeout waiting for graph notification %v", i) + t.Fatalf("timeout waiting for graph notifications, "+ + "only received %d/2 chanupds and %d/2 nodeanns", + numChannelUpds, numNodeAnns) } } diff --git a/routing/router.go b/routing/router.go index 14c084a0d..a3c913888 100644 --- a/routing/router.go +++ b/routing/router.go @@ -72,6 +72,10 @@ type ChannelGraphSource interface { // the target node. IsStaleNode(node Vertex, timestamp time.Time) bool + // IsPublicNode determines whether the given vertex is seen as a public + // node in the graph from the graph's source node's point of view. + IsPublicNode(node Vertex) (bool, error) + // IsKnownEdge returns true if the graph source already knows of the // passed channel ID. IsKnownEdge(chanID lnwire.ShortChannelID) bool @@ -2222,6 +2226,14 @@ func (r *ChannelRouter) IsStaleNode(node Vertex, timestamp time.Time) bool { return r.assertNodeAnnFreshness(node, timestamp) != nil } +// IsPublicNode determines whether the given vertex is seen as a public node in +// the graph from the graph's source node's point of view. +// +// NOTE: This method is part of the ChannelGraphSource interface. +func (r *ChannelRouter) IsPublicNode(node Vertex) (bool, error) { + return r.cfg.Graph.IsPublicNode(node) +} + // IsKnownEdge returns true if the graph source already knows of the passed // channel ID. // diff --git a/rpcserver.go b/rpcserver.go index 5afcc6cc1..a9052af1d 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -2759,12 +2759,34 @@ func (r *rpcServer) AddInvoice(ctx context.Context, } if !link.EligibleToForward() { - rpcsLog.Debugf("Skipping link %v due to not "+ + rpcsLog.Debugf("Skipping channel %v due to not "+ "being eligible to forward payments", chanPoint) continue } + // To ensure we don't leak unadvertised nodes, we'll + // make sure our counterparty is publicly advertised + // within the network. Otherwise, we'll end up leaking + // information about nodes that intend to stay + // unadvertised, like in the case of a node only having + // private channels. + var remotePub [33]byte + copy(remotePub[:], channel.IdentityPub.SerializeCompressed()) + isRemoteNodePublic, err := graph.IsPublicNode(remotePub) + if err != nil { + rpcsLog.Errorf("Unable to determine if node %x "+ + "is advertised: %v", remotePub, err) + continue + } + + if !isRemoteNodePublic { + rpcsLog.Debugf("Skipping channel %v due to "+ + "counterparty %x being unadvertised", + chanPoint, remotePub) + continue + } + // Fetch the policies for each end of the channel. chanID := channel.ShortChanID().ToUint64() info, p1, p2, err := graph.FetchChannelEdgesByID(chanID) @@ -2778,8 +2800,7 @@ func (r *rpcServer) AddInvoice(ctx context.Context, // Now, we'll need to determine which is the correct // policy for HTLCs being sent from the remote node. var remotePolicy *channeldb.ChannelEdgePolicy - remotePub := channel.IdentityPub.SerializeCompressed() - if bytes.Equal(remotePub, info.NodeKey1Bytes[:]) { + if bytes.Equal(remotePub[:], info.NodeKey1Bytes[:]) { remotePolicy = p1 } else { remotePolicy = p2