[ConfigManager] Deliver sei.toml for Decoded Sections 2/3 - #4044
Draft
bdchatham wants to merge 2 commits into
Draft
[ConfigManager] Deliver sei.toml for Decoded Sections 2/3#4044bdchatham wants to merge 2 commits into
bdchatham wants to merge 2 commits into
Conversation
|
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 @@
## plt-775-install-app #4044 +/- ##
=====================================================
Coverage 60.27% 60.28%
=====================================================
Files 2056 2058 +2
Lines 176963 177299 +336
=====================================================
+ Hits 106671 106887 +216
- Misses 60499 60588 +89
- Partials 9793 9824 +31
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
Author
|
@seidroid review |
…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>
bdchatham
force-pushed
the
plt-775-install-decode
branch
from
August 27, 2026 22:09
f70df38 to
ae60b14
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Delivers the values an operator's
sei.tomlsupplies for the sections whose reader decodes itsfile whole. 1673 lines. Second of three, on top of the install.
Why these need a second delivery
The 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 installing a value
into that source reaches nothing for them. Measured on a real boot: viper answers
19999for akey whose live struct field holds
26656.Rehearse into a copy, then publish through the pointers that exist
The values are decoded into a deep copy of the live configuration first. The copy has to be of the
node's own configuration rather than a fresh one, because what a decoder writes depends on what the
target already holds.
Publishing then assigns each section through the pointer already there, rather than replacing the
configuration wholesale. Components in the node take a section rather than the configuration
holding it, and that type carries nine section pointers, so replacing the top level hands every
holder a section frozen at its old values. Assigning through the pointers keeps their identity, so
a component reads the delivered values whichever side of the delivery it was built on.
One section at a time
A decode is all or nothing for whatever it is handed, so a single value a decoder refuses would
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 log level goes first
Before any of the reporting, because a refusal is reported at a level an operator may have raised
the threshold above. Doing it after would mean the one setting somebody changes in order to see a
refusal is the setting a refusal suppresses. An exported level still wins, matching the order a
node already resolves that setting by.
One report corrected
The line saying the file supplied no declared value describes the lookup delivery alone, and both
deliveries run in one pass. An operator whose file moved a setting through a decode was told the
file supplied nothing, a few lines later, in the same boot. It is now scoped to the delivery it
describes.
What review changed
A value that decodes cleanly and means something else. The guard for this checked the sign and
not the magnitude, so the pre-flight command approved two files that change what a node runs:
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. Both callers of that guard also printed one of
its reasons for all of them, so 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 handed.
Publishing. The delivery replaced the whole configuration struct, which swapped all nine
section pointers. It was correct only because the delivery happens to run before the components
that hold those pointers are built, and nothing stated that order or would fail when it changed.
Each section is now assigned through its own pointer.
Two things the reports got wrong. A password reached the log: the transaction index takes a
PostgreSQL connection string, and this report is the only place the running configuration is
written down. Nothing logs that string today, and a value carrying a password now has it removed,
detected in the value rather than from a list of keys somebody keeps in step. And an unread key was
reported as a key that did not move, because a missing map entry meant "could not read": absent on
both sides, it compared equal, which is what a key an operator wrote and got looks like. The read
now names what it could not read.
Assertions that could not fail. The copy walks the node's 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 comparison had no case for an interface, so eleven of the twenty-six paths it
enumerated could never fail. A separate 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, because the node's
own configuration holds the decoded sections and it had been handed every declared key. The read
now fails a test rather than answering partially.
Two tests that skipped on the precondition they exist to 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.tomland the node's own files is written down.Notes for review
Formatters,
go vetandgolangci-lintclean;-count=2 -shuffleclean.The verification of that last fix is worth recording, because the first attempt was a false pass:
the mutation pattern had not applied, so an unmutated run was read as proof it was covered. With
the mutation genuinely in place nothing failed, which is how the test for it came to exist.
The divergence record measures a node's second boot rather than its first. A flag bound to the
debug listener address hides that divergence on the first boot only, because the file has not been
read yet for the flag's empty default to lose to, so measuring the first boot measures the one
boot no running node repeats.