Open
Conversation
Greptile SummaryThis PR updates the Key changes:
One memory leak was identified in Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant RecheckMempool
participant CosmosTxStore
participant LegacyTxStore
participant ExtMempool
Note over Caller,ExtMempool: Replacement tx insert flow
Caller->>RecheckMempool: Insert(replacementTx)
RecheckMempool->>RecheckMempool: ante handler check
RecheckMempool->>ExtMempool: Insert(replacementTx)
Note over ExtMempool: replaces old tx internally
RecheckMempool->>RecheckMempool: markTxInserted(replacementTx)
RecheckMempool->>CosmosTxStore: InvalidateFrom(replacementTx)
CosmosTxStore->>CosmosTxStore: lookup key in s.keys (signer/nonce)
alt key exists (replacement)
CosmosTxStore->>CosmosTxStore: build nonce thresholds map
CosmosTxStore->>CosmosTxStore: filter out all txs with nonce >= threshold
CosmosTxStore->>CosmosTxStore: reindex remaining txs
CosmosTxStore-->>RecheckMempool: removed > 0
Note over RecheckMempool: replacement NOT added to snapshot yet
else key missing (fresh insert)
CosmosTxStore-->>RecheckMempool: 0
RecheckMempool->>CosmosTxStore: AddTx(tx)
end
Note over Caller,ExtMempool: Legacypool replacement path
Caller->>LegacyTxStore: RemoveTx(addr, oldTx)
LegacyTxStore->>LegacyTxStore: RemoveTxsFromNonce(addr, oldTx.Nonce())
LegacyTxStore->>LegacyTxStore: filter txs with nonce >= minNonce
LegacyTxStore->>LegacyTxStore: delete from lookup map
LegacyTxStore->>LegacyTxStore: total -= numRemoved
Note over Caller,ExtMempool: Next recheck rebuilds snapshot
Caller->>RecheckMempool: TriggerRecheckSync(newHead)
RecheckMempool->>CosmosTxStore: StartNewHeight (clears store)
loop each tx in ExtMempool
RecheckMempool->>RecheckMempool: RecheckCosmos(tx)
RecheckMempool->>CosmosTxStore: markTxRechecked (AddTx)
end
CosmosTxStore-->>Caller: recheckedTxs now includes replacement
Last reviewed commit: 64c1ccf |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…-snapshot-invalidation
technicallyty
commented
Mar 12, 2026
| require.Equal(t, uint64(1), result[addr1][0].Tx.Nonce()) | ||
| } | ||
|
|
||
| func TestTxStoreConcurrentRemove(t *testing.T) { |
Contributor
Author
There was a problem hiding this comment.
not sure if this test makes sense to have anymore
technicallyty
commented
Mar 12, 2026
|
|
||
| result := store.Txs(txpool.PendingFilter{}) | ||
| require.Len(t, result[addr1], 500) | ||
| require.Len(t, result[addr1], 0) |
Contributor
Author
There was a problem hiding this comment.
updating this bc i think the behavior we want now is if a tx is removed, all txs with a greater nonce should now be considered invalid and removed as well. they cannot be included in a block proposal.
Contributor
Author
|
@greptile re-review |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1074 +/- ##
==========================================
+ Coverage 65.45% 65.54% +0.09%
==========================================
Files 331 331
Lines 23262 23346 +84
==========================================
+ Hits 15225 15303 +78
+ Misses 6894 6880 -14
- Partials 1143 1163 +20
🚀 New features to boost your workflow:
|
Contributor
|
@greptile re-review |
Comment on lines
+153
to
170
| next := txs[:0] | ||
| numRemoved := 0 | ||
| for _, existing := range txs { | ||
| if existing.Nonce() >= minNonce { | ||
| delete(t.lookup, existing.Hash()) | ||
| numRemoved++ | ||
| continue | ||
| } | ||
| next = append(next, existing) | ||
| } | ||
| t.total -= uint64(numRemoved) | ||
|
|
||
| if len(next) == 0 { | ||
| delete(t.txs, addr) | ||
| return | ||
| } | ||
| t.txs[addr] = next | ||
| } |
There was a problem hiding this comment.
Memory leak: removed pointers retained in backing array
next := txs[:0] reuses the same backing array as txs. After the in-place filter loop, positions [len(next):len(txs)] of that array still hold references to the removed *types.Transaction pointers. Because t.txs[addr] = next keeps the same underlying array alive (with cap == len(txs)), those pointer slots are never collected by the GC — they stay pinned for as long as the address has any txs in the map.
For example: if the store holds 6 txs for an address and a replacement at nonce 4 causes txs 4–6 to be removed, the backing array still holds 3 live *types.Transaction references that the GC can never reclaim.
Zero out the tail before returning to release those references:
Suggested change
| next := txs[:0] | |
| numRemoved := 0 | |
| for _, existing := range txs { | |
| if existing.Nonce() >= minNonce { | |
| delete(t.lookup, existing.Hash()) | |
| numRemoved++ | |
| continue | |
| } | |
| next = append(next, existing) | |
| } | |
| t.total -= uint64(numRemoved) | |
| if len(next) == 0 { | |
| delete(t.txs, addr) | |
| return | |
| } | |
| t.txs[addr] = next | |
| } | |
| next := txs[:0] | |
| numRemoved := 0 | |
| for _, existing := range txs { | |
| if existing.Nonce() >= minNonce { | |
| delete(t.lookup, existing.Hash()) | |
| numRemoved++ | |
| continue | |
| } | |
| next = append(next, existing) | |
| } | |
| // Zero out the dead tail so the GC can collect the removed tx pointers. | |
| tail := txs[len(next):] | |
| for i := range tail { | |
| tail[i] = nil | |
| } | |
| t.total -= uint64(numRemoved) | |
| if len(next) == 0 { | |
| delete(t.txs, addr) | |
| return | |
| } | |
| t.txs[addr] = next |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
updates the TxStore to handle tx replacements. txs that get replaced as a result of an insert should invalidate all same-sender txs with a higher nonce value, as they were dependent on the original tx's execution.
Closes: STACK-2479
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
mainbranch