feat(app): v0.54 upgrade perf optimizations + app-mempool feature#2091
Merged
Conversation
- bump cosmos-sdk v0.53.4→v0.54.3, ibc-go v10→v11, cometbft v0.38→v0.39.3,
Go 1.25.8→1.25.9
- migrate cosmossdk.io/{store,log,x/evidence,x/feegrant,x/upgrade,x/tx}
standalone modules to in-tree github.com/cosmos/cosmos-sdk paths
(store/v2, log/v2, contrib/x/crisis)
- pin replaces: cosmos-sdk fork (PR #1812 staking queue optimization),
ethermint fork (PR #894), cronos-store fork (PR #57)
- app.go: drop traceStore from New(); SetCommitMultiStoreTracer removed;
ICAControllerKeeper/ICAHostKeeper/TransferKeeper as pointer; drop
subspaces from IBC/ICA/Transfer NewKeeper; gov adds
CalculateVoteResultsAndVotingPower; RegisterNodeService takes
EarliestVersion func; remove ParamKeyTable for ibcclient/transfer/ica
- ExtTxSelector: drop gasWanted arg; remove SelectTxForProposalFast (no
Noopmempool path)
- IBCConversionModule: forward SetICS4Wrapper to underlying app
- ibccallbacks v11: NewIBCMiddleware(ck, max) +
SetUnderlyingApplication + SetICS4Wrapper composition pattern
- blockstm.NewSTMRunner → txnrunner.NewSTMRunner
- AppCreator/AppExporter: drop io.Writer
- AccountKeeper.NextAccountNumber takes (ctx, AccountI)
- e2ee/keyring: ErrUnknownBacked → ErrUnknownBackend
- add v6.0.0-sdk54 upgrade handler with staking v6 migration + StoreUpgrades
- bump CI Go to 1.25.9
Plan: docs/architecture/cosmos-sdk-v0.54-upgrade-plan.md
IBC v2 routes deferred to follow-up PR.
Signed-off-by: JayT106 <JayT106@users.noreply.github.com>
- fix(exchange): add Name() to ExchangeContract to satisfy vm.PrecompiledContract - fix(upgrades): move PopulateQueuePendingSlots after RunMigrations so indexes are built on fully-migrated queue keys - test(proposal): add test proving ExtTxSelector validates with original memTx but forwards nil to parent to bypass block-gas enforcement - docs(middleware): clarify IBCConversionModule only implements IBCModule, not full porttypes.Middleware (missing SetUnderlyingApplication)
The Added set is empty (no store-key churn from v0.53→v0.54 in this app), so wiring UpgradeStoreLoader was strictly redundant — and worse than letting the caller fall back to MaxVersionStoreLoader, which is cronos's custom loader and the loader used at every other height. Always return false so the caller installs MaxVersionStoreLoader for both regular and upgrade-height boots. Drops the upgradeInfo read, the StoreUpgrades literal, and the storetypes import.
Previous test asserted parent.lastMemTx==nil (old buggy behavior). Replace with: - assertion that real memTx reaches parent unmodified - gas-cap integration test: gasCapSelector rejects second tx when first already consumed 60k of 100k budget, proving the nil-was-bug fix
CI repeatedly hits transient nixpkgs check-phase failures inside python3.13-cherrypy-18.10.0 (e.g. ToolTests::testCombinedTools), which cascades into cachecontrol → poetry → shell.nix and breaks the integration_tests entry point. The CI test runner uses test-env (a poetry2nix-built environment from pyproject.toml/poetry.lock) — the poetry binary itself is not invoked at runtime, only useful for local dev when updating dependencies. Drop it from shell.nix so a flaky upstream cherrypy unit test cannot fail the whole suite. Devs who need poetry for lock updates can `nix-shell -p poetry` or `nix run nixpkgs#poetry` ad hoc.
JayT106
requested review from
ApacCronos and
thomas-nguy
and removed request for
a team
May 29, 2026 17:42
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds app-side mempool support with sharded tx-decode cache and encoder cache, new InsertTx/ReapTxs ABCI handlers and fast proposal paths, extensive unit and integration tests, and CI/testground/build configuration updates. ChangesApp-side mempool with transaction decode and encode caches
Sequence Diagram(s)sequenceDiagram
participant CometBFT
participant InsertHandler as InsertTx Handler
participant AppRunTx as App.RunTx
participant SeenCache
participant EncoderCache
CometBFT->>InsertHandler: InsertTx(req.Tx)
InsertHandler->>SeenCache: HasAtHeight(hash, height)
alt hit
SeenCache-->>InsertHandler: true
InsertHandler->>CometBFT: OK
else miss
InsertHandler->>AppRunTx: RunTx(tx, ExecModeCheck)
AppRunTx-->>InsertHandler: success/error
alt success
InsertHandler->>SeenCache: Add(hash)
InsertHandler->>EncoderCache: Register(tx, canonicalBytes)
InsertHandler->>CometBFT: OK
else failure
InsertHandler->>CometBFT: CodeTypeRetry or error
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
This comment has been minimized.
This comment has been minimized.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
JayT106
changed the title
- upgrades.go: extract planName const; clarify PopulateQueuePendingSlots is idempotent - precompiles/ica: early-return baseCost on unknown method ID before SubmitMsgsMethodName check - middleware/conversion: invert ack.Success() guard to early-return on failure, flatten nesting
JayT106
force-pushed
the
feat/cosmos-sdk-v0.54-upgrade-wip
branch
from
May 29, 2026 17:53
ac47a5d to
584bce6
Compare
Commit a0fc282 updated tests to verify memTx is forwarded to the parent selector (for gas-cap enforcement), but left three gaps: - gasOnlyTx helper type was never defined - proposal.go still passed nil to parent - old test asserted the old nil-forwarding behavior Add gasOnlyTx (implements sdk.FeeTx), update SelectTxForProposal to pass memTx through to parent, and replace the stale nil-assertion test with one that checks the real memTx reaches both ValidateTx and the parent.
cosmos-sdk v0.54 removed NewDefaultProposalHandlerFast, regressing per-block proposer cost for nodes running NoOp mempool (archives / non-priority validators) by re-running TxDecode + AnteHandler per tx. Reintroduce the fast path locally: when the mempool is nil or NoOp, filter req.Txs at byte-level via the cronos block-list ValidateTx and honor req.MaxTxBytes; otherwise delegate to the upstream default handler so priority mempool ordering semantics are preserved. Adds unit tests covering: non-NoOp delegation, NoOp filtering with order preservation, MaxTxBytes boundary, MaxTxBytes <= 0, nil mempool fast path, and empty req.Txs.
Five subtests duplicated the same `t.Fatal` default-handler stub and three duplicated the `acceptAll` validator. Hoist `mustNotInvoke` and `acceptAll` to the parent test scope so each subtest only states what is unique about its case. No behavior change; same six subtests still pass.
golangci-lint 2.1.6 nolintlint flags these as unused (SA1019 does not fire on these telemetry/govcli/sim-config calls), so --fix strips them. Match CI to keep the working tree clean.
cronos_app_no_replace used base_port 26401, overlapping cronos_app_mempool's 26400 range. pystarport spaces validators by 10 (node i = base + i*10, each spanning base..base+8), so a 3-validator chain at 26400 occupies 26400-26428. Both fixtures are module-scoped and alive simultaneously, so node1/node2 of the 26401 chain failed to bind -> crash-loop (FATAL) -> no first block -> 'wait for block 1 timeout'. Move to 26450 (clear of the 26400 range).
thomas-nguy
reviewed
Jun 19, 2026
The type owns recheck/TTL eviction in addition to admission, so Admitter understated its role. Rename type, constructors, App accessor/field, and the admitter.go/admitter_test.go files to match. Addresses PR #2091 review nitpick.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
revive (exported rule) flags mempool.MempoolManager as a stutter. Rename the type to Manager (constructors NewManager/newManager); package-qualified cronosmempool.Manager keeps the reviewer's intent. App.MempoolManager() accessor unchanged — it doesn't stutter.
JayT106
added a commit
that referenced
this pull request
Jun 22, 2026
App.Commit() ran RecheckTxs() synchronously, so the O(N) pool scan plus RunTx(ReCheck) batch landed on the consensus-critical path every block and CometBFT could not advance until it finished — a regression vs upstream flood, which rechecks after Commit returns. Move recheck to a single-flight async worker: - Manager gains a buffered-1 recheckTrigger; Commit fires TriggerRecheck() (non-blocking, coalescing) instead of calling RecheckTxs() inline. - recheckWorker drains triggers on one goroutine; recheckMu serializes the worker, the inline fallback, and direct test calls, keeping the lock-free arrival map single-writer. - Staging is the source of truth, so a dropped/coalesced wakeup loses no senders. Cross-block overlap stays safe: RunTx(ReCheck) under a.mu still serializes against the next Commit's checkState reset. - App.Close() stops the worker before BaseApp/store teardown so a late recheck never reads a closing store. Tests built via newManager keep recheckTrigger nil and run synchronously, so existing recheck tests are unaffected. Refs PR #2091 discussion r3413717119.
JayT106
force-pushed
the
feat/cosmos-sdk-v0.54-upgrade-wip
branch
from
June 23, 2026 00:20
f4e31ed to
e3e0a4b
Compare
thomas-nguy
enabled auto-merge
June 23, 2026 06:50
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jun 23, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jun 23, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jun 23, 2026
JayT106
added a commit
to JayT106/cronos
that referenced
this pull request
Jun 23, 2026
App.Commit() ran RecheckTxs() synchronously, so the O(N) pool scan plus RunTx(ReCheck) batch landed on the consensus-critical path every block and CometBFT could not advance until it finished — a regression vs upstream flood, which rechecks after Commit returns. Move recheck to a single-flight async worker: - Manager gains a buffered-1 recheckTrigger; Commit fires TriggerRecheck() (non-blocking, coalescing) instead of calling RecheckTxs() inline. - recheckWorker drains triggers on one goroutine; recheckMu serializes the worker, the inline fallback, and direct test calls, keeping the lock-free arrival map single-writer. - Staging is the source of truth, so a dropped/coalesced wakeup loses no senders. Cross-block overlap stays safe: RunTx(ReCheck) under a.mu still serializes against the next Commit's checkState reset. - App.Close() stops the worker before BaseApp/store teardown so a late recheck never reads a closing store. Tests built via newManager keep recheckTrigger nil and run synchronously, so existing recheck tests are unaffected. Refs PR crypto-org-chain#2091 discussion r3413717119.
This was referenced Jun 24, 2026
pull Bot
pushed a commit
to learn2grid/cronos
that referenced
this pull request
Jul 11, 2026
…to-org-chain#2118) * perf(mempool): run post-Commit recheck async off consensus path App.Commit() ran RecheckTxs() synchronously, so the O(N) pool scan plus RunTx(ReCheck) batch landed on the consensus-critical path every block and CometBFT could not advance until it finished — a regression vs upstream flood, which rechecks after Commit returns. Move recheck to a single-flight async worker: - Manager gains a buffered-1 recheckTrigger; Commit fires TriggerRecheck() (non-blocking, coalescing) instead of calling RecheckTxs() inline. - recheckWorker drains triggers on one goroutine; recheckMu serializes the worker, the inline fallback, and direct test calls, keeping the lock-free arrival map single-writer. - Staging is the source of truth, so a dropped/coalesced wakeup loses no senders. Cross-block overlap stays safe: RunTx(ReCheck) under a.mu still serializes against the next Commit's checkState reset. - App.Close() stops the worker before BaseApp/store teardown so a late recheck never reads a closing store. Tests built via newManager keep recheckTrigger nil and run synchronously, so existing recheck tests are unaffected. Refs PR crypto-org-chain#2091 discussion r3413717119. * chore(changelog): add crypto-org-chain#2118 async recheck entry * chore(changelog): align crypto-org-chain#2118 entry to PR title * style(mempool): tighten recheckMu/recheckTrigger field comments * fix(mempool): address PR crypto-org-chain#2118 review feedback - Group recheckTrigger/stop/workerDone/stopOnce into recheckAsync sub-struct so recheck worker lifecycle fields are clearly scoped (per thomas-nguy). - Fix shutdown race: recheckWorker re-checks async.stop after dequeuing a trigger, so a Close() racing a buffered trigger skips the redundant recheck instead of blocking store teardown (per songgaoye). - Document stale-tx window on TriggerRecheck: PrepareProposal may race the async worker; behavior matches CometBFT flood-mempool (per songgaoye). - Add async lifecycle tests: worker wakes on trigger, coalesced triggers preserve staged senders, concurrent commits are race-safe, Close waits for in-flight RecheckTxs (per songgaoye). * style(mempool): tighten async recheck comments * feat(mempool): add WaitForRecheck so PrepareProposal sees a clean pool The async recheck worker could still be running when PrepareProposal starts on the next block, letting stale-nonce/balance txs enter proposals. Changes: - Each TriggerRecheck call creates a per-trigger ready gate (chan struct{}) and threads it through the trigger channel so the worker closes exactly the right gate after each run — no shared-field close race. - WaitForRecheck() reads the latest gate under readyMu and blocks on it; if no recheck is pending the gate is pre-closed and the call is instant. - PrepareProposal now calls WaitForRecheck() before inner proposal handler. - Add TestWaitForRecheck_BlocksUntilWorkerDone to cover the new path. * fix(mempool): WaitForRecheck respects ctx deadline; deduplicate test setup - WaitForRecheck(ctx) now selects on ctx.Done() alongside the ready gate so a stuck recheck worker cannot halt block production past the proposal deadline. - PrepareProposal passes its sdk.Context to WaitForRecheck. - Extract startAsyncWorker helper to remove 6-line async field setup duplicated in TestClose_WaitsForInFlight and TestWaitForRecheck_BlocksUntilWorkerDone. * refactor(mempool/test): simplify async test helpers - newAsyncRecheckFixture now calls startAsyncWorker to eliminate 5-line duplication of async field setup between the two helpers. - Extract waitUntil(t, cond, timeout, msg) to replace 4 identical deadline-poll loops across the test file. * fix(app): return empty proposal if WaitForRecheck times out If the async recheck worker doesn't finish before the PrepareProposal context deadline, return an empty block rather than selecting from a potentially-stale pool. An empty block is preferable to including txs with invalid nonces/balances that would fail at EVM execution. In practice this path is never taken: recheck finishes well before the proposal deadline in the normal inter-block gap. * fix(mempool/test): prevent test hang on assertion failure Both blocking-runner tests (TestClose_WaitsForInFlight and TestWaitForRecheck_BlocksUntilWorkerDone) could hang indefinitely if an assertion failed while RunTx was blocked: deferred Close() would wait on the worker, which was waiting on the unblock channel that nothing would ever close. Fix by wrapping unblock in sync.Once (idempotent close) and calling it before Close() in cleanup/defer, so shutdown is safe regardless of which code path exits the test. Also fixes goroutine leak in TestClose_WaitsForInFlight: startAsyncWorker had no cleanup, so a waitUntil timeout would leave the worker goroutine running. Also fixes struct field comment alignment in recheckAsync (golangci-lint). * style(mempool): trim async recheck comments to why-only Remove test function doc comments; keep only design rationale in async-specific code (recheckAsync struct, TriggerRecheck, recheckWorker, Close, WaitForRecheck, PrepareProposal/Commit/FinalizeBlock wiring). * feat(app): count recheck-timeout empty proposals for alerting Operators can alert on cronos_mempool_recheck_proposal_timeout to detect when recheck consistently misses the proposal deadline. * refactor(mempool): extract recheckWorker struct with recheck/run/stop/wait Move async recheck lifecycle out of Manager into a recheckWorker struct with focused methods: - recheck(): coalescing trigger dispatch - run(fn): worker goroutine loop - stop(): idempotent shutdown - wait(ctx): gate for PrepareProposal Manager.TriggerRecheck/Close/WaitForRecheck become thin wrappers. Struct and methods live in recheck_worker.go. * style(mempool): clarify worker nil-trigger comment * update comment * fix(mempool): remove unused recheckAsync type * refactor(mempool): move worker init into recheckWorker.init() * fix(mempool): close orphaned recheck gate on quit race, bound PrepareProposal wait - recheckWorker.run(): outer select could pick the quit case over an already-buffered trigger, leaving that gate unclosed forever; any WaitForRecheck blocked on it would hang. Drain-and-close on quit too. - app.go PrepareProposal: cosmos-sdk's ctx carries no deadline, so the documented "ctx cancellation bounds the wait" safety net was dead code. Wrap it with a real context.WithTimeout before calling WaitForRecheck. * style(mempool): trim test doc comments * fix(mempool): check recheck-wait ctx error before canceling it cancel() unconditionally sets ctx.Err(), so checking Err() after calling it always looked timed out. Every PrepareProposal call took the empty-proposal branch, so no tx was ever included in a block. * refactor(mempool): extract duplicated eviction and sender-merge logic selectTxs had two nearly identical eviction blocks (timeout vs TTL), and StageSkippedSenders/StageRecheckSenders duplicated the same merge-not-overwrite logic for recheckSenders. Pulled both into named helpers (evictForRecheck, mergeRecheckSenders); no behavior change. * test(mempool): extract WaitForRecheckTimedOut, add cancel-order regression test The cancel()-before-Err() bug fixed in 1ddf52f lived entirely inside app.go's PrepareProposal closure, which no unit test touched — it only surfaced through a full integration test run. Extracted the wait/cancel/check sequence into Manager.WaitForRecheckTimedOut so it's directly testable, and added tests asserting timedOut=false when recheck completes in time and timedOut=true when it's still stuck past the deadline. * test(mempool): cover recheckWorker.stop idempotency stop() claims to be idempotent via sync.Once; add a direct test for it. * style(mempool): trim recheck comments Cut duplication (worker.wait vs Manager.WaitForRecheck said the same thing twice) and shortened a few multi-line comments to one line, keeping the non-obvious why. * fix(mempool): remove ReCheck failures from the pool, not just the encoder cache runRecheck only called a.encCache.Evict(tx) on a ReCheck(RunTx) failure. BaseApp itself removes a tx from the pool on ante-handler recheck failure, but not on a msg-execution-level failure (post-ante) — that error path returns without touching the mempool. So a tx that failed full ReCheck execution could stay in the pool and get reselected into a future proposal. Switch to a.evict(tx), the existing helper that removes from both the pool and the encoder cache, matching the pattern already used elsewhere (e.g. selectTxs's TTL/timeout eviction). Predates PR crypto-org-chain#2118 — this bug was in the prior synchronous recheck path too — but fixing it here since it surfaced during review of the async recheck change. Added TestRecheckTxs_MsgExecFailureEvictsFromPool: the existing test stub always removed from the pool on failure (mirroring only the ante path), so none of the existing tests actually covered the msg-execution-failure gap. * style(mempool): trim recheckRunner comment, drop test doc comment * fix(mempool): close WaitForRecheckTimedOut boundary race, dedup test worker init wait() picked ctx.Done() over an already-closed ready gate at random when both fired near-simultaneously, misreporting a completed recheck as timed out right at the 1.5s PrepareProposal boundary. wait() now returns whether ctx fired first and rechecks ready non-blocking before answering, so a near-simultaneous completion is no longer misreported. This also drops the ctx.Err()-after-cancel snapshot Manager relied on. startAsyncWorker in recheck_async_test.go duplicated recheckWorker.init()'s body instead of calling it; now calls init() directly. * style(mempool): trim wait comment * refactor(mempool): encapsulate PrepareProposal wiring in AppMempoolProposalHandler Moves the inline PrepareProposal closure out of app.go into a named AppMempoolProposalHandler type in proposal.go, per review feedback on PR crypto-org-chain#2118. Same behavior, just given a name and a home next to the other proposal-handling types. * docs(mempool): make a few comments plainer to read Reworded a handful of comments in manager.go and recheck_worker.go that leaned on jargon or shorthand (ctor, "single-writer; see recheckMu", "orphaned unclosed") into plainer sentences. No behavior change. * fix(mempool): rename AppMempoolProposalHandler to MempoolProposalHandler golangci-lint's revive rule flagged the name as stuttering (app.AppMempoolProposalHandler). Renamed per the linter's own suggestion. * style(mempool): tighten MempoolProposalHandler doc comment * refactor(mempool): move ExtTxSelector + signer-adapter wiring into NewMempoolProposalHandler Caller now just builds a DefaultProposalHandler and passes it in; the constructor wires the tx selector and signer extraction adapter, so app.go's call site is a plain wrapper over DefaultProposalHandler. Addresses review comments on PR crypto-org-chain#2118. * small optimisation * rework on comments * rework comments * restore deleted code by mistake * fix(mempool): address review nits on recheckWorker lifecycle and evictForRecheck Store fn as a recheckWorker field instead of threading it through init/run. Replace init with a newRecheckWorker constructor and split goroutine launch into start(), mirroring the existing stop() lifecycle. Document readyMu's dual role guarding ready and serializing recheck() against stop(). Hoist the recheckSenders nil-check in evictForRecheck out of the per-signer loop. * style: fix goimports formatting in app.go golangci-lint --fix removed a stray blank line inside the import block. * fix(mempool): address review nits on ExtTxSelector.Clear and CacheProposalTxVerifier embed Clear gateSkipped alongside the selector's other per-round state so Clear() fully resets it regardless of caller. Narrow CacheProposalTxVerifier's embed from *baseapp.BaseApp to the baseapp.ProposalTxVerifier interface it actually implements, so it doesn't promote BaseApp's whole method set. * fix(mempool): rename CacheProposalTxVerifier constructor param, take interface app was a misleading name for a baseapp.ProposalTxVerifier; also widen the param type from the concrete *baseapp.BaseApp to the interface it needs. * fix(mempool): fix regressions from Clear/embed review-nit commit Clear() zeroing gateSkipped wiped it before the wrapper could drain it, since DefaultProposalHandler defers Clear() inside h.inner() first — silently disabled the stranded-sender recheck feature. Narrowing CacheProposalTxVerifier's embed to an interface made PrepareProposalVerifyTx panic on a cache hit: forming a bound method value off a nil interface panics in Go, unlike a nil pointer embed. --------- Signed-off-by: JayT106 <JayT106@users.noreply.github.com> Co-authored-by: Thomas Nguy <thomas.nguy@crypto.com>
pull Bot
pushed a commit
to SPPRAGUE/cronos
that referenced
this pull request
Jul 11, 2026
…hain#2119) * feat(mempool): submit EVM RPC txs via app-mempool direct insert Under mempool.type=app, route eth_sendRawTransaction/sendTransaction/etherbase straight into the app mempool via ethermint's MempoolTxInserter (crypto-org-chain/ethermint#1016) instead of BroadcastTx -> CheckTx. EVM RPC now gets a correct synchronous ABCI response and back-pressure (CodeTypeRetry on capacity), sharing one admission path with p2p gossip. - Manager: extract shared admit(); InsertTxHandler becomes a thin adapter; add Manager.InsertTx returning code+codespace+log for the RPC caller. - App: add InsertMempoolTx + MempoolInsertEnabled. The ethermint server gates registration on MempoolInsertEnabled(), so the default flood mempool keeps the unchanged BroadcastTx submission path. - Bump ethermint pin to the gated-interface follow-up. Addresses PR crypto-org-chain#2091 review r3361761130 (divergent RPC vs gossip admission). * chore(changelog): fill PR number crypto-org-chain#2119 * chore(changelog): align crypto-org-chain#2119 entry to PR title * style(mempool): tighten InsertMempoolTx/admit comments * fix(app): guard InsertMempoolTx against nil mempool manager Address PR crypto-org-chain#2119 review: InsertMempoolTx relied on ethermint gating the call on MempoolInsertEnabled(), a cross-repo contract. Add a nil guard returning ErrNotSupported (matching the other mempoolManager call sites) so a stray call returns a code instead of panicking. Add wrapped-capacity and flood-gate tests. * fix(ci): regenerate gomod2nix.toml for ethermint pin + drop redundant uint32 casts - gomod2nix.toml still pinned the pre-crypto-org-chain#1003 ethermint, so the Nix build failed with 'undefined: ethermintserver.MempoolTxInserter'. Regenerate to match the go.mod pin (crypto-org-chain#1016, b8b15b3); also pulls the new transitive deps. - Drop unnecessary uint32(abci.CodeTypeOK) conversions flagged by unconvert. * test(integration): pass hex block number to eth_getBlockReceipts The ethermint bump changes GetBlockReceipts param from BlockNumber to BlockNumberOrHash. BlockNumber accepted a plain decimal int (cast.ToUint64 fallback on missing 0x prefix); BlockNumberOrHash does not, so the raw int arg now returns an error and the response has no 'result'. Pass hex(). * docs(mempool): tighten insert-tx comments, align changelog wording * refactor(mempool): drop MempoolInsertEnabled, decline direct insert via nil response InsertMempoolTx now returns (nil, nil) when mempool.type != app, so ethermint falls back to BroadcastTx — no separate predicate gating registration. Re-pin ethermint to the matching single-method MempoolTxInserter interface. * remove wordy comments * chore(deps): repin ethermint to appmempool refactor; use appmempool.Inserter Ethermint moved the app-mempool contracts into a new appmempool package and dropped the package-global inserter registration (stack crypto-org-chain#1016+crypto-org-chain#1019+crypto-org-chain#1011). - Bump ethermint pin to 562fa2a2 (go.mod, go.sum, gomod2nix.toml). - Repoint root.go interface assertion: server.MempoolTxInserter -> appmempool.Inserter. App.InsertMempoolTx signature is unchanged, so ethermint StartJSONRPC casts it automatically; no app wiring change. * refactor(mempool): move InsertTx response build into Manager Address review: App.InsertMempoolTx no longer constructs the sdk.TxResponse. Manager.InsertTx now returns *sdk.TxResponse; App is a thin delegate (nil-decline gate + forward), keeping mempool response logic in the mempool package. * refactor(mempool): expose inserter via appmempool.InserterProvider The InsertMempoolTx method now lives on the mempool Manager, which satisfies ethermint's appmempool.Inserter directly. App implements the new InserterProvider (MempoolInserter), returning the manager or nil under flood mode. This keeps all mempool response logic in the mempool package and removes the (nil,nil) decline sentinel from app.go. Repin ethermint to 2ae3aaf8 (adds appmempool.InserterProvider). * chore(deps): repin ethermint to crypto-org-chain#1016 head (1389b15f) crypto-org-chain#2119 depends only on appmempool.Inserter/InserterProvider, which live in ethermint crypto-org-chain#1016. Pin directly to that PR head instead of the full sig-preverifier stack tip. Inserter API unchanged, so no cronos code change. * test(mempool): isolate tamper-reject case; tighten inserter comments - admission_test: tamper a fresh account so the forged-From rejection isn't confounded by nonce/check-state from the prior accepted tx (CodeRabbit r3483469252). - app.go / root.go: trim MempoolInserter / InserterProvider comments to the design decision (avoid BaseApp.InsertTx clash). * chore(deps): repin ethermint crypto-org-chain#1016; adapt to MempoolClient interface PR crypto-org-chain#1016 replaced appmempool.Inserter/InserterProvider with MempoolClient (InsertTx + PendingTxs) and MempoolClientProvider. - repin ethermint to d61c063f (PR head); regen gomod2nix.toml - Manager: InsertMempoolTx -> InsertTx; add PendingTxs (nil; app mempool holds generic sdk.Tx, not decoded MsgEthereumTx) - App: MempoolInserter() -> MempoolClient() via MempoolClientProvider (promoted BaseApp.InsertTx still blocks direct MempoolClient impl) * fix(lint): gci import formatting in app/app.go and app/mempool/manager.go - Remove extra blank line in app/app.go import section (line 52) - Reorder imports in app/mempool/manager.go to follow gci section rules: standard → default → cosmossdk.io → github.com/cosmos/cosmos-sdk * chore(deps): repin ethermint crypto-org-chain#1016 head (66efb289) Picks up main merge into the PR branch (go-metrics 0.5.4 -> 0.6.0). appmempool.MempoolClient interface unchanged; no code change. Regenerate gomod2nix.toml. * refactor: move MempoolClientProvider assertion to app.go Compile-time assert lives next to *App and its MempoolClient method; drop the now-unused appmempool import from root.go. * chore(deps): repin ethermint crypto-org-chain#1016 head (2f1ceeab) MempoolClientProvider removed; pass MempoolClient explicitly. Adapt: - Remove MempoolClientProvider compile-time assertion in app/app.go - Manager.PendingTxs() return type: []*MsgEthereumTx → []sdk.Tx (drops evmtypes dep) * docs(mempool): note nil-tx path is intentional when encCache disabled BaseApp.RunTx accepts nil sdk.Tx and falls back to txBytes; cacheTx is a no-op without encCache, so passing nil is safe. * refactor(mempool): trim verbose comments on InsertTx/PendingTxs * chore(deps): repin ethermint crypto-org-chain#1016 head (6f932cde)
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Jul 11, 2026
* feat(mempool): submit EVM RPC txs via app-mempool direct insert Under mempool.type=app, route eth_sendRawTransaction/sendTransaction/etherbase straight into the app mempool via ethermint's MempoolTxInserter (crypto-org-chain/ethermint#1016) instead of BroadcastTx -> CheckTx. EVM RPC now gets a correct synchronous ABCI response and back-pressure (CodeTypeRetry on capacity), sharing one admission path with p2p gossip. - Manager: extract shared admit(); InsertTxHandler becomes a thin adapter; add Manager.InsertTx returning code+codespace+log for the RPC caller. - App: add InsertMempoolTx + MempoolInsertEnabled. The ethermint server gates registration on MempoolInsertEnabled(), so the default flood mempool keeps the unchanged BroadcastTx submission path. - Bump ethermint pin to the gated-interface follow-up. Addresses PR #2091 review r3361761130 (divergent RPC vs gossip admission). * chore(changelog): fill PR number #2119 * chore(changelog): align #2119 entry to PR title * style(mempool): tighten InsertMempoolTx/admit comments * fix(app): guard InsertMempoolTx against nil mempool manager Address PR #2119 review: InsertMempoolTx relied on ethermint gating the call on MempoolInsertEnabled(), a cross-repo contract. Add a nil guard returning ErrNotSupported (matching the other mempoolManager call sites) so a stray call returns a code instead of panicking. Add wrapped-capacity and flood-gate tests. * fix(ci): regenerate gomod2nix.toml for ethermint pin + drop redundant uint32 casts - gomod2nix.toml still pinned the pre-#1003 ethermint, so the Nix build failed with 'undefined: ethermintserver.MempoolTxInserter'. Regenerate to match the go.mod pin (#1016, b8b15b3); also pulls the new transitive deps. - Drop unnecessary uint32(abci.CodeTypeOK) conversions flagged by unconvert. * test(integration): pass hex block number to eth_getBlockReceipts The ethermint bump changes GetBlockReceipts param from BlockNumber to BlockNumberOrHash. BlockNumber accepted a plain decimal int (cast.ToUint64 fallback on missing 0x prefix); BlockNumberOrHash does not, so the raw int arg now returns an error and the response has no 'result'. Pass hex(). * docs(mempool): tighten insert-tx comments, align changelog wording * refactor(mempool): drop MempoolInsertEnabled, decline direct insert via nil response InsertMempoolTx now returns (nil, nil) when mempool.type != app, so ethermint falls back to BroadcastTx — no separate predicate gating registration. Re-pin ethermint to the matching single-method MempoolTxInserter interface. * remove wordy comments * chore(deps): repin ethermint to appmempool refactor; use appmempool.Inserter Ethermint moved the app-mempool contracts into a new appmempool package and dropped the package-global inserter registration (stack #1016+#1019+#1011). - Bump ethermint pin to 562fa2a2 (go.mod, go.sum, gomod2nix.toml). - Repoint root.go interface assertion: server.MempoolTxInserter -> appmempool.Inserter. App.InsertMempoolTx signature is unchanged, so ethermint StartJSONRPC casts it automatically; no app wiring change. * refactor(mempool): move InsertTx response build into Manager Address review: App.InsertMempoolTx no longer constructs the sdk.TxResponse. Manager.InsertTx now returns *sdk.TxResponse; App is a thin delegate (nil-decline gate + forward), keeping mempool response logic in the mempool package. * refactor(mempool): expose inserter via appmempool.InserterProvider The InsertMempoolTx method now lives on the mempool Manager, which satisfies ethermint's appmempool.Inserter directly. App implements the new InserterProvider (MempoolInserter), returning the manager or nil under flood mode. This keeps all mempool response logic in the mempool package and removes the (nil,nil) decline sentinel from app.go. Repin ethermint to 2ae3aaf8 (adds appmempool.InserterProvider). * chore(deps): repin ethermint to #1016 head (1389b15f) #2119 depends only on appmempool.Inserter/InserterProvider, which live in ethermint #1016. Pin directly to that PR head instead of the full sig-preverifier stack tip. Inserter API unchanged, so no cronos code change. * test(mempool): isolate tamper-reject case; tighten inserter comments - admission_test: tamper a fresh account so the forged-From rejection isn't confounded by nonce/check-state from the prior accepted tx (CodeRabbit r3483469252). - app.go / root.go: trim MempoolInserter / InserterProvider comments to the design decision (avoid BaseApp.InsertTx clash). * chore(deps): repin ethermint #1016; adapt to MempoolClient interface PR #1016 replaced appmempool.Inserter/InserterProvider with MempoolClient (InsertTx + PendingTxs) and MempoolClientProvider. - repin ethermint to d61c063f (PR head); regen gomod2nix.toml - Manager: InsertMempoolTx -> InsertTx; add PendingTxs (nil; app mempool holds generic sdk.Tx, not decoded MsgEthereumTx) - App: MempoolInserter() -> MempoolClient() via MempoolClientProvider (promoted BaseApp.InsertTx still blocks direct MempoolClient impl) * fix(lint): gci import formatting in app/app.go and app/mempool/manager.go - Remove extra blank line in app/app.go import section (line 52) - Reorder imports in app/mempool/manager.go to follow gci section rules: standard → default → cosmossdk.io → github.com/cosmos/cosmos-sdk * chore(deps): repin ethermint #1016 head (66efb289) Picks up main merge into the PR branch (go-metrics 0.5.4 -> 0.6.0). appmempool.MempoolClient interface unchanged; no code change. Regenerate gomod2nix.toml. * refactor: move MempoolClientProvider assertion to app.go Compile-time assert lives next to *App and its MempoolClient method; drop the now-unused appmempool import from root.go. * chore(deps): repin ethermint #1016 head (2f1ceeab) MempoolClientProvider removed; pass MempoolClient explicitly. Adapt: - Remove MempoolClientProvider compile-time assertion in app/app.go - Manager.PendingTxs() return type: []*MsgEthereumTx → []sdk.Tx (drops evmtypes dep) * docs(mempool): note nil-tx path is intentional when encCache disabled BaseApp.RunTx accepts nil sdk.Tx and falls back to txBytes; cacheTx is a no-op without encCache, so passing nil is safe. * refactor(mempool): trim verbose comments on InsertTx/PendingTxs * chore(deps): repin ethermint #1016 head (6f932cde) * feat(mempool): back txpool RPC with app mempool PendingTxs Repin ethermint to PR #1019 head (stacked on #1016), which wires the txpool RPC namespace (content/inspect/status/contentFrom) to MempoolClient.PendingTxs. Implement Manager.PendingTxs: lock-free PoolSnapshot scan returning pooled EVM messages; skips non-EVM/multi-message txs; nil mpool reports none. * docs(changelog): add #2127 txpool RPC PendingTxs entry * fix(mempool): address PendingTxs review nits - Rename PoolSnapshot telemetry label recheck→pool; snapshot is now called from both recheck and txpool RPC paths so the label was misleading - Drop over-eager capacity hint in PendingTxs; EVM:non-EVM ratio unknown - Add missing blank line in manager_test.go (gofmt / CI lint fix) * test(mempool): integration tests for txpool RPC namespace Covers txpool_content, txpool_inspect, txpool_status, txpool_contentFrom: - shape tests (always pass): verify endpoints respond with {pending,queued} - test_txpool_pending_tx_visible: submit tx, query pool immediately, assert tx appears in all four endpoints; @flaky(3) for reap_interval race. * refactor(test): simplify txpool shape tests; add inspect xfail guard Extract _assert_pool_dict_shape to deduplicate the 3 identical dict-shape assertions. Add missing pytest.xfail guard for inspect_sender None case (timing race path identified in code review). * fix(lint): black format + wrap long lines in txpool tests * fix(test): enable txpool JSON-RPC namespace for app-mempool suite default.jsonnet's api list omits txpool, overriding ethermint's default namespace set. mempool_app.jsonnet never restored it, so all txpool_* RPC calls in test_app_mempool.py hit an unregistered namespace and returned no "result" key. * chore(deps): repin ethermint #1034 head (de2396e91b19) github.com/crypto-org-chain/ethermint v0.22.1-0.20260707200522-de2396e91b19 * fix(test): move txpool api override to app-config in mempool_app.jsonnet json-rpc lives under app-config in pystarport's schema, not config (config maps to config.toml; app-config maps to app.toml). The previous override nested json-rpc under config, so it silently patched an unused config.toml section instead of app.toml, and the txpool namespace was never actually enabled on the running node. * chore(deps): repin ethermint #1051 head (ea6791fbb8f0) Ethermint's MempoolClient interface gained CountTx() for O(1) txpool_status. Add Manager.CountTx() to satisfy it. * test(mempool): cover Manager.CountTx unit + integration Add TestManagerCountTx (nil mpool -> 0, populated pool -> exact count) and tighten test_txpool_pending_tx_visible's status assertion from a loose >= 1 to a before/after delta, so it actually verifies CountTx() counted the submitted tx instead of just "something is pending". * fix(nix): bump go_1_25 overlay pin to 1.25.11 go.mod's go directive moved to 1.25.11 (ethermint #1051 repin), but the nix overlay still pinned go1.25.9 and CI has no network access to fetch the newer toolchain on demand. Every nix-based build/integration job failed with "toolchain not available". Bump the overlay's version and source hash to match. * fix(ci): bump stale go-version 1.25.9 pins to 1.25.11 lint.yml, codeql-analysis.yml, and sims.yml each hardcode actions/setup-go's go-version separately from the nix toolchain. go.mod now requires go >= 1.25.11 (bumped by the ethermint #1051 repin), so these jobs failed with GOTOOLCHAIN=local: toolchain not available. * fix(testground): bump golang base image 1.25.9 -> 1.25.11 Same toolchain mismatch as the CI workflows: go.mod requires go >= 1.25.11 since the ethermint #1051 repin.
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
6 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.
Summary
Builds on #2089 (cosmos-sdk v0.54 upgrade): v0.54 perf optimizations + opt-in app-side mempool. Default mempool stays
flood; all app-mempool hooks dormant untilmempool.type=app.mempool.type=appopt-in app-side mempool; unrecognized value panics at startupAdmitterruns the AnteHandler viaRunTx(ExecModeCheck)at admission (p2pInsertTx+ RPCCheckTx), serialized by one mutex — fixes the shared-checkStaterace under CometBFT's concurrentInsertTxApp.Commitholds the admission mutex only acrossBaseApp.Commit's checkState reset; the O(pool) snapshot + expired sweep + candidate selection run lock-free (mempool + encCache are self-locked). Only theRunTx(ReCheck)batch re-acquires the mutex — deep-pool scans no longer stall ingest for the full commit cycleUpdate()is a no-op) — re-runs ante inExecModeReCheckfor senders touched by the block, plus aTimeoutHeightsweep that evicts expired txs; candidate cap defaults to one block's tx budget so stale txs don't accumulatemempool-ttl-num-blocks(default 120,0disables) evicts pool txs older than N blocks by arrival height, independent of per-txTimeoutHeight(EVM txs carryTimeoutHeight 0= never expire). Drains txs the proposal keeps skipping (baseFee gate, blocklist) whose sender never commits and would otherwise pin a pool slot forever. Arrival tracked lazily on first RecheckTxs sighting (no admission-path cost, no new lock — RecheckTxs is serial per Commit); the arrival map rebuilds from each cycle's snapshot so included/evicted txs self-reconcile. Eviction is a plain pool+cache drop, no AnteHandler rerunReapTxsHandlerwith throttle (mempool-gossip-ttl/mempool-txs-per-block) — spreads a large pool across ticks instead of flooding libp2pEncoderCacheof canonical bytes — eliminates double-decode/proto.Marshalacross PrepareProposal/Reap/FinalizeBlock; fast PrepareProposal skipsPrepareProposalVerifyTx(ante already ran at insert)mempool.type=appuses theNoCheckProposalTxVerifierfast path; flood and app+cache-disabled wire the SDKDefaultProposalHandlerdirectly.ExtTxSelectorenforces the blocklist + gas/byte caps in all paths (no bespoke no-op handler)COSMOS_SDK_CONFIG_SCOPE— skipsos.Executable/os.Hostnameon the bech32 hot path (~34× fastersdk.GetConfig)v0.39.4-…-22d5a9fConfig
mempool.type(CometBFTconfig.toml):flood(default) /""app[cronos](app.toml) — new keys:tx-cache-size0(derive)0= derive frommempool-txs-per-block(2×, or disabled when unlimited);-1disables bothtx-cache-max-tx-bytes65536mempool.max_tx_bytes(panics at startup if it does)mempool-gossip-ttl15smempool.type=app); a reaped tx isn't re-broadcast until it elapses;<=0uses defaultmempool-txs-per-block2900Commitcycle;0= unlimited (also disables tx-cache-size auto-derive)mempool-ttl-num-blocks120mempool.type=apptxs older than N blocks by arrival height (independent of per-txTimeoutHeight); drains proposal-skipped txs whose sender never commits;0disablesKnown limitations (mempool.type=app)