From f7fa823f17c4c539c04be169978a9658c78e9777 Mon Sep 17 00:00:00 2001 From: Svarog Date: Thu, 26 Nov 2020 12:12:01 +0200 Subject: [PATCH] [NOD-1551] Requirements for performance tests (#1154) * [NOD-1551] Add NewTestConsensusWithDataDir to factory * [NOD-1551] Cache transaction ID * [NOD-1551] Should return err if err != nil * [NOD-1551] BuildBlockWithParents returns the blocks pastUTXOData * [NOD-1551] Set BlockCoinbaseMaturity to 0 in TestDoubleSpends * [NOD-1551] Fix comments * --amend Co-authored-by: Ori Newman --- domain/consensus/consensus_test.go | 5 ++-- domain/consensus/factory.go | 15 ++++++++-- domain/consensus/finality_test.go | 6 ++-- .../model/externalapi/transaction.go | 4 +++ .../model/interface_processes_blockbuilder.go | 5 +++- .../consensus/model/testapi/test_consensus.go | 3 +- .../blockbuilder/test_block_builder.go | 29 ++++++++++--------- .../block_header_in_context_test.go | 5 ++-- .../resolve_block_status_test.go | 2 ++ .../pastmediantimemanager_test.go | 5 ++-- .../transaction_in_context.go | 8 ++--- domain/consensus/test_consensus.go | 5 ++-- .../consensusserialization/transaction.go | 10 ++++++- 13 files changed, 65 insertions(+), 37 deletions(-) diff --git a/domain/consensus/consensus_test.go b/domain/consensus/consensus_test.go index af64f73e9..63c1d1527 100644 --- a/domain/consensus/consensus_test.go +++ b/domain/consensus/consensus_test.go @@ -1,13 +1,14 @@ package consensus import ( + "testing" + "github.com/kaspanet/kaspad/domain/consensus/model/externalapi" "github.com/kaspanet/kaspad/domain/consensus/ruleerrors" "github.com/kaspanet/kaspad/domain/consensus/utils/consensusserialization" "github.com/kaspanet/kaspad/domain/consensus/utils/testutils" "github.com/kaspanet/kaspad/domain/dagconfig" "github.com/pkg/errors" - "testing" ) func TestConsensus_GetBlockInfo(t *testing.T) { @@ -20,7 +21,7 @@ func TestConsensus_GetBlockInfo(t *testing.T) { } defer teardown() - invalidBlock, err := consensus.BuildBlockWithParents([]*externalapi.DomainHash{params.GenesisHash}, nil, nil) + invalidBlock, _, err := consensus.BuildBlockWithParents([]*externalapi.DomainHash{params.GenesisHash}, nil, nil) if err != nil { t.Fatal(err) } diff --git a/domain/consensus/factory.go b/domain/consensus/factory.go index 5d68536de..9f7015617 100644 --- a/domain/consensus/factory.go +++ b/domain/consensus/factory.go @@ -48,6 +48,8 @@ import ( type Factory interface { NewConsensus(dagParams *dagconfig.Params, db infrastructuredatabase.Database) (externalapi.Consensus, error) NewTestConsensus(dagParams *dagconfig.Params, testName string) (tc testapi.TestConsensus, teardown func(), err error) + NewTestConsensusWithDataDir(dagParams *dagconfig.Params, dataDir string) ( + tc testapi.TestConsensus, teardown func(), err error) } type factory struct{} @@ -312,11 +314,18 @@ func (f *factory) NewConsensus(dagParams *dagconfig.Params, db infrastructuredat func (f *factory) NewTestConsensus(dagParams *dagconfig.Params, testName string) ( tc testapi.TestConsensus, teardown func(), err error) { - testDatabaseDir, err := ioutil.TempDir("", testName) + dataDir, err := ioutil.TempDir("", testName) if err != nil { return nil, nil, err } - db, err := ldb.NewLevelDB(testDatabaseDir) + + return f.NewTestConsensusWithDataDir(dagParams, dataDir) +} + +func (f *factory) NewTestConsensusWithDataDir(dagParams *dagconfig.Params, dataDir string) ( + tc testapi.TestConsensus, teardown func(), err error) { + + db, err := ldb.NewLevelDB(dataDir) if err != nil { return nil, nil, err } @@ -338,7 +347,7 @@ func (f *factory) NewTestConsensus(dagParams *dagconfig.Params, testName string) tstConsensus.testBlockBuilder = blockbuilder.NewTestBlockBuilder(consensusAsImplementation.blockBuilder, tstConsensus) teardown = func() { db.Close() - os.RemoveAll(testDatabaseDir) + os.RemoveAll(dataDir) } return tstConsensus, teardown, nil diff --git a/domain/consensus/finality_test.go b/domain/consensus/finality_test.go index 5cf6532e6..f5ba1a48e 100644 --- a/domain/consensus/finality_test.go +++ b/domain/consensus/finality_test.go @@ -27,7 +27,7 @@ func TestFinality(t *testing.T) { defer teardown() buildAndInsertBlock := func(parentHashes []*externalapi.DomainHash) (*externalapi.DomainBlock, error) { - block, err := consensus.BuildBlockWithParents(parentHashes, nil, nil) + block, _, err := consensus.BuildBlockWithParents(parentHashes, nil, nil) if err != nil { return nil, err } @@ -184,7 +184,7 @@ func TestBoundedMergeDepth(t *testing.T) { } checkViolatingMergeDepth := func(consensus testapi.TestConsensus, parents []*externalapi.DomainHash) (*externalapi.DomainBlock, bool) { - block, err := consensus.BuildBlockWithParents(parents, nil, nil) + block, _, err := consensus.BuildBlockWithParents(parents, nil, nil) if err != nil { t.Fatalf("TestBoundedMergeDepth: BuildBlockWithParents failed: %v", err) return nil, false // fo some reason go doesn't recognize that t.Fatalf never returns @@ -210,7 +210,7 @@ func TestBoundedMergeDepth(t *testing.T) { } buildAndInsertBlock := func(consensus testapi.TestConsensus, parentHashes []*externalapi.DomainHash) *externalapi.DomainBlock { - block, err := consensus.BuildBlockWithParents(parentHashes, nil, nil) + block, _, err := consensus.BuildBlockWithParents(parentHashes, nil, nil) if err != nil { t.Fatalf("TestBoundedMergeDepth: Failed building block: %v", err) } diff --git a/domain/consensus/model/externalapi/transaction.go b/domain/consensus/model/externalapi/transaction.go index 32959ec6d..9928ad732 100644 --- a/domain/consensus/model/externalapi/transaction.go +++ b/domain/consensus/model/externalapi/transaction.go @@ -17,6 +17,10 @@ type DomainTransaction struct { Fee uint64 Mass uint64 + + // ID is a field that is used to cache the transaction ID. + // Always use consensusserialization.TransactionID instead of accessing this field directly + ID *DomainTransactionID } // Clone returns a clone of DomainTransaction diff --git a/domain/consensus/model/interface_processes_blockbuilder.go b/domain/consensus/model/interface_processes_blockbuilder.go index 469c176ae..59c7bf60a 100644 --- a/domain/consensus/model/interface_processes_blockbuilder.go +++ b/domain/consensus/model/interface_processes_blockbuilder.go @@ -10,6 +10,9 @@ type BlockBuilder interface { // TestBlockBuilder adds to the main BlockBuilder methods required by tests type TestBlockBuilder interface { BlockBuilder + + // BuildBlockWithParents builds a block with provided parents, coinbaseData and transactions, + // and returns the block together with its past UTXO-diff from the virtual. BuildBlockWithParents(parentHashes []*externalapi.DomainHash, coinbaseData *externalapi.DomainCoinbaseData, - transactions []*externalapi.DomainTransaction) (*externalapi.DomainBlock, error) + transactions []*externalapi.DomainTransaction) (*externalapi.DomainBlock, *UTXODiff, error) } diff --git a/domain/consensus/model/testapi/test_consensus.go b/domain/consensus/model/testapi/test_consensus.go index e898e6e62..f87cb9f37 100644 --- a/domain/consensus/model/testapi/test_consensus.go +++ b/domain/consensus/model/testapi/test_consensus.go @@ -11,8 +11,7 @@ type TestConsensus interface { DatabaseContext() model.DBReader - BuildBlockWithParents(parentHashes []*externalapi.DomainHash, coinbaseData *externalapi.DomainCoinbaseData, - transactions []*externalapi.DomainTransaction) (*externalapi.DomainBlock, error) + BuildBlockWithParents(parentHashes []*externalapi.DomainHash, coinbaseData *externalapi.DomainCoinbaseData, transactions []*externalapi.DomainTransaction) (*externalapi.DomainBlock, *model.UTXODiff, error) // AddBlock builds a block with given information, solves it, and adds to the DAG. // Returns the hash of the added block diff --git a/domain/consensus/processes/blockbuilder/test_block_builder.go b/domain/consensus/processes/blockbuilder/test_block_builder.go index 667dd23e9..08943658c 100644 --- a/domain/consensus/processes/blockbuilder/test_block_builder.go +++ b/domain/consensus/processes/blockbuilder/test_block_builder.go @@ -27,8 +27,11 @@ func NewTestBlockBuilder(baseBlockBuilder model.BlockBuilder, testConsensus test } } -func (bb *testBlockBuilder) BuildBlockWithParents(parentHashes []*externalapi.DomainHash, coinbaseData *externalapi.DomainCoinbaseData, - transactions []*externalapi.DomainTransaction) (*externalapi.DomainBlock, error) { +// BuildBlockWithParents builds a block with provided parents, coinbaseData and transactions, +// and returns the block together with its past UTXO-diff from the virtual. +func (bb *testBlockBuilder) BuildBlockWithParents(parentHashes []*externalapi.DomainHash, + coinbaseData *externalapi.DomainCoinbaseData, transactions []*externalapi.DomainTransaction) ( + *externalapi.DomainBlock, *model.UTXODiff, error) { onEnd := logger.LogAndMeasureExecutionTime(log, "BuildBlockWithParents") defer onEnd() @@ -69,9 +72,7 @@ func (bb *testBlockBuilder) buildHeaderWithParents(parentHashes []*externalapi.D }, nil } -func (bb *testBlockBuilder) buildBlockWithParents( - parentHashes []*externalapi.DomainHash, coinbaseData *externalapi.DomainCoinbaseData, - transactions []*externalapi.DomainTransaction) (*externalapi.DomainBlock, error) { +func (bb *testBlockBuilder) buildBlockWithParents(parentHashes []*externalapi.DomainHash, coinbaseData *externalapi.DomainCoinbaseData, transactions []*externalapi.DomainTransaction) (*externalapi.DomainBlock, *model.UTXODiff, error) { defer bb.testConsensus.DiscardAllStores() @@ -87,43 +88,43 @@ func (bb *testBlockBuilder) buildBlockWithParents( err := bb.ghostdagManager.GHOSTDAG(tempBlockHash) if err != nil { - return nil, err + return nil, nil, err } ghostdagData, err := bb.ghostdagDataStore.Get(bb.databaseContext, tempBlockHash) if err != nil { - return nil, err + return nil, nil, err } selectedParentStatus, err := bb.testConsensus.ConsensusStateManager().ResolveBlockStatus(ghostdagData.SelectedParent) if err != nil { - return nil, err + return nil, nil, err } if selectedParentStatus == externalapi.StatusDisqualifiedFromChain { - return nil, errors.Errorf("Error building block with selectedParent %s with status DisqualifiedFromChain", + return nil, nil, errors.Errorf("Error building block with selectedParent %s with status DisqualifiedFromChain", ghostdagData.SelectedParent) } - _, acceptanceData, multiset, err := bb.consensusStateManager.CalculatePastUTXOAndAcceptanceData(tempBlockHash) + pastUTXO, acceptanceData, multiset, err := bb.consensusStateManager.CalculatePastUTXOAndAcceptanceData(tempBlockHash) if err != nil { - return nil, err + return nil, nil, err } bb.acceptanceDataStore.Stage(tempBlockHash, acceptanceData) coinbase, err := bb.coinbaseManager.ExpectedCoinbaseTransaction(tempBlockHash, coinbaseData) if err != nil { - return nil, err + return nil, nil, err } transactionsWithCoinbase := append([]*externalapi.DomainTransaction{coinbase}, transactions...) header, err := bb.buildHeaderWithParents(parentHashes, transactionsWithCoinbase, acceptanceData, multiset) if err != nil { - return nil, err + return nil, nil, err } return &externalapi.DomainBlock{ Header: header, Transactions: transactionsWithCoinbase, - }, nil + }, pastUTXO, nil } diff --git a/domain/consensus/processes/blockvalidator/block_header_in_context_test.go b/domain/consensus/processes/blockvalidator/block_header_in_context_test.go index fab862ba2..4f46a8c4f 100644 --- a/domain/consensus/processes/blockvalidator/block_header_in_context_test.go +++ b/domain/consensus/processes/blockvalidator/block_header_in_context_test.go @@ -2,6 +2,8 @@ package blockvalidator_test import ( "errors" + "testing" + "github.com/kaspanet/kaspad/domain/consensus" "github.com/kaspanet/kaspad/domain/consensus/model" "github.com/kaspanet/kaspad/domain/consensus/model/externalapi" @@ -9,7 +11,6 @@ import ( "github.com/kaspanet/kaspad/domain/consensus/utils/consensusserialization" "github.com/kaspanet/kaspad/domain/consensus/utils/testutils" "github.com/kaspanet/kaspad/domain/dagconfig" - "testing" ) func TestValidateMedianTime(t *testing.T) { @@ -22,7 +23,7 @@ func TestValidateMedianTime(t *testing.T) { defer teardown() addBlock := func(blockTime int64, parents []*externalapi.DomainHash, expectedErr error) (*externalapi.DomainBlock, *externalapi.DomainHash) { - block, err := tc.BuildBlockWithParents(parents, nil, nil) + block, _, err := tc.BuildBlockWithParents(parents, nil, nil) if err != nil { t.Fatalf("BuildBlockWithParents: %+v", err) } diff --git a/domain/consensus/processes/consensusstatemanager/resolve_block_status_test.go b/domain/consensus/processes/consensusstatemanager/resolve_block_status_test.go index 4bcd70d20..3dae149b0 100644 --- a/domain/consensus/processes/consensusstatemanager/resolve_block_status_test.go +++ b/domain/consensus/processes/consensusstatemanager/resolve_block_status_test.go @@ -17,6 +17,8 @@ import ( func TestDoubleSpends(t *testing.T) { testutils.ForAllNets(t, true, func(t *testing.T, params *dagconfig.Params) { + params.BlockCoinbaseMaturity = 0 + factory := consensus.NewFactory() consensus, teardown, err := factory.NewTestConsensus(params, "TestUTXOCommitment") diff --git a/domain/consensus/processes/pastmediantimemanager/pastmediantimemanager_test.go b/domain/consensus/processes/pastmediantimemanager/pastmediantimemanager_test.go index aa0184ab0..5cce8cca9 100644 --- a/domain/consensus/processes/pastmediantimemanager/pastmediantimemanager_test.go +++ b/domain/consensus/processes/pastmediantimemanager/pastmediantimemanager_test.go @@ -1,12 +1,13 @@ package pastmediantimemanager_test import ( + "testing" + "github.com/kaspanet/kaspad/domain/consensus" "github.com/kaspanet/kaspad/domain/consensus/model/externalapi" "github.com/kaspanet/kaspad/domain/consensus/utils/consensusserialization" "github.com/kaspanet/kaspad/domain/consensus/utils/testutils" "github.com/kaspanet/kaspad/domain/dagconfig" - "testing" ) func TestPastMedianTime(t *testing.T) { @@ -24,7 +25,7 @@ func TestPastMedianTime(t *testing.T) { blockTime := params.GenesisBlock.Header.TimeInMilliseconds for i := uint32(1); i < numBlocks; i++ { blockTime += 1000 - block, err := tc.BuildBlockWithParents([]*externalapi.DomainHash{blockHashes[i-1]}, nil, nil) + block, _, err := tc.BuildBlockWithParents([]*externalapi.DomainHash{blockHashes[i-1]}, nil, nil) if err != nil { t.Fatalf("BuildBlockWithParents: %s", err) } diff --git a/domain/consensus/processes/transactionvalidator/transaction_in_context.go b/domain/consensus/processes/transactionvalidator/transaction_in_context.go index 3f7f9a5bb..edf886921 100644 --- a/domain/consensus/processes/transactionvalidator/transaction_in_context.go +++ b/domain/consensus/processes/transactionvalidator/transaction_in_context.go @@ -18,24 +18,24 @@ func (v *transactionValidator) ValidateTransactionInContextAndPopulateMassAndFee err := v.checkTransactionCoinbaseMaturity(povBlockHash, tx) if err != nil { - return nil + return err } totalSompiIn, err := v.checkTransactionInputAmounts(tx) if err != nil { - return nil + return err } totalSompiOut, err := v.checkTransactionOutputAmounts(tx, totalSompiIn) if err != nil { - return nil + return err } tx.Fee = totalSompiIn - totalSompiOut err = v.checkTransactionSequenceLock(povBlockHash, tx, selectedParentMedianTime) if err != nil { - return nil + return err } err = v.validateTransactionScripts(tx) diff --git a/domain/consensus/test_consensus.go b/domain/consensus/test_consensus.go index b01dc58fb..d845b4e6f 100644 --- a/domain/consensus/test_consensus.go +++ b/domain/consensus/test_consensus.go @@ -14,8 +14,7 @@ type testConsensus struct { testConsensusStateManager model.TestConsensusStateManager } -func (tc *testConsensus) BuildBlockWithParents(parentHashes []*externalapi.DomainHash, coinbaseData *externalapi.DomainCoinbaseData, - transactions []*externalapi.DomainTransaction) (*externalapi.DomainBlock, error) { +func (tc *testConsensus) BuildBlockWithParents(parentHashes []*externalapi.DomainHash, coinbaseData *externalapi.DomainCoinbaseData, transactions []*externalapi.DomainTransaction) (*externalapi.DomainBlock, *model.UTXODiff, error) { // Require write lock because BuildBlockWithParents stages temporary data tc.lock.Lock() @@ -31,7 +30,7 @@ func (tc *testConsensus) AddBlock(parentHashes []*externalapi.DomainHash, coinba tc.lock.Lock() defer tc.lock.Unlock() - block, err := tc.testBlockBuilder.BuildBlockWithParents(parentHashes, coinbaseData, transactions) + block, _, err := tc.testBlockBuilder.BuildBlockWithParents(parentHashes, coinbaseData, transactions) if err != nil { return nil, err } diff --git a/domain/consensus/utils/consensusserialization/transaction.go b/domain/consensus/utils/consensusserialization/transaction.go index 4a99ce12b..fbb6f0e25 100644 --- a/domain/consensus/utils/consensusserialization/transaction.go +++ b/domain/consensus/utils/consensusserialization/transaction.go @@ -65,6 +65,12 @@ func TransactionHash(tx *externalapi.DomainTransaction) *externalapi.DomainHash // TransactionID generates the Hash for the transaction without the signature script and payload field. func TransactionID(tx *externalapi.DomainTransaction) *externalapi.DomainTransactionID { + + // If transaction ID is already cached, return it + if tx.ID != nil { + return tx.ID + } + // Encode the transaction, replace signature script with zeroes, cut off // payload and calculate double sha256 on the result. var encodingFlags txEncoding @@ -80,7 +86,9 @@ func TransactionID(tx *externalapi.DomainTransaction) *externalapi.DomainTransac } transactionID := externalapi.DomainTransactionID(*writer.Finalize()) - return &transactionID + tx.ID = &transactionID + + return tx.ID } func serializeTransaction(w io.Writer, tx *externalapi.DomainTransaction, encodingFlags txEncoding) error {