lnrpc/peers: handle net address changes in updateNodeAnnouncement

lnrpc/peersrpc/peers_server.go

@@ -6,6 +6,7 @@ package peersrpc
 import (
 	"context"
 	"fmt"
+	"net"
 	"sync/atomic"
 
 	"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
@@ -161,6 +162,85 @@ func (r *ServerShell) CreateSubServer(configRegistry lnrpc.SubServerConfigDispat
 	return subServer, macPermissions, nil
 }
 
+// updateAddresses computes the new address set after executing the update
+// actions.
+func (s *Server) updateAddresses(currentAddresses []net.Addr,
+	updates []*UpdateAddressAction) ([]net.Addr, *lnrpc.Op, error) {
+
+	// net.Addr is not comparable so we cannot use the default map
+	// (map[net.Addr]struct{}) so we have to use arrays and a helping
+	// function.
+	findAddr := func(addr net.Addr, slice []net.Addr) bool {
+		for _, sAddr := range slice {
+			if sAddr.Network() != addr.Network() {
+				continue
+			}
+
+			if sAddr.String() == addr.String() {
+				return true
+			}
+		}
+		return false
+	}
+
+	// Preallocate enough memory for both arrays.
+	removeAddr := make([]net.Addr, 0, len(updates))
+	addAddr := make([]net.Addr, 0, len(updates))
+	for _, update := range updates {
+		addr, err := s.cfg.ParseAddr(update.Address)
+		if err != nil {
+			return nil, nil, fmt.Errorf("unable to resolve "+
+				"address %v: %v", update.Address, err)
+		}
+
+		switch update.Action {
+		case UpdateAction_ADD:
+			addAddr = append(addAddr, addr)
+		case UpdateAction_REMOVE:
+			removeAddr = append(removeAddr, addr)
+		default:
+			return nil, nil, fmt.Errorf("invalid address update "+
+				"action: %v", update.Action)
+		}
+	}
+
+	// Look for any inconsistency trying to add AND remove the same address.
+	for _, addr := range removeAddr {
+		if findAddr(addr, addAddr) {
+			return nil, nil, fmt.Errorf("invalid updates for "+
+				"removing AND adding %v", addr)
+		}
+	}
+
+	ops := &lnrpc.Op{Entity: "addresses"}
+	newAddrs := make([]net.Addr, 0, len(updates)+len(currentAddresses))
+
+	// Copy current addresses excluding the ones that need to be removed.
+	for _, addr := range currentAddresses {
+		if findAddr(addr, removeAddr) {
+			ops.Actions = append(
+				ops.Actions,
+				fmt.Sprintf("%s removed", addr.String()),
+			)
+			continue
+		}
+		newAddrs = append(newAddrs, addr)
+	}
+
+	// Add new adresses if needed.
+	for _, addr := range addAddr {
+		if !findAddr(addr, newAddrs) {
+			ops.Actions = append(
+				ops.Actions,
+				fmt.Sprintf("%s added", addr.String()),
+			)
+			newAddrs = append(newAddrs, addr)
+		}
+	}
+
+	return newAddrs, ops, nil
+}
+
 // UpdateNodeAnnouncement allows the caller to update the node parameters
 // and broadcasts a new version of the node announcement to its peers.
 func (s *Server) UpdateNodeAnnouncement(_ context.Context,
@@ -217,7 +297,21 @@ func (s *Server) UpdateNodeAnnouncement(_ context.Context,
 		}
 	}
 
-	// TODO(positiveblue): apply addresses modifications
+	if len(req.AddressUpdates) > 0 {
+		newAddrs, ops, err := s.updateAddresses(
+			currentNodeAnn.Addresses,
+			req.AddressUpdates,
+		)
+		if err != nil {
+			return nil, fmt.Errorf("error trying to update node "+
+				"addresses: %w", err)
+		}
+		resp.Ops = append(resp.Ops, ops)
+		nodeModifiers = append(
+			nodeModifiers,
+			netann.NodeAnnSetAddrs(newAddrs),
+		)
+	}
 
 	if len(nodeModifiers) == 0 {
 		return nil, fmt.Errorf("unable detect any new values to " +

lntest/itest/assertions.go

@@ -1865,6 +1865,24 @@ func assertNodeAnnouncement(t *harnessTest, n1, n2 *lnrpc.NodeUpdate) {
 
 	// Color should match.
 	require.Equal(t.t, n1.Color, n2.Color, "color don't match")
+
+	// NodeAddresses  should match.
+	require.Equal(
+		t.t, len(n1.NodeAddresses), len(n2.NodeAddresses),
+		"node addresses don't match",
+	)
+
+	addrs := make(map[string]struct{}, len(n1.NodeAddresses))
+	for _, nodeAddr := range n1.NodeAddresses {
+		addrs[nodeAddr.Addr] = struct{}{}
+	}
+
+	for _, nodeAddr := range n2.NodeAddresses {
+		if _, ok := addrs[nodeAddr.Addr]; !ok {
+			t.Fatalf("address %v not found in node announcement",
+				nodeAddr.Addr)
+		}
+	}
 }
 
 // assertUpdateNodeAnnouncementResponse is a helper function to assert

lntest/itest/lnd_channel_graph_test.go

@@ -810,6 +810,17 @@ func testUpdateNodeAnnouncement(net *lntest.NetworkHarness, t *harnessTest) {
 	defer close(aliceSub.quit)
 
 	var lndArgs []string
+
+	// Add some exta addresses to the default ones.
+	extraAddrs := []string{
+		"192.168.1.1:8333",
+		"[2001:db8:85a3:8d3:1319:8a2e:370:7348]:8337",
+		"bkb6azqggsaiskzi.onion:9735",
+		"fomvuglh6h6vcag73xo5t5gv56ombih3zr2xvplkpbfd7wrog4swjwid.onion:1234",
+	}
+	for _, addr := range extraAddrs {
+		lndArgs = append(lndArgs, "--externalip="+addr)
+	}
 	dave := net.NewNode(t.t, "Dave", lndArgs)
 	defer shutdownAndAssert(net, t, dave)
 
@@ -819,9 +830,21 @@ func testUpdateNodeAnnouncement(net *lntest.NetworkHarness, t *harnessTest) {
 	resp, err := dave.GetInfo(ctxt, nodeInfoReq)
 	require.NoError(t.t, err, "unable to get dave's information")
 
+	defaultAddrs := make([]*lnrpc.NodeAddress, 0, len(resp.Uris))
+	for _, uri := range resp.GetUris() {
+		values := strings.Split(uri, "@")
+		defaultAddrs = append(
+			defaultAddrs, &lnrpc.NodeAddress{
+				Addr:    values[1],
+				Network: "tcp",
+			},
+		)
+	}
+
 	defaultDaveNodeAnn := &lnrpc.NodeUpdate{
-		Alias: resp.Alias,
-		Color: resp.Color,
+		Alias:         resp.Alias,
+		Color:         resp.Color,
+		NodeAddresses: defaultAddrs,
 	}
 
 	// Dave must have an open channel before he can send a node
@@ -869,25 +892,57 @@ func testUpdateNodeAnnouncement(net *lntest.NetworkHarness, t *harnessTest) {
 	newAlias := "new-alias"
 	newColor := "#2288ee"
 
+	newAddresses := []string{
+		"192.168.1.10:8333",
+		"192.168.1.11:8333",
+	}
+
+	updateAddressActions := []*peersrpc.UpdateAddressAction{
+		{
+			Action:  peersrpc.UpdateAction_ADD,
+			Address: newAddresses[0],
+		},
+		{
+			Action:  peersrpc.UpdateAction_ADD,
+			Address: newAddresses[1],
+		},
+		{
+			Action:  peersrpc.UpdateAction_REMOVE,
+			Address: defaultAddrs[0].Addr,
+		},
+	}
+
 	nodeAnnReq := &peersrpc.NodeAnnouncementUpdateRequest{
-		Alias: newAlias,
-		Color: newColor,
+		Alias:          newAlias,
+		Color:          newColor,
+		AddressUpdates: updateAddressActions,
 	}
 
 	response, err := dave.UpdateNodeAnnouncement(ctxt, nodeAnnReq)
 	require.NoError(t.t, err, "unable to update dave's node announcement")
 
 	expectedOps := map[string]int{
-		"color": 1,
-		"alias": 1,
+		"color":     1,
+		"alias":     1,
+		"addresses": 3,
 	}
 	assertUpdateNodeAnnouncementResponse(t, response, expectedOps)
 
+	newNodeAddresses := []*lnrpc.NodeAddress{}
+	// We removed the first address.
+	newNodeAddresses = append(newNodeAddresses, defaultAddrs[1:]...)
+	newNodeAddresses = append(
+		newNodeAddresses,
+		&lnrpc.NodeAddress{Addr: newAddresses[0], Network: "tcp"},
+		&lnrpc.NodeAddress{Addr: newAddresses[1], Network: "tcp"},
+	)
+
 	// After updating the node we expect the update to contain
 	// the requested color, requested alias and the new added addresses.
 	newDaveNodeAnn := &lnrpc.NodeUpdate{
-		Alias: newAlias,
-		Color: newColor,
+		Alias:         newAlias,
+		Color:         newColor,
+		NodeAddresses: newNodeAddresses,
 	}
 
 	// We'll then wait for Alice to receive dave's node announcement

netann/node_announcement.go

@@ -15,7 +15,7 @@ import (
 type NodeAnnModifier func(*lnwire.NodeAnnouncement)
 
 // NodeAnnSetAlias is a functional option that sets the alias of the
-// given node announcment.
+// given node announcement.
 func NodeAnnSetAlias(alias lnwire.NodeAlias) func(*lnwire.NodeAnnouncement) {
 	return func(nodeAnn *lnwire.NodeAnnouncement) {
 		nodeAnn.Alias = alias
@@ -31,7 +31,7 @@ func NodeAnnSetAddrs(addrs []net.Addr) func(*lnwire.NodeAnnouncement) {
 }
 
 // NodeAnnSetColor is a functional option that sets the color of the
-// given node announcment.
+// given node announcement.
 func NodeAnnSetColor(newColor color.RGBA) func(*lnwire.NodeAnnouncement) {
 	return func(nodeAnn *lnwire.NodeAnnouncement) {
 		nodeAnn.RGBColor = newColor