[ConfigManager] Generate a Node's sei.toml - #4094
Conversation
…e on The decode side recorded four keys where a declared value and a boot-generated file disagree. It resolved one kind of node, so it covered a quarter of the space, and the lookup side had no record at all. Measured across every kind of node and both deliveries. Thirty-six rows in four sets, and eight of them are decode-side rows three kinds of node have that a validator does not. Two are worth naming: p2p.laddr and rpc.laddr bind loopback and are declared on every interface, so a full or archive node adopting a sparse file reaches the network where it did not. One row is a third writer rather than the second. A key with a flag of its own on the start command is answered by that flag's default whether a file mentions it or not. That is pruning, in every kind of node: the flag prunes and the declaration keeps all state history. Eleven further declared keys are left unanswered by a generated file. Their readers each take the source's answer only when there is one, so the comparison has nothing to read for them and they are recorded as a named set instead. A key that stops being answered moves into that set and nothing else would say so. The two records are one, because they had the same shape and the same gap. The two-boot helper is shared rather than held inside one measurement. Each check verified by mutation: a dropped row, a wrong value, an extra row, a dropped kind of node, a row that no longer diverges, reading a decode key off the wrong delivery, and both directions of the unanswered set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A node under this manager answers every declared key from its sei.toml, so a sparse file moves every key it leaves out to the value this binary declares, and there are more than two hundred. Writing that file by hand means finding each of those by reading two files against a binary's defaults. seid config generate writes it from what the node answers today. Printed by default, since the file is read for every setting a node has and an operator reading it before it takes effect is the ordinary case. --write places it, and leaves an existing file alone. Two things make the written file run what the node ran. Every value it states is the node's own, read where that setting's reader reads it: the keys a decode delivers off the struct config.toml is decoded into, and every other key off app.toml over the start command's flag defaults. And a key nothing answers is not stated, because there is no value to state and its reader holds a default of its own. Reading the flag defaults is not an optimisation. Measured: app.toml over those defaults answers all 165 lookup keys exactly as a boot's own source does, and pruning is answered by a flag alone. A source built without them leaves pruning out of the file and silently stops a node pruning. The environment is not read. A variable that answers a declared key goes on answering it afterwards at the same precedence, so writing it into the file would state a value twice and leave the copy with no effect. Verified by starting a node from the written file: 154 keys a lookup answers and 116 a decode delivers, all unchanged across it. A second test holds which keys the file states, because a file full of lines restating the declaration passes a round trip while telling an operator that two hundred settings were decisions. Each check verified by mutation, with the mutation confirmed to compile: a source built without the flag defaults, a file stating nothing, a file stating every answered key, the decode keys omitted, an accepted empty home, an accepted unknown kind of node, and a replaced existing file. The home resolution, the node's own configuration reader and the typed read off that struct move to where both commands share them. Check's own tests pass unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4094 +/- ##
==========================================
- Coverage 61.33% 60.29% -1.05%
==========================================
Files 2184 2078 -106
Lines 191492 179119 -12373
==========================================
- Hits 117457 108003 -9454
+ Misses 62951 61066 -1885
+ Partials 11084 10050 -1034
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…delivery The round trip verifying this command was passing on a node that delivered nothing. A boot refuses a sei.toml whose kind of node disagrees with the kind the node runs as, and it refuses the whole file. The measurement wrote a file for a validator against a node whose own file recorded a full node, so nothing was installed, every key read the same before and after, and the check held for any file at all. Found by breaking the writer so it stated declared values instead of the node's, and getting a pass. The node's own configuration file records the kind, so the disagreement is answerable before anything is written. Refused now, with the shared answer rather than a second copy of the rule, so the one pairing that is not a disagreement stays accepted: the kind that keeps every version of history has no name in that file. A home no node was created in is refused as well. Both files are absent there, so every value would come from a flag's default and the file would describe this binary while looking like a node's configuration. The measurements now prepare a node that runs as the kind they generate for, and let the boot write both files rather than rendering a third. The two writers in this binary disagree on several keys, so a rendered file produces a node matching neither and the divergence record would be measuring the test. Verified by mutation on a boot-generated node and a hand-tuned one, each caught by both: stating declared values instead of the node's, stating nothing, never reading app.toml, never reading config.toml. The kind guard is verified in both directions, including a direct comparison that would refuse an archive node. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR SummaryMedium Risk Overview Shared
Reviewed by Cursor Bugbot for commit c67c288. Bugbot is set up for automated code reviews on this repo. Configure here. |
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
seid config generate is well-guarded (home, node presence, mode agreement, existing-file refusal) and backed by strong round-trip tests through a real boot, including a hand-tuned node. Two non-blocking gaps remain: the command never reads an existing sei.toml when deciding what the node "already runs", and the never-replace guarantee is a stat-then-rename check.
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.
| if err := theKindThisNodeAlreadyRuns(mode, own); err != nil { | ||
| return err | ||
| } | ||
| running, err := whatThisNodeAlreadyRuns(cmd, home, own) |
There was a problem hiding this comment.
[suggestion] whatThisNodeAlreadyRuns derives the node's current values from app.toml and config.toml only; an existing sei.toml is never read. The delivery installs resolved values into viper and into the config struct at boot and writes nothing back to those two files, so on a node already running under SEI_CONFIG_MANAGER=v2 the printed file describes the pre-manager state rather than what the node runs, and the godoc claim above ("a node started against the written file runs what it ran before") does not hold for it.
--write refuses when a sei.toml exists, so nothing is silently replaced — but that refusal message tells the operator to "Print this instead and compare them", and the printed file was computed as if the existing file did not exist. Comparing the two then shows differences that are artefacts of the read, not decisions. Every other state where the output would be plausible-but-wrong (no home, no node, disagreeing mode) is refused up front; this is the remaining one. Either resolve through the existing file, or refuse/annotate when config/sei.toml is present.
| // Asked before the document is rendered, so a home that already holds a file costs nothing and the | ||
| // refusal is the same whatever the file would have said. | ||
| path := filepath.Join(home, "config", seiTomlName) | ||
| if _, err := os.Stat(path); err == nil { |
There was a problem hiding this comment.
[suggestion] The "an existing file is never replaced" guarantee is a stat followed by file.Save, which installs via os.Rename and will happily replace a regular file created in the window between the two. Creating the destination with O_CREATE|O_EXCL (or renaming onto a path proven absent by the create itself) makes the guarantee hold rather than depend on timing. Low likelihood for an operator-run CLI, but the comment states it as an invariant.
1 participant
The problem
A node under this manager answers every declared key from its
sei.toml. So asparse file is a large change to a node whose own files were tuned: every
declared key the file leaves out moves to the value this binary declares, and
there are more than two hundred of them.
Writing that file by hand means finding each of those by reading
app.tomlandconfig.tomlagainst a binary's defaults, per kind of node. Nothing in thestack so far does that.
The command
seid config generate --mode <kind>writes the file from what the node answerstoday. It prints by default, because the file is read for every setting a node
has and reading it before it takes effect is the ordinary case.
--writeplacesit at
config/sei.toml, and leaves an existing file alone.A validator that let the boot generate its files:
Why it writes what it writes
Two things make the file run what the node ran, and the short output is neither
of them.
Every value stated is the node's own, read where that setting's reader reads
it. The keys a decode delivers come off the struct
config.tomlis decodedinto. Every other key comes off
app.tomlread over the start command's flagdefaults.
A key nothing answers is not stated, because there is no value to state. Those
readers each start from a default of their own and take the source's answer
only when there is one, so stating this binary's declaration instead is the one
way a writer here changes a setting nobody decided to change.
What the short output does is leave out a key whose answer is already the
declared value. Those lines would state what the declaration states, so every
line in the file marks a decision.
The flag defaults are not an optimisation
A boot binds its start command's flags into the source it builds and then reads
app.tomlover them, so a key with a flag of its own is answered by that flag'sdefault whether the file mentions it or not.
Measured:
app.tomlover those defaults answers all 165 lookup keys exactly asa boot's own source does, precedence included.
pruningis answered by a flagalone and by no file. A source built without the flag defaults leaves
pruningout of the written file, and a node adopting that file stops pruning, because
the declaration keeps all state history.
The command refuses rather than proceeds if it cannot find the start command,
for that reason.
The command refuses three things
Each one produces a plausible file rather than an error, which is what makes them
worth naming.
A kind of node the node's own file contradicts. A boot declines a
sei.tomlwhose kind disagrees with the kind the node runs as, and it declines the whole
file rather than the keys that differ, so the operator holds a file that changes
nothing. The one pairing that is not a disagreement stays accepted: the kind that
keeps every version of history has no name in the node's own file.
A home no node was created in. Both files are absent, so every value would
come from a flag's default and the file would describe this binary.
A missing or unknown
--mode. Every value in the file resolves for one kind,and the kinds differ on whether a node prunes and whether it serves queries.
It also leaves an existing
sei.tomlalone, because it holds what somebodydecided and the comments beside those decisions.
The environment is not read
A variable that answers a declared key goes on answering it after this runs, at
the same precedence. Writing it into the file would state the value twice and
leave the copy in the file with no effect.
What the record found
The first commit extends the divergence record, which is what makes the writer
reviewable: it is the set the writer has to produce.
The decode side recorded four keys and resolved one kind of node, so it covered
a quarter of the space. The lookup side had no record. Measured across all four
kinds and both deliveries, there are thirty-six rows, and eight are decode-side
rows that three kinds of node have and a validator does not.
Two are worth naming.
p2p.laddrandrpc.laddrbind loopback and aredeclared on every interface, so a full or archive node adopting a sparse file
reaches the network where it did not.
Eleven further declared keys are left unanswered by a generated file and are
recorded as a named set, because the comparison has nothing to read for them.
Verification
A node that really delivers the file: 154 keys a lookup answers and 116 a decode
delivers, all unchanged across it. Measured twice, on a boot-generated node and
on one whose
app.tomlandconfig.tomlwere edited by hand, because the valuesan operator chose agree with neither writer and are the ones a divergence record
cannot cover.
A second test holds which keys the file states, for the direction the round trip
cannot see. A file full of lines restating the declaration passes a round trip
while telling an operator that two hundred settings were decisions.
Every check was broken on purpose and confirmed to fail for its stated reason,
with the mutation confirmed to compile each time: stating declared values instead
of the node's, stating nothing, stating every answered key, never reading
app.toml, never readingconfig.toml, the decode keys omitted, an acceptedempty home, an accepted home with no node, an accepted unknown kind of node, a
dropped kind guard, a kind guard comparing directly and so refusing an archive
node, a replaced existing file, a dropped record row, a wrong recorded value, an
extra row, a dropped kind of node, and a row that no longer diverges.
One of those mutations is why this is worth reading. Stating declared values
instead of the node's passed the round trip, because the node under measurement
was refusing the whole file: a boot declines a sei.toml whose kind disagrees with
the kind the node runs as, the measurement generated for a validator against a
node recording a full node, and nothing was installed. The check held for any
file at all. The command now refuses that disagreement itself, and the
measurement prepares a node of the kind it generates for.
The home resolution, the node's own configuration reader and the typed read off
that struct move to where both commands share them. Check's own tests pass
unchanged, which is the proof the extraction changed nothing.
go test -racegreen on the touched packages,make fmtcheckclean,golangci-lint0 issues. Run end to end against a real single-node chain: thenode produced blocks under the legacy loader,
config generate --writeplaced afile stating 3 of its 281 declared keys, and the node then produced blocks under
SEI_CONFIG_MANAGER=v2from that file with no refusals.Not in this
seid initwriting asei.tomlfor a fresh node. That node has no files toread, so it is a different question and a separate change.