multi: Break coinbase dep on standardness rules.

Standardness rules are not the same as consensus rules and it is
important to keep clear separation between them, because standardness
rules can and do change, while the consensus rules must not change
without a specific vote, and even then, the old rules must be kept
around for validation of all blocks prior to any changes introduced by a
successful vote.

Prior to this commit, the blockchain code which enforces the consensus
rule that requires the second output of the coinbase transaction to
contain the height of the block in a provably pruneable nulldata script
push was relying on code in txscript/standard.go, which, as the name of
the file suggests, is specifically intended for code related to
standardness checks.

This introduces a new function in the txscript package named
ExtractCoinbaseNullData which does not rely on any of the code related
to standardness checks and modifies the blockchain code accordingly to
make use of the new function instead.  It also removes the
standardness-dependent GetNullDataContent function which is no longer
used by anything.

Finally, it adds tests in the txscript package to ensure the new
function has the required semantics.

blockchain/accept.go

@@ -17,8 +17,8 @@ import (
 	"github.com/decred/dcrd/txscript"
 )
 
-// checkCoinbaseUniqueHeight checks to ensure that for all blocks height > 1
-// that the coinbase contains the height encoding to make coinbase hash collisions
+// checkCoinbaseUniqueHeight checks to ensure that for all blocks height > 1 the
+// coinbase contains the height encoding to make coinbase hash collisions
 // impossible.
 func checkCoinbaseUniqueHeight(blockHeight int64, block *dcrutil.Block) error {
 	// Coinbase TxOut[0] is always tax, TxOut[1] is always
@@ -30,21 +30,26 @@ func checkCoinbaseUniqueHeight(blockHeight int64, block *dcrutil.Block) error {
 		return ruleError(ErrFirstTxNotCoinbase, str)
 	}
 
-	// The first 4 bytes of the NullData output must be the
-	// encoded height of the block, so that every coinbase
-	// created has a unique transaction hash.
-	nullData, err := txscript.GetNullDataContent(
-		block.MsgBlock().Transactions[0].TxOut[1].Version,
-		block.MsgBlock().Transactions[0].TxOut[1].PkScript)
-	if err != nil {
-		str := fmt.Sprintf("block %v txOut 1 has wrong pkScript "+
-			"type", block.Hash())
+	// Only version 0 scripts are currently valid.
+	nullDataOut := block.MsgBlock().Transactions[0].TxOut[1]
+	if nullDataOut.Version != 0 {
+		str := fmt.Sprintf("block %v output 1 has wrong script version",
+			block.Hash())
 		return ruleError(ErrFirstTxNotCoinbase, str)
 	}
 
+	// The first 4 bytes of the null data output must be the encoded height
+	// of the block, so that every coinbase created has a unique transaction
+	// hash.
+	nullData, err := txscript.ExtractCoinbaseNullData(nullDataOut.PkScript)
+	if err != nil {
+		str := fmt.Sprintf("block %v output 1 has wrong script type",
+			block.Hash())
+		return ruleError(ErrFirstTxNotCoinbase, str)
+	}
 	if len(nullData) < 4 {
-		str := fmt.Sprintf("block %v txOut 1 has too short nullData "+
-			"push to contain height", block.Hash())
+		str := fmt.Sprintf("block %v output 1 data push too short to "+
+			"contain height", block.Hash())
 		return ruleError(ErrFirstTxNotCoinbase, str)
 	}
 
@@ -52,7 +57,7 @@ func checkCoinbaseUniqueHeight(blockHeight int64, block *dcrutil.Block) error {
 	cbHeight := binary.LittleEndian.Uint32(nullData[0:4])
 	if cbHeight != uint32(blockHeight) {
 		prevBlock := block.MsgBlock().Header.PrevBlock
-		str := fmt.Sprintf("block %v txOut 1 has wrong height in "+
+		str := fmt.Sprintf("block %v output 1 has wrong height in "+
 			"coinbase; want %v, got %v; prevBlock %v, header height %v",
 			block.Hash(), blockHeight, cbHeight, prevBlock,
 			block.MsgBlock().Header.Height)

txscript/consensus.go

@@ -5,10 +5,54 @@
 
 package txscript
 
+import (
+	"fmt"
+)
+
 const (
 	// LockTimeThreshold is the number below which a lock time is
 	// interpreted to be a block number.  Since an average of one block
 	// is generated per 10 minutes, this allows blocks for about 9,512
 	// years.
 	LockTimeThreshold = 5e8 // Tue Nov 5 00:53:20 1985 UTC
+
+	// maxUniqueCoinbaseNullDataSize is the maximum number of bytes allowed
+	// in the pushed data output of the coinbase output that is used to
+	// ensure the coinbase has a unique hash.
+	maxUniqueCoinbaseNullDataSize = 256
 )
+
+// ExtractCoinbaseNullData ensures the passed script is a nulldata script as
+// required by the consensus rules for the coinbase output that is used to
+// ensure the coinbase has a unique hash and returns the data it pushes.
+func ExtractCoinbaseNullData(pkScript []byte) ([]byte, error) {
+	pops, err := parseScript(pkScript)
+	if err != nil {
+		return nil, fmt.Errorf("script parse failure")
+	}
+
+	// The nulldata in the coinbase must be a single OP_RETURN followed by a
+	// data push up to maxUniqueCoinbaseNullDataSize bytes.
+	//
+	// NOTE: This is intentionally not using GetScriptClass and the related
+	// functions because those are specifically for standardness checks which
+	// can change over time and this function is specifically intended to be
+	// used by the consensus rules.
+	//
+	// Also of note is that technically normal nulldata scripts support encoding
+	// numbers via small opcodes, however the consensus rules require the block
+	// height to be encoded as a 4-byte little-endian uint32 pushed via a normal
+	// data push, as opposed to using the normal number handling semantics of
+	// scripts, so this is specialized to accomodate that.
+	if len(pops) == 1 && pops[0].opcode.value == OP_RETURN {
+		return nil, nil
+	}
+	if len(pops) == 2 && pops[0].opcode.value == OP_RETURN &&
+		pops[1].opcode.value <= OP_PUSHDATA4 && len(pops[1].data) <=
+		maxUniqueCoinbaseNullDataSize {
+
+		return pops[1].data, nil
+	}
+
+	return nil, fmt.Errorf("not a properly-formed nulldata script")
+}

txscript/consensus_test.go

@@ -0,0 +1,75 @@
+// Copyright (c) 2018 The Decred developers
+// Use of this source code is governed by an ISC
+// license that can be found in the LICENSE file.
+
+package txscript
+
+import (
+	"bytes"
+	"testing"
+)
+
+// TestExtractCoinbaseNullData ensures the ExtractCoinbaseNullData function
+// produces the expected extracted data under both valid and invalid scenarios.
+func TestExtractCoinbaseNullData(t *testing.T) {
+	tests := []struct {
+		name   string
+		script []byte
+		valid  bool
+		result []byte
+	}{{
+		name:   "block 2, height only",
+		script: mustParseShortForm("RETURN DATA_4 0x02000000"),
+		valid:  true,
+		result: hexToBytes("02000000"),
+	}, {
+		name:   "block 2, height and extra nonce data",
+		script: mustParseShortForm("RETURN DATA_36 0x02000000000000000000000000000000000000000000000000000000ffa310d9a6a9588e"),
+		valid:  true,
+		result: hexToBytes("02000000000000000000000000000000000000000000000000000000ffa310d9a6a9588e"),
+	}, {
+		name:   "block 2, height and reduced extra nonce data",
+		script: mustParseShortForm("RETURN DATA_12 0x02000000ffa310d9a6a9588e"),
+		valid:  true,
+		result: hexToBytes("02000000ffa310d9a6a9588e"),
+	}, {
+		name:   "no push",
+		script: mustParseShortForm("RETURN"),
+		valid:  true,
+		result: nil,
+	}, {
+		// Normal nulldata scripts support special handling of small data,
+		// however the coinbase nulldata in question does not.
+		name:   "small data",
+		script: mustParseShortForm("RETURN OP_2"),
+		valid:  false,
+		result: nil,
+	}, {
+		name:   "almost correct",
+		script: mustParseShortForm("OP_TRUE RETURN DATA_12 0x02000000ffa310d9a6a9588e"),
+		valid:  false,
+		result: nil,
+	}, {
+		name:   "almost correct 2",
+		script: mustParseShortForm("DATA_12 0x02000000 0xffa310d9a6a9588e"),
+		valid:  false,
+		result: nil,
+	}}
+
+	for _, test := range tests {
+		nullData, err := ExtractCoinbaseNullData(test.script)
+		if test.valid && err != nil {
+			t.Errorf("test '%s' unexpected error: %v", test.name, err)
+			continue
+		} else if !test.valid && err == nil {
+			t.Errorf("test '%s' passed when it should have failed", test.name)
+			continue
+		}
+
+		if !bytes.Equal(nullData, test.result) {
+			t.Errorf("test '%s' mismatched result - got %x, want %x", test.name,
+				nullData, test.result)
+			continue
+		}
+	}
+}

txscript/standard.go

@@ -1343,22 +1343,6 @@ func ExtractPkScriptAltSigType(pkScript []byte) (int, error) {
 	return -1, fmt.Errorf("bad signature scheme type")
 }
 
-// GetNullDataContent returns the content of a NullData (OP_RETURN) data push
-// and an error if the script is not a NullData script.
-func GetNullDataContent(version uint16, pkScript []byte) ([]byte, error) {
-	class := GetScriptClass(version, pkScript)
-	if class != NullDataTy {
-		return nil, fmt.Errorf("not nulldata script")
-	}
-
-	pops, err := parseScript(pkScript)
-	if err != nil {
-		return nil, fmt.Errorf("script parse failure")
-	}
-
-	return pops[1].data, nil
-}
-
 // AtomicSwapDataPushes houses the data pushes found in atomic swap contracts.
 type AtomicSwapDataPushes struct {
 	RecipientHash160 [20]byte