From f06513aad7c1b6edb857113d4ad895b52783bb82 Mon Sep 17 00:00:00 2001 From: Ori Newman Date: Thu, 14 Feb 2019 18:05:11 +0200 Subject: [PATCH] [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 | 35 +++++++ blockdag/external_dag_test.go | 162 +++++++++++++++++++++++++++++- blockdag/indexers/txindex_test.go | 2 +- blockdag/validate.go | 26 ----- mining/test_utils.go | 20 +++- util/testtools/testtools.go | 2 +- 6 files changed, 213 insertions(+), 34 deletions(-) diff --git a/blockdag/dag.go b/blockdag/dag.go index ee2cc1189..49be30d2c 100644 --- a/blockdag/dag.go +++ b/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 { diff --git a/blockdag/external_dag_test.go b/blockdag/external_dag_test.go index ed7e66e8a..ad79b7bee 100644 --- a/blockdag/external_dag_test.go +++ b/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, ¶ms, parentHashes, nil, false) + msgBlock, err := mining.PrepareBlockForTest(dag, ¶ms, 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, ¶ms, []daghash.Hash{*params.GenesisHash}, nil, false) + block1, err := mining.PrepareBlockForTest(dag, ¶ms, []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, ¶ms, []daghash.Hash{block1.BlockHash()}, []*wire.MsgTx{tx, chainedTx}, true) + block2, err := mining.PrepareBlockForTest(dag, ¶ms, []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, ¶ms, []daghash.Hash{block1.BlockHash()}, []*wire.MsgTx{nonChainedTx}, false) + block3, err := mining.PrepareBlockForTest(dag, ¶ms, []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: ¶ms, + 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, ¶ms, gasLimit) + if err != nil { + t.Fatalf("could not register network: %s", err) + } + + fundsBlock, err := mining.PrepareBlockForTest(dag, ¶ms, 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, ¶ms, 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, ¶ms, 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, ¶ms, 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, ¶ms, 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") + } +} diff --git a/blockdag/indexers/txindex_test.go b/blockdag/indexers/txindex_test.go index b5567f06a..852e35857 100644 --- a/blockdag/indexers/txindex_test.go +++ b/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, ¶ms, parentHashes, transactions, false) + block, err := mining.PrepareBlockForTest(dag, ¶ms, parentHashes, transactions, false, 1) if err != nil { t.Fatalf("TestTxIndexConnectBlock: block %v got unexpected error from PrepareBlockForTest: %v", blockName, err) } diff --git a/blockdag/validate.go b/blockdag/validate.go index bf81f52b8..369c78bce 100644 --- a/blockdag/validate.go +++ b/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 } diff --git a/mining/test_utils.go b/mining/test_utils.go index 6937bc3c9..57f1fde21 100644 --- a/mining/test_utils.go +++ b/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) diff --git a/util/testtools/testtools.go b/util/testtools/testtools.go index 2d3724484..d39a7f648 100644 --- a/util/testtools/testtools.go +++ b/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 }