From 76196db70ef32e3fdf2e1df42eef2e8fb06abdd6 Mon Sep 17 00:00:00 2001 From: positiveblue Date: Thu, 20 Jan 2022 15:04:04 -0800 Subject: [PATCH] lnrpc/peers: handle net address changes in updateNodeAnnouncement --- lnrpc/peersrpc/peers_server.go | 96 +++++++++++++++++++++++++- lntest/itest/assertions.go | 18 +++++ lntest/itest/lnd_channel_graph_test.go | 71 ++++++++++++++++--- netann/node_announcement.go | 4 +- 4 files changed, 178 insertions(+), 11 deletions(-) diff --git a/lnrpc/peersrpc/peers_server.go b/lnrpc/peersrpc/peers_server.go index aae96bc1b..16cc20af6 100644 --- a/lnrpc/peersrpc/peers_server.go +++ b/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 " + diff --git a/lntest/itest/assertions.go b/lntest/itest/assertions.go index 01fe6b211..dac88010d 100644 --- a/lntest/itest/assertions.go +++ b/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 diff --git a/lntest/itest/lnd_channel_graph_test.go b/lntest/itest/lnd_channel_graph_test.go index 7c19883e0..212ae00f1 100644 --- a/lntest/itest/lnd_channel_graph_test.go +++ b/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 diff --git a/netann/node_announcement.go b/netann/node_announcement.go index 48dc8411e..e0fbb9df3 100644 --- a/netann/node_announcement.go +++ b/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