[ConfigManager] Deliver sei.toml for Decoded Sections 2/3 by bdchatham · Pull Request #4044 · sei-protocol/sei-chain

bdchatham · 2026-08-27T20:05:17Z

Delivers the resolved values for the ten sections whose reader decodes its file whole. Second of
three, on top of the install.

Why a second delivery exists

The boot decodes config.toml into a struct before any lookup happens, so a value installed into
the key-value source reaches nothing for those keys. Measured on a real boot: viper answers 19999
for a key whose live struct field holds 26656.

The split is exact. 115 of the 274 declared keys live in that struct and 159 do not, and the two
sets share no key.

Decode into a copy, then publish through the pointers that exist

A decoder gathers errors and keeps going, so decoding into the live struct would leave it holding
some new values and some old ones on a refusal. Values decode into a deep copy first.

Publishing assigns each section through the pointer already there. Components take a section rather
than the configuration holding it, so replacing the top level would freeze every holder at its old
values.

A decode is all or nothing for what it is handed, so one section is delivered at a time.

Values that decode cleanly and mean something else

Five shapes are refused, because nothing later objects to any of them:

[p2p] max-connections = 1e20    truncates, so the ceiling bounds nothing
[mempool] size = 1.5            becomes 1
[mempool] max-tx-bytes = ""     becomes 0, so the node accepts no transactions

An empty value is what an unfilled template variable renders as. The checks now read all three
sources; only the file carries a typed number.

Verified

Formatters, go vet and golangci-lint clean. -race -count=2 -shuffle clean. Each refusal is
mutation-verified.

github-actions · 2026-08-27T20:06:57Z

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 31, 2026, 9:55 PM

codecov · 2026-08-27T20:08:49Z

Codecov Report

❌ Patch coverage is 69.69697% with 110 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.31%. Comparing base (1b522ed) to head (ed5b3a9).

Files with missing lines	Patch %	Lines
cmd/seid/cmd/configmanager/tendermint_copy.go	70.17%	46 Missing and 22 partials ⚠️
cmd/seid/cmd/configmanager/tendermint.go	66.66%	31 Missing and 11 partials ⚠️

Additional details and impacted files

@@                  Coverage Diff                  @@
##           plt-775-install-app    #4044    +/-   ##
=====================================================
  Coverage                60.30%   60.31%            
=====================================================
  Files                     2057     2059     +2     
  Lines                   177115   177412   +297     
=====================================================
+ Hits                    106809   107004   +195     
- Misses                   60447    60520    +73     
- Partials                  9859     9888    +29

Flag	Coverage Δ
sei-chain-pr	`69.89% <69.69%> (-5.72%)`	⬇️
sei-db	`69.80% <ø> (ø)`

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

Files with missing lines	Coverage Δ
cmd/seid/cmd/configmanager/install.go	`76.35% <100.00%> (+1.05%)`	⬆️
config/registry/delivery.go	`100.00% <100.00%> (ø)`
cmd/seid/cmd/configmanager/tendermint.go	`66.66% <66.66%> (ø)`
cmd/seid/cmd/configmanager/tendermint_copy.go	`70.17% <70.17%> (ø)`

... 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.

bdchatham · 2026-08-27T20:22:58Z

@seidroid review

cursor · 2026-08-31T14:30:39Z

PR Summary

High Risk
Changes node boot configuration for all Tendermint decoded sections, including overwriting undeclared keys with registry defaults and applying values through reflection-heavy copy/publish paths; mistakes affect production node behavior on restart.

Overview
Adds the second configuration delivery for sections whose settings are read from config.toml via struct decode, not Viper lookup. During install, resolved values for those sections are now decoded into a deep copy of the live node config, validated, then published in-place (preserving section pointer identity) instead of only logging that the install path cannot reach them.

Registry behavior changes: ResolvedAndOwnedByDecodedSections replaces the supplied-only variant and hands every resolved key in decoded sections—including declared defaults for keys omitted from sei.toml—so sparse sei.toml replaces config.toml for those settings rather than patching it.

Safety and observability: Per-section all-or-nothing apply; pre-decode guards for misleading numeric shapes (bare durations, negative uints, overflow, fractions); ValidateBasic on the copy; password redaction in change reports; resolved log level applied before other reporting; docs updated for two delivery paths and precedence.

Tests: Broad boot/integration coverage (node_delivery_test.go, copy/publish property tests, report tests) and stricter boot tests (fatals instead of skips on missing preconditions).

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

seidroid

The second delivery is well-structured: per-section rehearse-into-a-copy, publish through the existing section pointers, and a pre-decode guard for values that decode cleanly into something else. No blockers found; the main gap is that the published candidate never goes through Tendermint's own ValidateBasic, plus a few smaller doc/redaction/test-hygiene items.

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

Blockers

