TestBuildBlockErrorCases and remove redundant check of coinbase script length (#1427)

* Write structure of TestBlockBuilderErrorCases

* Remove double verification of script length in serializeCoinbasePayload

* Remove redundant code in TestBuildBlockErrorCases

* Rename povTransactionHash -> povBlockHash

* Convert coinbasePayloadScriptPublicKeyMaxLength to uint8

* Re-use consensus in TestBuildBlockErrorCases

domain/consensus/model/interface_processes_transactionvalidator.go

@@ -9,5 +9,5 @@ import (
 type TransactionValidator interface {
 	ValidateTransactionInIsolation(transaction *externalapi.DomainTransaction) error
 	ValidateTransactionInContextAndPopulateMassAndFee(tx *externalapi.DomainTransaction,
-		povTransactionHash *externalapi.DomainHash, selectedParentMedianTime int64) error
+		povBlockHash *externalapi.DomainHash, selectedParentMedianTime int64) error
 }

domain/consensus/processes/blockbuilder/block_builder_test.go

@@ -0,0 +1,56 @@
+package blockbuilder_test
+
+import (
+	"testing"
+
+	"github.com/kaspanet/kaspad/domain/consensus/ruleerrors"
+
+	"github.com/kaspanet/kaspad/domain/consensus"
+	"github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
+	"github.com/kaspanet/kaspad/domain/consensus/utils/testutils"
+	"github.com/kaspanet/kaspad/domain/dagconfig"
+	"github.com/pkg/errors"
+)
+
+func TestBuildBlockErrorCases(t *testing.T) {
+	testutils.ForAllNets(t, true, func(t *testing.T, params *dagconfig.Params) {
+		factory := consensus.NewFactory()
+		testConsensus, teardown, err := factory.NewTestConsensus(
+			params, false, "TestBlockBuilderErrorCases")
+		if err != nil {
+			t.Fatalf("Error initializing consensus for: %+v", err)
+		}
+		defer teardown(false)
+
+		tests := []struct {
+			name              string
+			coinbaseData      *externalapi.DomainCoinbaseData
+			transactions      []*externalapi.DomainTransaction
+			expectedErrorType error
+		}{
+			{
+				"scriptPublicKey too long",
+				&externalapi.DomainCoinbaseData{
+					ScriptPublicKey: &externalapi.ScriptPublicKey{
+						Script:  make([]byte, params.CoinbasePayloadScriptPublicKeyMaxLength+1),
+						Version: 0,
+					},
+					ExtraData: nil,
+				},
+				nil,
+				ruleerrors.ErrBadCoinbasePayloadLen,
+			},
+		}
+
+		for _, test := range tests {
+			_, err = testConsensus.BlockBuilder().BuildBlock(test.coinbaseData, test.transactions)
+			if err == nil {
+				t.Errorf("%s: No error from BuildBlock", test.name)
+				return
+			}
+			if test.expectedErrorType != nil && !errors.Is(err, test.expectedErrorType) {
+				t.Errorf("%s: Expected error '%s', but got '%s'", test.name, test.expectedErrorType, err)
+			}
+		}
+	})
+}

domain/consensus/processes/coinbasemanager/coinbasemanager.go

@@ -11,7 +11,7 @@ import (
 type coinbaseManager struct {
 	subsidyReductionInterval                uint64
 	baseSubsidy                             uint64
-	coinbasePayloadScriptPublicKeyMaxLength uint64
+	coinbasePayloadScriptPublicKeyMaxLength uint8
 
 	databaseContext     model.DBReader
 	ghostdagDataStore   model.GHOSTDAGDataStore
@@ -129,7 +129,7 @@ func New(
 
 	subsidyReductionInterval uint64,
 	baseSubsidy uint64,
-	coinbasePayloadScriptPublicKeyMaxLength uint64,
+	coinbasePayloadScriptPublicKeyMaxLength uint8,
 
 	ghostdagDataStore model.GHOSTDAGDataStore,
 	acceptanceDataStore model.AcceptanceDataStore) model.CoinbaseManager {

domain/consensus/processes/coinbasemanager/payload.go

@@ -2,7 +2,6 @@ package coinbasemanager
 
 import (
 	"encoding/binary"
-	"math"
 
 	"github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
 	"github.com/kaspanet/kaspad/domain/consensus/ruleerrors"
@@ -17,16 +16,13 @@ const lengthOfVersionScriptPubKey = uint16Len
 // serializeCoinbasePayload builds the coinbase payload based on the provided scriptPubKey and extra data.
 func (c *coinbaseManager) serializeCoinbasePayload(blueScore uint64, coinbaseData *externalapi.DomainCoinbaseData) ([]byte, error) {
 	scriptLengthOfScriptPubKey := len(coinbaseData.ScriptPublicKey.Script)
-	if uint64(scriptLengthOfScriptPubKey) > c.coinbasePayloadScriptPublicKeyMaxLength {
+	if scriptLengthOfScriptPubKey > int(c.coinbasePayloadScriptPublicKeyMaxLength) {
 		return nil, errors.Wrapf(ruleerrors.ErrBadCoinbasePayloadLen, "coinbase's payload script public key is "+
 			"longer than the max allowed length of %d", c.coinbasePayloadScriptPublicKeyMaxLength)
 	}
 
 	payload := make([]byte, uint64Len+lengthOfVersionScriptPubKey+lengthOfscriptPubKeyLength+scriptLengthOfScriptPubKey+len(coinbaseData.ExtraData))
 	binary.LittleEndian.PutUint64(payload[:uint64Len], blueScore)
-	if len(coinbaseData.ScriptPublicKey.Script) > math.MaxUint8 {
-		return nil, errors.Errorf("script public key is bigger than %d", math.MaxUint8)
-	}
 
 	payload[uint64Len] = uint8(coinbaseData.ScriptPublicKey.Version)
 	payload[uint64Len+lengthOfVersionScriptPubKey] = uint8(len(coinbaseData.ScriptPublicKey.Script))
@@ -49,7 +45,7 @@ func (c *coinbaseManager) ExtractCoinbaseDataAndBlueScore(coinbaseTx *externalap
 	scriptPubKeyVersion := uint16(coinbaseTx.Payload[uint64Len])
 	scriptPubKeyScriptLength := coinbaseTx.Payload[uint64Len+lengthOfVersionScriptPubKey]
 
-	if uint64(scriptPubKeyScriptLength) > c.coinbasePayloadScriptPublicKeyMaxLength {
+	if scriptPubKeyScriptLength > c.coinbasePayloadScriptPublicKeyMaxLength {
 		return 0, nil, errors.Wrapf(ruleerrors.ErrBadCoinbasePayloadLen, "coinbase's payload script public key is "+
 			"longer than the max allowed length of %d", c.coinbasePayloadScriptPublicKeyMaxLength)
 	}

domain/dagconfig/params.go

@@ -175,7 +175,7 @@ type Params struct {
 	MaxMassAcceptedByBlock uint64
 
 	// CoinbasePayloadScriptPublicKeyMaxLength is the maximum allowed script public key in the coinbase's payload
-	CoinbasePayloadScriptPublicKeyMaxLength uint64
+	CoinbasePayloadScriptPublicKeyMaxLength uint8
 
 	// BaseSubsidy is the starting subsidy amount for mined blocks.
 	BaseSubsidy uint64

infrastructure/config/network.go

@@ -3,14 +3,15 @@ package config
 import (
 	"encoding/json"
 	"fmt"
+	"math/big"
+	"os"
+	"time"
+
 	"github.com/jessevdk/go-flags"
 	"github.com/kaspanet/kaspad/domain/consensus/model"
 	"github.com/kaspanet/kaspad/domain/dagconfig"
 	"github.com/kaspanet/kaspad/util/difficulty"
 	"github.com/pkg/errors"
-	"math/big"
-	"os"
-	"time"
 )
 
 // NetworkFlags holds the network configuration, that is which network is selected.
@@ -33,7 +34,7 @@ type overrideDAGParamsConfig struct {
 	MassPerTxByte                           *uint64      `json:"massPerTxByte"`
 	MassPerScriptPubKeyByte                 *uint64      `json:"massPerScriptPubKeyByte"`
 	MassPerSigOp                            *uint64      `json:"massPerSigOp"`
-	CoinbasePayloadScriptPublicKeyMaxLength *uint64      `json:"coinbasePayloadScriptPublicKeyMaxLength"`
+	CoinbasePayloadScriptPublicKeyMaxLength *uint8       `json:"coinbasePayloadScriptPublicKeyMaxLength"`
 	PowMax                                  *string      `json:"powMax"`
 	BlockCoinbaseMaturity                   *uint64      `json:"blockCoinbaseMaturity"`
 	SubsidyReductionInterval                *uint64      `json:"subsidyReductionInterval"`