From 7cabe2667ff55d0d990c2d1ba2822a81e4d93af5 Mon Sep 17 00:00:00 2001
From: Conner Fromknecht <conner@lightning.engineering>
Date: Thu, 13 Jun 2019 17:33:24 -0700
Subject: [PATCH] watchtower/wtclient/client: use existing session with same
 TxPolicy

This commit modifies the client's filtering when selecting from existing
sessions. The new logic compares the configured TxPolicy with the
TxPolicy of the candidate sessions, which has the effect of ignoring
operational parameters such as MaxUpdates. Prior, changing MaxUpdates
would cause the client to request a new session even if it had perfectly
good slots available in a policy with an equal TxPolicy.
---
 watchtower/wtclient/client.go      |  7 ++--
 watchtower/wtclient/client_test.go | 61 ++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/watchtower/wtclient/client.go b/watchtower/wtclient/client.go
index 7b1653b9e..09bb54846 100644
--- a/watchtower/wtclient/client.go
+++ b/watchtower/wtclient/client.go
@@ -519,9 +519,10 @@ func (c *TowerClient) nextSessionQueue() *sessionQueue {
 		delete(c.candidateSessions, id)
 
 		// Skip any sessions with policies that don't match the current
-		// configuration. These can be used again if the client changes
-		// their configuration back.
-		if sessionInfo.Policy != c.cfg.Policy {
+		// TxPolicy, as they would result in different justice
+		// transactions from what is requested. These can be used again
+		// if the client changes their configuration and restarting.
+		if sessionInfo.Policy.TxPolicy != c.cfg.Policy.TxPolicy {
 			continue
 		}
 
diff --git a/watchtower/wtclient/client_test.go b/watchtower/wtclient/client_test.go
index c7c589771..499c4610a 100644
--- a/watchtower/wtclient/client_test.go
+++ b/watchtower/wtclient/client_test.go
@@ -1266,6 +1266,67 @@ var clientTests = []clientTest{
 			h.assertUpdatesForPolicy(hints, h.clientCfg.Policy)
 		},
 	},
+	{
+		// Asserts that the client will not request a new session if
+		// already has an existing session with the same TxPolicy. This
+		// permits the client to continue using policies that differ in
+		// operational parameters, but don't manifest in different
+		// justice transactions.
+		name: "create session change policy same txpolicy",
+		cfg: harnessCfg{
+			localBalance:  localBalance,
+			remoteBalance: remoteBalance,
+			policy: wtpolicy.Policy{
+				TxPolicy: wtpolicy.TxPolicy{
+					BlobType:     blob.TypeDefault,
+					SweepFeeRate: 1,
+				},
+				MaxUpdates: 10,
+			},
+		},
+		fn: func(h *testHarness) {
+			const (
+				chanID     = 0
+				numUpdates = 6
+			)
+
+			// Generate the retributions that will be backed up.
+			hints := h.advanceChannelN(chanID, numUpdates)
+
+			// Now, queue the first half of the retributions.
+			h.backupStates(chanID, 0, numUpdates/2, nil)
+
+			// Wait for the server to collect the first half.
+			h.waitServerUpdates(hints[:numUpdates/2], time.Second)
+
+			// Stop the client, which should have no more backups.
+			h.client.Stop()
+
+			// Record the policy that the first half was stored
+			// under. We'll expect the second half to also be stored
+			// under the original policy, since we are only adjusting
+			// the MaxUpdates. The client should detect that the
+			// two policies have equivalent TxPolicies and continue
+			// using the first.
+			expPolicy := h.clientCfg.Policy
+
+			// Restart the client with a new policy.
+			h.clientCfg.Policy.MaxUpdates = 20
+			h.startClient()
+			defer h.client.ForceQuit()
+
+			// Now, queue the second half of the retributions.
+			h.backupStates(chanID, numUpdates/2, numUpdates, nil)
+
+			// Wait for all of the updates to be populated in the
+			// server's database.
+			h.waitServerUpdates(hints, 5*time.Second)
+
+			// Assert that the server has updates for the client's
+			// original policy.
+			h.assertUpdatesForPolicy(hints, expPolicy)
+		},
+	},
 	{
 		// Asserts that the client will deduplicate backups presented by
 		// a channel both in memory and after a restart. The client