None at the file/PR level.

Non-blocking

[suggestion] applyResolvedLogLevel (cmd/seid/cmd/configmanager/tendermint.go:243) is new production logic with three distinct outcomes — level applied, deferred because SEI_LOG_LEVEL is exported, applied anyway because a --log_level flag was typed — and none of them is asserted anywhere. TestTheReportSurvivesAQuietNode is the only test that puts log-level in a sei.toml, and it asserts the reporting floor rather than the delivered level, so it passes whether or not this function does anything. Given the PR's own standard about tests that measure nothing, a test reading back the process level after a boot (and one with SEI_LOG_LEVEL set) would close the last untested channel in the delivery.
5 suggestion(s)/nit(s) flagged inline on specific lines.
1 non-blocking pre-existing issue(s) listed below under pre-existing issues.

Pre-existing issues

[suggestion] interceptConfigs (sei-cosmos/server/util.go:250-291) calls conf.ValidateBasic() only on the branch that creates a fresh config.toml from defaults. When the file already exists it is read and unmarshalled with no validation, so an operator's config.toml can put the node into a state Tendermint declares invalid without any startup complaint. This is why the sei.toml finding above is parity rather than a regression.

bdchatham · 2026-08-31T17:23:40Z

@seidroid review

seidroid

The second delivery is well constructed: the rehearse-into-a-copy/publish-through-existing-pointers split is correct, the per-section refusal and the pre-decode "decodes to something else" guard are backed by real boot-driven tests, and the type-driven detach/publish walks match the tests that hold them. Two non-blocking gaps remain, both in the reporting path: the password redaction fails open on libpq escaping and on unparseable URLs, and describe renders pointer-typed leaves as addresses.

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

Blockers

None at the file/PR level.

Non-blocking

None at the file/PR level.
2 suggestion(s)/nit(s) flagged inline on specific lines.

