[ConfigManager] Register Node Sections 2/4 - #3993
Draft
bdchatham wants to merge 3 commits into
Draft
Conversation
Neither varies by node kind. How long a node waits at each step of a round has to agree across the validator set for the set to reach a decision, and what a node holds before a transaction is decided is a limit on its own memory. The consensus struct carries fifteen fields the node removed as settings, and it marks each one deprecated. They are excluded: declaring one would offer a key that changes nothing about how the node runs. So the section declares nine of its twenty-four paths. The reader has a check that names the removed settings an operator wrote, and it reaches eight of the fifteen. Six are durations or booleans, where a written zero and an unwritten field hold the same value, so no check can tell them apart. One more it omits. Nothing calls the check in any case. A test holds which eight it reaches, so making it complete fails rather than leaving the count stale, and a second test holds every exclusion to the struct's own deprecated marking rather than to that check. 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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## plt-775-node-sections-1 #3993 +/- ##
===========================================================
- Coverage 57.64% 57.60% -0.04%
===========================================================
Files 2231 2230 -1
Lines 187818 187668 -150
===========================================================
- Hits 108259 108112 -147
+ Misses 69691 69689 -2
+ Partials 9868 9867 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
# Conflicts: # config/tendermintbase/tendermintbase.go # config/tendermintbase/tendermintbase_test.go
…ory out Both carry the root directory field the node fills from the command line after the file is read, so both stated the empty string for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
What this does
Declares the consensus table and the mempool table, 31 keys. Nothing consumes
them, so a node's configuration reads exactly as it did.
Neither varies by node kind
How long a node waits at each step of a round has to agree across the validator set
for the set to reach a decision, so a value that followed from the kind of node
asking would be this change proposing that they disagree. What a node holds before a
transaction is decided is a limit on its own memory and bandwidth, and nothing in
the binary makes one follow from what the node is for.
Fifteen consensus keys are removed settings
The consensus struct carries fifteen fields the node removed as settings and marks
each one deprecated. The fields are kept so a decode can tell that an operator set
one. They are left out here, because declaring any of them would offer a key that
changes nothing about how the node runs.
That leaves nine of the table's twenty-four paths declared, and the root directory makes ten
excluded: both these sections carry the field the node fills from the command line after the
file is read, so both stated the empty string for it.
Counting them by looking for unset pointers gives nine, and that is wrong. Six
of the fifteen are durations or booleans, where a written zero and an unwritten
field hold the same value. The test reads the struct's own deprecated marking
instead, which is the only signal that covers all fifteen.
The reader's own check reaches eight of them, and nothing calls it
There is a function that names the removed settings an operator wrote. It reaches
eight of the fifteen: the six described above cannot be detected at all, for the
same reason, and one more it omits while naming that field's seven siblings.
Neither that function nor the wrapper around it has a caller anywhere in the tree.
So an operator who still has
timeout-commitin their file today gets no error andno warning, and the value is quietly ignored.
That is what makes leaving these out load-bearing rather than tidy. It keeps the key
out of the new format instead of relying on a diagnostic that never runs.
Two tests hold this. One ties every exclusion to the struct's deprecated marking, so
a field that stopped being deprecated fails. The other holds which eight the check
reaches, so making the check complete fails rather than leaving the count stale.
Verification
gofmt,goimports,go vetclean.golangci-lintreports nothing on the touchedpackage.
config/...andcmd/seid/...pass.Reviewing this
Second of four, stacked on the first, so the diff shows only this step.