Fix overflow when checking coinbase maturity and don't ban peers that send transactions with immature spend (#1722)

* Fix overflow when checking coinbase maturity and don't ban peers that send transactions with immature spend

* Fix tests

Co-authored-by: Svarog <feanorr@gmail.com>
(cherry picked from commit a18f2f8802d52261c691ba1051d7b3134ddf8a78)

domain/consensus/processes/transactionvalidator/transaction_in_context.go

@@ -68,8 +68,7 @@ func (v *transactionValidator) checkTransactionCoinbaseMaturity(stagingArea *mod
 			missingOutpoints = append(missingOutpoints, &input.PreviousOutpoint)
 		} else if utxoEntry.IsCoinbase() {
 			originDAAScore := utxoEntry.BlockDAAScore()
-			daaScoreSincePrev := povDAAScore - originDAAScore
+			if originDAAScore+v.blockCoinbaseMaturity > povDAAScore {
-			if daaScoreSincePrev < v.blockCoinbaseMaturity {
 				return errors.Wrapf(ruleerrors.ErrImmatureSpend, "tried to spend coinbase "+
 					"transaction output %s from DAA score %d "+
 					"to DAA score %d before required maturity "+

domain/consensus/processes/transactionvalidator/transactionvalidator_test.go

@@ -78,7 +78,17 @@ func TestValidateTransactionInContextAndPopulateMassAndFee(t *testing.T) {
 				true,
 				uint64(5)),
 		}
-		txInputWithMaxSequence := externalapi.DomainTransactionInput{
+		immatureInput := externalapi.DomainTransactionInput{
+			PreviousOutpoint: prevOutPoint,
+			SignatureScript:  []byte{},
+			Sequence:         constants.MaxTxInSequenceNum,
+			UTXOEntry: utxo.NewUTXOEntry(
+				100_000_000, // 1 KAS
+				scriptPublicKey,
+				true,
+				uint64(6)),
+		}
+		txInputWithSequenceLockTimeIsSeconds := externalapi.DomainTransactionInput{
 			PreviousOutpoint: prevOutPoint,
 			SignatureScript:  []byte{},
 			Sequence:         constants.SequenceLockTimeIsSeconds,
@@ -110,7 +120,7 @@ func TestValidateTransactionInContextAndPopulateMassAndFee(t *testing.T) {
 
 		validTx := externalapi.DomainTransaction{
 			Version:      constants.MaxTransactionVersion,
-			Inputs:       []*externalapi.DomainTransactionInput{&txInputWithMaxSequence},
+			Inputs:       []*externalapi.DomainTransactionInput{&txInputWithSequenceLockTimeIsSeconds},
 			Outputs:      []*externalapi.DomainTransactionOutput{&txOut},
 			SubnetworkID: subnetworks.SubnetworkIDRegistry,
 			Gas:          0,
@@ -127,7 +137,7 @@ func TestValidateTransactionInContextAndPopulateMassAndFee(t *testing.T) {
 
 		txWithImmatureCoinbase := externalapi.DomainTransaction{
 			Version:      constants.MaxTransactionVersion,
-			Inputs:       []*externalapi.DomainTransactionInput{&txInput},
+			Inputs:       []*externalapi.DomainTransactionInput{&immatureInput},
 			Outputs:      []*externalapi.DomainTransactionOutput{&txOut},
 			SubnetworkID: subnetworks.SubnetworkIDRegistry,
 			Gas:          0,
@@ -158,7 +168,15 @@ func TestValidateTransactionInContextAndPopulateMassAndFee(t *testing.T) {
 
 		povBlockHash := externalapi.NewDomainHashFromByteArray(&[32]byte{0x01})
 		tc.DAABlocksStore().StageDAAScore(stagingArea, povBlockHash, consensusConfig.BlockCoinbaseMaturity+txInput.UTXOEntry.BlockDAAScore())
-		tc.DAABlocksStore().StageDAAScore(stagingArea, povBlockHash, 10)
+
+		// Just use some stub ghostdag data
+		tc.GHOSTDAGDataStore().Stage(stagingArea, povBlockHash, model.NewBlockGHOSTDAGData(
+			0,
+			nil,
+			consensusConfig.GenesisHash,
+			nil,
+			nil,
+			nil))
 
 		tests := []struct {
 			name                     string
@@ -171,7 +189,7 @@ func TestValidateTransactionInContextAndPopulateMassAndFee(t *testing.T) {
 			{
 				name:                     "Valid transaction",
 				tx:                       &validTx,
-				povBlockHash:             model.VirtualBlockHash,
+				povBlockHash:             povBlockHash,
 				selectedParentMedianTime: 1,
 				isValid:                  true,
 				expectedError:            nil,
@@ -189,7 +207,7 @@ func TestValidateTransactionInContextAndPopulateMassAndFee(t *testing.T) {
 			{ // The total inputs amount is bigger than the allowed maximum (constants.MaxSompi)
 				name:                     "checkTransactionInputAmounts",
 				tx:                       &txWithLargeAmount,
-				povBlockHash:             model.VirtualBlockHash,
+				povBlockHash:             povBlockHash,
 				selectedParentMedianTime: 1,
 				isValid:                  false,
 				expectedError:            ruleerrors.ErrBadTxOutValue,
@@ -197,7 +215,7 @@ func TestValidateTransactionInContextAndPopulateMassAndFee(t *testing.T) {
 			{ // The total SompiIn (sum of inputs amount) is smaller than the total SompiOut (sum of outputs value) and hence invalid.
 				name:                     "checkTransactionOutputAmounts",
 				tx:                       &txWithBigValue,
-				povBlockHash:             model.VirtualBlockHash,
+				povBlockHash:             povBlockHash,
 				selectedParentMedianTime: 1,
 				isValid:                  false,
 				expectedError:            ruleerrors.ErrSpendTooHigh,
@@ -205,15 +223,15 @@ func TestValidateTransactionInContextAndPopulateMassAndFee(t *testing.T) {
 			{ // the selectedParentMedianTime is negative and hence invalid.
 				name:                     "checkTransactionSequenceLock",
 				tx:                       &validTx,
-				povBlockHash:             model.VirtualBlockHash,
+				povBlockHash:             povBlockHash,
 				selectedParentMedianTime: -1,
 				isValid:                  false,
 				expectedError:            ruleerrors.ErrUnfinalizedTx,
 			},
-			{ // The SignatureScript (in the txInput) is empty and hence invalid.
+			{ // The SignatureScript (in the immatureInput) is empty and hence invalid.
 				name:                     "checkTransactionScripts",
 				tx:                       &txWithInvalidSignature,
-				povBlockHash:             model.VirtualBlockHash,
+				povBlockHash:             povBlockHash,
 				selectedParentMedianTime: 1,
 				isValid:                  false,
 				expectedError:            ruleerrors.ErrScriptValidation,
@@ -226,7 +244,7 @@ func TestValidateTransactionInContextAndPopulateMassAndFee(t *testing.T) {
 			if test.isValid {
 				if err != nil {
 					t.Fatalf("Unexpected error on TestValidateTransactionInContextAndPopulateMassAndFee"+
-						" on test '%v': %v", test.name, err)
+						" on test '%v': %+v", test.name, err)
 				}
 			} else {
 				if err == nil || !errors.Is(err, test.expectedError) {

domain/miningmanager/mempool/error.go

@@ -49,6 +49,7 @@ const (
 	RejectInsufficientFee RejectCode = 0x42
 	RejectFinality        RejectCode = 0x43
 	RejectDifficulty      RejectCode = 0x44
+	RejectImmatureSpend   RejectCode = 0x45
 )
 
 // Map of reject codes back strings for pretty printing.
@@ -63,6 +64,7 @@ var rejectCodeStrings = map[RejectCode]string{
 	RejectFinality:        "REJECT_FINALITY",
 	RejectDifficulty:      "REJECT_DIFFICULTY",
 	RejectNotRequested:    "REJECT_NOTREQUESTED",
+	RejectImmatureSpend:   "REJECT_IMMATURESPEND",
 }
 
 // String returns the RejectCode in human-readable form.

domain/miningmanager/mempool/mempool.go

@@ -735,6 +735,9 @@ func (mp *mempool) maybeAcceptTransaction(tx *consensusexternalapi.DomainTransac
 		if errors.As(err, &missingOutpoints) {
 			return missingOutpoints.MissingOutpoints, nil, nil
 		}
+		if errors.Is(err, ruleerrors.ErrImmatureSpend) {
+			return nil, nil, txRuleError(RejectImmatureSpend, "one of the transaction inputs spends an immature UTXO")
+		}
 		if errors.As(err, &ruleerrors.RuleError{}) {
 			return nil, nil, newRuleError(err)
 		}

domain/miningmanager/miningmanager_test.go

@@ -1,6 +1,7 @@
 package miningmanager_test
 
 import (
+	"github.com/kaspanet/kaspad/domain/miningmanager/mempool"
 	"strings"
 	"testing"
 
@@ -70,11 +71,35 @@ func TestValidateAndInsertTransaction(t *testing.T) {
 	})
 }
 
+func TestImmatureSpend(t *testing.T) {
+	testutils.ForAllNets(t, true, func(t *testing.T, consensusConfig *consensus.Config) {
+		factory := consensus.NewFactory()
+		tc, teardown, err := factory.NewTestConsensus(consensusConfig, "TestValidateAndInsertTransaction")
+		if err != nil {
+			t.Fatalf("Error setting up TestConsensus: %+v", err)
+		}
+		defer teardown(false)
+
+		miningFactory := miningmanager.NewFactory()
+		miningManager := miningFactory.NewMiningManager(tc, &consensusConfig.Params)
+		tx := createTransactionWithUTXOEntry(t, 0)
+		err = miningManager.ValidateAndInsertTransaction(tx, false)
+		txRuleError := &mempool.TxRuleError{}
+		if !errors.As(err, txRuleError) || txRuleError.RejectCode != mempool.RejectImmatureSpend {
+			t.Fatalf("Unexpected error %+v", err)
+		}
+		transactionsFromMempool := miningManager.AllTransactions()
+		if contains(tx, transactionsFromMempool) {
+			t.Fatalf("Mempool contains a transaction with immature coinbase")
+		}
+	})
+}
+
 // TestInsertDoubleTransactionsToMempool verifies that an attempt to insert a transaction
 // more than once into the mempool will result in raising an appropriate error.
 func TestInsertDoubleTransactionsToMempool(t *testing.T) {
 	testutils.ForAllNets(t, true, func(t *testing.T, consensusConfig *consensus.Config) {
-
+		consensusConfig.BlockCoinbaseMaturity = 0
 		factory := consensus.NewFactory()
 		tc, teardown, err := factory.NewTestConsensus(consensusConfig, "TestInsertDoubleTransactionsToMempool")
 		if err != nil {
@@ -99,7 +124,7 @@ func TestInsertDoubleTransactionsToMempool(t *testing.T) {
 // TestHandleNewBlockTransactions verifies that all the transactions in the block were successfully removed from the mempool.
 func TestHandleNewBlockTransactions(t *testing.T) {
 	testutils.ForAllNets(t, true, func(t *testing.T, consensusConfig *consensus.Config) {
-
+		consensusConfig.BlockCoinbaseMaturity = 0
 		factory := consensus.NewFactory()
 		tc, teardown, err := factory.NewTestConsensus(consensusConfig, "TestHandleNewBlockTransactions")
 		if err != nil {
@@ -165,7 +190,7 @@ func domainBlocksToBlockIds(blocks []*externalapi.DomainTransaction) []*external
 // will be removed from the mempool.
 func TestDoubleSpends(t *testing.T) {
 	testutils.ForAllNets(t, true, func(t *testing.T, consensusConfig *consensus.Config) {
-
+		consensusConfig.BlockCoinbaseMaturity = 0
 		factory := consensus.NewFactory()
 		tc, teardown, err := factory.NewTestConsensus(consensusConfig, "TestDoubleSpends")
 		if err != nil {
@@ -309,7 +334,7 @@ func createTransactionWithUTXOEntry(t *testing.T, i int) *externalapi.DomainTran
 			100000000, // 1 KAS
 			scriptPublicKey,
 			true,
-			uint64(5)),
+			uint64(0)),
 	}
 	txOut := externalapi.DomainTransactionOutput{
 		Value:           10000,