…coded A node's own configuration file is read into a struct by the boot's handler before this runs, and nothing consults the key-value source for those settings afterwards. So the install of the previous change is no delivery at all for them: the values have to be decoded into that struct instead. Decoded into a deep copy of the live configuration and published by replacing it, rather than decoded in place. What a decoder writes depends on what the target already holds, so the copy has to be of the node's own configuration and not a fresh one. One section at a time. A decode is all or nothing for whatever it is handed, so a single value a decoder refuses would otherwise cost every key in the file rather than the keys of the section it appeared in. An operator who fixes one setting and mistypes another has to end up with the first one applied. The resolved log level is applied before any of the reporting, because a refusal is reported at a level an operator may have raised the threshold above, and doing it after would mean the one setting somebody changes in order to see a refusal is the setting a refusal suppresses. One report corrected. The line saying the file supplied no declared value describes the lookup delivery alone, and both run in one pass, so an operator whose file moved a setting through a decode was told the file supplied nothing a few lines later. It is now scoped to the delivery it describes. Verified by mutation, and the first attempt at that verification was a false pass worth recording: the pattern had not applied, so an unmutated run was read as proof. With the mutation genuinely in place nothing failed, which is how the new test came to be written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…that decode to something else The delivery replaced the node's whole configuration struct. Every section under it sits behind a pointer of its own, and components take the section rather than the configuration holding it, so replacing the top level swapped all nine pointers for fresh ones. Anything already holding a section went on reading the values that section had before the delivery. It was correct only because the delivery happens to run before those components are built, and nothing stated that order. Each section is now assigned through its own pointer, so identity survives and a holder reads the delivered value whichever side of the delivery it took its pointer from. The guard on values that decode to something other than what they say checked the sign and not the magnitude. Measured through the pre-flight command on real files: [p2p] max-connections = 1e20 approved, then applied as 18446744073709551615 [mempool] size = 1.5 approved, then applied as 1 The first saturates to the largest value the field holds, which is the same outcome the guard already refuses a minus one for. The second truncates, so a mempool written between one and two carries a single transaction. Both are refused now, and the walk that answers what a key's field is returns the field's type rather than a yes or no, so three walks over the same tags became one. Both callers of that guard printed one of its reasons for all of them. A negative number came out as "cannot be negative, and decodes to the largest value this setting can hold rather than to no limit is a length of time written as a plain number, which reads as nanoseconds". Each message already stands alone, so the callers print what they were given. A password reached the log. The transaction index takes a PostgreSQL connection string, and the report naming what a delivery changed is the only place the running configuration is written down. Nothing logs that string today. A value carrying a password now has it taken out, detected in the value rather than from a list of keys somebody keeps in step. An unread key reported as a key that did not move. Reading a value used a missing map entry to mean "could not read", so a key absent from both sides compared equal and was reported as unchanged, which is what a key an operator wrote and got looks like. The read now names what it could not read, and the caller says so. The copy walked the type and had no case for an array, so an array of pointers would have been shared. The test holding it to that promise had the same blind spot, and its share check had no case for an interface, so eleven of the twenty-six paths it enumerated could not fail. All three are fixed. A test comparing a hundred and fifty keys was comparing the absence of a value with the absence of a value for a hundred and forty of them: the node's own configuration holds the decoded sections and it was handed every declared key. The read now fails the test rather than answering partially, and the test compares each key through the delivery that owns it. Two tests skipped on a precondition that is the thing they measure, dead code, a signature whose second argument was never read, and two doc comments claiming a guarantee their bodies do not give are all corrected. The precedence between sei.toml and the node's own files is written down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The decode delivery imports go-viper/mapstructure directly, and go.mod still carried it as indirect. The lint job runs a `go mod tidy` check, which failed on that line for this branch and every branch above it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…rries one in The redaction only handled the userinfo of a URL. PostgreSQL accepts a password three other ways, and all three reached the log: postgres://seid@host/idx?password=... postgres://seid@host/idx?sslpassword=... host=... user=seid password=... dbname=idx Four of five forms leaked. A named password field is now removed wherever it appears, with the prefix kept so sslpassword still reads as itself, and the value running only to the next separator so nothing after it is swallowed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…his delivery makes untrue The doc comment on the decode guard still opened with the name it had before the rename, so it did not name the function it documents. Swept the rest of the package: every other doc comment names its own subject. The install's report of keys it holds back goes away here, because this is where they stop being held back. The delivery it describes now exists, so the report that said those values do not arrive would be the false one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A value can decode cleanly, mean exactly what it says, and still be one the node refuses. A transaction-size ceiling of minus one is a valid int, so none of the four shapes the decode guard covers applies to it. It decoded to -1 and published, and the node then measured every transaction against it and found all of them larger, so it would have accepted none. Around thirty checks of that kind live in the node's own validation and nothing above this could see any of them. Checked on the rehearsal copy, which is the one place a copy exists to check, so it inherits the section-scoped refusal: a bad value costs its own section rather than the file. This is parity rather than a regression, since an existing config.toml is not validated either, but the copy makes it cheap to close for the channel added here. A quoted password leaked its tail. PostgreSQL accepts a keyword value in quotes and a password may hold spaces, so a run that stopped at the first one redacted the first word and left the rest in the log line. The test could not see it either, because it compared the whole secret and the first word was genuinely gone; it now asserts on every word. The production file no longer imports testing. The helper that needed it takes the two methods it uses as an interface instead, which keeps the property that mattered: it can still end a test, so it cannot answer without having measured. The type walk beside it moved into the test that is its only caller. Five other dependencies already pull testing into this binary, so that part of the concern was not new, but a production file importing it is worth not adding to. Three doc comments named the mechanism this delivery deliberately does not use, and a test variable carried a number that meant nothing to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two tests here called an accessor the install PR removed once a single read replaced it. They go through the same one read now, so a test cannot describe a registry the boot would not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…r leaf holds The redaction failed open on three inputs. A backslash escape inside a quoted keyword value ended the match at the escaped quote, so the tail of the password stayed in the line, and the same held for an escaped space in an unquoted value. Separately, a password holding a byte a URL parser refuses in userinfo made the parse fail, and a connection string in that form carries no named field either, so the whole credential was logged. Escapes are consumed with what follows them now, and a value that does not parse as a URL has its userinfo removed by pattern. The test could not see any of it, so it grew the three cases. Removing either half fails it on two and three leaked words. A pointer to anything other than a struct is left as a pointer by the decoder, so rendering one gives an address, and publishing assigns a fresh pointer for a leaf that is not a struct. The report would have named such a key as moved on every boot and printed two addresses. Nothing reaches it today because every pointer leaf is left undeclared, but the walk around it is driven from the type so that a field added later is covered, and this keeps that true for one more shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…wn rules The node's rules answer for the whole configuration, and a boot never applies them to an existing config.toml, so a node can already hold a value they reject. Refusing on that blamed the section being delivered for a failure it did not cause and left every later change unable to land on that node. The copy is now compared against what the node already holds: a failure that was already there is reported and the section is applied, and only a failure this section introduced refuses it. The keys neither side could be read for are dropped before the comparison. Naming them and then comparing them anyway let the report say the section matches the node's own file, which is the statement naming them exists to withhold. The filter is its own step so it can be measured, because it had no test when it was inline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…a source wrote A section read by a decode holds what its own file said, and config.toml is not consulted for a declared key under this manager. So every key the resolution answered is handed to the decode, including the ones that took their declared value. A key sei.toml leaves out would otherwise keep whatever config.toml said, which makes the file a patch on the configuration rather than the configuration. That replaces what an operator's config.toml said for a key their sei.toml does not mention, and it is meant to. A path rendering sei.toml from a node's existing files is what makes it safe, and it has to land before this is switched on anywhere. The accessor is named for what it now answers, the delivery no longer returns whether it did anything, and two tests that asserted the old model now assert this one. The one whose failure message said it plainest read "the node's own file turned the metrics listener on, sei.toml said nothing about it, and the node runs with it off" — which is now the correct outcome. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit ed5b3a9. Configure here.}

