mempool: inline call to has (#646)

ava-labs · Sep 5, 2024 · b34bbde · b34bbde
1 parent d1615a9
commit b34bbde
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 34 deletions.
diff --git a/plugin/evm/gossiper_atomic_gossiping_test.go b/plugin/evm/gossiper_atomic_gossiping_test.go
@@ -71,7 +71,7 @@ func TestMempoolAtmTxsAppGossipHandling(t *testing.T) {
 	txGossipedLock.Lock()
 	assert.Equal(0, txGossiped, "tx should not have been gossiped")
 	txGossipedLock.Unlock()
-	assert.True(vm.mempool.has(tx.ID()))
+	assert.True(vm.mempool.Has(tx.ID()))
 
 	vm.ctx.Lock.Unlock()
 
@@ -101,7 +101,7 @@ func TestMempoolAtmTxsAppGossipHandling(t *testing.T) {
 	txGossipedLock.Lock()
 	assert.Equal(0, txGossiped, "tx should not have been gossiped")
 	txGossipedLock.Unlock()
-	assert.False(vm.mempool.has(conflictingTx.ID()), "conflicting tx should not be in the atomic mempool")
+	assert.False(vm.mempool.Has(conflictingTx.ID()), "conflicting tx should not be in the atomic mempool")
 }
 
 // show that txs already marked as invalid are not re-requested on gossiping
@@ -142,7 +142,7 @@ func TestMempoolAtmTxsAppGossipHandlingDiscardedTx(t *testing.T) {
 	mempool.DiscardCurrentTx(txID)
 
 	// Check the mempool does not contain the discarded transaction
-	assert.False(mempool.has(txID))
+	assert.False(mempool.Has(txID))
 
 	// Gossip the transaction to the VM and ensure that it is not added to the mempool
 	// and is not re-gossipped.
@@ -164,7 +164,7 @@ func TestMempoolAtmTxsAppGossipHandlingDiscardedTx(t *testing.T) {
 	assert.Zero(txGossiped, "tx should not have been gossiped")
 	txGossipedLock.Unlock()
 
-	assert.False(mempool.has(txID))
+	assert.False(mempool.Has(txID))
 
 	vm.ctx.Lock.Unlock()
 
@@ -182,6 +182,6 @@ func TestMempoolAtmTxsAppGossipHandlingDiscardedTx(t *testing.T) {
 	assert.Equal(1, txGossiped, "conflicting tx should have been gossiped")
 	txGossipedLock.Unlock()
 
-	assert.False(mempool.has(txID))
-	assert.True(mempool.has(conflictingTx.ID()))
+	assert.False(mempool.Has(txID))
+	assert.True(mempool.Has(conflictingTx.ID()))
 }
diff --git a/plugin/evm/mempool.go b/plugin/evm/mempool.go
@@ -115,13 +115,6 @@ func (m *Mempool) length() int {
 	return m.txHeap.Len() + len(m.issuedTxs)
 }
 
-// has indicates if a given [txID] is in the mempool and has not been
-// discarded.
-func (m *Mempool) has(txID ids.ID) bool {
-	_, dropped, found := m.GetTx(txID)
-	return found && !dropped
-}
-
 // atomicTxGasPrice is the [gasPrice] paid by a transaction to burn a given
 // amount of [AVAXAssetID] given the value of [gasUsed].
 func (m *Mempool) atomicTxGasPrice(tx *Tx) (uint64, error) {
@@ -430,10 +423,8 @@ func (m *Mempool) GetTx(txID ids.ID) (*Tx, bool, bool) {
 
 // Has returns true if the mempool contains [txID] or it was issued.
 func (m *Mempool) Has(txID ids.ID) bool {
-	m.lock.RLock()
-	defer m.lock.RUnlock()
-
-	return m.has(txID)
+	_, dropped, found := m.GetTx(txID)
+	return found && !dropped
 }
 
 // IssueCurrentTx marks [currentTx] as issued if there is one

diff --git a/plugin/evm/mempool_atomic_gossiping_test.go b/plugin/evm/mempool_atomic_gossiping_test.go
@@ -47,18 +47,18 @@ func TestMempoolAddLocallyCreateAtomicTx(t *testing.T) {
 			// add a tx to the mempool
 			err := vm.mempool.AddLocalTx(tx)
 			assert.NoError(err)
-			has := mempool.has(txID)
+			has := mempool.Has(txID)
 			assert.True(has, "valid tx not recorded into mempool")
 
 			// try to add a conflicting tx
 			err = vm.mempool.AddLocalTx(conflictingTx)
 			assert.ErrorIs(err, errConflictingAtomicTx)
-			has = mempool.has(conflictingTxID)
+			has = mempool.Has(conflictingTxID)
 			assert.False(has, "conflicting tx in mempool")
 
 			<-issuer
 
-			has = mempool.has(txID)
+			has = mempool.Has(txID)
 			assert.True(has, "valid tx not recorded into mempool")
 
 			// Show that BuildBlock generates a block containing [txID] and that it is
@@ -71,7 +71,7 @@ func TestMempoolAddLocallyCreateAtomicTx(t *testing.T) {
 
 			assert.Equal(txID, evmBlk.atomicTxs[0].ID(), "block does not include expected transaction")
 
-			has = mempool.has(txID)
+			has = mempool.Has(txID)
 			assert.True(has, "tx should stay in mempool until block is accepted")
 
 			err = blk.Verify(context.Background())
@@ -80,7 +80,7 @@ func TestMempoolAddLocallyCreateAtomicTx(t *testing.T) {
 			err = blk.Accept(context.Background())
 			assert.NoError(err)
 
-			has = mempool.has(txID)
+			has = mempool.Has(txID)
 			assert.False(has, "tx shouldn't be in mempool after block is accepted")
 		})
 	}
@@ -105,13 +105,13 @@ func TestMempoolMaxMempoolSizeHandling(t *testing.T) {
 	mempool.maxSize = 0
 
 	assert.ErrorIs(mempool.AddTx(tx), errTooManyAtomicTx)
-	assert.False(mempool.has(tx.ID()))
+	assert.False(mempool.Has(tx.ID()))
 
 	// shortcut to simulated empty mempool
 	mempool.maxSize = defaultMempoolSize
 
 	assert.NoError(mempool.AddTx(tx))
-	assert.True(mempool.has(tx.ID()))
+	assert.True(mempool.Has(tx.ID()))
 }
 
 // mempool will drop transaction with the lowest fee
@@ -136,22 +136,22 @@ func TestMempoolPriorityDrop(t *testing.T) {
 		t.Fatal(err)
 	}
 	assert.NoError(mempool.AddTx(tx1))
-	assert.True(mempool.has(tx1.ID()))
+	assert.True(mempool.Has(tx1.ID()))
 
 	tx2, err := vm.newImportTx(vm.ctx.XChainID, testEthAddrs[1], initialBaseFee, []*secp256k1.PrivateKey{testKeys[1]})
 	if err != nil {
 		t.Fatal(err)
 	}
 	assert.ErrorIs(mempool.AddTx(tx2), errInsufficientAtomicTxFee)
-	assert.True(mempool.has(tx1.ID()))
-	assert.False(mempool.has(tx2.ID()))
+	assert.True(mempool.Has(tx1.ID()))
+	assert.False(mempool.Has(tx2.ID()))
 
 	tx3, err := vm.newImportTx(vm.ctx.XChainID, testEthAddrs[1], new(big.Int).Mul(initialBaseFee, big.NewInt(2)), []*secp256k1.PrivateKey{testKeys[1]})
 	if err != nil {
 		t.Fatal(err)
 	}
 	assert.NoError(mempool.AddTx(tx3))
-	assert.False(mempool.has(tx1.ID()))
-	assert.False(mempool.has(tx2.ID()))
-	assert.True(mempool.has(tx3.ID()))
+	assert.False(mempool.Has(tx1.ID()))
+	assert.False(mempool.Has(tx2.ID()))
+	assert.True(mempool.Has(tx3.ID()))
 }
diff --git a/plugin/evm/tx_gossip_test.go b/plugin/evm/tx_gossip_test.go
@@ -590,5 +590,5 @@ func TestAtomicTxPushGossipInbound(t *testing.T) {
 	inboundGossipMsg := append(binary.AppendUvarint(nil, atomicTxGossipProtocol), inboundGossipBytes...)
 
 	require.NoError(vm.AppGossip(ctx, ids.EmptyNodeID, inboundGossipMsg))
-	require.True(vm.mempool.has(tx.ID()))
+	require.True(vm.mempool.Has(tx.ID()))
 }
diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go
@@ -1102,9 +1102,9 @@ func TestReissueAtomicTxHigherGasPrice(t *testing.T) {
 				t.Fatalf("Expected to fail with err: %s, but found err: %s", errConflictingAtomicTx, err)
 			}
 
-			assert.True(t, vm.mempool.has(importTx1.ID()))
-			assert.True(t, vm.mempool.has(importTx2.ID()))
-			assert.False(t, vm.mempool.has(reissuanceTx1.ID()))
+			assert.True(t, vm.mempool.Has(importTx1.ID()))
+			assert.True(t, vm.mempool.Has(importTx2.ID()))
+			assert.False(t, vm.mempool.Has(reissuanceTx1.ID()))
 
 			reissuanceTx2, err := vm.newImportTxWithUTXOs(vm.ctx.XChainID, testEthAddrs[0], new(big.Int).Mul(big.NewInt(4), initialBaseFee), kc, []*avax.UTXO{utxo1, utxo2})
 			if err != nil {