[ConfigManager] Declare the Config Values a Node Actually Reads - #4096
Draft
bdchatham wants to merge 1 commit into
Draft
[ConfigManager] Declare the Config Values a Node Actually Reads#4096bdchatham wants to merge 1 commit into
bdchatham wants to merge 1 commit into
Conversation
…hing declares A node's own configuration files state 276 keys and this binary declared 281, and 22 of the file's keys were not among them. An undeclared key goes on resolving from the file that states it, which is the one thing sei.toml cannot reach: the resolution has nothing to say about it, so writing that file changes nothing for it and neither does writing it again on every start. A node enrolled in automatic configuration management was held to its kind's defaults for every key but those 22. Four of them are used. The gRPC-web settings are read by the upstream server, and one of them is assigned per kind of node by the same mode rules the rest of this package follows, so a rule already targeted a key the declaration could not express. Declared now, which adds two rows to the agreement record: the interface is open on a generated file and declared closed for a validator and a seed, the way the REST interface and gRPC already are. Twelve are not used, and each now carries its reason. The query settings are the find: the reader that builds the server configuration from the source fills that whole section from its own defaults and never asks the source for any of its five keys. Measured, a written page ceiling of 500 arrives as the default 1000 and a written allowlist arrives empty, while a gRPC-web address written beside it arrives. The file's own comment tells an operator to set one of them. Rosetta's six are an unsupported feature, pending removal rather than declared. The remaining six were already decided, and those rows name the constant holding each reason rather than restating it. The measurement is what keeps this closed. A key written into a template with no section to declare it now fails rather than answering from app.toml quietly, and a key recorded here that the binary starts declaring fails as a record of a hole that is closed. Verified by mutation: a dropped row, a row naming a declared key, and the gRPC-web registration removed, which reports all four of its keys and is the gap this started from. 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-generate #4096 +/- ##
====================================================
- Coverage 60.29% 60.27% -0.03%
====================================================
Files 2078 2076 -2
Lines 179119 178923 -196
====================================================
- Hits 108003 107848 -155
+ Misses 61066 61041 -25
+ Partials 10050 10034 -16
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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.
Stacked on #4094. Review that one first; this adds two commits on top.
What was wrong
A node's own configuration files state 276 keys. This binary declared 281, and
22 of the file's keys were not among them.
An undeclared key goes on resolving from the file that states it, and that is the
one thing
sei.tomlcannot reach. The resolution has nothing to say about thekey, so writing that file changes nothing for it, and neither does writing it
again on every start. A node enrolled in automatic configuration management was
held to its kind's defaults for every key except those 22.
Four of them are used
The gRPC-web settings are read by the upstream server:
server/start.gostartsthe listener and
server/grpc/grpc_web.goreads the address, the CORS switch andthe connection ceiling. And
params.SetAppConfigByModeassignsgrpc-web.enableper kind of node, so a mode rule already targeted a key the declaration could not
express.
Declared now, which adds two rows to the agreement record: the interface is open
on a generated file and declared closed for a validator and a seed, exactly as
the REST interface and gRPC already are.
Twelve are not used, and now say why
The query settings are the find. The reader that builds the server configuration
from the source fills that whole section from its own defaults:
It never asks the source for any of the five keys. Measured:
The file's own comment tells an operator to "set
disable-limitson nodes notexposed to untrusted callers", and there is validation warning that an overly
broad
trusted-cidrsentry lets public callers bypass pagination limits. Neithervalue ever arrives. The compiled defaults are the restrictive ones, so limits stay
on and nobody is trusted: the failure is that an operator cannot grant trusted
access or move a limit, while believing they have. Filed separately; not declared
here, because declaring a key nothing reads offers a setting that changes nothing.
Rosetta's six are an unsupported feature. Recorded as pending removal rather than
declared, so the record closes without waiting on a product removal.
wasm.lru_sizehas no reader.eth_replay.eth_replay_contract_state_checksis atemplate line whose name the reader does not look up.
The remaining six were already decided in
config/tendermintbase, which keeps anamed constant and a reason for every key it leaves out. Those rows name the
constant rather than restating it, so the two cannot drift.
What keeps it closed
config/tendermintbasehad this discipline andconfig/cosmosbasehad none: itregistered five sections and every key outside them was undeclared with nothing
recorded. That asymmetry is why
rosetta.*,query.*andgrpc-web.*all satoutside the declaration for no stated reason.
The new measurement holds both directions. A key written into a template with no
section to declare it fails, rather than answering from
app.tomlquietly. A keyrecorded as undeclared that the binary starts declaring also fails, as a record of
a hole that is closed.
Verification
Every check broken on purpose and confirmed to fail for its stated reason, with
the mutation confirmed to apply: a dropped row, a row naming a key the binary
declares, and the gRPC-web registration removed — which reports all four of its
keys and is the gap this started from.
The first attempt at that last mutation silently failed to apply and reported a
pass; the retry is why the claim stands.
go test -racegreen on the touched packages,make fmtcheckclean,golangci-lint0 issues.Not in this
without the removal.
and wants its own review.