[amir-deris]

Summary

• Wire tendermint_mempool_pending_nonce OTel counter at the actual pending-nonce lifecycle points in sei-tendermint/internal/mempool/tx.go (added, expired, rejected, accepted)
• Remove the dead evm_pending_nonce counter from x/evm/ante/metrics.go (defined but never incremented since Removed callbacks from TxMempool #3410 moved pending-nonce tracking into the mempool)

Closes PLT-1017 (sei-chain side).

Related PR

• Dashboards: sei-protocol/platform#1564 — updates App/EVM Monitoring (OTel) panels to query sei_chain_tendermint_mempool_pending_nonce_total

Deploy and merge this PR before (or together with) the platform PR so the new series exists when dashboards roll out.

[github-actions]

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	✅ passed	✅ passed	✅ passed	Aug 24, 2026, 3:56 PM

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.96%. Comparing base (776d565) to head (56d97bb).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3991      +/-   ##
==========================================
- Coverage   59.08%   57.96%   -1.12%     
==========================================
  Files        2305     2198     -107     
  Lines      197021   184825   -12196     
==========================================
- Hits       116402   107143    -9259     
+ Misses      69858    67895    -1963     
+ Partials    10761     9787     -974     

Flag	Coverage Δ
sei-chain-pr	83.40% <100.00%> (?)
sei-db	69.80% <ø> (ø)
sei-db-state-db	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
sei-tendermint/internal/mempool/metrics.go	100.00% <100.00%> (ø)
sei-tendermint/internal/mempool/tx.go	92.06% <100.00%> (+0.23%)	⬆️
x/evm/ante/metrics.go	50.00% <ø> (ø)

... and 174 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 668bd1d. Configure here.}

[seidroid]

Moves the pending-nonce counter from the dead evm_pending_nonce in x/evm/ante to real lifecycle points in the mempool, which is the right place for it; the removal side is clean and the insert signature change is correctly threaded through both callers. Remaining concerns are all metric-accuracy and readability: the accepted suppression condition leans on loop-variable shadowing with a redundant third term, rejected is not gated by recordAdded (so per-block compaction evictions land in it) while duplicate/same-nonce rejections are never counted, added fires before the mempool-full eviction check, and no test pins any of it.

Findings: 0 blocking | 4 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• [suggestion] No test pins the new counter semantics, and the interesting part of this change is exactly the conditional suppression (recordAdded, the wtx == newTx identity check, per-block compact() re-entry). A characterization test over an sdk/metric manual reader — insert a ready tx, insert a gapped pending tx then its filler, expire one, feed an old nonce, and assert the four label counts — would fix the intended behavior in a diff rather than leaving it implied by the condition. There is precedent for that harness in the repo (ratelimiter/registry_test.go, sei-tendermint/rpc/jsonrpc/server/rate_limit_middleware_test.go).
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] This suppression condition is hard to verify and the insertedAtNextNonce term is either redundant or wrong. Inside the loop wtx is the shadowed loop variable, so wtx == newTx already uniquely identifies the newly inserted transaction — and by the store's invariant (the previous insert advanced nextNonce as far as the byNonce chain and balances allow) the new transaction can only be reached by this loop when its nonce equals the pre-insert nextNonce. So the extra term never changes the outcome today; if a path ever did reach newTx at a higher nonce, it would emit both added (line 451) and accepted for a single insertion.

Suggest dropping insertedAtNextNonce (and the newTx alias, which only exists to work around the shadowing) and instead deciding after the loop from newTx.readyEl: record added unconditionally, and accepted for promotions of transactions other than the one just inserted. That reads as two named steps rather than a triple-negated condition that depends on loop-variable shadowing.

[seidroid]

[suggestion] Unlike added and accepted, rejected is not gated by recordAdded, so it also fires from compact() — which Update runs every block with clearAccounts: true, re-reading firstNonce from app state. A queued transaction whose nonce was consumed on chain by some other means is then re-inserted, hits errOldNonce, and is counted as a rejection even though it is really an eviction of a stale entry. Gating this on recordAdded (or adding a distinct label for the compaction path) would keep the series meaning "incoming transaction rejected".

Conversely — and as Codex noted — this is the only rejection path counted: errDuplicateTx (duplicate EVM hash) and errSameNonce (replacement lost on balance or priority) return without incrementing anything, so same-nonce replacement rejections, arguably the most interesting pending-nonce rejection, never appear on the dashboard. Worth either covering them with the same label or documenting that rejected means "nonce below account nonce" only.

[seidroid]

[suggestion] added is recorded here, before Insert checks the hard limit and runs the overflow compact() (lines 516-523). When that compaction evicts the transaction just inserted, Insert returns errMempoolFull but the added increment has already landed, so the series counts transactions that never entered the mempool. Recording added in Insert after the hard-limit check (rather than inside insert) would avoid that; it would also drop the need to thread recordAdded through for this event.

More generally, the four labels are not a closed lifecycle: added fires for every EVM insert including ones that go straight to ready, while accepted deliberately excludes those, so added - accepted - expired - rejected is not pending-queue depth. Since the linked dashboard PR graphs this series, please state the per-label meaning in the metric description in metrics.go so the panels are not derived from an assumed conservation law.

[cursor]

PR Summary

Low Risk
Observability-only: increments OTel counters at existing mempool insert/update paths and removes a dead metric. No consensus or tx-handling behavior change.

Overview
Wires tendermint_mempool_pending_nonce so EVM pending-nonce lifecycle is actually counted in the mempool (added, expired, rejected, accepted).

insert now takes recordAdded so compact/rebuild does not double-count adds. Rejects on old nonce, expiry removals, and later nonce-gap fills increment the matching event. User inserts of EVM txs record added; becoming ready is not counted as accepted when the tx was already at nextNonce.

Removes the unused evm_pending_nonce counter from EVM ante metrics (never incremented after pending-nonce tracking moved into the mempool). Dashboards should query the mempool series instead.

^{Reviewed by Cursor Bugbot for commit 56d97bb. Bugbot is set up for automated code reviews on this repo. Configure here.}

[bdchatham]

Any concern with this going away? Any alerts?

[amir-deris]

I believe there shouldn't be any concern, as this metric was already not being emitted.

The metric was already dead
On main, evm_pending_nonce in x/evm/ante/metrics.go is defined but never incremented. Nothing in x/evm calls evmAnteMetrics.pendingNonce:

sig.go → nonceMismatch
fee.go → effectiveGasPrice
preprocess.go → associationError
Instrumentation moved out of ante when #3410 removed mempool callbacks. At the same time, app/ante/evm_checktx.go stopped emitting app_pending_nonce (that metric definition was removed too). Pending-nonce lifecycle now lives entirely in the Tendermint mempool (tx.go), which PR 3991 wires up.

So removing the dead definition is cleanup, not a behavioral change — production has not been emitting either old series since #3410.