[ConfigManager] Register Node Sections 3/4 - #3994
Merged
Merged
Conversation
Four sections, each registered by the package that owns its struct, so the struct, the values and the keys come from one place and cannot drift apart. The keys derive from the mapstructure tags, which is what makes the registry's spelling and each reader's own constants the same strings, and each package's test holds the two against each other rather than against a written-out list. admin_server 2 keys giga_executor 2 keys receipt-store 6 keys wasm 3 keys Three register their own struct. wasm needs a schema, because the upstream type carries no mapstructure tags at all, so keys derived from it would be field names rather than the ones the module reads. Its simulation gas limit is text because the field it stands for is an optional number and absent is a meaning of its own: unset means the consensus block gas limit applies, which no number can say. Two of that type's settings declare nothing, one having no key any reader resolves and the other written into app.toml by the template and read by nothing. Registering the receipt store needed a distinction the registry did not draw. mapstructure reads a tag of "-" as skip this field, and a configuration struct uses it for a field something else assigns: KeepRecent comes from the global min-retain-blocks flag at the app layer, ExternalPruning from whatever constructs the garbage collector. The registry read that as a missing name and refused the whole section. Such a field now declares no key, which is narrower than declaring one that resolves to a default and safer for the same reason: a declared key is written at override precedence, so a default would land on top of the value that code assigned, and a node with min-retain-blocks set would silently keep nothing. A field with no tag at all stays a defect, because that is the opposite intent, a key nothing names reaching no field. The skip lives in tagOf, which both walks already share, so the declared keys and the rendered defaults describe the same fields. Reverting either walk's skip on its own fails a test. The recorded configuration surface does not move: nothing consumes the registry yet, and no golden changed. 100% of statements in config/registry, race clean. Three mutations each fail a named test: refusing a dash again, skipping an untagged field, and letting the two walks disagree.
Four sections owned by the app package, each registered where its struct lives. genesis 2 keys light_invariance 1 key state-commit 20 keys state-store 12 keys light_invariance registers the type its reader fills. The other three declare a schema, because keys derived from the type the reader fills are not the keys the reader looks up. The genesis import type carries no mapstructure tags at all. State store and state commit both tag their fields with something other than the name resolved from configuration, and state commit nests its settings under three inner structs while the keys are flat names on the section, apart from the one flat key-value setting that has a segment of its own. State commit's write mode is text rather than the reader's named type, because the reader parses a written name into that type itself. Declaring the named type would have one key answer as a named string from these defaults and as a plain one from an operator's file. Each section's test holds its resolved keys and values against the reader's own constants and its own defaults. Resolving is what it compares, rather than the registered struct, because the resolved map carries the key a tag produced and the value that tag's field held: a comparison of struct to struct agrees with itself while two tags sit on the wrong fields, since each field still holds the value the test names for it. Putting the genesis tags on each other's fields leaves the key set identical and fails the test. State store's declared defaults are not what its reader produces for a file missing those keys. It starts from the declared defaults, then assigns eleven of its twelve fields straight from a lookup with no check that the key was present, so an absent key casts to a zero and clobbers the default beside it: the store reads as disabled, with no backend, keeping every version, and committing synchronously. Only the snapshot key is guarded, and its own comment at the read says why. A node whose app.toml predates one of the other keys therefore runs the clobbered value today and the declared default once something installs this section, and guarding the remaining reads is what makes those the same thing. The recorded configuration surface does not move, because nothing consumes the registry on a boot path yet.
Four sections, each registered by the package that owns its struct. eth_blocktest 2 keys eth_replay 4 keys evm 57 keys evm_query 1 key All four register the struct their reader fills. None needs a schema, because in each of them the mapstructure tags already spell the keys the reader looks up, so the registry derives what a node reads and nothing restates a list of fifty-seven keys. Each package's test holds the derived keys against the reader's own constants, which are the second statement of the same set in the same file: a rename that moves one and not the other compiles. The EVM section resolves the same values for every mode. Nothing consults a node's kind while reading these keys, so a file missing them serves both interfaces whatever kind of node it is, and that is what these resolve to. A node seid init provisioned is the other case and needs nothing here, since that path writes the two interface toggles per mode and a written value is what resolves. Two of that section's values come from the machine rather than from a decision. The simulation call limit is the processor count and the worker pool is twice it, capped, so they describe whichever host resolved them. That is stated where they are declared, because a caller rendering them into a file carries one host's sizing to whatever reads that file next. Replay declares three of its four keys under the name the template writes and one under a different one. The template renders eth_replay_contract_state_checks and the reader looks up contract_state_checks, so every generated file already carries a name nothing resolves. The declared key is the one a value reaches a reader through, and a test refuses the other: declaring it would add a key an operator can set and no reader answers, which is worse than the mismatch, because the value would look as though it applied. The recorded configuration surface does not move, because nothing consumes the registry on a boot path yet.
Five sections whose keys belong to the Cosmos server, and the two registry capabilities they need. api 8 keys base 14 keys grpc 11 keys state-sync 3 keys telemetry 7 keys These sections have no owning package here. Their structs and their readers live in sei-cosmos, which this repository vendors rather than authors, so there is nowhere upstream to put a registration this registry would see. Four of the five register the upstream struct directly, because its mapstructure tags already name the keys the reader resolves. A section can now declare keys at the root of the file. The node-wide settings are written at the top of app.toml and read as pruning and halt-height, with no segment in front, so a section carrying a name into every key would rename all fourteen and an operator's existing file would reach none of them. A section therefore has a name it is looked up by and a prefix its keys carry, and for a root section the prefix is empty. Both walks build a key through one function, so a root key gains no separator on either side; reverting either one on its own fails a test, the value walk through the check that a rendered default states one value per declared key. Two keys can now collide where two prefixes never could. A key two sections both declare has one default rendered over the other, and which one depends on the order the sections are walked. And a root key that is also a section's name cannot be written at all, because a file holding both a value for that name and a table under it is not valid TOML, so one of the two is unreachable and nothing says which. Both are refused, in either registration order. A section can now say that an environment variable cannot supply one of its keys. The metric label set is a list of name and value rows and its reader asserts that exact shape rather than casting what it finds, so no single string satisfies it, and the assertion is the first statement of the whole server configuration. A resolved variable would install a value that stops the node; leaving the channel out means the file's value applies and the node runs. The reason is required rather than optional, because an operator whose variable is ignored has to be told why, and a refusal with no reason is itself refused. The metric section is the one here that needs a schema, and for one field's shape rather than for a spelling. Its label set is declared as untyped rows to match what the reader takes. A test holds every other field to the upstream field's name, tag and type, and holds the count of differing types at one, so a second divergence is a failure and a converged upstream type leaves the schema with nothing to justify it. Nothing here varies a default by mode. seid init writes the two interface toggles and the block retention per mode, so a node it provisioned carries those as written values, and these are what a node with nothing written runs. One declared value is not what a running node uses, and it is worth knowing which. The pruning strategy is declared as keeping everything, while the command line registers a flag of the same name defaulting to the standard strategy, and a bound flag is a source of its own below the file. A node started with no pruning key written prunes on the standard schedule. Whoever resolves for a running node has to supply the flag values to get the answer that node uses. The recorded configuration surface does not move, because nothing consumes the registry on a boot path yet.
The rule was right and the reason recorded for it was not, which matters because it is the reason the remaining sections will cite. It said a declared key would be written at override precedence and land on top of the value that code assigned. That cannot happen for either field it named: the app layer assigns the receipt store's retention after the reader has returned, so the assignment is last and wins. It also said such a node would silently keep nothing, and the field's own comment says the opposite, that keeping zero versions means keeping everything. An operator handed that sentence during an incident looks for missing receipts and finds a full disk. The reproducible reason is the one every other refusal here rests on. A key for such a field is one an operator can write that the assignment then discards, so it reaches no field. A field with no tag stays a defect for the same reason read from the other end, because it would declare a key derived from a field name and no operator writes that. The two look alike in a diff and mean opposite things, which is why the package's own contract now states the distinction rather than leaving it in a comment beside one branch. Two of the four sections held their declared keys against a written-out list of the same strings. That is a second statement of the key set, which is what a section exists to remove: a tag and the list move together and the reader keeps asking for the old spelling. Both now hold against the constants their reader passes to Get. The admin server's reader was spelling its two keys inline, so it has constants for them now, and its registration no longer recites the derived keys in prose that drifts the moment a tag moves. One assertion is gone because it could not fail, the exact key set having been compared three lines above it. Two declared values are also stated elsewhere in the binary, and each now says so where it is declared. The wasm query gas limit resolves to ten times what the template writes into a generated file, so a node provisioned by the binary runs the smaller number and a node whose file predates the section runs the declared one; whoever renders declared values into a file has to decide which survives, and that limit bounds the work one smart query can ask of a node serving queries to anyone. The receipt store's database directory resolves to an empty string, and the emptiness carries the meaning: the app layer fills it from the host, keeping the former path for a node that already holds the store there. A path written into a file is one host's answer and names an empty directory on another. One comment said the contract debug switch has no key any reader resolves. It is read from the node-wide trace flag, so its key belongs to the root of the file rather than to that section, which also means the section's three keys do not determine the configuration the module ends up with. A new test asks the whole set at once. Two refusals depend on what else has registered, neither is visible from inside either section, and the section that loses is dropped whole with every key it declared. Registering a section whose key collides with the receipt store's leaves all four section suites green and fails only this one.
Two of the state store's settings mean something different depending on what kind of node asks, and this section answered the same for all four. An archive node exists to keep history, and it was declaring a retention of a hundred thousand versions. The binary already says otherwise, in the mode rules it applies when it writes a file: an archive node keeps everything, and a validator and a seed run with the store off. Those rules are now where this section's answer comes from, so a change to them moves this too. That is the correction the pinned defect asks for. The record of it says to pin how configuration resolves today and correct it in the versioned manager, and this registry is the versioned manager, so declaring the rendered value would have pinned the defect a second time in the place meant to fix it. Nothing runs differently yet, because nothing consumes the registry on a boot path, and the direction matters more than the timing: state store pruning deletes inside the store on a timer, the archive volumes are protected against being deleted rather than against being emptied, and pruning frees disk so nothing that watches disk would fire. The rest of this change replaces prose with measurement. The declared values for the two storage sections are not what their readers produce for a file missing the keys, and the comment describing which keys those are was wrong three ways: it named four of the six store settings, missed two, named a commitment setting that does not in fact differ, and missed the one that selects how a node commits. The write mode is read through a presence check and then rewritten unconditionally, so a node with nothing written commits in the derived mode rather than the one that key carries. A comment cannot fail when it is wrong. So the set is measured against the readers now, per mode, and recorded as data: a key that starts diverging fails, and so does one that stops, which means guarding a read has to account for its row rather than quietly making a sentence stale. Each key set is also held against this package's own read-site record, which is kept for another purpose and held against a golden file, rather than against a list written beside it in the same commit. The record spells its keys with the reader's constants and the section derives them from tags, so a rename on either side alone fails. One comment said the other keys under the commitment section's flat key-value name have no reader. Four of them are read by the Cosmos server's own reader, so they belong to whoever registers that section, and saying they reach nothing would have closed the door on declaring them. The whole-registry defect sweep here is now scoped to the four sections this file registers. A refusal that depends on what else has registered is not this package's to answer for, and the sweep that covers it lives where every section is linked.
Four comments said more than their subject. The light-invariance default now states the mode decision and leaves what the check compares to the check. The genesis schema says why it exists and leaves what holds it to the test that holds it. The helper that resolves says once that it renders every section, so a failure naming another one is read correctly. And the write-mode default is asked for every mode rather than one, which is how the tests beside it ask.
The failure messages for an excluded field still claimed the mechanism the comment above them no longer does: that a declared key would land over whatever assigns the field. They now say what is true, that such a key is one an operator can write and the assignment discards. The half of that test covering an untagged field asserted only that some refusal was recorded, so it passed on a refusal raised for any other reason. It now requires the message to name the field, and changing the name it looks for fails it. The registry's contract adds the one thing its new rule leaves unsaid: the exclusion tag is meaningful only on an exported field, because an unexported one carrying any tag is refused before the tag is read. The giga executor's default says why this section does not vary by mode without stating it as a rule for every section, since another section in this work does vary and the binary is what decides which. The fixture in the registry's own spec no longer describes that package as one that would register, because it now does. The wasm section's defaults function takes a name that does not collide with a local of the same name elsewhere in that package, and its test asks every mode rather than one.
Three of these settings mean something different depending on what kind of node asks, and all five sections answered the same for every one. The binary already states the rules, in what it applies when it writes a file, and every section here now answers through them. Each of the three matters in a different direction. A full node and an archive node exist to serve queries, and both interfaces that serve them were declared closed. A validator is meant to expose as little as it can, and gRPC was declared open on every one of them, which is the opposite of what the rule beside it says it is for. And the number of blocks a node retains was declared as keeping everything for a full node, where the rule prunes at a hundred thousand. The rules are read rather than restated, so one added later moves these sections with nothing here changing, and the test writes the three values out by kind of node so a change to the rules fails and gets looked at. Resolving every mode as a validator, opening gRPC on a validator, and changing the retention each fail it. The two sections no rule touches answer through the same function, so there is one place a mode is applied rather than a decision per section about whether to apply it.
…aces per mode A resolved list was the section's own list. A section's default is usually a package-level variable, so a slice field handed out the array that variable holds, and one in-place write by a caller rewrote it for the whole process: every later resolution, and every reader that copies the same struct. Two of the five lists this reaches are deny lists, so the rewrite is silent and it is a security control. The registry already copies a section's keys for exactly this reason and said so in a comment; values now get the same guarantee, at the one function both walks pass through. Removing it fails a test that writes into a resolved list and asks the section's default what it holds. The EVM interfaces answer per kind of node. A full node and an archive node serve queries, which is what those interfaces are for. A validator and a seed serve none, and this section declared both open for every kind, which puts a public request surface on the node that holds a signing key. The rule already exists in this binary. It could not be read from here, because the package that owns the node mode imports this one, so the rule moved to the registry, which is a leaf both sides reach, and the node-mode type now delegates to it rather than stating it twice. Forgetting archive in that one statement now fails a test. Each section also holds its keys to the values their fields hold, not to its own defaults struct compared with itself. That comparison agreed with itself while two tags sat on the wrong fields: the key set stays identical and every field still holds the value it always did, so an endpoint and a directory, or a deny list and an origin list, change places unnoticed. Both of those swaps now fail. Each section reports its own refusal. A registration the registry cannot use is recorded rather than raised, and these tests inferred it from a lookup coming back empty, which threw away the sentence saying why. Two tests are gone because they restated checks that already exist a few files away, where the message is better. Two comments are corrected: enabling replay opens a client without reaching the endpoint, so an unreachable one surfaces during replay rather than at startup, and the two machine-derived values are not one case, because the worker pool re-measures when it is given a value that is not positive and the simulation limit reads zero as no limit at all. The registry now states what a resolved value's type depends on, because it resolves values and does not convert them: a default arrives as its field's type, a file as whatever the format decodes to, and an environment variable as one string.
…d nothing A resolution answered for any string. A section's defaults answer per mode, and a mode this package does not know reached whatever each section does with an argument it cannot match, which for these five is the rules answering as though it were a full node. So an empty string, a capitalised name, or one with a trailing space resolved the interfaces a full node serves onto whichever node asked, with no error. It is refused now, naming the four. A refusal of the environment channel is recorded by a key, so a slip in the spelling named a key no section declares. The channel would never have offered that key, so the refusal covered nothing while reading as though it did, and the key it was written for went on resolving from a variable. Both sets exist for the first time when something resolves, because a refusal may be recorded before the section declaring its key registers, so that is where they are compared. The reason a refusal carries is required and had no consumer. The channel was skipped before the variable was read, so the one fact a diagnostic needs, that an operator set it, was discarded at the cheapest possible point. The variable is read now and its value still thrown away, and the key comes back named, so a required reason is one somebody can be told. A refused key nobody set is not reported, because a value nobody chose is not news. A refusal also names the section that declares the key, so a refused key is attributable the way every other defect is. It was putting the key where the section belongs, which made a defect read as though a key had registered and made a scoped sweep skip it. Four of the metric section's seven hand-copied values were held against nothing. That is the one section here that has to restate its values, so it is the one where a field can be assigned from its neighbour, and assigning the hostname toggle from the enabled toggle survived the suite. Every one of the seven is now held as the key it resolves to rather than as a struct field, because a struct compared with itself agrees while two values sit on the wrong fields. Five comments said things the code does not. The node-wide settings claimed to be unchanged by mode while one of their own keys answers per mode. A count of non-zero defaults was wrong. Two different counts of six read as one, and the pair the sentence lost is read through a clamp that does nothing for an absent key. The package's reason for existing named a vendored tree, when other sections register inside one and the real obstacle is an import edge. And a paragraph named two sections that belong to another change.
Four statements in the package contract described the previous shape. A second entry point exists, for the settings written at the top of a file rather than inside a table, and the contract showed one. The list of what makes a registration unusable no longer enumerated: two sections declaring one key and a top-level key sharing a section's name both became possible once a key could sit at the root, and a refusal of the environment carrying no reason is refused too. The resolution order had gained a per-key hole in one channel and did not say so. And the first step of adding a section told an author to use the name as the first segment of every key, which is false for a section whose keys have none. A mode this package does not declare is also refused now, and the contract says that where it says a default answers per mode.
refuseOverlap refused two collisions and only one of them could happen. Two sections declaring one key is already refused by the environment check, which two identical keys reach by answering to one variable, so that arm was a second guard on a case already covered. The other arm, a key at the top of the file sharing a section's name, was the only one it alone caught, and it has no instance: one section declares keys at the top of the file and none of its fourteen names is a section's. So the code goes and the fact stays. The contract names the collision among the things this package does not guard, with what makes it reachable, because a second such section is where it starts to matter. The prototype found that out by hand: it named the section holding config.toml's top-level keys after the file rather than after the node, because the client file declares a top-level key called node and a node section could not have coexisted with it. The one case the removed guard described better is named better now where it is still refused. Two identical keys were being reported as two spellings of one environment variable, and the reason a dot and a hyphen are the same character to the environment is not the reason a key collides with itself.
…sures where a node differs These sections said their values were what a node with nothing written resolves. They are not, and the difference was carried in four paragraphs of prose with one of the counts wrong. What they are is what seid init writes for a kind of node: the upstream defaults with the binary's own mode rules applied is exactly the pipeline that renders a generated app.toml, so a declared value is what that file would have held. That is a claim about a real pipeline in this binary rather than a judgement, so it can be held, and it is what a caller writing a configuration file wants. Where a node with nothing written resolves something else is now measured. The reader is driven with the start command's flags bound, the way a booting node binds them, because seventeen of these keys are also flags and a flag's registration default is what an absent key reaches before the lookup comes back empty. Twelve keys differ, and the measurement corrected the prose twice over: seven keys the paragraphs implied differ do not once the flags are bound, and the gRPC toggle does, which no paragraph named. Of the two interface toggles it is the only one that diverges, because its flag defaults the interface on while a generated validator file writes it off. A key that starts diverging fails, and so does one that stops, so guarding a read has to account for its row. Dropping the flag binding fails it too, which is what keeps the record measuring what a node gets rather than what the reader says in isolation.
This binary writes an app.toml two ways and they disagree on four keys. The provisioning command applies the mode rules and renders the result; a node starting without a file runs a second pipeline that applies no mode rules at all and carries overrides of its own. So it writes the standard pruning strategy where the command writes keeping everything, a metric retention of sixty against seven thousand two hundred, the REST interface on for a validator against off, and a pruning interval drawn at random on every run. A declared value follows the command an operator runs to provision a node. That was already true and the comment said only that seid writes it, which is ninety per cent of a fact.
A declared value is what the seid init command writes for a kind of node. This section departs from that once: the retention an archive node keeps. The mode rules set it to keep everything and the command does not write that, because the type it renders declares a state store field of its own and fills it from the mode-blind default, so the rule is applied and then discarded. PLT-955 records that and records the decision, which is to pin what a node resolves today and correct it in the versioned declaration rather than at the point that loses it. So the departure is intended, and it is now held rather than asserted. It fails if the command starts carrying the rule, which is the day the departure should be deleted. It fails if this section stops departing, which would declare a retention on the one kind of node whose purpose is keeping what it would prune. Both directions are checked, because a departure nothing measures cannot be told from an oversight.
These four said their values are what a node with nothing written resolves. They are what the seid init command writes for a kind of node, which is a different statement and a checkable one: that command applies the same mode rule to this section's own defaults and renders the result, and for the EVM section it passes what it applied through rather than refilling it from a mode-blind copy, so a declared value here is the value that reaches the file.
…de runs The section declared this module's own default and the command writes a tenth of it, so the two disagreed by a factor of ten on the only bound on the work one smart query can ask of a node that serves queries to anyone. Declaring the larger one meant a caller rendering a file from these values would loosen that bound, and a test named for the module's default invited exactly that reading. The number now lives once, beside the section, and the command reads it. So a declared value is what reaches an operator's file, and the generated file is unchanged, which the command's own characterization suite confirms. Raising it to the module's default fails a test rather than silently widening a public surface. The module's default is still what a node whose file carries no wasm section resolves. That is a different question, and it is held as one. Three other sections say what their declared values are rather than what a node with nothing written resolves.
# Conflicts: # config/registry/doc.go
tagOf carried its rationale inline, which meant the steps were never named. Each condition that needed explaining is now a predicate whose name says why it is there, and the rationale moved to that predicate's doc comment. The tag excluding a field from configuration and the spelling no written key can match are the two that carried comments. Splitting the tag into a name and the squash option is a step of its own, so the body reads as a sequence rather than parsing in place. Behaviour is unchanged and no test moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ions The node's own configuration file carries 141 keys and no section declared any of them. These two are the first, and the registry needed two things before they could be. A section can now leave out a path the struct carries. Two kinds of field earn it. One a reader refuses outright, where writing the key stops the node, so declaring it would put a setting in the space whose only effect is an outage. And one whose absence is itself the setting, where a default would be this package inventing one. An exclusion naming no field the struct carries is refused, because the field it named can be renamed away and leave the exclusion reading as a deliberate omission while excluding nothing. A field that collects what the decode matched no field for now declares no key. What lands in it is what an operator misspelled, so giving it a key would offer the collector itself as a setting to write. Neither the package defining these settings nor the package deciding them can register them: the struct belongs to the node's configuration package and the rules that vary it by node kind live in the parameters package, which imports that struct. So a third package does it, the same shape the upstream server sections already use. Four keys vary by node kind and a test holds all four by value. Two are listen addresses, so a rule that stopped varying would leave a validator binding the address a query-serving node binds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
State sync, the transaction index, instrumentation, the signing key paths and self remediation. That is every table in the node's own configuration file. The transaction index varies by node kind: a node that serves queries indexes transactions so it can answer them, and a validator and a seed serve none, so they index nothing and keep the write. State sync leaves one path out. The servers to fetch a snapshot from are the operator's own peers, so there is no value to inherit, and an address written here would name a host this binary cannot know about. A test now walks the section names this package owns rather than a list kept beside them, so the next section is covered by registering it. Another asserts the registry refused nothing, which is the check no single section can make: two of the refusals depend on what else has registered, and a section that loses is dropped whole rather than reported by itself. 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).
|
Five of these sections carry a root directory field tagged the same as the key at the top of the file, and the node fills every one of them from the command line after the file is read. So the file never carries the value and each section states the empty string for it. Declaring it hands whatever delivers these values an empty root to write over a running node's, and a node that cannot find its data directory, its genesis file or its signing key does not start. Two of the five are here. The test checks every registered section rather than the five, so a section added later that carries the same field fails rather than shipping the same hole. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # config/tendermintbase/tendermintbase.go # config/tendermintbase/tendermintbase_test.go
…he zero The record of what the reader's deprecation check cannot name said a written zero cannot be told from an unwritten field. That is backwards for one of the eight, and it is the one an operator is most likely to write: the stateless leader election setting defaults to true, so an unwritten field decodes to true, a written true is what cannot be told apart, and a written false is detectable. False is exactly what an operator writes after reading a generated file and trying to turn the behaviour off. Measured across the eight: six default to the zero, one to true, and one is a pointer where any written value is distinguishable. The rule that covers all three is the field's own default, so that is what both copies now say. The point of stating the split is that someone closing the gap knows which keys need non-zero-write detection and which need four lines in a check that already tests eight pointers. As written it told them to skip the field whose detectable case is the dangerous one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
The five registrations are correct — section names match the tmcfg.Config tags, the 32-key count matches declaredAgainst's exclusion arithmetic, tx-index.indexer is genuinely the only mode-varying key, and SetRoot/seid init leave these sections untouched so forMode's invariant holds. Both previously unresolved threads are properly closed; one non-blocking note remains on the newly scoped template probe.
Findings: 0 blocking | 2 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 1 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] Nothing in the repo imports
config/tendermintbaseorconfig/cosmosbase(verified by grep — the only references are inside the packages themselves), so the binary-wide sweep incmd/seid/cmd/registry_sections_test.gonever sees their sections. The two registry refusals that depend on what else has registered — one key declared by two sections, and two keys collapsing onto oneSEID_*variable — therefore go unchecked for every tendermint/cosmos base key. No collision exists today (statesync.*,state-sync.*andself-remediation.statesync-*all produce distinct env names), and the newTestEverySectionThisPackageRegistersIsUsablenow readsregistry.Defects()for this package's own registrations, but the cross-package direction stays uncovered and this PR grows the surface by 32 keys. A blank import of the two packages fromcmd/seid/cmdwould fold them into the existing sweep.
…wrong Almost every comment this branch adds narrated what an earlier revision of it did wrong: a loop that used to end in a skip, a parse that gave four answers across forty calls, a predicate that reported seven live settings as dead. The repository's own guide rules that out, and the git log holds it. Rewritten from each declaration rather than edited. Where a sentence was protecting a property a future editor could break, the property is stated in the present tense and the story around it is gone: that only a top-level declaration is read, so a struct inside a function cannot answer; that the deprecation note has to open a line, because a comment mentioning another field's deprecation is not a marking; that the returned marking map belongs to the caller, because the parse behind it is shared. Rationale that was doing no work in a godoc moved to the line that needs it or went. Two docs shed a policy argument that is not a property of the variable it sat on. One kept its second paragraph, because a coverage limit is the kind of second paragraph that earns itself. Two accuracy fixes fell out of the rewrite. The consensus list said its fields are kept so a decode can tell that an operator set one, which is true of the nine held as pointers and false of the six held as values. The interface note said the removed settings are each a pointer to an empty interface, which is true of eight of sixteen. One guard removed as dead. Filtering parsed files by a _test package name cannot fire: Go requires an external test package to live in a _test.go file, and the filename filter above it already excludes those. The invariant it claimed to keep is real and is kept by the filename. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t-775-node-sections-2
…y walk The walk that finds deprecated fields took the first tag segment unconditionally, where the walk that derives keys drops a field naming no key and one collecting what the decode matched nothing for. So the two read the same tags by different rules. The reachable case is the ordinary next step for a dead field: deprecate a consensus setting and take it out of the reader by retagging it to name no key. The marking set then gains a row for that name, and the check reports that the section declares a key which does not exist. Adding it to the removed list makes it worse, since an exclusion matching no derived path is refused and the whole section leaves the registry. Reproduced before fixing and confirmed after: the retagged field now passes, a live declared key marked deprecated is still caught, and reverting the alignment brings the false failure back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # config/tendermintbase/tendermintbase_test.go
Two carried the same pattern the branch below just corrected. One narrated the reasoning an earlier revision used for excluding the snapshot servers and then said both halves of it are measured below, which is commentary about the test rather than about the subject. The other counted the exclusions justified by something other than a marking, and that count is four here and five one branch up, which is what a number nothing measures does. Rewritten from each declaration. Both keep the property a future editor would break: that an excluded path whose field stops being deprecated is a setting the node honours and the space refuses, and that the state sync keys are a set an operator fills together with the scratch directory the one the template leaves out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Applied the godoc discipline from 3993 to what this branch adds, rather than waiting for it to be found here. Removed: a comment attached to no declaration (the root-key exclusions had one sitting between the list it described and the next declaration's doc), three closing paragraphs that were commentary about the comment rather than its subject, a count that reads four on one branch and five on the next, and a paragraph narrating the reasoning an earlier revision used before this one changed it. Kept, in the present tense: that an excluded path whose field stops being deprecated is a setting the node honours and the space refuses; that the state sync keys are a set an operator fills together with the scratch directory the one the template omits; that any reused name fails because no section states which file it belongs to; and that a section's markings come from a set of structs because the root schema carries none of its own. |
Contributor
Author
|
@seidroid review |
Scoping the search to a table left one case answering false for a table the file does not contain, and a caller asserting a key is unwritten then passes while measuring nothing. That is the shape the scoping was added to remove, reintroduced one branch later: the two peer callers both assert false, so renaming the table or folding those settings elsewhere would have silenced both. Demonstrated in both directions by renaming the table in the template: the assertions pass vacuously without this and fail naming the missing table with it. The empty table name still asks about the region above the first table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
The five new registrations are correct: section names match the tmcfg.Config mapstructure tags, the 32-key total matches declaredAgainst's exclusion arithmetic (16+2+3+6+5), tx-index.indexer is genuinely the only mode-varying key among them, SetTendermintConfigByMode touches none of the other four sections, and the state-sync template claims the new test measures hold on today's tree. Two non-blocking notes: the previously reported vacuous-pass in generatedFileCarries is still open, and one new const doc contradicts the test that consumes it.
Findings: 0 blocking | 3 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.
- 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.
Pre-existing issues
- [suggestion] Nothing in the repo imports
config/tendermintbaseorconfig/cosmosbase(verified by grep — the only references are inside the packages themselves), so the binary-wide sweep incmd/seid/cmd/registry_sections_test.gonever sees their sections. The two registry refusals that depend on what else has registered — one key declared by two sections, and two keys collapsing onto oneSEID_*variable — therefore go unchecked for every tendermint/cosmos base key. No collision exists today (statesync.*,state-sync.*andself-remediation.statesync-*all produce distinctEnvNamespellings), and the newTestEverySectionThisPackageRegistersIsUsablereadsregistry.Defects()for this package's own registrations, but the cross-package direction stays uncovered and this PR grows the surface by 32 keys. A blank import of the two packages fromcmd/seid/cmdwould fold them into the existing sweep.
The doc said a generated file writes all of that section's keys but the scratch directory. Measured: the section declares sixteen, the file writes seven, and nine are absent. The body checked six, so nothing failed. The claim now lives on the table that measures it, and the table gained the one key it was missing. Those keys were also spelled across three overlapping tables in one function, up to three times each, so a typo in one could have drifted the three checks onto different keys. One table, one spelling. Two docs corrected. The constant for the unmarked mempool path called itself the only exclusion no marking accounts for and then said every other one is marked or carries a constant, which makes it one of several; it now states what is actually distinctive. The metric prefix constant recorded what its declared value used to be. Two counts removed rather than corrected. The root directory is carried by five sections here and six once the root section registers, so a number in the doc is wrong on one branch either way, and one of the two docs contradicted itself within three lines. Verified by mutation: renaming the peer table, mis-spelling a state sync key, shipping a value for the trust hash, and dropping a deprecation note each fail with the reason that names them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@seidroid review |
The consensus defaults said the same values hold for every mode because the timings have to agree across the validator set. That is true of the paths this section leaves out, which moved to the consensus parameters, and not of the seven it declares: a log path, a proposer's own empty-block policy, a gossip mode, two reactor sleeps and a local restart check. Nodes running different values for those still reach the same decision. The reason is the one the mempool defaults already give, that nothing in the binary derives one from a node kind, which is also what the mode-variance check measures. The record of what the reader's deprecation check cannot name gave one reason for eight rows, and measured against the value the test writes it fits one of them. Seven of the eight probes differ from the field's default, so the ambiguity does not arise for them; they are silent because the check never tests those fields, and it would fire on exactly these writes. Only the leader election setting has a probe equal to its default. Stated as the three reasons it is, because the split is what tells someone closing the gap which rows are cheap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
The five new registrations are correct: section names match the tmcfg.Config mapstructure tags, the 32-key total matches declaredAgainst's exclusion arithmetic (16+2+3+6+5), tx-index.indexer is genuinely the only mode-varying key, SetRoot and seid init touch none of these sections so forMode's invariant holds, and the state-sync template claims the new test measures are true on today's tree. Both previously unresolved threads are properly closed; two non-blocking notes remain — one declared key nothing reads, and one doc clause left dangling by this PR's edit.
Findings: 0 blocking | 3 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.
- 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.
Pre-existing issues
- [suggestion] Nothing in the repo imports
config/tendermintbaseorconfig/cosmosbase(verified by grep — the only references are inside the packages themselves), so the binary-wide sweep incmd/seid/cmd/registry_sections_test.gonever sees their sections. The two registry refusals that depend on what else has registered — one key declared by two sections, and two keys collapsing onto oneSEID_*variable — therefore go unchecked for every tendermint/cosmos base key. No collision exists today (statesync.*,state-sync.*andself-remediation.statesync-*all produce distinctEnvNamespellings), andTestEverySectionThisPackageRegistersIsUsablereadsregistry.Defects()for this package's own registrations, but the cross-package direction stays uncovered and this PR grows the surface by 32 keys. A blank import of the two packages fromcmd/seid/cmdwould fold them into the existing sweep.
Four of that section's five settings reach a reactor, three to block sync and one to state sync. The fifth reaches neither: the node checks its bound while validating and nothing else reads it, so a written value changes nothing about when the node restarts. The template writes it into every operator's file under a comment describing what it would do. That is the criterion the mempool list already applies, and the precedence it already settles: a generated file carrying the key is not on its own what makes it declarable. Its field carries no deprecation note either, so no check on markings accounted for it and nothing held it either way. The root directory doc is rewritten rather than patched. Removing a count from it a round earlier left the clause that reconciled two quantities with nothing to reconcile, and adding this branch's fifth exclusion made those quantities equal, so the clause was explaining a mismatch that no longer existed. It also described the section at the top of the file leaving the path out under its own name, which no section in this tree does yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
yzang2019
approved these changes
Aug 27, 2026
amir-deris
approved these changes
Aug 27, 2026
declaredHere did not say where "here" was: the package, the file or the repository. Named for what it returns instead, which is the keys of the sections this package registers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # config/tendermintbase/tendermintbase.go # config/tendermintbase/tendermintbase_test.go
bdchatham
enabled auto-merge
August 27, 2026 16:47
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 27, 2026
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
4 participants
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.
Declares the last five tables: state sync, the transaction index, instrumentation, the signing
key paths, and self remediation. 31 keys. Nothing consumes them.
After this every table in the file is declared; the keys at its top are the remaining step.
State sync is declared as a set
All 16 keys, with nothing left out. The snapshot servers were previously excluded on the
reasoning that a key with no default cannot be declared, and three siblings in the same table
disprove it: the trust height, the trust hash and the scratch directory each state a zero value
and are declared.
Refusing one made it a key every operator's own file reports as unknown, which is the channel
that reports a typo. Turning state sync on means writing the whole set, so the test holds the
set rather than the one exclusion, with each key spelled once so the three checks on it cannot
drift onto different keys.
What is left out
The metric prefix on the instrumentation table. The node marks the field deprecated and states
that its metrics always use one fixed prefix, so the value is not an operator's to set.
One self remediation setting, because no reactor reads it. The other four in that group reach
one: three the block sync reactor and one the state sync reactor. The node checks this one's
bound while validating and nothing else reads it, so a written value changes nothing about when
the node restarts, while the template writes it into every operator's file under a comment
describing what it would do. Its field carries no deprecation note either, so no check on
markings accounted for it.
The signing key table leaves out the root directory, for the reason the other sections carrying
it do.
One key varies by node kind
tx-index.indexeris[kv]for a node that serves queries and[null]for a validator or aseed. The other four tables answer the same for every mode, and each says why in the source.
Two checks that were measuring less than they claimed
The helper asking whether a generated file carries a key matched a bare name against every line
of the file. A key name is not unique there — the listen address and the connection ceiling
each appear under more than one table — so an unscoped match answered for whichever table
writes the name first. This branch added five generic names to it, one of which is
enable,and the wrong answer would have been a pass. It now takes the table, and a region it measured
no key in is fatal rather than absent.
An exclusion whose field stops being deprecated is a setting the node honours and the key space
refuses, and the check beside it walks declared keys so it could never see that. Deprecation is
now the default justification and an exclusion resting on anything else names the constant
carrying its reason, so a new exclusion either is marked or fails until its reason is written
down. That check is what refused the self remediation exclusion above until its reason was
recorded.
Formatters,
go vetandgolangci-lintclean. Mutation-verified: renaming a table in thetemplate, mis-spelling a state sync key, shipping a value for the trust hash, and dropping a
deprecation note each fail with the reason that names them.