From 486b464e1c00278f92df0e6ff132ff94db4dc4d9 Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Mon, 2 Oct 2017 21:58:34 -0700
Subject: [PATCH] routing: move path caching into FindRoutes

This commit modifies the way we do path caching. Rather than only
caching within SendPayment, we now cache routes within FindRoutes. This
is more natural as SendPayment eventually calls FindRoute. As a result
of this commit, queries to FindRoute are now properly cached, speeding
up applications which are focused on graph visualization or querying
rather than sending payments.
---
 routing/router.go | 63 ++++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/routing/router.go b/routing/router.go
index 6c0838fc6..dc23fd7ff 100644
--- a/routing/router.go
+++ b/routing/router.go
@@ -121,11 +121,11 @@ type routeTuple struct {
 }
 
 // newRouteTuple creates a new route tuple from the target and amount.
-func newRouteTuple(amt lnwire.MilliSatoshi, dest *btcec.PublicKey) routeTuple {
+func newRouteTuple(amt lnwire.MilliSatoshi, dest []byte) routeTuple {
 	r := routeTuple{
 		amt: amt,
 	}
-	copy(r.dest[:], dest.SerializeCompressed())
+	copy(r.dest[:], dest)
 
 	return r
 }
@@ -820,6 +820,25 @@ func (r *ChannelRouter) FindRoutes(target *btcec.PublicKey,
 	dest := target.SerializeCompressed()
 	log.Debugf("Searching for path to %x, sending %v", dest, amt)
 
+	// Before attempting to perform a series of graph traversals to find
+	// the k-shortest paths to the destination, we'll first consult our
+	// path cache
+	rt := newRouteTuple(amt, dest)
+	r.routeCacheMtx.RLock()
+	routes, ok := r.routeCache[rt]
+	r.routeCacheMtx.RUnlock()
+
+	// If we already have a cached route, then we'll return it directly as
+	// there's no need to repeat the computation.
+	if ok {
+		return routes, nil
+	}
+
+	// If we don't have a set of routes cached, we'll query the graph for a
+	// set of potential routes to the destination node that can support our
+	// payment amount. If no such routes can be found then an error will be
+	// returned.
+
 	// We can short circuit the routing by opportunistically checking to
 	// see if the target vertex event exists in the current graph.
 	if _, exists, err := r.cfg.Graph.HasLightningNode(target); err != nil {
@@ -882,6 +901,12 @@ func (r *ChannelRouter) FindRoutes(target *btcec.PublicKey,
 		}),
 	)
 
+	// Populate the cache with this set of fresh routes so we can
+	// reuse them in the future.
+	r.routeCacheMtx.Lock()
+	r.routeCache[rt] = validRoutes
+	r.routeCacheMtx.Unlock()
+
 	return validRoutes, nil
 }
 
@@ -986,35 +1011,11 @@ func (r *ChannelRouter) SendPayment(payment *LightningPayment) ([32]byte, *Route
 		preImage  [32]byte
 	)
 
-	// TODO(roasbeef): consult KSP cache before dispatching
-
-	// Before attempting to perform a series of graph traversals to find
-	// the k-shortest paths to the destination, we'll first consult our
-	// path cache
-	rt := newRouteTuple(payment.Amount, payment.Target)
-
-	r.routeCacheMtx.RLock()
-	routes, ok := r.routeCache[rt]
-	r.routeCacheMtx.RUnlock()
-
-	// If we don't have a set of routes cached, we'll query the graph for a
-	// set of potential routes to the destination node that can support our
-	// payment amount. If no such routes can be found then an error will be
-	// returned.
-	if !ok {
-		// TODO(roasbeef): put cache handling into FindRoutes
-		freshRoutes, err := r.FindRoutes(payment.Target, payment.Amount)
-		if err != nil {
-			return preImage, nil, err
-		}
-
-		// Populate the cache with this set of fresh routes so we can
-		// reuse them in the future.
-		r.routeCacheMtx.Lock()
-		r.routeCache[rt] = freshRoutes
-		r.routeCacheMtx.Unlock()
-
-		routes = freshRoutes
+	// First, we'll kick off the payment attempt by attempting to query the
+	// known channel graph for a route to the destination.
+	routes, err := r.FindRoutes(payment.Target, payment.Amount)
+	if err != nil {
+		return preImage, nil, err
 	}
 
 	// For each eligible path, we'll attempt to successfully send our