From 0731f2ddc979708df658492bd947addaa79e114c Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Fri, 21 Oct 2016 10:03:14 -0500
Subject: [PATCH] txscript: Cleanup and improve NullDataScript tests.

This modifies the recently-added NullDataScript function in several
ways in an effort to make them more consistent with the tests in the
rest of the code base and improve/correct the logic:

- Use the hexToBytes and mustParseShortForm functions
- Consistently format the test errors
- Replace the valid bool flag with an expected error and test against it
- Ensure the returned script type is the expected type in all cases
---
 txscript/standard.go      |   4 +-
 txscript/standard_test.go | 105 +++++++++++++++++++-------------------
 2 files changed, 54 insertions(+), 55 deletions(-)

diff --git a/txscript/standard.go b/txscript/standard.go
index 69a869b07..7980b13c3 100644
--- a/txscript/standard.go
+++ b/txscript/standard.go
@@ -351,8 +351,8 @@ func PayToAddrScript(addr btcutil.Address) ([]byte, error) {
 	return nil, ErrUnsupportedAddress
 }
 
-// NullDataScript creates a provably prunable script
-// containing OP_RETURN followed by the passed data.
+// NullDataScript creates a provably-prunable script containing OP_RETURN
+// followed by the passed data.
 func NullDataScript(data []byte) ([]byte, error) {
 	if len(data) > MaxDataCarrierSize {
 		return nil, ErrStackLongScript
diff --git a/txscript/standard_test.go b/txscript/standard_test.go
index 7a37c5dfd..226af1725 100644
--- a/txscript/standard_test.go
+++ b/txscript/standard_test.go
@@ -1031,86 +1031,85 @@ func TestStringifyClass(t *testing.T) {
 
 // TestNullDataScript tests whether NullDataScript returns a valid script.
 func TestNullDataScript(t *testing.T) {
-	tiny := hexToBytes("01")
-	short := hexToBytes("0102030405060708090a0b0c0d0e0f1" +
-		"01112131415161718")
-	shortExp := hexToBytes("6a180102030405060708090a0b0c0d0e0f1" +
-		"01112131415161718")
-	justRight := hexToBytes("000102030405060708090a0b0c0d0e0f" +
-		"101112131415161718191a1b1c1d1e1f20212223242526272829" +
-		"2a2b2c2d2e2f303132333435363738393a3b3c3d3e3f40414243" +
-		"4445464748494a4b4c4d4e4f")
-	tooLong := hexToBytes("000102030405060708090a0b0c0d0e0f" +
-		"101112131415161718191a1b1c1d1e1f20212223242526272829" +
-		"2a2b2c2d2e2f303132333435363738393a3b3c3d3e3f40414243" +
-		"4445464748494a4b4c4d4e4f50")
-
 	tests := []struct {
 		name     string
 		data     []byte
-		valid    bool
 		expected []byte
+		err      error
+		class    ScriptClass
 	}{
 		{
-			name:     "small data",
-			data:     tiny,
-			valid:    true,
-			expected: []byte{0x6a, 0x51},
+			name:     "small int",
+			data:     hexToBytes("01"),
+			expected: mustParseShortForm("RETURN 1"),
+			err:      nil,
+			class:    NullDataTy,
 		},
 		{
-			name:     "data of size before OP_PUSHDATA1 is needed",
-			data:     short,
-			valid:    true,
-			expected: shortExp,
+			name:     "max small int",
+			data:     hexToBytes("10"),
+			expected: mustParseShortForm("RETURN 16"),
+			err:      nil,
+			class:    NullDataTy,
 		},
 		{
-			name:  "just right",
-			data:  justRight,
-			valid: true,
-			expected: append([]byte{
-				OP_RETURN, OP_PUSHDATA1, 80},
-				justRight...),
+			name: "data of size before OP_PUSHDATA1 is needed",
+			data: hexToBytes("0102030405060708090a0b0c0d0e0f10111" +
+				"2131415161718"),
+			expected: mustParseShortForm("RETURN 0x18 0x01020304" +
+				"05060708090a0b0c0d0e0f101112131415161718"),
+			err:   nil,
+			class: NullDataTy,
 		},
 		{
-			name:     "too big",
-			data:     tooLong,
-			valid:    false,
+			name: "just right",
+			data: hexToBytes("000102030405060708090a0b0c0d0e0f101" +
+				"112131415161718191a1b1c1d1e1f202122232425262" +
+				"728292a2b2c2d2e2f303132333435363738393a3b3c3" +
+				"d3e3f404142434445464748494a4b4c4d4e4f"),
+			expected: mustParseShortForm("RETURN PUSHDATA1 0x50 " +
+				"0x000102030405060708090a0b0c0d0e0f101112131" +
+				"415161718191a1b1c1d1e1f20212223242526272829" +
+				"2a2b2c2d2e2f303132333435363738393a3b3c3d3e3" +
+				"f404142434445464748494a4b4c4d4e4f"),
+			err:   nil,
+			class: NullDataTy,
+		},
+		{
+			name: "too big",
+			data: hexToBytes("000102030405060708090a0b0c0d0e0f101" +
+				"112131415161718191a1b1c1d1e1f202122232425262" +
+				"728292a2b2c2d2e2f303132333435363738393a3b3c3" +
+				"d3e3f404142434445464748494a4b4c4d4e4f50"),
 			expected: nil,
+			err:      ErrStackLongScript,
+			class:    NonStandardTy,
 		},
 	}
 
-	for _, test := range tests {
+	for i, test := range tests {
 		script, err := NullDataScript(test.data)
-
-		if !test.valid {
-			if err == nil {
-				t.Errorf("NullDataScript test %v: expected error but none was returned. ",
-					test.name)
-			}
-
-			continue
-		}
-
-		if err != nil {
-			t.Errorf("NullDataScript test %v: Unexpected error returned %v ",
-				test.name, err)
-
+		if err != test.err {
+			t.Errorf("NullDataScript: #%d (%s) unexpected error: "+
+				"got %v, want %v", i, test.name, err, test.err)
 			continue
 		}
 
 		// Check that the expected result was returned.
 		if !bytes.Equal(script, test.expected) {
-			t.Errorf("NullDataScript test %v: Expected %x, got %x",
-				test.name, test.expected, script)
-
+			t.Errorf("NullDataScript: #%d (%s) wrong result\n"+
+				"got: %x\nwant: %x", i, test.name, script,
+				test.expected)
 			continue
 		}
 
 		// Check that the script has the correct type.
 		scriptType := GetScriptClass(script)
-		if scriptType != NullDataTy {
-			t.Errorf("NullDataScript test %v: Expected NullDataTy, got %v",
-				test.name, scriptType)
+		if scriptType != test.class {
+			t.Errorf("GetScriptClass: #%d (%s) wrong result -- "+
+				"got: %v, want: %v", i, test.name, scriptType,
+				test.class)
+			continue
 		}
 	}
 }