From 259e8ca751810a65473c4aa47a76530c30faac7d Mon Sep 17 00:00:00 2001 From: bdchatham Date: Mon, 24 Aug 2026 11:59:20 -0700 Subject: [PATCH 1/2] feat(config): declare the node configuration file's root keys Fourteen keys sit at the top of that file with no table of their own. They are declared against a schema rather than the node's top-level type, because that type carries the nine tables as well and declaring against it would declare every table's keys a second time. The schema squashes the same base group that type squashes, so those spellings still come from the node's own tags, and it restates the two fields held beside that group. A test holds those two to the type they came from by name, tag and type, and holds the count, so a third one appearing there fails rather than going undeclared. Two paths are left out. The home directory is where this file is found, so a value inside it would be the file naming its own location, and the command line already carries it. The node mode is the fact the file states at the top under its own name, and a second spelling would let the two disagree, with the resolution answering for one and the node reading the other. A test also checks that no root key is another section's name. Nothing refuses that collision, and the two settings it produces cannot both be written because no file holds a value for a name and a table under it. This is the first change to declare root keys beside another package's, so the check lives here until it has somewhere better to be. Co-Authored-By: Claude Opus 5 (1M context) --- config/tendermintbase/tendermintbase.go | 41 +++++++ config/tendermintbase/tendermintbase_test.go | 110 +++++++++++++++++++ 2 files changed, 151 insertions(+) diff --git a/config/tendermintbase/tendermintbase.go b/config/tendermintbase/tendermintbase.go index 55dd538a32..40ba306fae 100644 --- a/config/tendermintbase/tendermintbase.go +++ b/config/tendermintbase/tendermintbase.go @@ -18,8 +18,34 @@ const ( InstrumentationSectionName = "instrumentation" PrivValidatorSectionName = "priv-validator" SelfRemediationSectionName = "self-remediation" + + // RootSectionName identifies the keys that sit at the top of the file with no table of their own. The + // name is for lookups and reports and is not part of any key. + RootSectionName = "node_base" ) +// notWritableInThisFile are root paths this section does not declare. +// +// Neither is a setting an operator can usefully write here. The home directory is where this file is found, +// so a value inside it would be the file naming its own location, and the command line already carries it. +// The node mode is the same fact the file states at the top under its own name, and declaring a second +// spelling would let the two disagree, with the resolution answering for one and the node reading the +// other. +var notWritableInThisFile = []string{"home", "mode"} + +// nodeRootSchema declares the keys that sit at the root of the node's configuration file. +// +// The node's own top-level type carries these and the nine tables both, so declaring against it directly +// would declare every table's keys a second time. This squashes the same base group that type squashes, so +// fourteen spellings still come from the node's own tags, and restates only the two fields it holds beside +// that group. A test holds those two against it. +type nodeRootSchema struct { + tmcfg.BaseConfig `mapstructure:",squash"` + + AutobahnConfigFile string `mapstructure:"autobahn-config-file"` + HashVaultDisabledUnsafe bool `mapstructure:"hash-vault-disabled-unsafe"` +} + // removedSettings are the consensus paths this section does not declare. // // Every one is a setting the node removed, and the struct marks each field deprecated. The fields are kept @@ -72,6 +98,8 @@ func init() { registry.RegisterSection(PrivValidatorSectionName, &tmcfg.PrivValidatorConfig{}, privValidatorDefaults) registry.RegisterSection(SelfRemediationSectionName, &tmcfg.SelfRemediationConfig{}, selfRemediationDefaults) + registry.RegisterRootKeysExcluding(RootSectionName, &nodeRootSchema{}, rootDefaults, + notWritableInThisFile...) } // forMode is the configuration the seid init command writes for a kind of node. @@ -152,6 +180,19 @@ func instrumentationDefaults(mode registry.Mode) any { return *forMode(mode).Ins // not make. func privValidatorDefaults(mode registry.Mode) any { return *forMode(mode).PrivValidator } +// rootDefaults is what a generated file carries at the top of the node's configuration file. +// +// The same values for every mode. These name where a node keeps its data and how it logs, and nothing in +// the binary makes either follow from what kind of node is asking. +func rootDefaults(mode registry.Mode) any { + live := forMode(mode) + return nodeRootSchema{ + BaseConfig: live.BaseConfig, + AutobahnConfigFile: live.AutobahnConfigFile, + HashVaultDisabledUnsafe: live.HashVaultDisabledUnsafe, + } +} + // selfRemediationDefaults is what a generated file carries for the self remediation section. // // The same values for every mode. These are the thresholds at which a node restarts itself, and each one diff --git a/config/tendermintbase/tendermintbase_test.go b/config/tendermintbase/tendermintbase_test.go index d12c9e3c6f..a706c3eec3 100644 --- a/config/tendermintbase/tendermintbase_test.go +++ b/config/tendermintbase/tendermintbase_test.go @@ -389,3 +389,113 @@ func TestEverySectionThisPackageRegistersIsUsable(t *testing.T) { t.Errorf("the registry refused %s: %v", d.Section, d.Err) } } + +// TestTheRootSchemaCarriesWhatTheNodesOwnTypeCarries closes the one place a spelling is restated. +// +// The root section declares against a schema rather than the node's top-level type, because that type +// carries the nine tables as well and declaring against it would declare their keys twice. The schema +// squashes the same base group, so fourteen keys still derive from the node's own tags, and it restates two +// fields by hand. This holds those two to the type they came from: name, tag and type each, and the count of +// non-table fields, so a third one appearing there fails here rather than going undeclared. +func TestTheRootSchemaCarriesWhatTheNodesOwnTypeCarries(t *testing.T) { + live := reflect.TypeOf(tmcfg.Config{}) + schema := reflect.TypeOf(nodeRootSchema{}) + + var restated int + for i := 0; i < live.NumField(); i++ { + f := live.Field(i) + tag, ok := f.Tag.Lookup("mapstructure") + if !ok { + continue + } + // The squashed base group and the nine tables are not restated; everything else is. + if strings.Contains(tag, "squash") { + continue + } + if f.Type.Kind() == reflect.Pointer && f.Type.Elem().Kind() == reflect.Struct { + continue + } + restated++ + + got, found := schema.FieldByName(f.Name) + if !found { + t.Errorf("the node's type carries %s (%s) at its root and the schema does not, so the key is "+ + "not declared at all", f.Name, tag) + continue + } + if want := got.Tag.Get("mapstructure"); want != tag { + t.Errorf("%s is tagged %q on the node's type and %q here, so the declared key is not the one "+ + "the reader decodes", f.Name, tag, want) + } + if got.Type != f.Type { + t.Errorf("%s is a %s on the node's type and a %s here, so the declared value has a shape the "+ + "reader does not read", f.Name, f.Type, got.Type) + } + } + + // The schema holds the squashed group plus exactly the restated fields, so a field added here that the + // node's type does not carry fails too. + if want := restated + 1; schema.NumField() != want { + t.Errorf("the schema carries %d fields and the node's type has %d root fields beside the squashed "+ + "group, so %d were expected", schema.NumField(), restated, want) + } +} + +// TestTheRootPathsLeftOutAreTheOnesTheFileAlreadyStates names why two root paths are not declared. +// +// The home directory is where this file is found, so a value inside it would be the file naming its own +// location, and the command line already carries it. The node mode is the fact the file states at the top +// under its own name, and a second spelling would let the two disagree: the resolution answers for one and +// the node reads the other. +// +// Written out here rather than read from the list the registration uses. Comparing that list against itself +// agrees however it changes, so a path dropped from it would leave this passing while the key became +// declared. +func TestTheRootPathsLeftOutAreTheOnesTheFileAlreadyStates(t *testing.T) { + registered, ok := registry.Lookup(RootSectionName) + if !ok { + t.Fatalf("%s is not registered; Defects: %v", RootSectionName, registry.Defects()) + } + declared := map[string]bool{} + for _, key := range registered.Keys { + declared[key] = true + } + for key, why := range map[string]string{ + "home": "the file's own location, which the command line carries", + "mode": "the fact the file states at the top under its own name", + } { + if declared[key] { + t.Errorf("%q is declared at the root and it is %s, so an operator can write a second value "+ + "for something already settled", key, why) + } + } + if len(registered.Excluded) != 2 { + t.Errorf("the root section excludes %v and two paths were expected", registered.Excluded) + } +} + +// TestNoRootKeyCollidesWithAnotherSectionsName covers the collision the registry does not refuse. +// +// A key at the top of the file that is also a section's name cannot be written: no file holds both a value +// for that name and a table under it, so one of the two settings is unreachable and nothing says which. The +// registry does not catch it, and this package is the first to declare root keys beside another package's, +// so the check belongs here until it moves. +func TestNoRootKeyCollidesWithAnotherSectionsName(t *testing.T) { + sections := map[string]bool{} + for _, s := range registry.Sections() { + if s.Prefix != "" { + sections[s.Prefix] = true + } + } + for _, s := range registry.Sections() { + if s.Prefix != "" { + continue + } + for _, key := range s.Keys { + if sections[key] { + t.Errorf("%s declares %q at the top of the file and a section is named %q, so one of the "+ + "two cannot be written and nothing reports which", s.Name, key, key) + } + } + } +} From ad5474c2b4e3d364a93b16234d638a56f86bf152 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Tue, 25 Aug 2026 14:22:33 -0700 Subject: [PATCH 2/2] fix(config): the set of sections this package owns records itself A list written beside the registrations was a second statement of the same fact, and the root-key section was registered and left off it. So the test that asks whether every section this package registers is usable, and the filter that decides which keys belong to it, both skipped that section. The registration records itself now. Dropping a section from the recording fails the tests, where dropping it from a hand-kept list did not. Found while counting sections for a diagram, which is a poor substitute for a check and the reason there is one now. Co-Authored-By: Claude Opus 5 (1M context) --- config/tendermintbase/tendermintbase.go | 43 +++++++++++++++----- config/tendermintbase/tendermintbase_test.go | 8 +--- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/config/tendermintbase/tendermintbase.go b/config/tendermintbase/tendermintbase.go index 1c39637fdf..e11816429b 100644 --- a/config/tendermintbase/tendermintbase.go +++ b/config/tendermintbase/tendermintbase.go @@ -84,27 +84,50 @@ var removedSettings = []string{ // The keys derive from the struct's mapstructure tags, which is what the node's reader decodes through, so // a key here is a key that reader resolves rather than a second spelling of it. func init() { - registry.RegisterSectionExcluding(P2PSectionName, &tmcfg.P2PConfig{}, p2pDefaults, + declareSection(P2PSectionName, &tmcfg.P2PConfig{}, p2pDefaults, filledFromTheCommandLine, "max-outbound-connections") - registry.RegisterSectionExcluding(RPCSectionName, &tmcfg.RPCConfig{}, rpcDefaults, + declareSection(RPCSectionName, &tmcfg.RPCConfig{}, rpcDefaults, filledFromTheCommandLine) - registry.RegisterSectionExcluding(ConsensusSectionName, &tmcfg.ConsensusConfig{}, consensusDefaults, + declareSection(ConsensusSectionName, &tmcfg.ConsensusConfig{}, consensusDefaults, append([]string{filledFromTheCommandLine}, removedSettings...)...) - registry.RegisterSectionExcluding(MempoolSectionName, &tmcfg.MempoolConfig{}, mempoolDefaults, + declareSection(MempoolSectionName, &tmcfg.MempoolConfig{}, mempoolDefaults, filledFromTheCommandLine) - registry.RegisterSectionExcluding(StateSyncSectionName, &tmcfg.StateSyncConfig{}, stateSyncDefaults, + declareSection(StateSyncSectionName, &tmcfg.StateSyncConfig{}, stateSyncDefaults, "rpc-servers") - registry.RegisterSection(TxIndexSectionName, &tmcfg.TxIndexConfig{}, txIndexDefaults) - registry.RegisterSection(InstrumentationSectionName, &tmcfg.InstrumentationConfig{}, + declareSection(TxIndexSectionName, &tmcfg.TxIndexConfig{}, txIndexDefaults) + declareSection(InstrumentationSectionName, &tmcfg.InstrumentationConfig{}, instrumentationDefaults) - registry.RegisterSectionExcluding(PrivValidatorSectionName, &tmcfg.PrivValidatorConfig{}, + declareSection(PrivValidatorSectionName, &tmcfg.PrivValidatorConfig{}, privValidatorDefaults, filledFromTheCommandLine) - registry.RegisterSection(SelfRemediationSectionName, &tmcfg.SelfRemediationConfig{}, + declareSection(SelfRemediationSectionName, &tmcfg.SelfRemediationConfig{}, selfRemediationDefaults) - registry.RegisterRootKeysExcluding(RootSectionName, &nodeRootSchema{}, rootDefaults, + declareRootKeys(RootSectionName, &nodeRootSchema{}, rootDefaults, notWritableInThisFile...) } +// registeredHere are the sections this package put in the registry, recorded as each one is registered. +// +// A test needs to know which sections are this package's, and a list written beside the registrations is a +// second statement of the same fact: a section registered and left off the list is one nothing here checks, +// which is the case the list exists to prevent. Recorded by the registration itself instead, so the two +// cannot disagree. +var registeredHere []string + +// declareSection registers a section and records that it belongs to this package. +func declareSection(name string, prototype any, defaults func(registry.Mode) any, excluding ...string) { + registry.RegisterSectionExcluding(name, prototype, defaults, excluding...) + registeredHere = append(registeredHere, name) +} + +// declareRootKeys registers a section whose keys sit at the root of the file, and records it the same way. +func declareRootKeys(name string, prototype any, defaults func(registry.Mode) any, excluding ...string) { + registry.RegisterRootKeysExcluding(name, prototype, defaults, excluding...) + registeredHere = append(registeredHere, name) +} + +// SectionsRegisteredHere returns the sections this package registered, for the tests that hold the set. +func SectionsRegisteredHere() []string { return append([]string(nil), registeredHere...) } + // forMode is the configuration the seid init command writes for a kind of node. // // Pinned to that command's own pipeline rather than restated here: the defaults the node's package diff --git a/config/tendermintbase/tendermintbase_test.go b/config/tendermintbase/tendermintbase_test.go index f3904e701f..d6568d2fe8 100644 --- a/config/tendermintbase/tendermintbase_test.go +++ b/config/tendermintbase/tendermintbase_test.go @@ -53,13 +53,7 @@ var whatVariesByNodeKind = map[string]map[registry.Mode]string{ // declaredSections are the sections this package registers, so a test walks the set rather than a list that // has to be extended alongside it. -func declaredSections() []string { - return []string{ - P2PSectionName, RPCSectionName, ConsensusSectionName, MempoolSectionName, - StateSyncSectionName, TxIndexSectionName, InstrumentationSectionName, - PrivValidatorSectionName, SelfRemediationSectionName, - } -} +func declaredSections() []string { return SectionsRegisteredHere() } // ours reports whether a key belongs to a section this package registers. func ours(key string) bool {