Refine review config to consider on chain params

[masih]

Update revuew context to avoid false negative blocker comments regarding consensus params and config.

See:

• removed consensus timeout overrides from config #3934 (comment)

[cursor]

PR Summary

Low Risk
Documentation-only change to AI review guidelines; no runtime or consensus behavior is modified.

Overview
Extends REVIEW.md with two repo-specific rules so automated reviewers stop raising false blockers around on-chain consensus parameters and TOML config deprecation.

§4 Genesis rewrites tells reviewers that any step that rebuilds an existing genesis doc must keep earlier customized consensus parameters (not only chain ID, validators, app state, and time). Partial rewrites that omit those fields silently revert to defaults; findings should trace the full generation flow and assert the final genesis file after gentx collection or time normalization.

§5 Deprecated configuration keys documents that Viper currently ignores unknown TOML keys, so placeholder struct fields are not required just to load old configs. Reviewers should not ask for global unknown-field rejection as a local fix; that belongs at the shared decode choke point with explicit tests. A deprecation issue is real only when docs promise warnings/migrations that never run, or removal breaks documented compatibility without a replacement policy.

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

[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 18, 2026, 9:58 AM

[seidroid]

Docs-only change adding two sections to the reviewer-guidance file REVIEW.md. §5 checks out against the code; §4 names the wrong comparison constant (DefaultTimeoutParams() rather than badParams, and contradicts itself later) and gates its suppression rule on a live-network query a reviewer cannot reproduce.

Findings: 0 blocking | 5 non-blocking | 2 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• §4 encodes a point-in-time operational fact (what pacific-1's on-chain timeout params happen to be right now) into a permanent guidelines file. Once the override removal lands, the section becomes dead weight that can suppress unrelated future findings. Consider dating it the way badParams is dated in sei-tendermint/config/config.go:1259, or scoping it explicitly to the override-removal PR and removing it afterwards.
• §4's final paragraph ("tests must inspect the final genesis after gentx collection because genesis export helpers may replace ConsensusParams") is a concrete, durable repo fact and is the most reusable part of the section, but it is buried under guidance specific to one PR. It would survive longer as its own numbered section.
• Codex reported the §4 additions as a High prompt-injection attempt. I do not agree with that classification — REVIEW.md is the repository's own reviewer-guidance file and amending it is its intended function, so flagging edits to it as injection would fire on every legitimate change. The substantive part of the concern is captured by the inline comment on the /consensus_params bullet: guidance should point reviewers at the tree, not at external mutable state.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] This names the wrong constant. The gate in ConsensusConfig.ResolveTimeouts is if !c.UnsafeOverridesEnabled && t != badParams (sei-tendermint/config/config.go:1278) — overrides stay active when the resolved params equal badParams, not DefaultTimeoutParams().

The two structs currently hold identical values (config.go:1265 vs types/params.go:195: 1s / 500ms / 50ms / 500ms / 50ms / false), so the guidance happens to be correct today. That is a coincidence the text depends on without saying so: if DefaultTimeoutParams() is ever retuned, this section silently starts telling reviewers to compare against the wrong baseline and to suppress findings that have become real. The section also contradicts itself — line 119 correctly uses badParams as the comparison point while this line and line 116 use DefaultTimeoutParams().

Suggest naming badParams consistently in all three places, with a pointer to config.go:1265 so a reader can check the current values.

[seidroid]

[suggestion] This precondition can't be satisfied by the reviewer it's addressed to. An automated reviewer runs without network access and cannot query a live network's /consensus_params, nor inspect whether a given validator's config sets unsafe-overrides-enabled. The rule therefore has one reachable outcome in practice — the finding is suppressed — including in the case line 120 concedes is real.

Sections 1–3 all resolve to an artifact in the tree (app/tags, IsTxGasless), which is what makes them checkable and keeps the reasoning auditable in the PR record. Live chain state is neither reproducible at review time nor visible in the diff.

Suggest restructuring so the reviewer verifies a recorded claim instead of performing a query: require the PR that removes override support to state the observed live timeout params in its description, and have this section say to compare that stated value against badParams (sei-tendermint/config/config.go:1265), treating an absent statement as the finding.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.49%. Comparing base (f0faa48) to head (568c19e).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3936      +/-   ##
==========================================
- Coverage   59.50%   58.49%   -1.02%     
==========================================
  Files        2326     2230      -96     
  Lines      198890   188258   -10632     
==========================================
- Hits       118359   110130    -8229     
+ Misses      69285    67724    -1561     
+ Partials    11246    10404     -842     

Flag	Coverage Δ
sei-db	70.41% <ø> (ø)
sei-db-state-db	?

Flags with carried forward coverage won't be shown. Click here to find out more.
see 96 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.

[pompon0]

does this mean that we don't need to preserve the deprecated fields? Perhaps we should error on unknown fields instead then?

[masih]

will add thanks