Skip to content

Commit

Permalink
[EVM] Avoid returning nil transactions on ForEach (#197)
Browse files Browse the repository at this point in the history
* remove heapIndex to avoid nil scenario

* avoid returning nil in loop (mimic Peek)
  • Loading branch information
stevenlanders authored and udpatil committed Mar 26, 2024
1 parent f75db54 commit 12f4a52
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
15 changes: 9 additions & 6 deletions internal/mempool/priority_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,11 @@ func (pq *TxPriorityQueue) popTxUnsafe() *WrappedTx {
}
tx := x.(*WrappedTx)

// this situation is primarily for a test case that inserts nils
if tx == nil {
return nil
}

// non-evm transactions do not have txs waiting on a nonce
if !tx.isEVM {
return tx
Expand Down Expand Up @@ -338,6 +343,9 @@ func (pq *TxPriorityQueue) ForEachTx(handler func(wtx *WrappedTx) bool) {

for i := 0; i < numTxs; i++ {
popped := pq.popTxUnsafe()
if popped == nil {
break
}
txs = append(txs, popped)
if !handler(popped) {
return
Expand Down Expand Up @@ -377,9 +385,7 @@ func (pq *TxPriorityQueue) PeekTxs(max int) []*WrappedTx {
//
// NOTE: A caller should never call Push. Use PushTx instead.
func (pq *TxPriorityQueue) Push(x interface{}) {
n := len(pq.txs)
item := x.(*WrappedTx)
item.heapIndex = n
pq.txs = append(pq.txs, item)
}

Expand All @@ -390,8 +396,7 @@ func (pq *TxPriorityQueue) Pop() interface{} {
old := pq.txs
n := len(old)
item := old[n-1]
old[n-1] = nil // avoid memory leak
item.heapIndex = -1 // for safety
old[n-1] = nil // avoid memory leak
pq.txs = old[0 : n-1]
return item
}
Expand Down Expand Up @@ -420,6 +425,4 @@ func (pq *TxPriorityQueue) Less(i, j int) bool {
// Swap implements the Heap interface. It swaps two transactions in the queue.
func (pq *TxPriorityQueue) Swap(i, j int) {
pq.txs[i], pq.txs[j] = pq.txs[j], pq.txs[i]
pq.txs[i].heapIndex = i
pq.txs[j].heapIndex = j
}
14 changes: 14 additions & 0 deletions internal/mempool/priority_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,20 @@ func TestTxPriorityQueue_ReapHalf(t *testing.T) {
}
}

func TestAvoidPanicIfTransactionIsNil(t *testing.T) {
pq := NewTxPriorityQueue()
pq.Push(&WrappedTx{sender: "1", isEVM: true, evmAddress: "0xabc", evmNonce: 1, priority: 10})
pq.txs = append(pq.txs, nil)

var count int
pq.ForEachTx(func(tx *WrappedTx) bool {
count++
return true
})

require.Equal(t, 1, count)
}

func TestTxPriorityQueue_PriorityAndNonceOrdering(t *testing.T) {
testCases := []TxTestCase{
{
Expand Down

0 comments on commit 12f4a52

Please sign in to comment.