[DEV-345] Enforce requirement for transactions from 1(DAGCoin) or 2(sub network registry) sub network to have gas = 0 (#157)

* [DEV-345] Validate that gas and payload are 0 when required by sub-network

* [DEV-345] Remove check for txOut.Value < 0, since txOut.Value is a uint64

* [DEV-345] Added tests for CheckTransactionSanity

* [DEV-345] Remove checks for Gas and Payload validity in wire.MsgTx.Decode

* [DEV-345] Verify that payload in Gas sub-network is always 8 bytes (uint64).

* [DEV-345] Renamed tstCheck{Script/Rule}Error to check{Script/Rule}Error

* [DEV-345] Improved formatting

blockdag/common_test.go

@@ -7,9 +7,11 @@ package blockdag
 import (
 	"compress/bzip2"
 	"encoding/binary"
+	"fmt"
 	"io"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"time"
 
@@ -220,3 +222,35 @@ func buildNodeGenerator(phantomK uint32, withChildren bool) func(parents blockSe
 	}
 	return buildNode
 }
+
+// checkRuleError ensures the type of the two passed errors are of the
+// same type (either both nil or both of type RuleError) and their error codes
+// match when not nil.
+func checkRuleError(gotErr, wantErr error) error {
+	// Ensure the error code is of the expected type and the error
+	// code matches the value specified in the test instance.
+	if reflect.TypeOf(gotErr) != reflect.TypeOf(wantErr) {
+		return fmt.Errorf("wrong error - got %T (%[1]v), want %T",
+			gotErr, wantErr)
+	}
+	if gotErr == nil {
+		return nil
+	}
+
+	// Ensure the want error type is a script error.
+	werr, ok := wantErr.(RuleError)
+	if !ok {
+		return fmt.Errorf("unexpected test error type %T", wantErr)
+	}
+
+	// Ensure the error codes match.  It's safe to use a raw type assert
+	// here since the code above already proved they are the same type and
+	// the want error is a script error.
+	gotErrorCode := gotErr.(RuleError).ErrorCode
+	if gotErrorCode != werr.ErrorCode {
+		return fmt.Errorf("mismatched error code - got %v (%v), want %v",
+			gotErrorCode, gotErr, werr.ErrorCode)
+	}
+
+	return nil
+}

blockdag/error.go

@@ -220,6 +220,10 @@ const (
 	// by subnetwork
 	ErrInvalidGas
 
+	// ErrInvalidPayload transaction includes a payload in a sub-network that doesn't allow
+	// a Payload
+	ErrInvalidPayload
+
 	// ErrSubNetwork indicates that a block doesn't adhere to the sub-network
 	// registry rules
 	ErrSubNetworkRegistry
@@ -270,6 +274,7 @@ var errorCodeStrings = map[ErrorCode]string{
 	ErrFinality:                  "ErrFinality",
 	ErrTransactionsNotSorted:     "ErrTransactionsNotSorted",
 	ErrInvalidGas:                "ErrInvalidGas",
+	ErrInvalidPayload:            "ErrInvalidPayload",
 }
 
 // String returns the ErrorCode as a human-readable name.

blockdag/error_test.go

@@ -58,6 +58,7 @@ func TestErrorCodeStringer(t *testing.T) {
 		{ErrFinality, "ErrFinality"},
 		{ErrTransactionsNotSorted, "ErrTransactionsNotSorted"},
 		{ErrInvalidGas, "ErrInvalidGas"},
+		{ErrInvalidPayload, "ErrInvalidPayload"},
 		{0xffff, "Unknown ErrorCode (65535)"},
 	}
 

blockdag/test_utils.go

@@ -3,11 +3,12 @@ package blockdag
 import (
 	"encoding/binary"
 	"fmt"
-	"github.com/daglabs/btcd/util/subnetworkid"
 	"os"
 	"path/filepath"
 	"time"
 
+	"github.com/daglabs/btcd/util/subnetworkid"
+
 	"github.com/daglabs/btcd/dagconfig"
 	"github.com/daglabs/btcd/dagconfig/daghash"
 	"github.com/daglabs/btcd/database"
@@ -104,6 +105,41 @@ func DAGSetup(dbName string, config Config) (*BlockDAG, func(), error) {
 // OpTrueScript is script returning TRUE
 var OpTrueScript = []byte{txscript.OpTrue}
 
+type txSubnetworkData struct {
+	subnetworkID subnetworkid.SubNetworkID
+	Gas          uint64
+	Payload      []byte
+}
+
+func createTxForTest(numInputs uint32, numOutputs uint32, outputValue uint64, subnetworkData *txSubnetworkData) *wire.MsgTx {
+	tx := wire.NewMsgTx(wire.TxVersion)
+
+	for i := uint32(0); i < numInputs; i++ {
+		tx.AddTxIn(&wire.TxIn{
+			PreviousOutPoint: *wire.NewOutPoint(&daghash.Hash{}, i),
+			SignatureScript:  []byte{},
+			Sequence:         wire.MaxTxInSequenceNum,
+		})
+	}
+	for i := uint32(0); i < numOutputs; i++ {
+		tx.AddTxOut(&wire.TxOut{
+			PkScript: OpTrueScript,
+			Value:    outputValue,
+		})
+	}
+
+	if subnetworkData != nil {
+		tx.SubNetworkID = subnetworkData.subnetworkID
+		tx.Gas = subnetworkData.Gas
+		tx.Payload = subnetworkData.Payload
+	} else {
+		tx.SubNetworkID = wire.SubNetworkDAGCoin
+		tx.Gas = 0
+		tx.Payload = []byte{}
+	}
+	return tx
+}
+
 // createCoinbaseTxForTest returns a coinbase transaction with the requested number of
 // outputs paying an appropriate subsidy based on the passed block height to the
 // address associated with the harness.  It automatically uses a standard

blockdag/validate.go

@@ -204,11 +204,6 @@ func CheckTransactionSanity(tx *util.Tx) error {
 	var totalSatoshi uint64
 	for _, txOut := range msgTx.TxOut {
 		satoshi := txOut.Value
-		if satoshi < 0 {
-			str := fmt.Sprintf("transaction output has negative "+
-				"value of %v", satoshi)
-			return ruleError(ErrBadTxOutValue, str)
-		}
 		if satoshi > util.MaxSatoshi {
 			str := fmt.Sprintf("transaction output value of %v is "+
 				"higher than max allowed value of %v", satoshi,
@@ -267,6 +262,30 @@ func CheckTransactionSanity(tx *util.Tx) error {
 		}
 	}
 
+	// Transactions in native and SubNetworkRegistry SubNetworks must have Gas = 0
+	if (msgTx.SubNetworkID == wire.SubNetworkDAGCoin ||
+		msgTx.SubNetworkID == wire.SubNetworkRegistry) &&
+		msgTx.Gas > 0 {
+
+		return ruleError(ErrInvalidGas, "transaction in the native or "+
+			"registry sub-networks has gas > 0 ")
+	}
+
+	if msgTx.SubNetworkID == wire.SubNetworkDAGCoin &&
+		len(msgTx.Payload) > 0 {
+
+		return ruleError(ErrInvalidPayload,
+			"transaction in the native sub-network includes a payload")
+	}
+
+	if msgTx.SubNetworkID == wire.SubNetworkRegistry &&
+		len(msgTx.Payload) != 8 {
+
+		return ruleError(ErrInvalidPayload,
+			"transaction in the sub-network registry include a payload "+
+				"with length != 8 bytes")
+	}
+
 	return nil
 }
 

blockdag/validate_test.go

@@ -1145,3 +1145,51 @@ var BlockWithWrongTxOrder = wire.MsgBlock{
 		},
 	},
 }
+
+func TestCheckTransactionSanity(t *testing.T) {
+	tests := []struct {
+		name                   string
+		numInputs              uint32
+		numOutputs             uint32
+		outputValue            uint64
+		subnetworkData         *txSubnetworkData
+		extraModificationsFunc func(*wire.MsgTx)
+		expectedErr            error
+	}{
+		{"good one", 1, 1, 1, nil, nil, nil},
+		{"no inputs", 0, 1, 1, nil, nil, ruleError(ErrNoTxInputs, "")},
+		{"no outputs", 1, 0, 1, nil, nil, ruleError(ErrNoTxOutputs, "")},
+		{"too big", 100000, 1, 1, nil, nil, ruleError(ErrTxTooBig, "")},
+		{"too much satoshi in one output", 1, 1, util.MaxSatoshi + 1, nil, nil, ruleError(ErrBadTxOutValue, "")},
+		{"too much satoshi in total outputs", 1, 2, util.MaxSatoshi - 1, nil, nil, ruleError(ErrBadTxOutValue, "")},
+		{"duplicate inputs", 2, 1, 1, nil,
+			func(tx *wire.MsgTx) { tx.TxIn[1].PreviousOutPoint.Index = 0 },
+			ruleError(ErrDuplicateTxInputs, "")},
+		{"non-zero gas in DAGCoin", 1, 1, 0,
+			&txSubnetworkData{wire.SubNetworkDAGCoin, 1, []byte{}},
+			nil, ruleError(ErrInvalidGas, "")},
+		{"non-zero gas in subnetwork registry", 1, 1, 0,
+			&txSubnetworkData{wire.SubNetworkRegistry, 1, []byte{}},
+			nil, ruleError(ErrInvalidGas, "")},
+		{"non-zero payload in DAGCoin", 1, 1, 0,
+			&txSubnetworkData{wire.SubNetworkDAGCoin, 0, []byte{1}},
+			nil, ruleError(ErrInvalidPayload, "")},
+		{"payload in subnetwork registry isn't 8 bytes", 1, 1, 0,
+			&txSubnetworkData{wire.SubNetworkRegistry, 0, []byte{1, 2, 3, 4, 5, 6, 7}},
+			nil, ruleError(ErrInvalidPayload, "")},
+	}
+
+	for _, test := range tests {
+		tx := createTxForTest(test.numInputs, test.numOutputs, test.outputValue, test.subnetworkData)
+
+		if test.extraModificationsFunc != nil {
+			test.extraModificationsFunc(tx)
+		}
+
+		err := CheckTransactionSanity(util.NewTx(tx))
+		if e := checkRuleError(err, test.expectedErr); e != nil {
+			t.Errorf("TestCheckTransactionSanity: '%s': %v", test.name, e)
+			continue
+		}
+	}
+}

txscript/engine_test.go

@@ -156,7 +156,7 @@ func TestCheckErrorCondition(t *testing.T) {
 			}
 
 			err = vm.CheckErrorCondition(test.finalScript)
-			if e := tstCheckScriptError(err, test.expectedErr); e != nil {
+			if e := checkScriptError(err, test.expectedErr); e != nil {
 				t.Errorf("TestCheckErrorCondition: %d: %s", i, e)
 			}
 		}()
@@ -443,7 +443,7 @@ func TestDisasmPC(t *testing.T) {
 
 	for i, test := range tests {
 		actual, err := vm.DisasmPC()
-		if e := tstCheckScriptError(err, test.expectedErr); e != nil {
+		if e := checkScriptError(err, test.expectedErr); e != nil {
 			t.Errorf("TestDisasmPC: %d: %s", i, e)
 		}
 
@@ -505,7 +505,7 @@ func TestDisasmScript(t *testing.T) {
 
 	for _, test := range tests {
 		actual, err := vm.DisasmScript(test.index)
-		if e := tstCheckScriptError(err, test.expectedErr); e != nil {
+		if e := checkScriptError(err, test.expectedErr); e != nil {
 			t.Errorf("TestDisasmScript: %d: %s", test.index, e)
 		}
 

txscript/script_test.go

@@ -3654,7 +3654,7 @@ func TestUnparsingInvalidOpcodes(t *testing.T) {
 
 	for _, test := range tests {
 		_, err := test.pop.bytes()
-		if e := tstCheckScriptError(err, test.expectedErr); e != nil {
+		if e := checkScriptError(err, test.expectedErr); e != nil {
 			t.Errorf("Parsed opcode test '%s': %v", test.name, e)
 			continue
 		}

txscript/scriptnum_test.go

@@ -180,7 +180,7 @@ func TestMakeScriptNum(t *testing.T) {
 		// code matches the value specified in the test instance.
 		gotNum, err := makeScriptNum(test.serialized,
 			test.numLen)
-		if e := tstCheckScriptError(err, test.err); e != nil {
+		if e := checkScriptError(err, test.err); e != nil {
 			t.Errorf("makeScriptNum(%#x): %v", test.serialized, e)
 			continue
 		}

txscript/stack_test.go

@@ -12,10 +12,10 @@ import (
 	"testing"
 )
 
-// tstCheckScriptError ensures the type of the two passed errors are of the
+// checkScriptError ensures the type of the two passed errors are of the
 // same type (either both nil or both of type Error) and their error codes
 // match when not nil.
-func tstCheckScriptError(gotErr, wantErr error) error {
+func checkScriptError(gotErr, wantErr error) error {
 	// Ensure the error code is of the expected type and the error
 	// code matches the value specified in the test instance.
 	if reflect.TypeOf(gotErr) != reflect.TypeOf(wantErr) {
@@ -827,7 +827,7 @@ func TestStack(t *testing.T) {
 
 		// Ensure the error code is of the expected type and the error
 		// code matches the value specified in the test instance.
-		if e := tstCheckScriptError(err, test.err); e != nil {
+		if e := checkScriptError(err, test.err); e != nil {
 			t.Errorf("%s: %v", test.name, e)
 			continue
 		}

txscript/standard_test.go

@@ -464,7 +464,7 @@ func TestCalcScriptInfo(t *testing.T) {
 		pkScript := mustParseShortForm(test.pkScript)
 
 		si, err := CalcScriptInfo(sigScript, pkScript, test.isP2SH)
-		if e := tstCheckScriptError(err, test.scriptInfoErr); e != nil {
+		if e := checkScriptError(err, test.scriptInfoErr); e != nil {
 			t.Errorf("scriptinfo test %q: %v", test.name, e)
 			continue
 		}
@@ -610,7 +610,7 @@ func TestPayToAddrScript(t *testing.T) {
 	t.Logf("Running %d tests", len(tests))
 	for i, test := range tests {
 		pkScript, err := PayToAddrScript(test.in)
-		if e := tstCheckScriptError(err, test.err); e != nil {
+		if e := checkScriptError(err, test.err); e != nil {
 			t.Errorf("PayToAddrScript #%d unexpected error - "+
 				"got %v, want %v", i, err, test.err)
 			continue
@@ -716,7 +716,7 @@ func TestMultiSigScript(t *testing.T) {
 	t.Logf("Running %d tests", len(tests))
 	for i, test := range tests {
 		script, err := MultiSigScript(test.keys, test.nrequired)
-		if e := tstCheckScriptError(err, test.err); e != nil {
+		if e := checkScriptError(err, test.err); e != nil {
 			t.Errorf("MultiSigScript #%d: %v", i, e)
 			continue
 		}
@@ -768,7 +768,7 @@ func TestCalcMultiSigStats(t *testing.T) {
 	for i, test := range tests {
 		script := mustParseShortForm(test.script)
 		_, _, err := CalcMultiSigStats(script)
-		if e := tstCheckScriptError(err, test.err); e != nil {
+		if e := checkScriptError(err, test.err); e != nil {
 			t.Errorf("CalcMultiSigStats #%d (%s): %v", i, test.name,
 				e)
 			continue
@@ -1077,7 +1077,7 @@ func TestNullDataScript(t *testing.T) {
 
 	for i, test := range tests {
 		script, err := NullDataScript(test.data)
-		if e := tstCheckScriptError(err, test.err); e != nil {
+		if e := checkScriptError(err, test.err); e != nil {
 			t.Errorf("NullDataScript: #%d (%s): %v", i, test.name,
 				e)
 			continue

wire/msgtx.go

@@ -476,24 +476,12 @@ func (msg *MsgTx) BtcDecode(r io.Reader, pver uint32) error {
 			return err
 		}
 
-		isRegistrySubNetwork := msg.SubNetworkID == SubNetworkRegistry
-
-		if isRegistrySubNetwork && msg.Gas != 0 {
-			str := fmt.Sprintf("Transactions from subnetwork %v should have 0 gas", msg.SubNetworkID)
-			return messageError("MsgTx.BtcDecode", str)
-		}
-
 		payloadLength, err := ReadVarInt(r, pver)
 		if err != nil {
 			returnScriptBuffers()
 			return err
 		}
 
-		if isRegistrySubNetwork && payloadLength != 8 {
-			str := fmt.Sprintf("For registry sub network the payload should always be uint64 (8 bytes length)")
-			return messageError("MsgTx.BtcDecode", str)
-		}
-
 		msg.Payload = make([]byte, payloadLength)
 		_, err = io.ReadFull(r, msg.Payload)
 	}

wire/msgtx_test.go

@@ -624,50 +624,6 @@ func TestTxSerializeErrors(t *testing.T) {
 	if err == nil || err.Error() != expectedErr.Error() {
 		t.Errorf("TestTxSerializeErrors: expected error %v but got %v", expectedErr, err)
 	}
-
-	registryWithGasTxEncoded := []byte{
-		0x01, 0x00, 0x00, 0x00, // Version
-		0x00,                                           // Varint for number of input transactions
-		0x00,                                           // Varint for number of output transactions
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // Lock time
-		0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x00, 0x00, // Sub Network ID
-		0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // Gas
-		0x08,                                           // Payload length varint
-		0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // Payload / Gas limit
-	}
-
-	r = bytes.NewReader(registryWithGasTxEncoded)
-	err = tx.Deserialize(r)
-
-	str = fmt.Sprintf("Transactions from subnetwork %v should have 0 gas", SubNetworkRegistry)
-	expectedErr = messageError("MsgTx.BtcDecode", str)
-	if err == nil || err.Error() != expectedErr.Error() {
-		t.Errorf("TestTxSerializeErrors: expected error %v but got %v", expectedErr, err)
-	}
-
-	registryWithWrongPayloadTxEncoded := []byte{
-		0x01, 0x00, 0x00, 0x00, // Version
-		0x00,                                           // Varint for number of input transactions
-		0x00,                                           // Varint for number of output transactions
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // Lock time
-		0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-		0x00, 0x00, 0x00, 0x00, // Sub Network ID
-		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // Gas
-		0x03,             // Payload length varint
-		0x01, 0x02, 0x03, // Payload / Gas limit
-	}
-
-	r = bytes.NewReader(registryWithWrongPayloadTxEncoded)
-	err = tx.Deserialize(r)
-
-	str = fmt.Sprintf("For registry sub network the payload should always be uint64 (8 bytes length)")
-	expectedErr = messageError("MsgTx.BtcDecode", str)
-	if err == nil || err.Error() != expectedErr.Error() {
-		t.Errorf("TestTxSerializeErrors: expected error %v but got %v", expectedErr, err)
-	}
 }
 
 // TestTxOverflowErrors performs tests to ensure deserializing transactions