feat(mempool): support direct insert tx via app mempool

[JayT106]

Under mempool.type=app, EVM RPC txs now insert straight into the app mempool instead of going through BroadcastTx -> CheckTx (which returns an empty response and spawns a goroutine per tx). Uses ethermint's appmempool.MempoolClient (crypto-org-chain/ethermint#1016).

Why:

• Correct sync response for eth_sendRawTransaction, eth_sendTransaction, and etherbase, plus back-pressure (CodeTypeRetry when full).
• RPC and p2p gossip share one admission path. Addresses feat(app): v0.54 upgrade perf optimizations + app-mempool feature #2091 review r3361761130.

Changes:

• Manager: extract admit(); InsertTxHandler becomes a thin adapter over it; add Manager.InsertTx (RPC path) and PendingTxs (txpool stub — app mempool holds generic sdk.Tx, not decoded MsgEthereumTx, so it reports none).
• App.MempoolClient(): returns the Manager, or nil under flood mode so ethermint falls back to BroadcastTx. Method on *App avoids a clash with the promoted BaseApp.InsertTx.

Depends on crypto-org-chain/ethermint#1016.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds direct RPC-based EVM transaction insertion into the app mempool by refactoring Manager to share a common admit path between gossip (InsertTxHandler) and a new InsertTx method. App gains a MempoolClient() accessor satisfying appmempool.MempoolClientProvider. Dependencies are bumped to pick up the upstream appmempool interface, and an integration test fixes eth_getBlockReceipts to pass a hex-encoded block number.

Changes

EVM direct mempool insertion

Layer / File(s)	Summary
Shared admission and RPC response app/mempool/manager.go	InsertTxHandler and InsertTx both route through the new admit(txBytes) helper that decodes before locking, runs RunTx(ExecModeCheck), maps mempool-full to CodeTypeRetry, caches accepted txs, and returns (code, codespace, log).
App mempool client exposure app/app.go	Adds App.MempoolClient() returning nil when mempoolManager is unset, otherwise returning it as appmempool.MempoolClient; includes a compile-time MempoolClientProvider assertion.
Manager admission tests app/mempool/manager_test.go	Seven new unit tests cover OK admission, rejection, mempool-full retry (direct and wrapped), canonical byte caching, suppressed caching on reject, and concurrent gossip+RPC admission under the manager mutex.
App-level insertion tests app/mempool/admission_test.go	Two tests verify end-to-end insertion via MempoolClient().InsertTx (valid tx accepted, tampered tx rejected) and that MempoolClient() is nil when using a non-app mempool type.
Dependency metadata and release note go.mod, gomod2nix.toml, CHANGELOG.md	Bumps ethermint replace target to crypto-org-chain/ethermint fork with appmempool wiring, updates several indirect deps, and adds the UNRELEASED changelog entry.

Receipt RPC argument formatting

Layer / File(s)	Summary
Receipt call argument update integration_tests/test_basic.py	eth_getBlockReceipts now receives hex(block_number) instead of the raw integer.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• crypto-org-chain/cronos#1823: Overlaps at assert_receipt_transaction_and_block in integration_tests/test_basic.py where eth_getBlockReceipts usage was previously modified.

Suggested labels

cronos

Suggested reviewers

• yihuang
• calvinaco

🐇 A tx hops in through the RPC door,
admit() checks it, then caches the core.
The gossip and RPC both share the same lock,
No race in the mempool, no slip through the clock.
The rabbit says: insert with delight! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: adding direct tx insertion support through the app mempool.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jt/ethermint-insertTx

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[JayT106]

@claude review

[github-actions]

Claude finished @JayT106's task in 2m 33s —— View job

---

PR #2119 Review

• Gather context and read PR changes
• Review app/mempool/manager.go
• Review app/app.go and cmd/cronosd/cmd/root.go
• Review tests
• Check for state-breaking changes
• Post final review

---

Summary

Good, well-scoped PR. The refactor is clean — extracting admit() as a shared path avoids duplication between the gossip and RPC admission paths, and the compile-time interface assertion in root.go is the right way to enforce the ethermint contract. No consensus/state-breaking changes detected (this is a pure mempool path, not state machine logic).

---

Issues Found

Medium — Nil-safety gap in InsertMempoolTx

app/app.go:1307 (inline comment posted)

InsertMempoolTx calls app.mempoolManager.InsertTx(txBytes) without a nil guard. The doc comment says "Only called when MempoolInsertEnabled() is true", but that contract is on ethermint's side. If ethermint calls this without gating on MempoolInsertEnabled(), the process panics. All other mempoolManager call sites in app.go explicitly guard with != nil checks (lines 527, 1637, 1662). This one should too.

