From f726fc2bc8410960a6e1585cf2de3eab0633f5d2 Mon Sep 17 00:00:00 2001
From: carla <kirkcohenc@gmail.com>
Date: Wed, 15 Apr 2020 09:10:21 +0200
Subject: [PATCH] loopd: add missing zero case to validateConfTarget

Update validate function to return default target for zero value
confirmation targets, as indicated by the comment. This change
introduces a behaviour change for direct rpc calls to loopd (ie,
those not made by the loop cli tool). Previously, these calls
would fail and indicate that the conf target must be > 2, now
they will succeed with the default conf target. The loop cli
tool is unaffected because we already set the default value.

When clients upgrade from a previous version of loopd which did
not have this check, preexisting loops will be unaffected, because
loop ins had the default of 6 confirmations set, and loop outs with
<2 conf target would not have been created.
---
 loopd/swapclient_server.go      |  3 ++
 loopd/swapclient_server_test.go | 72 +++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 loopd/swapclient_server_test.go

diff --git a/loopd/swapclient_server.go b/loopd/swapclient_server.go
index b03fafd..377c263 100644
--- a/loopd/swapclient_server.go
+++ b/loopd/swapclient_server.go
@@ -477,6 +477,9 @@ func (s *swapClientServer) processStatusUpdates(mainCtx context.Context) {
 // isn't specified (0 value), then the default target is used.
 func validateConfTarget(target, defaultTarget int32) (int32, error) {
 	switch {
+	case target == 0:
+		return defaultTarget, nil
+
 	// Ensure the target respects our minimum threshold.
 	case target < minConfTarget:
 		return 0, fmt.Errorf("a confirmation target of at least %v "+
diff --git a/loopd/swapclient_server_test.go b/loopd/swapclient_server_test.go
new file mode 100644
index 0000000..0494de4
--- /dev/null
+++ b/loopd/swapclient_server_test.go
@@ -0,0 +1,72 @@
+package loopd
+
+import "testing"
+
+// TestValidateConfTarget tests all failure and success cases for our conf
+// target validation function, including the case where we replace a zero
+// target with the default provided.
+func TestValidateConfTarget(t *testing.T) {
+	const (
+		// Various input confirmation values for tests.
+		zeroConf int32 = 0
+		oneConf  int32 = 1
+		twoConf  int32 = 2
+		fiveConf int32 = 5
+
+		// defaultConf is the default confirmation target we use for
+		// all tests.
+		defaultConf = 6
+	)
+
+	tests := []struct {
+		name           string
+		confTarget     int32
+		expectedTarget int32
+		expectErr      bool
+	}{
+		{
+			name:           "zero conf, get default",
+			confTarget:     zeroConf,
+			expectedTarget: defaultConf,
+			expectErr:      false,
+		},
+		{
+			name:       "one conf, get error",
+			confTarget: oneConf,
+			expectErr:  true,
+		},
+		{
+			name:           "two conf, ok",
+			confTarget:     twoConf,
+			expectedTarget: twoConf,
+			expectErr:      false,
+		},
+		{
+			name:           "five conf, ok",
+			confTarget:     fiveConf,
+			expectedTarget: fiveConf,
+			expectErr:      false,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+
+		t.Run(test.name, func(t *testing.T) {
+			target, err := validateConfTarget(
+				test.confTarget, defaultConf,
+			)
+
+			haveErr := err != nil
+			if haveErr != test.expectErr {
+				t.Fatalf("expected err: %v, got: %v",
+					test.expectErr, err)
+			}
+
+			if target != test.expectedTarget {
+				t.Fatalf("expected: %v, got: %v",
+					test.expectedTarget, target)
+			}
+		})
+	}
+}