[DEV-369] Revert CheckTransactionSanity and CheckBlockSanity + move all Gas logic somewhere else (#185)

* [DEV-369] Revert CheckTransactionSanity and CheckBlockSanity + move all Gas logic somewhere else

* [DEV-369] Add TestGasLimit

* [DEV-369] Handle zero coinbase output in PrepareBlockForTest

* [DEV-369] add comments to TestGasLimit

blockdag/dag.go

@@ -530,6 +530,11 @@ func (dag *BlockDAG) connectBlock(node *blockNode, block *util.Block, fastAdd bo
 		}
 	}
 
+	err := dag.validateGasLimit(block)
+	if err != nil {
+		return err
+	}
+
 	// Add the node to the virtual and update the UTXO set of the DAG.
 	utxoDiff, acceptedTxsData, err := dag.applyUTXOChanges(node, block, fastAdd)
 	if err != nil {
@@ -606,6 +611,36 @@ func (dag *BlockDAG) connectBlock(node *blockNode, block *util.Block, fastAdd bo
 	return nil
 }
 
+func (dag *BlockDAG) validateGasLimit(block *util.Block) error {
+	transactions := block.Transactions()
+	// Amount of gas consumed per sub-network shouldn't be more than the subnetwork's limit
+	gasUsageInAllSubnetworks := map[subnetworkid.SubnetworkID]uint64{}
+	for _, tx := range transactions {
+		msgTx := tx.MsgTx()
+		// In DAGCoin and Registry sub-networks all txs must have Gas = 0, and that is validated in checkTransactionSanity
+		// Therefore - no need to check them here.
+		if msgTx.SubnetworkID != wire.SubnetworkIDNative && msgTx.SubnetworkID != wire.SubnetworkIDRegistry {
+			gasUsageInSubnetwork := gasUsageInAllSubnetworks[msgTx.SubnetworkID]
+			gasUsageInSubnetwork += msgTx.Gas
+			if gasUsageInSubnetwork < gasUsageInAllSubnetworks[msgTx.SubnetworkID] { // protect from overflows
+				str := fmt.Sprintf("Block gas usage in subnetwork with ID %s has overflown", msgTx.SubnetworkID)
+				return ruleError(ErrInvalidGas, str)
+			}
+			gasUsageInAllSubnetworks[msgTx.SubnetworkID] = gasUsageInSubnetwork
+
+			gasLimit, err := dag.SubnetworkStore.GasLimit(&msgTx.SubnetworkID)
+			if err != nil {
+				return err
+			}
+			if gasUsageInSubnetwork > gasLimit {
+				str := fmt.Sprintf("Block wastes too much gas in subnetwork with ID %s", msgTx.SubnetworkID)
+				return ruleError(ErrInvalidGas, str)
+			}
+		}
+	}
+	return nil
+}
+
 // LastFinalityPointHash returns the hash of the last finality point
 func (dag *BlockDAG) LastFinalityPointHash() *daghash.Hash {
 	if dag.lastFinalityPoint == nil {

blockdag/external_dag_test.go

@@ -2,8 +2,11 @@ package blockdag_test
 
 import (
 	"fmt"
+	"math"
 	"testing"
 
+	"github.com/daglabs/btcd/util/subnetworkid"
+
 	"github.com/daglabs/btcd/dagconfig/daghash"
 	"github.com/daglabs/btcd/util/testtools"
 
@@ -43,7 +46,7 @@ func TestFinality(t *testing.T) {
 	}
 	defer teardownFunc()
 	buildNodeToDag := func(parentHashes []daghash.Hash) (*util.Block, error) {
-		msgBlock, err := mining.PrepareBlockForTest(dag, &params, parentHashes, nil, false)
+		msgBlock, err := mining.PrepareBlockForTest(dag, &params, parentHashes, nil, false, 1)
 		if err != nil {
 			return nil, err
 		}
@@ -179,7 +182,7 @@ func TestChainedTransactions(t *testing.T) {
 	}
 	defer teardownFunc()
 
-	block1, err := mining.PrepareBlockForTest(dag, &params, []daghash.Hash{*params.GenesisHash}, nil, false)
+	block1, err := mining.PrepareBlockForTest(dag, &params, []daghash.Hash{*params.GenesisHash}, nil, false, 1)
 	if err != nil {
 		t.Fatalf("PrepareBlockForTest: %v", err)
 	}
@@ -214,7 +217,7 @@ func TestChainedTransactions(t *testing.T) {
 		Value:    uint64(1),
 	})
 
-	block2, err := mining.PrepareBlockForTest(dag, &params, []daghash.Hash{block1.BlockHash()}, []*wire.MsgTx{tx, chainedTx}, true)
+	block2, err := mining.PrepareBlockForTest(dag, &params, []daghash.Hash{block1.BlockHash()}, []*wire.MsgTx{tx, chainedTx}, true, 1)
 	if err != nil {
 		t.Fatalf("PrepareBlockForTest: %v", err)
 	}
@@ -222,7 +225,7 @@ func TestChainedTransactions(t *testing.T) {
 	//Checks that dag.ProcessBlock fails because we don't allow a transaction to spend another transaction from the same block
 	isOrphan, err = dag.ProcessBlock(util.NewBlock(block2), blockdag.BFNoPoWCheck)
 	if err == nil {
-		t.Errorf("ProcessBlock expected an error\n")
+		t.Errorf("ProcessBlock expected an error")
 	} else if rErr, ok := err.(blockdag.RuleError); ok {
 		if rErr.ErrorCode != blockdag.ErrMissingTxOut {
 			t.Errorf("ProcessBlock expected an %v error code but got %v", blockdag.ErrMissingTxOut, rErr.ErrorCode)
@@ -245,7 +248,7 @@ func TestChainedTransactions(t *testing.T) {
 		Value:    uint64(1),
 	})
 
-	block3, err := mining.PrepareBlockForTest(dag, &params, []daghash.Hash{block1.BlockHash()}, []*wire.MsgTx{nonChainedTx}, false)
+	block3, err := mining.PrepareBlockForTest(dag, &params, []daghash.Hash{block1.BlockHash()}, []*wire.MsgTx{nonChainedTx}, false, 1)
 	if err != nil {
 		t.Fatalf("PrepareBlockForTest: %v", err)
 	}
@@ -259,3 +262,152 @@ func TestChainedTransactions(t *testing.T) {
 		t.Errorf("ProcessBlock: block3 got unexpectedly orphaned")
 	}
 }
+
+// TestGasLimit tests the gas limit rules
+func TestGasLimit(t *testing.T) {
+	params := dagconfig.SimNetParams
+	params.K = 1
+	params.BlockRewardMaturity = 1
+	dag, teardownFunc, err := blockdag.DAGSetup("TestSubnetworkRegistry", blockdag.Config{
+		DAGParams:    &params,
+		SubnetworkID: &wire.SubnetworkIDSupportsAll,
+	})
+	if err != nil {
+		t.Fatalf("Failed to setup DAG instance: %v", err)
+	}
+	defer teardownFunc()
+
+	// First we prepare a subnetwrok and a block with coinbase outputs to fund our tests
+	gasLimit := uint64(12345)
+	subnetworkID, err := testtools.RegisterSubnetworkForTest(dag, &params, gasLimit)
+	if err != nil {
+		t.Fatalf("could not register network: %s", err)
+	}
+
+	fundsBlock, err := mining.PrepareBlockForTest(dag, &params, dag.TipHashes(), nil, false, 2)
+	if err != nil {
+		t.Fatalf("PrepareBlockForTest: %v", err)
+	}
+	isOrphan, err := dag.ProcessBlock(util.NewBlock(fundsBlock), blockdag.BFNoPoWCheck)
+	if err != nil {
+		t.Fatalf("ProcessBlock: %v", err)
+	}
+	if isOrphan {
+		t.Fatalf("ProcessBlock: funds block got unexpectedly orphan")
+	}
+
+	cbTxValue := fundsBlock.Transactions[0].TxOut[0].Value
+	cbTxID := fundsBlock.Transactions[0].TxID()
+
+	tx1 := wire.NewMsgTx(wire.TxVersion)
+	tx1.AddTxIn(&wire.TxIn{
+		PreviousOutPoint: *wire.NewOutPoint(&cbTxID, 0),
+		Sequence:         wire.MaxTxInSequenceNum,
+	})
+	tx1.AddTxOut(&wire.TxOut{
+		Value:    cbTxValue,
+		PkScript: blockdag.OpTrueScript,
+	})
+	tx1.SubnetworkID = *subnetworkID
+	tx1.Gas = 10000
+
+	tx2 := wire.NewMsgTx(wire.TxVersion)
+	tx2.AddTxIn(&wire.TxIn{
+		PreviousOutPoint: *wire.NewOutPoint(&cbTxID, 1),
+		Sequence:         wire.MaxTxInSequenceNum,
+	})
+	tx2.AddTxOut(&wire.TxOut{
+		Value:    cbTxValue,
+		PkScript: blockdag.OpTrueScript,
+	})
+	tx2.SubnetworkID = *subnetworkID
+	tx2.Gas = 10000
+
+	// Here we check that we can't process a block that has transactions that exceed the gas limit
+	overLimitBlock, err := mining.PrepareBlockForTest(dag, &params, dag.TipHashes(), []*wire.MsgTx{tx1, tx2}, true, 1)
+	if err != nil {
+		t.Fatalf("PrepareBlockForTest: %v", err)
+	}
+	isOrphan, err = dag.ProcessBlock(util.NewBlock(overLimitBlock), blockdag.BFNoPoWCheck)
+	if err == nil {
+		t.Fatalf("ProcessBlock expected to have an error")
+	}
+	rErr, ok := err.(blockdag.RuleError)
+	if !ok {
+		t.Fatalf("ProcessBlock expected a RuleError, but got %v", err)
+	} else if rErr.ErrorCode != blockdag.ErrInvalidGas {
+		t.Fatalf("ProcessBlock expected error code %s but got %s", blockdag.ErrInvalidGas, rErr.ErrorCode)
+	}
+	if isOrphan {
+		t.Fatalf("ProcessBlock: overLimitBlock got unexpectedly orphan")
+	}
+
+	overflowGasTx := wire.NewMsgTx(wire.TxVersion)
+	overflowGasTx.AddTxIn(&wire.TxIn{
+		PreviousOutPoint: *wire.NewOutPoint(&cbTxID, 1),
+		Sequence:         wire.MaxTxInSequenceNum,
+	})
+	overflowGasTx.AddTxOut(&wire.TxOut{
+		Value:    cbTxValue,
+		PkScript: blockdag.OpTrueScript,
+	})
+	overflowGasTx.SubnetworkID = *subnetworkID
+	overflowGasTx.Gas = math.MaxUint64
+
+	// Here we check that we can't process a block that its transactions' gas overflows uint64
+	overflowGasBlock, err := mining.PrepareBlockForTest(dag, &params, dag.TipHashes(), []*wire.MsgTx{tx1, overflowGasTx}, true, 1)
+	if err != nil {
+		t.Fatalf("PrepareBlockForTest: %v", err)
+	}
+	isOrphan, err = dag.ProcessBlock(util.NewBlock(overflowGasBlock), blockdag.BFNoPoWCheck)
+	if err == nil {
+		t.Fatalf("ProcessBlock expected to have an error")
+	}
+	rErr, ok = err.(blockdag.RuleError)
+	if !ok {
+		t.Fatalf("ProcessBlock expected a RuleError, but got %v", err)
+	} else if rErr.ErrorCode != blockdag.ErrInvalidGas {
+		t.Fatalf("ProcessBlock expected error code %s but got %s", blockdag.ErrInvalidGas, rErr.ErrorCode)
+	}
+	if isOrphan {
+		t.Fatalf("ProcessBlock: overLimitBlock got unexpectedly orphan")
+	}
+
+	nonExistentSubnetwork := subnetworkid.SubnetworkID{123}
+	nonExistentSubnetworkTx := wire.NewMsgTx(wire.TxVersion)
+	nonExistentSubnetworkTx.AddTxIn(&wire.TxIn{
+		PreviousOutPoint: *wire.NewOutPoint(&cbTxID, 0),
+		Sequence:         wire.MaxTxInSequenceNum,
+	})
+	nonExistentSubnetworkTx.AddTxOut(&wire.TxOut{
+		Value:    cbTxValue,
+		PkScript: blockdag.OpTrueScript,
+	})
+	nonExistentSubnetworkTx.SubnetworkID = nonExistentSubnetwork
+	nonExistentSubnetworkTx.Gas = 1
+
+	nonExistentSubnetworkBlock, err := mining.PrepareBlockForTest(dag, &params, dag.TipHashes(), []*wire.MsgTx{nonExistentSubnetworkTx, overflowGasTx}, true, 1)
+	if err != nil {
+		t.Fatalf("PrepareBlockForTest: %v", err)
+	}
+
+	// Here we check that we can't process a block with a transaction from a non-existent subnetwork
+	isOrphan, err = dag.ProcessBlock(util.NewBlock(nonExistentSubnetworkBlock), blockdag.BFNoPoWCheck)
+	expectedErrStr := fmt.Sprintf("subnetwork '%s' not found", nonExistentSubnetwork)
+	if err.Error() != expectedErrStr {
+		t.Fatalf("ProcessBlock expected error %v but got %v", expectedErrStr, err)
+	}
+
+	// Here we check that we can process a block with a transaction that doesn't exceed the gas limit
+	validBlock, err := mining.PrepareBlockForTest(dag, &params, dag.TipHashes(), []*wire.MsgTx{tx1}, true, 1)
+	if err != nil {
+		t.Fatalf("PrepareBlockForTest: %v", err)
+	}
+	isOrphan, err = dag.ProcessBlock(util.NewBlock(validBlock), blockdag.BFNoPoWCheck)
+	if err != nil {
+		t.Fatalf("ProcessBlock: %v", err)
+	}
+	if isOrphan {
+		t.Fatalf("ProcessBlock: overLimitBlock got unexpectedly orphan")
+	}
+}

blockdag/indexers/txindex_test.go

@@ -54,7 +54,7 @@ func TestTxIndexConnectBlock(t *testing.T) {
 	}
 
 	prepareAndProcessBlock := func(parentHashes []daghash.Hash, transactions []*wire.MsgTx, blockName string) *wire.MsgBlock {
-		block, err := mining.PrepareBlockForTest(dag, &params, parentHashes, transactions, false)
+		block, err := mining.PrepareBlockForTest(dag, &params, parentHashes, transactions, false, 1)
 		if err != nil {
 			t.Fatalf("TestTxIndexConnectBlock: block %v got unexpected error from PrepareBlockForTest: %v", blockName, err)
 		}

blockdag/validate.go

@@ -584,32 +584,6 @@ func (dag *BlockDAG) checkBlockSanity(block *util.Block, flags BehaviorFlags) er
 		}
 	}
 
-	// Amount of gas consumed per sub-network shouldn't be more than the subnetwork's limit
-	gasUsageInAllSubnetworks := map[subnetworkid.SubnetworkID]uint64{}
-	for _, tx := range transactions {
-		msgTx := tx.MsgTx()
-		// In DAGCoin and Registry sub-networks all txs must have Gas = 0, and that is validated in checkTransactionSanity
-		// Therefore - no need to check them here.
-		if msgTx.SubnetworkID != wire.SubnetworkIDNative && msgTx.SubnetworkID != wire.SubnetworkIDRegistry {
-			gasUsageInSubnetwork := gasUsageInAllSubnetworks[msgTx.SubnetworkID]
-			gasUsageInSubnetwork += msgTx.Gas
-			if gasUsageInSubnetwork < gasUsageInAllSubnetworks[msgTx.SubnetworkID] { // protect from overflows
-				str := fmt.Sprintf("Block gas usage in subnetwork with ID %s has overflown", msgTx.SubnetworkID)
-				return ruleError(ErrInvalidGas, str)
-			}
-			gasUsageInAllSubnetworks[msgTx.SubnetworkID] = gasUsageInSubnetwork
-
-			gasLimit, err := dag.SubnetworkStore.GasLimit(&msgTx.SubnetworkID)
-			if err != nil {
-				return err
-			}
-			if gasUsageInSubnetwork > gasLimit {
-				str := fmt.Sprintf("Block wastes too much gas in subnetwork with ID %s", msgTx.SubnetworkID)
-				return ruleError(ErrInvalidGas, str)
-			}
-		}
-	}
-
 	return nil
 }
 

mining/test_utils.go

@@ -38,7 +38,7 @@ func (txs *fakeTxSource) HaveTransaction(txID *daghash.TxID) bool {
 }
 
 // PrepareBlockForTest generates a block with the proper merkle roots, coinbase/fee transactions etc. This function is used for test purposes only
-func PrepareBlockForTest(dag *blockdag.BlockDAG, params *dagconfig.Params, parentHashes []daghash.Hash, transactions []*wire.MsgTx, forceTransactions bool) (*wire.MsgBlock, error) {
+func PrepareBlockForTest(dag *blockdag.BlockDAG, params *dagconfig.Params, parentHashes []daghash.Hash, transactions []*wire.MsgTx, forceTransactions bool, coinbaseOutputs uint64) (*wire.MsgBlock, error) {
 	newVirtual, err := blockdag.GetVirtualFromParentsForTest(dag, parentHashes)
 	if err != nil {
 		return nil, err
@@ -88,10 +88,28 @@ func PrepareBlockForTest(dag *blockdag.BlockDAG, params *dagconfig.Params, paren
 			txsToAdd = append(txsToAdd, tx)
 		}
 	}
+	if coinbaseOutputs != 1 {
+		cb := template.Block.Transactions[0]
+		originalValue := cb.TxOut[0].Value
+		pkScript := cb.TxOut[0].PkScript
+		cb.TxOut = make([]*wire.TxOut, coinbaseOutputs)
+		if coinbaseOutputs != 0 {
+			newOutValue := originalValue / coinbaseOutputs
+			for i := uint64(0); i < coinbaseOutputs; i++ {
+				cb.TxOut[i] = &wire.TxOut{
+					Value:    newOutValue,
+					PkScript: pkScript,
+				}
+			}
+		}
+	}
 	if forceTransactions && len(txsToAdd) > 0 {
 		for _, tx := range txsToAdd {
 			template.Block.Transactions = append(template.Block.Transactions, tx)
 		}
+	}
+	updateMerkleRoots := coinbaseOutputs != 1 || (forceTransactions && len(txsToAdd) > 0)
+	if updateMerkleRoots {
 		utilTxs := make([]*util.Tx, len(template.Block.Transactions))
 		for i, tx := range template.Block.Transactions {
 			utilTxs[i] = util.NewTx(tx)

util/testtools/testtools.go

@@ -19,7 +19,7 @@ import (
 // RegisterSubnetworkForTest is used to register network on DAG with specified gas limit
 func RegisterSubnetworkForTest(dag *blockdag.BlockDAG, params *dagconfig.Params, gasLimit uint64) (*subnetworkid.SubnetworkID, error) {
 	buildNextBlock := func(parentHashes []daghash.Hash, txs []*wire.MsgTx) (*util.Block, error) {
-		msgBlock, err := mining.PrepareBlockForTest(dag, params, parentHashes, txs, false)
+		msgBlock, err := mining.PrepareBlockForTest(dag, params, parentHashes, txs, false, 1)
 		if err != nil {
 			return nil, err
 		}