func (app *App) InsertMempoolTx(txBytes []byte) (*sdk.TxResponse, error) {
    if app.mempoolManager == nil {
        return &sdk.TxResponse{Code: sdkerrors.ErrNotSupported.ABCICode(), RawLog: "mempool insert not enabled"}, nil
    }
    code, codespace, log := app.mempoolManager.InsertTx(txBytes)
    return &sdk.TxResponse{Code: code, Codespace: codespace, RawLog: log}, nil
}

Minor — Missing wrapped-error test for InsertTx

app/mempool/manager_test.go (inline comment posted)

TestInsertTxHandler_RetryOnWrappedMempoolFull covers the gossip handler's errors.Join(outer, ErrMempoolTxMaxCapacity) → CodeTypeRetry case. No equivalent test exists for InsertTx. Since both go through admit(), the behavior is correct, but a dedicated test would guard against future refactors splitting the paths.

---

Observations (non-blocking)

• evict() in manager.go:469 calls a.encCache.Evict(tx) unconditionally, but EncoderCache.Evict already has a nil-receiver guard (if e == nil { return }), so this is safe. No change needed.

• InsertTxHandler discards codespace and log (code, _, _ := a.admit(req.Tx)). This is correct for the P2P gossip path (CometBFT only uses the code), but worth confirming the ethermint interface only exposes the code from ResponseInsertTx — which the walkthrough confirms.

• Dependency bump adds kcalvinalvin/anet as an indirect dependency. This is pulled in transitively by the ethermint bump; worth a brief audit of what it is (it's a network utility lib used by ethermint's libp2p stack).

• TestManagerInsertTx_SharesAdmitWithHandler is a nice -race validator. One nit: it only tests interleaving; it doesn't assert that each unique tx actually lands in the pool (just that the raceRunner.state map has the right count). Still a good test.

• No breaking changes: InsertMempoolTx / MempoolInsertEnabled are new methods on App. admit() is unexported. The gossip InsertTxHandler behavior is preserved. This is mempool-local and does not affect consensus, store migrations, or the state machine.

---

[thomas-nguy]

please rework on the comment in this PR, they are too verbose

[JayT106]

please rework on the comment in this PR, they are too verbose

done!

[JayT106]

update pin once ethermint got merged.

[github-actions]

Claude finished @JayT106's task in 4m 11s —— View job

---

PR #2119 Review

• Gather context and read PR changes
• Review app/mempool/manager.go
• Review app/app.go
• Review tests
• Check for state-breaking changes
• Post final review

---

Summary

Clean, well-scoped PR. The admit() refactor correctly unifies gossip and RPC admission paths, and the MempoolClient() accessor pattern (returning the manager rather than implementing the interface on *App) neatly sidesteps the BaseApp.InsertTx promotion clash. No consensus/state-breaking changes — this is entirely a mempool/RPC path, with no state machine, message, or serialization impact.

---

Issues Found

Minor — No compile-time assertion for appmempool.MempoolClient

*Manager is documented as satisfying appmempool.MempoolClient (lines 142–151 in manager.go) but there's no var _ appmempool.MempoolClient = (*Manager)(nil) guard. app/app.go has the existing var _ runtime.AppI = (*App)(nil) pattern (lines 271–272). Adding this guard to manager.go (or at least to app.go where the import already exists) would catch upstream interface changes at compile time rather than at the ethermint integration point.

Minor — PendingTxs() stub silently empties eth_txpool_* RPCs

Posted as inline comment. The stub is correct for now, but eth_txpool_content and eth_txpool_status will always return empty. A TODO comment would capture this known gap for a future pass.

Minor — nil-tx path in admit() is implicit

Posted as inline comment. When encCache == nil, tx is nil and passed to RunTx. This matches the pre-refactor behavior in InsertTxHandler, so no regression. Just implicit.

---

Observations (non-blocking)

• ethermint pin is still at PR head SHA (v0.22.1-0.20260701023501-2f1ceeab1a02). The comment in go.mod correctly marks it as "PR head"; this pin should be updated to the merged tag/commit once ethermint Problem: memiavl persisted node queries is not optimal #1016 lands (already tracked in the PR description and in a prior coderabbit comment).

• kcalvinalvin/anet pulled in transitively via the ethermint bump — worth a brief audit to confirm it's a legitimate transitive dep (it is: anet is a network utility library used by ethermint's libp2p stack).

• InsertTxHandler codespace/log discard (code, _, _ := a.admit(req.Tx)) is correct: CometBFT's P2P path only uses the ABCI code, not codespace or log. No change needed.

• Test coverage is thorough: accept, reject, retry (direct + wrapped), cache register/no-register on reject, and the concurrent RPC+gossip race test. The prior coderabbit concern about test isolation (accounts[1] for the tampered case) has been correctly addressed in the current code.

• integration_tests/test_basic.py: hex(block_number) fix for eth_getBlockReceipts is a correct standalone bugfix included here.

• No state-breaking changes: all changes are mempool-local. No consensus impact, no store migrations, no message/signing changes.