From eb05726dacc7252ae8f1bc8689745bec0e9fc23f Mon Sep 17 00:00:00 2001
From: Mikael Lindlof <mikuz.dev@gmail.com>
Date: Sun, 14 Jun 2020 19:00:20 +0100
Subject: [PATCH] Nullable optional JSON-RPC parameters

Fix command marshalling dropping params following params with nil value.

#1591 Allow specifying null parameter value from command line.
---
 btcjson/cmdparse.go      | 16 ++++++---
 btcjson/cmdparse_test.go | 74 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/btcjson/cmdparse.go b/btcjson/cmdparse.go
index 48c6278a..117f72f1 100644
--- a/btcjson/cmdparse.go
+++ b/btcjson/cmdparse.go
@@ -16,19 +16,20 @@ import (
 func makeParams(rt reflect.Type, rv reflect.Value) []interface{} {
 	numFields := rt.NumField()
 	params := make([]interface{}, 0, numFields)
+	lastParam := -1
 	for i := 0; i < numFields; i++ {
 		rtf := rt.Field(i)
 		rvf := rv.Field(i)
+		params = append(params, rvf.Interface())
 		if rtf.Type.Kind() == reflect.Ptr {
 			if rvf.IsNil() {
-				break
+				// Omit optional null params unless a non-null param follows
+				continue
 			}
-			rvf.Elem()
 		}
-		params = append(params, rvf.Interface())
+		lastParam = i
 	}
-
-	return params
+	return params[:lastParam+1]
 }
 
 // MarshalCmd marshals the passed command to a JSON-RPC request byte slice that
@@ -255,6 +256,11 @@ func assignField(paramNum int, fieldName string, dest reflect.Value, src reflect
 		return nil
 	}
 
+	// Optional variables can be set null using "null" string
+	if destIndirects > 0 && src.String() == "null" {
+		return nil
+	}
+
 	// When the destination has more indirects than the source, the extra
 	// pointers have to be created.  Only create enough pointers to reach
 	// the same level of indirection as the source so the dest can simply be
diff --git a/btcjson/cmdparse_test.go b/btcjson/cmdparse_test.go
index dd951dcb..71547271 100644
--- a/btcjson/cmdparse_test.go
+++ b/btcjson/cmdparse_test.go
@@ -162,6 +162,18 @@ func TestAssignField(t *testing.T) {
 			src:      `{"1Address":1.5}`,
 			expected: map[string]float64{"1Address": 1.5},
 		},
+		{
+			name:     `null optional field - "null" -> *int32`,
+			dest:     btcjson.Int32(0),
+			src:      "null",
+			expected: nil,
+		},
+		{
+			name:     `null optional field - "null" -> *string`,
+			dest:     btcjson.String(""),
+			src:      "null",
+			expected: nil,
+		},
 	}
 
 	t.Logf("Running %d tests", len(tests))
@@ -175,6 +187,15 @@ func TestAssignField(t *testing.T) {
 			continue
 		}
 
+		// Check case where null string is used on optional field
+		if dst.Kind() == reflect.Ptr && test.src == "null" {
+			if !dst.IsNil() {
+				t.Errorf("Test #%d (%s) unexpected value - got %v, "+
+					"want nil", i, test.name, dst.Interface())
+			}
+			continue
+		}
+
 		// Inidirect through to the base types to ensure their values
 		// are the same.
 		for dst.Kind() == reflect.Ptr {
@@ -401,6 +422,59 @@ func TestNewCmdErrors(t *testing.T) {
 	}
 }
 
+// TestMarshalCmd tests the MarshalCmd function.
+func TestMarshalCmd(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name     string
+		id       interface{}
+		cmd      interface{}
+		expected string
+	}{
+		{
+			name:     "include all parameters",
+			id:       1,
+			cmd:      btcjson.NewGetNetworkHashPSCmd(btcjson.Int(100), btcjson.Int(2000)),
+			expected: `{"jsonrpc":"1.0","method":"getnetworkhashps","params":[100,2000],"id":1}`,
+		},
+		{
+			name:     "include padding null parameter",
+			id:       1,
+			cmd:      btcjson.NewGetNetworkHashPSCmd(nil, btcjson.Int(2000)),
+			expected: `{"jsonrpc":"1.0","method":"getnetworkhashps","params":[null,2000],"id":1}`,
+		},
+		{
+			name:     "omit single unnecessary null parameter",
+			id:       1,
+			cmd:      btcjson.NewGetNetworkHashPSCmd(btcjson.Int(100), nil),
+			expected: `{"jsonrpc":"1.0","method":"getnetworkhashps","params":[100],"id":1}`,
+		},
+		{
+			name:     "omit unnecessary null parameters",
+			id:       1,
+			cmd:      btcjson.NewGetNetworkHashPSCmd(nil, nil),
+			expected: `{"jsonrpc":"1.0","method":"getnetworkhashps","params":[],"id":1}`,
+		},
+	}
+
+	t.Logf("Running %d tests", len(tests))
+	for i, test := range tests {
+		bytes, err := btcjson.MarshalCmd(test.id, test.cmd)
+		if err != nil {
+			t.Errorf("Test #%d (%s) wrong error - got %T (%v)",
+				i, test.name, err, err)
+			continue
+		}
+		marshalled := string(bytes)
+		if marshalled != test.expected {
+			t.Errorf("Test #%d (%s) mismatched marshall result - got "+
+				"%v, want %v", i, test.name, marshalled, test.expected)
+			continue
+		}
+	}
+}
+
 // TestMarshalCmdErrors  tests the error paths of the MarshalCmd function.
 func TestMarshalCmdErrors(t *testing.T) {
 	t.Parallel()