diff --git a/internal/mempool/mempool.go b/internal/mempool/mempool.go index 90ef114c5..b49f3ace5 100644 --- a/internal/mempool/mempool.go +++ b/internal/mempool/mempool.go @@ -736,7 +736,7 @@ func (txmp *TxMempool) handleRecheckResult(tx types.Tx, res *abci.ResponseCheckT // Only evaluate transactions that have not been removed. This can happen // if an existing transaction is evicted during CheckTx and while this // callback is being executed for the same evicted transaction. - if !txmp.txStore.IsTxRemoved(wtx.hash) { + if !txmp.txStore.IsTxRemoved(wtx) { var err error if txmp.postCheck != nil { err = txmp.postCheck(tx, res.ResponseCheckTx) @@ -806,7 +806,7 @@ func (txmp *TxMempool) updateReCheckTxs(ctx context.Context) { // Only execute CheckTx if the transaction is not marked as removed which // could happen if the transaction was evicted. - if !txmp.txStore.IsTxRemoved(wtx.hash) { + if !txmp.txStore.IsTxRemoved(wtx) { res, err := txmp.proxyAppConn.CheckTx(ctx, &abci.RequestCheckTx{ Tx: wtx.tx, Type: abci.CheckTxType_Recheck, @@ -893,7 +893,7 @@ func (txmp *TxMempool) insertTx(wtx *WrappedTx) bool { } func (txmp *TxMempool) removeTx(wtx *WrappedTx, removeFromCache bool, shouldReenqueue bool, updatePriorityIndex bool) { - if txmp.txStore.IsTxRemoved(wtx.hash) { + if txmp.txStore.IsTxRemoved(wtx) { return } diff --git a/internal/mempool/tx.go b/internal/mempool/tx.go index 9366ac7fa..fdb0ac91c 100644 --- a/internal/mempool/tx.go +++ b/internal/mempool/tx.go @@ -147,15 +147,22 @@ func (txs *TxStore) GetTxByHash(hash types.TxKey) *WrappedTx { // IsTxRemoved returns true if a transaction by hash is marked as removed and // false otherwise. -func (txs *TxStore) IsTxRemoved(hash types.TxKey) bool { +func (txs *TxStore) IsTxRemoved(wtx *WrappedTx) bool { txs.mtx.RLock() defer txs.mtx.RUnlock() - wtx, ok := txs.hashTxs[hash] + // if this instance has already been marked, return true + if wtx.removed == true { + return true + } + + // otherwise if the same hash exists, return its state + wtx, ok := txs.hashTxs[wtx.hash] if ok { return wtx.removed } + // otherwise we haven't seen this tx return false } diff --git a/internal/mempool/tx_test.go b/internal/mempool/tx_test.go index 152730b33..0703acb5d 100644 --- a/internal/mempool/tx_test.go +++ b/internal/mempool/tx_test.go @@ -76,6 +76,80 @@ func TestTxStore_SetTx(t *testing.T) { require.Equal(t, wtx, res) } +func TestTxStore_IsTxRemoved(t *testing.T) { + // Initialize the store + txs := NewTxStore() + + // Current time for timestamping transactions + now := time.Now() + + // Tests setup as a slice of anonymous structs + tests := []struct { + name string + wtx *WrappedTx + setup func(*TxStore, *WrappedTx) // Optional setup function to manipulate store state + wantRemoved bool + }{ + { + name: "Existing transaction not removed", + wtx: &WrappedTx{ + tx: types.Tx("tx_hash_1"), + hash: types.Tx("tx_hash_1").Key(), + removed: false, + timestamp: now, + }, + setup: func(ts *TxStore, w *WrappedTx) { + ts.SetTx(w) + }, + wantRemoved: false, + }, + { + name: "Existing transaction marked as removed", + wtx: &WrappedTx{ + tx: types.Tx("tx_hash_2"), + hash: types.Tx("tx_hash_2").Key(), + removed: true, + timestamp: now, + }, + setup: func(ts *TxStore, w *WrappedTx) { + ts.SetTx(w) + }, + wantRemoved: true, + }, + { + name: "Non-existing transaction", + wtx: &WrappedTx{ + tx: types.Tx("tx_hash_3"), + hash: types.Tx("tx_hash_3").Key(), + removed: false, + timestamp: now, + }, + wantRemoved: false, + }, + { + name: "Non-existing transaction but marked as removed", + wtx: &WrappedTx{ + tx: types.Tx("tx_hash_4"), + hash: types.Tx("tx_hash_4").Key(), + removed: true, + timestamp: now, + }, + wantRemoved: true, + }, + } + + // Execute test scenarios + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setup != nil { + tt.setup(txs, tt.wtx) + } + removed := txs.IsTxRemoved(tt.wtx) + require.Equal(t, tt.wantRemoved, removed) + }) + } +} + func TestTxStore_GetOrSetPeerByTxHash(t *testing.T) { txs := NewTxStore() wtx := &WrappedTx{