fix(mempool): fall back to app.ChainID() for EVM sig preverifier when chain-id flag is unset

[JayT106]

What

app/app.go's mempool setup now registers the EVM signature preverifier (appmempool.NewEVMSigPreVerifier) with app.ChainID() instead of the raw --chain-id flag value.

Issue

The flag is empty on a bare cronosd start — the chain ID is only known from genesis at InitChain time. ethermint.ParseChainID fails on an empty string, so NewEVMSigPreVerifier silently returns nil and the preverifier never registers. Every tx admission then pays the full ecrecover cost inside the admission mutex instead of hitting the sender cache populated by the (never-running) preverifier — this was found while diagnosing a mempool-admission throughput ceiling.

newApp() builds baseappOptions via server.DefaultBaseappOptions(appOpts), which already resolves the flag-or-genesis chain ID and applies it with baseapp.SetChainID — and that option runs before this closure, since baseapp.NewBaseApp applies options in slice order. So app.ChainID() at this point already equals.

Solution

• Call appmempool.NewEVMSigPreVerifier(app.ChainID(), activeDecoder, senderCache) directly.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ef9556e7-00be-4784-a0a0-116c73eae518

📥 Commits

Reviewing files that changed from the base of the PR and between bfce946 and 7b45bf8.

📒 Files selected for processing (1)

• app/app.go

Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

The EVM signature pre-verifier now uses app.ChainID() during registration. A changelog entry records the fallback fix.

Changes

Pre-verifier chain ID resolution

Layer / File(s)	Summary
Resolve and record the pre-verifier chain ID app/app.go, CHANGELOG.md	EVM signature pre-verifier registration now uses the resolved BaseApp chain ID. The changelog records the fallback behavior.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 7b45b

This change restores EVM signature preverification when the chain-id flag is unset while preserving explicit flag behavior; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: thomas-nguy

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title clearly and concisely describes the main change: using app.ChainID() as a fallback when the chain-id flag is unset.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch worktree-fix-mempool-chainid-fallback

---

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 3m 11s —— View job

---

Code Review

• Read changed files
• Analyze code quality, correctness, and potential issues
• Post review findings

---

Summary

This is a clean, minimal fix. The bug is real: ethermint.ParseChainID("") fails, so NewEVMSigPreVerifier silently returned nil on bare cronosd start, leaving every tx admission to pay the full ecrecover cost. The fix is correct.

No breaking changes. This is purely a runtime/initialization fix — no state mutations, no consensus changes, no migrations required.

---

Correctness of the ordering claim

The key correctness invariant is that app.ChainID() is populated before the mempool setup closure executes. The code confirms this:

• baseAppOptions passed into New() (including server.DefaultBaseappOptions' SetChainID) occupy the head of the slice.
• The mempool closure is appended at line 531 — so it appears after all incoming options.
• baseapp.NewBaseApp applies them in order: SetChainID runs first, then the mempool closure calls app.ChainID().

This makes the fallback reliable for normal cronosd start invocations.

---

Observations

1. Edge case: both empty (resolvePreVerifierChainID("", "") → "")

If neither the flag nor app.ChainID() is set (e.g. a custom test harness that doesn't pass server.DefaultBaseappOptions), NewEVMSigPreVerifier receives an empty string and still returns nil — same silent failure as before. This is documented by the test case and is an acceptable degradation.

2. Test file: testify suite for a 3-case pure function

Using suite.Suite for resolvePreVerifierChainID (a 3-line pure function) adds boilerplate. A plain table-driven test with t.Run would be simpler and idiomatic Go:

func TestResolvePreVerifierChainID(t *testing.T) {
    for _, tc := range []struct{ ... }{ ... } {
        t.Run(tc.name, func(t *testing.T) {
            require.Equal(t, tc.expected, resolvePreVerifierChainID(tc.flagChainID, tc.appChainID))
        })
    }
}

This is non-blocking — if the suite pattern matches codebase conventions, keep it.

3. Removed // Register EVM module preverifier comment (line 585 before)

The comment deletion is fine — the code is self-descriptive and consistent with this repo's "no obvious comments" preference.

---

Verdict

LGTM. Logic is correct, tests cover the three meaningful cases, no state-breaking changes, changelog is in order. The test-file style note is optional.