diff --git a/data/pools/transactionPool.go b/data/pools/transactionPool.go index e218efc37f..336162a8cd 100644 --- a/data/pools/transactionPool.go +++ b/data/pools/transactionPool.go @@ -98,8 +98,8 @@ type TransactionPool struct { // exceed the txPoolMaxSize. This flag is reset to false OnNewBlock stateproofOverflowed bool - testEvalCtx *testEvalContext - testEvalCtxMu deadlock.RWMutex + testBlockEvaluator *eval.TestBlockEvaluator + testBlockEvaluatorMu deadlock.RWMutex // shutdown is set to true when the pool is being shut down. It is checked in exported methods // to prevent pool operations like remember and recomputing the block evaluator @@ -109,7 +109,6 @@ type TransactionPool struct { // BlockEvaluator defines the block evaluator interface exposed by the ledger package. type BlockEvaluator interface { - TestTransactionGroup(txgroup []transactions.SignedTxn) error Round() basics.Round PaySetSize() int TransactionGroup(txads []transactions.SignedTxnWithAD) error @@ -118,7 +117,8 @@ type BlockEvaluator interface { ResetTxnBytes() } -// testEvalContext implements the eval.TestEvalContext interface. +// testEvalContext implements the eval.TestEvalContext interface. It allows for concurrent +// calls to TestTransactionGroup for candidate transactions calling transactionPool.Test. type testEvalContext struct { ledger *ledger.Ledger block bookkeeping.Block @@ -126,22 +126,24 @@ type testEvalContext struct { specials transactions.SpecialAddresses } -func newTestEvalCtx(ledger *ledger.Ledger, block bookkeeping.Block) *testEvalContext { - return &testEvalContext{ - ledger: ledger, - block: block, - proto: config.Consensus[block.CurrentProtocol], - specials: transactions.SpecialAddresses{ - FeeSink: block.FeeSink, - RewardsPool: block.RewardsPool, - }, - } +func newTestBlockEvaluator(ledger *ledger.Ledger, block bookkeeping.Block) *eval.TestBlockEvaluator { + return &eval.TestBlockEvaluator{ + TestEvalContext: &testEvalContext{ + ledger: ledger, + block: block, + proto: config.Consensus[block.CurrentProtocol], + specials: transactions.SpecialAddresses{ + FeeSink: block.FeeSink, + RewardsPool: block.RewardsPool, + }, + }} } func (c *testEvalContext) Proto() config.ConsensusParams { return c.proto } func (c *testEvalContext) Specials() transactions.SpecialAddresses { return c.specials } func (c *testEvalContext) TxnContext() transactions.TxnContext { return c.block } func (c *testEvalContext) CheckDup(firstValid, lastValid basics.Round, txid transactions.Txid, txl ledgercore.Txlease) error { + // will call txTail.checkDup, which uses an RLock for concurrent access. return c.ledger.CheckDup(c.proto, c.block.BlockHeader.Round, firstValid, lastValid, txid, txl) } @@ -421,19 +423,20 @@ func (pool *TransactionPool) checkSufficientFee(txgroup []transactions.SignedTxn // Test performs basic duplicate detection and well-formedness checks // on a transaction group without storing the group. +// It may be called concurrently. func (pool *TransactionPool) Test(txgroup []transactions.SignedTxn) error { if err := pool.checkPendingQueueSize(txgroup); err != nil { return err } - pool.testEvalCtxMu.RLock() - defer pool.testEvalCtxMu.RUnlock() + pool.testBlockEvaluatorMu.RLock() + defer pool.testBlockEvaluatorMu.RUnlock() - if pool.testEvalCtx == nil { + if pool.testBlockEvaluator == nil { return fmt.Errorf("Test: testEvalCtx is nil") } - return eval.TestTransactionGroup(pool.testEvalCtx, txgroup) + return pool.testBlockEvaluator.TestTransactionGroup(txgroup) } type poolIngestParams struct { @@ -780,9 +783,9 @@ func (pool *TransactionPool) recomputeBlockEvaluator(committedTxIDs map[transact return } - pool.testEvalCtxMu.Lock() - pool.testEvalCtx = newTestEvalCtx(pool.ledger, next) - pool.testEvalCtxMu.Unlock() + pool.testBlockEvaluatorMu.Lock() + pool.testBlockEvaluator = newTestBlockEvaluator(pool.ledger, next) + pool.testBlockEvaluatorMu.Unlock() var asmStats telemetryspec.AssembleBlockMetrics asmStats.StartCount = len(txgroups) diff --git a/ledger/eval/eval.go b/ledger/eval/eval.go index 3a7ad1bd8b..5a1b7feb4d 100644 --- a/ledger/eval/eval.go +++ b/ledger/eval/eval.go @@ -932,12 +932,19 @@ func (eval *BlockEvaluator) CheckDup(firstValid, lastValid basics.Round, txid tr // on a transaction group, but does not actually add the transactions to the block // evaluator, or modify the block evaluator state in any other visible way. func (eval *BlockEvaluator) TestTransactionGroup(txgroup []transactions.SignedTxn) error { - return TestTransactionGroup(eval, txgroup) + return TestBlockEvaluator{eval}.TestTransactionGroup(txgroup) +} + +// TestBlockEvaluator uses a TestEvalContext to perform basic checks on transactions. +type TestBlockEvaluator struct { + TestEvalContext } // TestTransactionGroup performs basic duplicate detection and well-formedness checks -// on a transaction group. -func TestTransactionGroup(eval TestEvalContext, txgroup []transactions.SignedTxn) error { +// on a transaction group. It is passed a TestEvalContext, which prevents it from adding +// the transactions to the block evaluator, or modifying the block evaluator state in any +// other visible way. +func (eval TestBlockEvaluator) TestTransactionGroup(txgroup []transactions.SignedTxn) error { // Nothing to do if there are no transactions. if len(txgroup) == 0 { return nil @@ -952,7 +959,7 @@ func TestTransactionGroup(eval TestEvalContext, txgroup []transactions.SignedTxn var group transactions.TxGroup for gi, txn := range txgroup { - err := TestTransaction(eval, txn) + err := eval.TestTransaction(txn) if err != nil { return err } @@ -993,16 +1000,9 @@ func TestTransactionGroup(eval TestEvalContext, txgroup []transactions.SignedTxn return nil } -// TestTransaction performs basic duplicate detection and well-formedness checks -// on a single transaction, but does not actually add the transaction to the block -// evaluator, or modify the block evaluator state in any other visible way. -func (eval *BlockEvaluator) TestTransaction(txn transactions.SignedTxn) error { - return TestTransaction(eval, txn) -} - // TestTransaction performs basic duplicate detection and well-formedness checks // on a single transaction. -func TestTransaction(eval TestEvalContext, txn transactions.SignedTxn) error { +func (eval TestBlockEvaluator) TestTransaction(txn transactions.SignedTxn) error { // Transaction valid (not expired)? err := txn.Txn.Alive(eval.TxnContext()) if err != nil { @@ -1016,6 +1016,7 @@ func TestTransaction(eval TestEvalContext, txn transactions.SignedTxn) error { } // Transaction already in the ledger? + // BlockEvaluator.transaction will check again using cow.checkDup later, if the pool tries to add this transaction to the block. txid := txn.ID() err = eval.CheckDup(txn.Txn.First(), txn.Txn.Last(), txid, ledgercore.Txlease{Sender: txn.Txn.Sender, Lease: txn.Txn.Lease}) if err != nil { @@ -1199,6 +1200,7 @@ func (eval *BlockEvaluator) transaction(txn transactions.SignedTxn, evalParams * } // Transaction already in the ledger? + // this checks against the txns added to this evaluator; testTransaction currently only checks against committed txns. err = cow.checkDup(txn.Txn.First(), txn.Txn.Last(), txid, ledgercore.Txlease{Sender: txn.Txn.Sender, Lease: txn.Txn.Lease}) if err != nil { return err