add TestBlockEvaluator for methods to hang off of

Author: cce

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,30 +117,33 @@ 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
 	proto    config.ConsensusParams
 	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)

ledger/eval/eval.go

@@ -904,9 +904,8 @@ func (eval *BlockEvaluator) ResetTxnBytes() {
 	eval.blockTxBytes = 0
 }
 
-// TestEvalContext defines the evaluation context required to check for
-// well-formedness and duplicate detection used by TestTransactionGroup
-// and TestTransaction.
+// TestEvalContext defines the evaluation context required by TestBlockEvaluator
+// to check for well-formedness and duplicate detection.
 type TestEvalContext interface {
 	Proto() config.ConsensusParams
 	Specials() transactions.SpecialAddresses
@@ -928,16 +927,19 @@ func (eval *BlockEvaluator) CheckDup(firstValid, lastValid basics.Round, txid tr
 	return eval.state.checkDup(firstValid, lastValid, txid, txl)
 }
 
-// TestTransactionGroup performs basic duplicate detection and well-formedness checks
-// 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.
+// TestTransactionGroup is only called by tests.
 func (eval *BlockEvaluator) TestTransactionGroup(txgroup []transactions.SignedTxn) error {
-	return TestTransactionGroup(eval, txgroup)
+	return TestBlockEvaluator{eval}.TestTransactionGroup(txgroup)
 }
 
+// TestBlockEvaluator uses a TestEvalContext to perform basic transaction checks.
+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, but does not actually add the transactions to the block
+// evaluator, or modify the block evaluator state in any other visible way.
+// It uses a TestEvalContext to access needed recent ledger state.
+func (eval TestBlockEvaluator) TestTransactionGroup(txgroup []transactions.SignedTxn) error {
 	// Nothing to do if there are no transactions.
 	if len(txgroup) == 0 {
 		return nil
@@ -952,7 +954,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
 		}
@@ -996,13 +998,7 @@ func TestTransactionGroup(eval TestEvalContext, txgroup []transactions.SignedTxn
 // 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 {
@@ -1017,6 +1013,7 @@ func TestTransaction(eval TestEvalContext, txn transactions.SignedTxn) error {
 
 	// Transaction already in the ledger?
 	txid := txn.ID()
+	// BlockEvaluator.transaction will check again using cow.checkDup later, if the pool tries to add this transaction to the block.
 	err = eval.CheckDup(txn.Txn.First(), txn.Txn.Last(), txid, ledgercore.Txlease{Sender: txn.Txn.Sender, Lease: txn.Txn.Lease})
 	if err != nil {
 		return err
@@ -1199,6 +1196,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