From e7e4cdcf49617178be8b68fd53151807779fd5f9 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sat, 24 Feb 2018 19:35:58 -0800 Subject: [PATCH] discovery: avoid always validating ECDSA sigs by asking router if item is fresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In this commit, we reduce the amount of unnecessary work that the gossiper can carry out. When CPU profiling some nodes, I noticed that we’d spend a lot of time validating the signatures for an announcement, only to realize that the router already had it. To remedy this, we’ll use the new methods added to the channel router in order to avoid unnecessarily validating an announcement that is actually stale. This should reduce memory usage (since it uses big int’s under the scenes), and also idle CPU usage. --- discovery/gossiper.go | 35 ++++++++++++++++++++++++++++--- discovery/gossiper_test.go | 43 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/discovery/gossiper.go b/discovery/gossiper.go index cf629ad7..21f2a026 100644 --- a/discovery/gossiper.go +++ b/discovery/gossiper.go @@ -1299,7 +1299,17 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(nMsg *networkMsg) []n // information about a node in one of the channels we know about, or a // updating previously advertised information. case *lnwire.NodeAnnouncement: + timestamp := time.Unix(int64(msg.Timestamp), 0) + if nMsg.isRemote { + // We'll quickly ask the router if it already has a + // newer update for this node so we can skip validating + // signatures if not required. + if d.cfg.Router.IsStaleNode(msg.NodeID, timestamp) { + nMsg.err <- nil + return nil + } + if err := ValidateNodeAnn(msg); err != nil { err := errors.Errorf("unable to validate "+ "node announcement: %v", err) @@ -1312,7 +1322,7 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(nMsg *networkMsg) []n features := lnwire.NewFeatureVector(msg.Features, lnwire.GlobalFeatures) node := &channeldb.LightningNode{ HaveNodeAnnouncement: true, - LastUpdate: time.Unix(int64(msg.Timestamp), 0), + LastUpdate: timestamp, Addresses: msg.Addresses, PubKeyBytes: msg.NodeID, Alias: msg.Alias.String(), @@ -1382,6 +1392,13 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(nMsg *networkMsg) []n return nil } + // At this point, we'll now ask the router if this is a stale + // update. If so we can skip all the processing below. + if d.cfg.Router.IsKnownEdge(msg.ShortChannelID) { + nMsg.err <- nil + return nil + } + // If this is a remote channel announcement, then we'll validate // all the signatures within the proof as it should be well // formed. @@ -1445,7 +1462,7 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(nMsg *networkMsg) []n if err := d.cfg.Router.AddEdge(edge); err != nil { // If the edge was rejected due to already being known, // then it may be that case that this new message has a - // fresh channel proof, so we'll cechk. + // fresh channel proof, so we'll check. if routing.IsError(err, routing.ErrOutdated, routing.ErrIgnored) { @@ -1585,6 +1602,18 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(nMsg *networkMsg) []n return nil } + // Before we perform any of the expensive checks below, we'll + // make sure that the router doesn't already have a fresher + // announcement for this edge. + timestamp := time.Unix(int64(msg.Timestamp), 0) + if d.cfg.Router.IsStaleEdgePolicy( + msg.ShortChannelID, timestamp, msg.Flags, + ) { + + nMsg.err <- nil + return nil + } + // Get the node pub key as far as we don't have it in channel // update announcement message. We'll need this to properly // verify message signature. @@ -1668,7 +1697,7 @@ func (d *AuthenticatedGossiper) processNetworkAnnouncement(nMsg *networkMsg) []n update := &channeldb.ChannelEdgePolicy{ SigBytes: msg.Signature.ToSignatureBytes(), ChannelID: shortChanID, - LastUpdate: time.Unix(int64(msg.Timestamp), 0), + LastUpdate: timestamp, Flags: msg.Flags, TimeLockDelta: msg.TimeLockDelta, MinHTLC: msg.HtlcMinimumMsat, diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index 4a8502d4..18c69da5 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -199,6 +199,49 @@ func (r *mockGraphSource) GetChannelByID(chanID lnwire.ShortChannelID) ( return chanInfo, edges[0], edges[1], nil } +// IsStaleNode returns true if the graph source has a node announcement for the +// target node with a more recent timestamp. +func (r *mockGraphSource) IsStaleNode(nodePub routing.Vertex, timestamp time.Time) bool { + for _, node := range r.nodes { + if node.PubKeyBytes == nodePub { + return node.LastUpdate.After(timestamp) || + node.LastUpdate.Equal(timestamp) + } + } + + return false +} + +// IsKnownEdge returns true if the graph source already knows of the passed +// channel ID. +func (r *mockGraphSource) IsKnownEdge(chanID lnwire.ShortChannelID) bool { + _, ok := r.infos[chanID.ToUint64()] + return ok +} + +// IsStaleEdgePolicy returns true if the graph source has a channel edge for +// the passed channel ID (and flags) that have a more recent timestamp. +func (r *mockGraphSource) IsStaleEdgePolicy(chanID lnwire.ShortChannelID, + timestamp time.Time, flags lnwire.ChanUpdateFlag) bool { + + edges, ok := r.edges[chanID.ToUint64()] + if !ok { + return false + } + + switch { + + case len(edges) >= 1 && edges[0].Flags == flags: + return !edges[0].LastUpdate.Before(timestamp) + + case len(edges) >= 2 && edges[1].Flags == flags: + return !edges[1].LastUpdate.Before(timestamp) + + default: + return false + } +} + type mockNotifier struct { clientCounter uint32 epochClients map[uint32]chan *chainntnfs.BlockEpoch