From 348e4daffedf469cdb0cbc8f9f20121262af6da6 Mon Sep 17 00:00:00 2001 From: Ori Newman Date: Tue, 5 Dec 2023 19:31:31 +0200 Subject: [PATCH] Some fixes --- .../consensus/model/testapi/test_consensus.go | 3 + domain/consensus/test_consensus.go | 11 ++ .../revalidate_high_priority_transactions.go | 40 ++----- domain/miningmanager/miningmanager_test.go | 110 ++++++++++++++++-- 4 files changed, 122 insertions(+), 42 deletions(-) diff --git a/domain/consensus/model/testapi/test_consensus.go b/domain/consensus/model/testapi/test_consensus.go index f7be594067..6738a916ce 100644 --- a/domain/consensus/model/testapi/test_consensus.go +++ b/domain/consensus/model/testapi/test_consensus.go @@ -43,6 +43,9 @@ type TestConsensus interface { AddBlock(parentHashes []*externalapi.DomainHash, coinbaseData *externalapi.DomainCoinbaseData, transactions []*externalapi.DomainTransaction) (*externalapi.DomainHash, *externalapi.VirtualChangeSet, error) + AddBlockOnTips(coinbaseData *externalapi.DomainCoinbaseData, + transactions []*externalapi.DomainTransaction) (*externalapi.DomainHash, *externalapi.VirtualChangeSet, error) + AddUTXOInvalidHeader(parentHashes []*externalapi.DomainHash) (*externalapi.DomainHash, *externalapi.VirtualChangeSet, error) AddUTXOInvalidBlock(parentHashes []*externalapi.DomainHash) (*externalapi.DomainHash, diff --git a/domain/consensus/test_consensus.go b/domain/consensus/test_consensus.go index 6f67c47a3d..64d61afdfd 100644 --- a/domain/consensus/test_consensus.go +++ b/domain/consensus/test_consensus.go @@ -69,6 +69,17 @@ func (tc *testConsensus) AddBlock(parentHashes []*externalapi.DomainHash, coinba return consensushashing.BlockHash(block), virtualChangeSet, nil } +func (tc *testConsensus) AddBlockOnTips(coinbaseData *externalapi.DomainCoinbaseData, + transactions []*externalapi.DomainTransaction) (*externalapi.DomainHash, *externalapi.VirtualChangeSet, error) { + + tips, err := tc.Tips() + if err != nil { + return nil, nil, err + } + + return tc.AddBlock(tips, coinbaseData, transactions) +} + func (tc *testConsensus) AddUTXOInvalidHeader(parentHashes []*externalapi.DomainHash) (*externalapi.DomainHash, *externalapi.VirtualChangeSet, error) { diff --git a/domain/miningmanager/mempool/revalidate_high_priority_transactions.go b/domain/miningmanager/mempool/revalidate_high_priority_transactions.go index 820aa36e70..31e3ad046c 100644 --- a/domain/miningmanager/mempool/revalidate_high_priority_transactions.go +++ b/domain/miningmanager/mempool/revalidate_high_priority_transactions.go @@ -9,7 +9,6 @@ import ( func (mp *mempool) revalidateHighPriorityTransactions() ([]*externalapi.DomainTransaction, error) { type txNode struct { children map[externalapi.DomainTransactionID]struct{} - isInvalid bool nonVisitedParents int tx *model.MempoolTransaction visited bool @@ -31,7 +30,6 @@ func (mp *mempool) revalidateHighPriorityTransactions() ([]*externalapi.DomainTr node := &txNode{ children: make(map[externalapi.DomainTransactionID]struct{}), - isInvalid: false, nonVisitedParents: 0, tx: mp.transactionsPool.highPriorityTransactions[txID], } @@ -41,13 +39,7 @@ func (mp *mempool) revalidateHighPriorityTransactions() ([]*externalapi.DomainTr queue := make([]*txNode, 0, len(mp.transactionsPool.highPriorityTransactions)) for id, transaction := range mp.transactionsPool.highPriorityTransactions { - node := &txNode{ - children: make(map[externalapi.DomainTransactionID]struct{}), - isInvalid: false, - nonVisitedParents: 0, - tx: transaction, - } - txDAG[id] = node + node := maybeAddNode(id) parents := make(map[externalapi.DomainTransactionID]struct{}) for _, input := range transaction.Transaction().Inputs { @@ -76,9 +68,6 @@ func (mp *mempool) revalidateHighPriorityTransactions() ([]*externalapi.DomainTr continue } node.visited = true - if node.isInvalid { - continue - } transaction := node.tx isValid, err := mp.revalidateTransaction(transaction) @@ -86,24 +75,6 @@ func (mp *mempool) revalidateHighPriorityTransactions() ([]*externalapi.DomainTr return nil, err } - if !isValid { - // Invalidate the offspring of this transaction - invalidateQueue := []*txNode{node} - for len(invalidateQueue) > 0 { - var current *txNode - current, invalidateQueue = invalidateQueue[0], invalidateQueue[1:] - - if current.isInvalid { - continue - } - current.isInvalid = true - for child := range current.children { - invalidateQueue = append(invalidateQueue, txDAG[child]) - } - } - continue - } - for child := range node.children { childNode := txDAG[child] childNode.nonVisitedParents-- @@ -112,7 +83,9 @@ func (mp *mempool) revalidateHighPriorityTransactions() ([]*externalapi.DomainTr } } - validTransactions = append(validTransactions, transaction.Transaction().Clone()) + if isValid { + validTransactions = append(validTransactions, transaction.Transaction().Clone()) + } } return validTransactions, nil @@ -134,6 +107,11 @@ func (mp *mempool) revalidateTransaction(transaction *model.MempoolTransaction) return false, nil } + _, err = mp.validateAndInsertTransaction(transaction.Transaction(), false, false) + if err != nil { + return false, err + } + return true, nil } diff --git a/domain/miningmanager/miningmanager_test.go b/domain/miningmanager/miningmanager_test.go index d1b1b8e2e9..67074d9ea3 100644 --- a/domain/miningmanager/miningmanager_test.go +++ b/domain/miningmanager/miningmanager_test.go @@ -577,6 +577,76 @@ func TestRevalidateHighPriorityTransactions(t *testing.T) { }) } +func TestRevalidateHighPriorityTransactionsWithChain(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, "TestRevalidateHighPriorityTransactions") + if err != nil { + t.Fatalf("Failed setting up TestConsensus: %+v", err) + } + defer teardown(false) + + miningFactory := miningmanager.NewFactory() + mempoolConfig := mempool.DefaultConfig(&consensusConfig.Params) + tcAsConsensus := tc.(externalapi.Consensus) + tcAsConsensusPointer := &tcAsConsensus + consensusReference := consensusreference.NewConsensusReference(&tcAsConsensusPointer) + miningManager := miningFactory.NewMiningManager(consensusReference, &consensusConfig.Params, mempoolConfig) + + const chainSize = 10 + chain, err := createTxChain(tc, chainSize) + if err != nil { + t.Fatal(err) + } + + for i, transaction := range chain { + t.Logf("chain %d %s", i, consensushashing.TransactionID(transaction)) + } + + _, err = miningManager.ValidateAndInsertTransaction(chain[0], true, false) + if err != nil { + t.Fatal(err) + } + + blockHash, _, err := tc.AddBlockOnTips(nil, []*externalapi.DomainTransaction{chain[0].Clone()}) + if err != nil { + t.Fatal(err) + } + + block, _, err := tc.GetBlock(blockHash) + if err != nil { + t.Fatal(err) + } + + _, err = miningManager.HandleNewBlockTransactions(block.Transactions) + if err != nil { + t.Fatal(err) + } + + for _, transaction := range chain[1:] { + _, err = miningManager.ValidateAndInsertTransaction(transaction, true, false) + if err != nil { + t.Fatal(err) + } + } + + _, _, err = tc.AddBlockOnTips(nil, []*externalapi.DomainTransaction{chain[1].Clone()}) + if err != nil { + t.Fatal(err) + } + + revalidated, err := miningManager.RevalidateHighPriorityTransactions() + if err != nil { + t.Fatal(err) + } + + if len(revalidated) != chainSize-2 { + t.Fatalf("expected %d transactions to revalidate but instead only %d revalidated", chainSize-2, len(revalidated)) + } + }) +} + // TestModifyBlockTemplate verifies that modifying a block template changes coinbase data correctly. func TestModifyBlockTemplate(t *testing.T) { testutils.ForAllNets(t, true, func(t *testing.T, consensusConfig *consensus.Config) { @@ -904,40 +974,58 @@ func createArraysOfParentAndChildrenTransactions(tc testapi.TestConsensus) ([]*e func createParentAndChildrenTransactions(tc testapi.TestConsensus) (txParent *externalapi.DomainTransaction, txChild *externalapi.DomainTransaction, err error) { + chain, err := createTxChain(tc, 2) + if err != nil { + return nil, nil, err + } + + return chain[0], chain[1], nil +} + +func createTxChain(tc testapi.TestConsensus, numTxs int) ([]*externalapi.DomainTransaction, error) { // We will add two blocks by consensus before the parent transactions, in order to fund the parent transactions. tips, err := tc.Tips() if err != nil { - return nil, nil, err + return nil, err } _, _, err = tc.AddBlock(tips, nil, nil) if err != nil { - return nil, nil, errors.Wrapf(err, "AddBlock: %v", err) + return nil, errors.Wrapf(err, "AddBlock: %v", err) } tips, err = tc.Tips() if err != nil { - return nil, nil, err + return nil, err } fundingBlockHashForParent, _, err := tc.AddBlock(tips, nil, nil) if err != nil { - return nil, nil, errors.Wrap(err, "AddBlock: ") + return nil, errors.Wrap(err, "AddBlock: ") } fundingBlockForParent, _, err := tc.GetBlock(fundingBlockHashForParent) if err != nil { - return nil, nil, errors.Wrap(err, "GetBlock: ") + return nil, errors.Wrap(err, "GetBlock: ") } fundingTransactionForParent := fundingBlockForParent.Transactions[transactionhelper.CoinbaseTransactionIndex] - txParent, err = testutils.CreateTransaction(fundingTransactionForParent, 1000) + + transactions := make([]*externalapi.DomainTransaction, numTxs) + transactions[0], err = testutils.CreateTransaction(fundingTransactionForParent, 1000) if err != nil { - return nil, nil, err + return nil, err } - txChild, err = testutils.CreateTransaction(txParent, 1000) - if err != nil { - return nil, nil, err + + txParent := transactions[0] + for i := 1; i < numTxs; i++ { + transactions[i], err = testutils.CreateTransaction(txParent, 1000) + if err != nil { + return nil, err + } + + txParent = transactions[i] } - return txParent, txChild, nil + + return transactions, nil } func createChildAndParentTxsAndAddParentToConsensus(tc testapi.TestConsensus) (*externalapi.DomainTransaction, error) {