cursor · 2026-08-31T22:00:19Z

+	shown, omitted := capLoggedItems(moved)
+	log.Info("this section's settings now differ from what the node's own configuration file says",
+		"section", name, "count", len(moved), "changed", strings.Join(shown, "; "), "omitted", omitted)
+}


Delivery reports stay loud off start

Low Severity

reportWhatMoved always logs at info, while installResolved drops the ordinary install line to debug on every command that is not start. Every subcommand still runs delivery, so seid keys list and similar emit an info line per decoded section and bury the refusals that are meant to stay visible.

Additional Locations (1)

cmd/seid/cmd/configmanager/install.go#L105-L118

^{Reviewed by Cursor Bugbot for commit ed5b3a9. Configure here.}

bdchatham · 2026-09-02T16:55:12Z

Folded into #4043.

The split was on the wrong line. This PR deleted two functions #4043 added, renamed a third, and rewrote the package doctrine, so a reviewer of #4043 alone read about 130 lines that did not survive and a statement that decoded keys are "reported here but not delivered" which was true for one PR. The two deliveries are one design and now land together.

Nothing is lost: #4043 is a fast-forward to this branch, so every commit here is in it.

bdchatham force-pushed the plt-775-install-decode branch from f70df38 to ae60b14 Compare August 27, 2026 22:09

bdchatham added the non-app-hash-breaking label Aug 27, 2026

bdchatham force-pushed the plt-775-install-app branch from 0becc6b to 05f98d1 Compare August 31, 2026 14:29

bdchatham force-pushed the plt-775-install-decode branch from ae60b14 to 93d7b68 Compare August 31, 2026 14:29

bdchatham marked this pull request as ready for review August 31, 2026 14:29

cursor Bot reviewed Aug 31, 2026

View reviewed changes

Comment thread cmd/seid/cmd/configmanager/tendermint_copy.go

Comment thread cmd/seid/cmd/configmanager/tendermint.go

cursor Bot reviewed Aug 31, 2026

View reviewed changes

Comment thread cmd/seid/cmd/configmanager/tendermint.go

seidroid Bot approved these changes Aug 31, 2026

View reviewed changes

bdchatham force-pushed the plt-775-install-decode branch 3 times, most recently from a697060 to 48812ba Compare August 31, 2026 16:15

seidroid Bot approved these changes Aug 31, 2026

View reviewed changes

Comment thread cmd/seid/cmd/configmanager/tendermint.go Outdated

Comment thread cmd/seid/cmd/configmanager/tendermint_copy.go

bdchatham force-pushed the plt-775-install-decode branch 2 times, most recently from 9802cfe to f60169a Compare August 31, 2026 18:03

cursor Bot reviewed Aug 31, 2026

View reviewed changes

Comment thread cmd/seid/cmd/configmanager/tendermint.go

Comment thread cmd/seid/cmd/configmanager/tendermint.go Outdated

bdchatham force-pushed the plt-775-install-decode branch 2 times, most recently from df86468 to 4234426 Compare August 31, 2026 21:49

bdchatham and others added 8 commits August 31, 2026 14:51

bdchatham and others added 2 commits August 31, 2026 14:51

bdchatham force-pushed the plt-775-install-decode branch from 4234426 to ed5b3a9 Compare August 31, 2026 21:53

cursor Bot reviewed Aug 31, 2026

View reviewed changes

yzang2019 approved these changes Sep 2, 2026

View reviewed changes

bdchatham closed this Sep 2, 2026

Uh oh!

Conversation

bdchatham commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why a second delivery exists

Decode into a copy, then publish through the pointers that exist

Values that decode cleanly and mean something else

Verified

Uh oh!

github-actions Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bdchatham commented Aug 27, 2026

Uh oh!

cursor Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seidroid Bot left a comment

Choose a reason for hiding this comment

Blockers

Non-blocking

Pre-existing issues

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bdchatham commented Aug 31, 2026

Uh oh!

seidroid Bot left a comment

Choose a reason for hiding this comment

Blockers

Non-blocking

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 31, 2026

Choose a reason for hiding this comment

Delivery reports stay loud off start

Uh oh!

bdchatham commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bdchatham commented Aug 27, 2026 •

edited

Loading

github-actions Bot commented Aug 27, 2026 •

edited

Loading

codecov Bot commented Aug 27, 2026 •

edited

Loading

cursor Bot commented Aug 31, 2026 •

edited

Loading