[ConfigManager] Register Node Sections 2/4

[bdchatham]

Declares the consensus and mempool sections: 25 keys. Nothing consumes them.

What is left out, and why

The consensus table carries 24 paths and 7 are declared. Sixteen name settings the node
removed and one is the root directory. The fields are kept so a decode can tell that an
operator set one, and declaring any would offer a key that changes nothing about how the node
runs.

The mempool table declares 18 of 22. Three are settings the conversion into the running
mempool does not carry, so no written value arrives anywhere; that reason names the function
which would have to change for the key to matter. Two of the three are also marked dead at
their destination.

A deprecated field is found by its marking, not by its name

The exclusion list was held against a predicate that read the prefix on a field's name. The
node marks a deprecated field two ways: fifteen consensus fields carry that prefix, and seven
more across three other structs carry the standard comment above the field instead. Reflection
cannot see a comment, so those seven read as live settings.

The predicate now reads the source, which is what a deprecation marking is. One list per
section still serves both directions of the check, so no second list is needed. The check is
applied to every section rather than to consensus alone, through one table the key-count check
reads too.

That found the leader election setting, documented as retained for parsing compatibility and
ignored. Its declared value was the affirmative one, so an operator reading a generated file
would find the behaviour named and switchable and neither is true.

A probe that reported a pass while measuring nothing

The rows checking which removed settings the reader's own warning names composed their probe
value by matching the key's name. A name says nothing about a shape: the double-sign height is
an integer whose name begins like the boolean overrides do, and it decoded only because the
decoder accepts a boolean where an integer belongs. A refused shape was reported as a skip,
which ends the whole loop rather than one row, so every remaining row went unmeasured while the
run read as a pass. The value now comes from the field's own type and a refusal is fatal.

Formatters, go vet and golangci-lint clean. Mutation-verified: feeding an integer field a
list now raises a failure where it previously raised a skip.

[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 25, 2026, 10:49 PM

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.59%. Comparing base (561d5db) to head (5fad4c5).

Additional details and impacted files

@@                     Coverage Diff                     @@
##           plt-775-node-sections-1    #3993      +/-   ##
===========================================================
- Coverage                    57.62%   57.59%   -0.04%     
===========================================================
  Files                         2230     2230              
  Lines                       187157   186962     -195     
===========================================================
- Hits                        107854   107678     -176     
+ Misses                       69461    69448      -13     
+ Partials                      9842     9836       -6     

Flag	Coverage Δ
sei-chain-pr	100.00% <100.00%> (+4.08%)	⬆️
sei-db	69.80% <ø> (ø)

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

Files with missing lines	Coverage Δ
config/tendermintbase/tendermintbase.go	100.00% <100.00%> (ø)

... and 2 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]

PR Summary

Low Risk
Registry and test-only changes; no runtime wiring of consensus/mempool keys yet, so operator-facing behavior is unchanged until later ConfigManager steps land.

Overview
Registers consensus and mempool in the tendermint config registry (31 declared keys total), with mode-independent defaults via consensusDefaults / mempoolDefaults. Nothing in the binary consumes these sections yet—this is declaration for the ConfigManager pipeline.

Consensus deliberately omits 15 deprecated/no-op paths (removedSettings: legacy timeouts, unsafe overrides, stateless-leader-election, etc.) plus home, so generated config does not advertise keys that do not change node behavior. Mempool omits three paths (neverReachTheMempool) that never propagate into the running mempool.

Tests are expanded: a shared declaredAgainst table covers P2P, RPC, consensus, and mempool; deprecation is detected by parsing sei-tendermint/config source (field-name prefix and Deprecated: comments, not reflection alone). New checks tie consensus exclusions to struct deprecation, assert declared keys are not deprecated, and pin which removed settings the existing DeprecatedFieldWarning() path can still detect (eight of fifteen; the check is unused at runtime).

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

[seidroid]

The consensus/mempool registrations are derived cleanly from the reader's own structs and the exclusion machinery is well tested, but the "a key that changes nothing must not be declared" rule the PR states is enforced only by a Deprecated-name-prefix heuristic, and four ignored settings escape it — most notably consensus.stateless-leader-election, which the legacy template never offered. Two smaller test-robustness gaps are also worth closing.

Findings: 1 blocking | 5 non-blocking | 4 posted inline

Blockers

• None at the file/PR level.
• 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

• None at the file/PR level.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.
• 2 non-blocking pre-existing issue(s) listed below under pre-existing issues.

Pre-existing issues

• [suggestion] (*ConsensusConfig).DeprecatedFieldWarning in sei-tendermint/config/config.go:1345 tests DeprecatedSkipTimeoutCommit twice, so a file setting skip-timeout-commit names it twice in the error, and it never tests DeprecatedUnsafeBypassCommitTimeoutOverride even though that field is a *bool it could detect. Combined with having no caller anywhere in the binary, an operator with a removed timeout key gets nothing.
• [suggestion] registry.Lookup and registry.Sections (config/registry/registry.go:241, :254) copy Keys defensively — with a comment saying why — but return Excluded by reference, so a caller sorting or writing into it reaches the registry's own storage outside the mutex. The new tests are the first consumers of Excluded outside the package.

