Merge pull request #1981 from wpaulino/routing-hints-unadvertised-nodes

rpc+server: ensure we don't leak unadvertised nodes within invoice routing hints

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

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 {

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.

discovery/gossiper.go

@@ -1613,13 +1613,26 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(
 			return nil
 		}
 
-		// Node announcement was successfully proceeded and know it
+		// In order to ensure we don't leak unadvertised nodes, we'll
-		// might be broadcast to other connected nodes.
+		// make a quick check to ensure this node intends to publicly
-		announcements = append(announcements, networkMsg{
+		// advertise itself to the network.
-			peer:   nMsg.peer,
+		isPublic, err := d.cfg.Router.IsPublicNode(node.PubKeyBytes)
-			source: nMsg.source,
+		if err != nil {
-			msg:    msg,
+			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

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,
+func createAnnouncementWithoutProof(blockHeight uint32,
-	extraBytes ...[]byte) (*lnwire.ChannelAnnouncement, error) {
+	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 {
+	// First, we'll craft a valid remote channel announcement and send it to
-	case err = <-ctx.gossiper.ProcessRemoteAnnouncement(na, nodePeer):
+	// the gossiper so that it can be processed.
-	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.
 	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
+	// We'll then craft the channel policy of the remote party and also send
-	// side, and check that we broadcasted it to the other network, and
+	// it to the gossiper.
-	// added updates to the router.
 	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

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,
+	// If this channel is not meant to be announced to the greater network,
-	// wait until the funding tx has reached 6 confirmations before
+	// we'll only send our NodeAnnouncement to our counterparty to ensure we
-	// announcing it.
+	// 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
+		// Otherwise, we'll wait until the funding transaction has
-		// funding transaction reaches at least 6 confirmations.
+		// 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 "+

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
+	// channel announcement messages.
-	// node announcement.
+	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)

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
+	var numChannelUpds int
-	for i := 0; i < numExpectedUpdates; i++ {
+	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:
-
+			// Process all channel updates prsented in this update
-			if len(graphUpdate.ChannelUpdates) > 0 {
+			// message.
-				chanUpdate := graphUpdate.ChannelUpdates[0]
+			for _, chanUpdate := range graphUpdate.ChannelUpdates {
-				if chanUpdate.Capacity != int64(chanAmt) {
-					t.Fatalf("channel capacities mismatch:"+
-						" expected %v, got %v", chanAmt,
-						btcutil.Amount(chanUpdate.Capacity))
-				}
 				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 {
+			for _, nodeUpdate := range graphUpdate.NodeUpdates {
-				nodeUpdate := graphUpdate.NodeUpdates[0]
 				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 {
+			for _, nodeUpdate := range graphUpdate.NodeUpdates {
-				nodeUpdate := graphUpdate.NodeUpdates[0]
 				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 {
+			for _, chanUpdate := range graphUpdate.ChannelUpdates {
-				chanUpdate := graphUpdate.ChannelUpdates[0]
-				if chanUpdate.Capacity != int64(chanAmt) {
-					t.Fatalf("channel capacities mismatch:"+
-						" expected %v, got %v", chanAmt,
-						btcutil.Amount(chanUpdate.Capacity))
-				}
 				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)
 		}
 	}
 

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.
 //

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