[seidroid]

[blocker] consensus.stateless-leader-election is declared here, and it is a key that changes nothing about how the node runs — the same property removedSettings exists to keep out of the key space. ConsensusConfig.StatelessLeaderElection (sei-tendermint/config/config.go:1249) documents itself as "retained only for config parsing compatibility and is ignored", and grep confirms nothing outside config.go reads it. The legacy template does not write it either (sei-tendermint/config/toml.go:554 onward has no such line), so this PR newly offers operators a knob with no effect.

TestTheExcludedConsensusPathsAreTheRemovedOnes cannot catch it because deprecatedPaths keys off the Go field name starting with Deprecated, and this field's marking lives only in its doc comment. Note that simply appending the path to removedSettings will fail that test's !marked[rel] branch, so the exclusion set and the authority the test derives from need to move together — e.g. a second list for "decoded but ignored" paths with its own assertion, rather than widening the Deprecated prefix rule.

[bdchatham]

Confirmed and excluded. config.go:1246 states it is retained for parsing compatibility and ignored, and DefaultConsensusConfig sets it true, so a writer emitted an affirmative toggle for behaviour that is always on.

Your mechanical point was right and led somewhere better than a second list. The predicate keyed on the Deprecated name prefix because reflection cannot read a doc comment; it now parses the source, which is what a deprecation marking actually is. One list per section then serves both directions. The corrected predicate went on to find instrumentation.namespace and three root keys on the branches above, so it is applied to every section rather than to consensus alone.

[seidroid]

[suggestion] The mempool section declares three settings its own struct marks as having no effect: max-batch-bytes (config.go:889 — "XXX: Unused due to tendermint/tendermint#5796"), pending-ttl-duration and pending-ttl-num-blocks (config.go:921, :924 — "Deprecated: pending TTL is not used and this field has no effect"). None is read outside config.go / toml.go.

These are weaker than the consensus case, since the legacy template already writes all three, so declaring them preserves parity. But by the rule stated on removedSettings they are keys that change nothing, and unlike consensus there is no test tying mempool's exclusions to anything the struct says. Either exclude them, or state here why parity with the old file wins for mempool and not for consensus.

[bdchatham]

Confirmed and all three excluded, on stronger evidence than the marking. ToMempoolConfig at config.go:970 is the only path into the running mempool and copies thirteen fields; none of these three is among them, so no written value arrives anywhere. That names the function which would have to change for the key to matter.

Two are also marked dead at the destination. max-batch-bytes carries only the upstream-issue note, so it would not have been caught by the marking alone.

[seidroid]

[suggestion] "The same values for every mode" is the load-bearing claim for both new sections, and nothing measures it. TestWhatVariesByNodeKindIsTheRecordedSet skips every key not prefixed p2p. or rpc. (tendermintbase_test.go:65), so if a rule in SetTendermintConfigByMode ever started varying, say, mempool.size by node kind, no test would notice and this comment would go quietly wrong — the exact failure mode whatVariesByNodeKind was built to prevent. Dropping the prefix filter (or extending it to the two new sections) makes both doc comments checkable: they are mode-invariant today, so the recorded set stays at four rows.

[bdchatham]

Confirmed on the root keys, and the branch matters. At this branch the filter is the p2p./rpc. literal you describe. By the tip it derives from the registered section names, so tx-index.indexer is covered — but a section whose keys sit at the root of the file has no prefix for HasPrefix to match, so it covered 110 of 124 keys and was blind to all 14 root keys.

Fixed at the tip by reading the keys out of the registry rather than matching prefixes, which also stops the check assuming a section's Name and its Prefix are the same string. Mutation-verified: a root key made to vary by node kind is now caught.

[seidroid]

[suggestion] t.Skipf calls runtime.Goexit, and this helper is called from inside a range removedSettings loop in TestTheDeprecationWarningReachesTheRecordedSubset (and a range registered.Keys loop above). One path whose probe value fails to decode therefore ends the whole test, not that iteration: every remaining removed setting goes unchecked and the run reports SKIP rather than a failure. That is the same shape as the diagnostic-that-never-runs this file is written to guard against.

Returning the decode error to the caller so it can t.Errorf and continue, or making the probe value derive from the field's type rather than its name so a decode failure is a real defect, both keep the loop intact.

[bdchatham]

Confirmed on the mechanism and fixed structurally. Measured, no iteration is lost today — every row decodes — so the loops do reach their later cases. What made it worth fixing is that the failure mode is silent: a refused shape reported SKIP, not FAIL.

The rows survived only on weak typing. probeValueFor matched names, so the double-sign height, an int64, was fed true because it matches the double-sign- arm. The value now comes from the field's own type, so a refusal cannot happen by construction, and the skip is a t.Fatalf. Mutation-verified: feeding an integer field a list now raises a failure where it previously raised a skip.

Worth noting the removed settings are each *any, which is how the reader tells written from absent — that case is handled explicitly rather than falling through.