diff --git a/config/registry/detach_test.go b/config/registry/detach_test.go index 2ddd63e39a..00904f1e11 100644 --- a/config/registry/detach_test.go +++ b/config/registry/detach_test.go @@ -75,6 +75,7 @@ func TestAResolvedListIsTheCallersToWriteInto(t *testing.T) { // Driven through a second resolution rather than by inspecting the variable, because what matters is not // that a copy was taken but that the next caller is unaffected. func TestSortingAResolvedNestedListLeavesTheSectionsOwnDefaultAlone(t *testing.T) { + registry.Reset() type nested struct { Deny map[string][]string `mapstructure:"deny"` } diff --git a/config/registry/registry.go b/config/registry/registry.go index 253f93e0cd..df686aa6e3 100644 --- a/config/registry/registry.go +++ b/config/registry/registry.go @@ -51,6 +51,13 @@ type Section struct { Prefix string // Keys are the dotted paths this section declares, sorted. Keys []string + // Excluded are dotted paths the struct carries that this section deliberately does not declare, + // sorted. + // + // Kept rather than discarded because both walks have to agree. The type walk decides what is declared + // and the value walk decides what is stated, and a path dropped from one and not the other makes a + // section that either declares a key nothing answers or answers a key it never declared. + Excluded []string // Defaults returns the section's default for a mode. Defaults func(Mode) any } @@ -87,7 +94,31 @@ var ( // It never panics. A registration this package cannot use is recorded as a Defect and the // section is not registered. func RegisterSection(name string, prototype any, defaults func(Mode) any) { - record(name, name, prototype, defaults) + record(name, name, prototype, defaults, nil) +} + +// RegisterSectionExcluding records a section, leaving out paths the struct carries that are not settings. +// +// Each excluding path is relative to the section, so "max-outbound-connections" rather than the dotted key +// it becomes. A path matching nothing the struct declares is refused, because an exclusion covering nothing +// reads as though it covered something. +// +// Two kinds of field earn this. 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 any default would be this package inventing one. +// +// A field can also carry "-" as its mapstructure name, and that says something different rather than the +// same thing in another place. The tag says the field is not configuration and no reader offers it. An +// exclusion says the opposite about the field and less about this package: the field is configuration, the +// reader that owns its file decodes it, and this key space does not offer it. Tagging one of these paths +// would take the setting away from that reader. +// +// The tag is a statement of intent and not a barrier. The decoder in use honours "-" when it renders a +// struct into a map and not when it decodes a map into a struct, where it takes the name literally, so a +// key spelled "-" reaches every field in the struct tagged that way at once. Nothing offers such a key and +// no operator writes one, which is why this is stated rather than guarded. +func RegisterSectionExcluding(name string, prototype any, defaults func(Mode) any, excluding ...string) { + record(name, name, prototype, defaults, excluding) } // RegisterRootKeys records a section whose keys sit at the root of the file, with no section of their own. @@ -98,12 +129,30 @@ func RegisterSection(name string, prototype any, defaults func(Mode) any) { // Some settings are node-wide and are written at the top of a file rather than inside a table. Giving them // a section would rename them, and a renamed key is one an operator's existing file no longer reaches. func RegisterRootKeys(name string, prototype any, defaults func(Mode) any) { - record(name, "", prototype, defaults) + record(name, "", prototype, defaults, nil) +} + +// RegisterRootKeysExcluding records a root-key section, leaving out paths that are not settings. +// +// RegisterSectionExcluding says which fields earn an exclusion and how a path is spelled. +func RegisterRootKeysExcluding(name string, prototype any, defaults func(Mode) any, excluding ...string) { + record(name, "", prototype, defaults, excluding) } // record is the one path both registrations take. -func record(name, prefix string, prototype any, defaults func(Mode) any) { +func record(name, prefix string, prototype any, defaults func(Mode) any, excluding []string) { keys, err := deriveKeys(name, prefix, prototype) + var excluded []string + if err == nil { + keys, excluded, err = withoutExcluded(prefix, keys, excluding) + } + // Refused after the exclusions are known, because deriveKeys can only see a struct that declares + // nothing and this catches the section whose every declared path was then excluded. Such a section + // answers no key while still taking its name, so the real registration under that name is later + // refused as a duplicate, and its defaults are still rendered on every resolution. + if err == nil && len(keys) == 0 { + err = fmt.Errorf("every path it declares is excluded (%v), so the section declares nothing", excluded) + } mu.Lock() defer mu.Unlock() @@ -121,10 +170,53 @@ func record(name, prefix string, prototype any, defaults func(Mode) any) { defects = append(defects, Defect{Section: name, Err: err}) return } - sections[name] = Section{Name: name, Prefix: prefix, Keys: keys, Defaults: defaults} + sections[name] = Section{ + Name: name, Prefix: prefix, Keys: keys, Excluded: excluded, Defaults: defaults, + } } } +// withoutExcluded splits derived paths into the ones a section declares and the ones it does not. +// +// An exclusion is spelled relative to the section, so this is where it becomes the dotted key both walks +// compare against. One matching no derived path is refused: the field it named was renamed or removed, and +// an exclusion for a field that is gone stops excluding anything while still reading as a deliberate +// omission. +func withoutExcluded(prefix string, derived, excluding []string) (keys, excluded []string, err error) { + if len(excluding) == 0 { + return derived, nil, nil + } + drop := make(map[string]bool, len(excluding)) + for _, rel := range excluding { + key := rel + if prefix != "" { + key = prefix + "." + rel + } + if drop[key] { + return nil, nil, fmt.Errorf("%s is excluded twice, and the second one covers nothing", key) + } + drop[key] = true + } + for _, key := range derived { + if drop[key] { + excluded = append(excluded, key) + delete(drop, key) + continue + } + keys = append(keys, key) + } + if len(drop) > 0 { + missing := make([]string, 0, len(drop)) + for key := range drop { + missing = append(missing, key) + } + sort.Strings(missing) + return nil, nil, fmt.Errorf("%v is excluded and the struct declares no such key, so the "+ + "exclusion covers nothing", missing) + } + return keys, excluded, nil +} + // envNamesAreDistinct refuses keys that share one environment spelling. Callers hold mu. // // Dots and hyphens both become underscores, so two keys differing only in that punctuation answer to @@ -160,14 +252,24 @@ func envNamesAreDistinct(adding []string) error { return nil } +// detached returns a copy of the section that shares no storage with the registry's own. +// +// Every path out of the mutex goes through this, so a slice field added to Section later is copied +// without anyone having to remember to copy it at each accessor. A caller that sorts or writes into +// what it was handed reaches its own storage, and both walks keep reading what registration decided. +func (s Section) detached() Section { + s.Keys = append([]string(nil), s.Keys...) + s.Excluded = append([]string(nil), s.Excluded...) + return s +} + // Sections returns every registered section, sorted by name. func Sections() []Section { mu.RLock() defer mu.RUnlock() out := make([]Section, 0, len(sections)) for _, s := range sections { - s.Keys = append([]string(nil), s.Keys...) - out = append(out, s) + out = append(out, s.detached()) } sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) return out @@ -178,10 +280,7 @@ func Lookup(name string) (Section, bool) { mu.RLock() defer mu.RUnlock() s, ok := sections[name] - // Copied, so a caller sorting or writing into Keys cannot reach the registry's own storage from - // outside the mutex. Defects copies for the same reason. - s.Keys = append([]string(nil), s.Keys...) - return s, ok + return s.detached(), ok } // Defects returns every registration this package could not use. @@ -367,7 +466,10 @@ func tagOf(f reflect.StructField, label string) (name string, squash, skip bool, "unreachable through their tags", label, f.Name) } - name, squash = parseTag(tag) + name, squash, remain := parseTag(tag) + if remain { + return "", false, true, nil + } if squash { if name != "" { return "", false, false, fmt.Errorf("%s.%s is squashed and also names %q; one or the other", @@ -394,15 +496,23 @@ func tagOf(f reflect.StructField, label string) (name string, squash, skip bool, return name, false, false, nil } -// parseTag splits a mapstructure tag into the name it gives a field and whether it squashes. -func parseTag(tag string) (name string, squash bool) { +// parseTag splits a mapstructure tag into the name it gives a field and the options that change what the +// field is. +// +// A squashed field contributes its own fields at this level rather than a segment of its own. A remaining +// field is where the decode puts what it matched no field for, so it declares no key: what lands in it is +// what an operator misspelled, and giving it a key would offer the collector itself as a setting. +func parseTag(tag string) (name string, squash, remain bool) { parts := strings.Split(tag, ",") for _, opt := range parts[1:] { - if opt == "squash" { + switch opt { + case "squash": squash = true + case "remain": + remain = true } } - return parts[0], squash + return parts[0], squash, remain } // assignedOutsideConfiguration reports whether a tag excludes its field from configuration. diff --git a/config/registry/resolve.go b/config/registry/resolve.go index 536d088a2a..3beda34c7d 100644 --- a/config/registry/resolve.go +++ b/config/registry/resolve.go @@ -48,6 +48,12 @@ type Resolved struct { // supply. LookupEnv is a function rather than a map because an environment cannot be enumerated for a // prefix, since a variable is only findable if you already know its name. type Sources struct { + // File is one flat map whose keys are whole dotted paths, matched without regard to case. + // + // Flat rather than nested, because that is the shape the other two sources carry: a flag name and + // an environment variable are each one string naming one key. A decoded configuration file is the + // wrong shape, since a table there is a nested map and every key an operator wrote sits a level + // below where anything matches it. Resolve refuses that shape rather than resolving past it. File map[string]any LookupEnv func(string) (string, bool) Flags map[string]any @@ -90,6 +96,17 @@ func Resolve(mode Mode, from Sources) (Resolved, error) { mode, Modes()) } + // Refused before anything is resolved, because a registration this package could not use is a + // section missing from the key space, and every key it declared then reads as one no section + // declares. An operator's written value for such a key is reported as unknown rather than applied, + // which is the hole this function promises never to hand out. Two sections colliding is the case + // that matters: the loser is dropped whole and which one loses follows package initialisation + // order, so the same binary can answer differently for reasons no caller can see. + if bad := Defects(); len(bad) > 0 { + return out, fmt.Errorf("the registry could not use %d registration(s), so the key space is "+ + "incomplete: %v", len(bad), bad) + } + // One snapshot, read once and passed everywhere below. Every part of the answer has to describe the // same registry: asking again leaves a window a concurrent registration fits through, and a section // arriving in that window is declared by one part of the answer and not by another. @@ -99,6 +116,9 @@ func Resolve(mode Mode, from Sources) (Resolved, error) { return out, err } declared := declaredKeys(registered) + if err := refuseANestedFile(from.File, declared); err != nil { + return out, err + } undeliverable := keysNoVariableCanCarry(defaults) out.Values = make(map[string]any, len(declared)) @@ -171,6 +191,11 @@ func defaultValues(mode Mode, registered []Section) (map[string]any, error) { if err != nil { return out, fmt.Errorf("section %q default for mode %q: %w", s.Name, mode, err) } + // The same paths the declaration left out. Both walks read the one struct, so a path dropped from + // the declared side and kept here would be a value under a key nothing declares. + for _, key := range s.Excluded { + delete(values, key) + } if err := matchesDeclaration(s.Keys, values); err != nil { return out, fmt.Errorf("section %q default for mode %q: %w", s.Name, mode, err) } @@ -451,6 +476,34 @@ func envValues(declared map[string]bool, undeliverable map[string]string, // // A source enumerates lower-cased while a file may not be written that way, and a key that differed // only in case would resolve as unknown while the operator's value went nowhere. +// refuseANestedFile refuses a file source whose tables were left nested. +// +// The shape a configuration reader hands back, and it resolves to pure defaults without complaint: every +// key an operator wrote sits one level below where a dotted path matches it, so nothing is applied, and +// the table's own name is the one thing reported, as a key no section declares. A caller that then +// installs what it resolved writes the whole declared set over the file it was trying to read. +// +// Detected by a table holding a declared key, which is that mistake and nothing else. A declared key +// whose own field is a map is left alone, since the value an operator writes for it is a table. +func refuseANestedFile(values map[string]any, declared map[string]bool) error { + for key, v := range values { + if _, isTable := v.(map[string]any); !isTable { + continue + } + lower := strings.ToLower(key) + if declared[lower] { + continue + } + for d := range declared { + if strings.HasPrefix(d, lower+".") { + return fmt.Errorf("the file source holds a table at %q carrying %q, so it is nested "+ + "where this reads one flat map of whole dotted keys", key, d) + } + } + } + return nil +} + func fileValues(values map[string]any) map[string]any { if values == nil { return nil diff --git a/config/registry/spec_test.go b/config/registry/spec_test.go index 9091b8a965..ad1d68331b 100644 --- a/config/registry/spec_test.go +++ b/config/registry/spec_test.go @@ -2,6 +2,7 @@ package registry_test import ( "reflect" + "slices" "sort" "strings" "sync" @@ -1033,33 +1034,43 @@ func requireEnvCollisionRefused(t *testing.T, spelling string) { // every other caller's declared set. func TestAReadKeySliceCannotReachTheRegistry(t *testing.T) { registry.Reset() - registry.RegisterSection("s", &struct { - B string `mapstructure:"b"` - A string `mapstructure:"a"` - }{}, func(registry.Mode) any { - return struct { - B string `mapstructure:"b"` - A string `mapstructure:"a"` - }{} - }) + type shape struct { + B string `mapstructure:"b"` + A string `mapstructure:"a"` + Gone string `mapstructure:"gone"` + } + registry.RegisterSectionExcluding("s", &shape{}, + func(registry.Mode) any { return shape{} }, "gone") requireNoDefects(t) - want := []string{"s.a", "s.b"} + // Both slices, through both accessors. The exclusion list is read on every Resolve, so a caller + // that reached it would not corrupt one key: it would drop a path from one walk and not the other, + // and every later resolution would fail for every mode. for _, read := range []struct { - name string - keys func() []string + name string + want []string + slice func() []string }{ - {"Lookup", func() []string { s, _ := registry.Lookup("s"); return s.Keys }}, - {"Sections", func() []string { return registry.Sections()[0].Keys }}, + {"Lookup.Keys", []string{"s.a", "s.b"}, + func() []string { s, _ := registry.Lookup("s"); return s.Keys }}, + {"Lookup.Excluded", []string{"s.gone"}, + func() []string { s, _ := registry.Lookup("s"); return s.Excluded }}, + {"Sections.Keys", []string{"s.a", "s.b"}, + func() []string { return registry.Sections()[0].Keys }}, + {"Sections.Excluded", []string{"s.gone"}, + func() []string { return registry.Sections()[0].Excluded }}, } { t.Run(read.name, func(t *testing.T) { - got := read.keys() + // Written by index rather than appended, because every section registered today has a + // capacity equal to its length, so an append reallocates and would pass while the + // registry's own storage was still reachable. + got := read.slice() for i := range got { got[i] = "clobbered" } - if after := read.keys(); !reflect.DeepEqual(after, want) { - t.Errorf("writing into the slice %s returned left the registry declaring %v, want %v", - read.name, after, want) + if after := read.slice(); !reflect.DeepEqual(after, read.want) { + t.Errorf("writing into the slice %s returned left the registry holding %v, want %v", + read.name, after, read.want) } }) } @@ -1406,3 +1417,129 @@ func TestAFieldExcludedFromConfigDeclaresNoKey(t *testing.T) { t.Errorf("the refusal reads %q and does not name the untagged field", got) } } + +// TestAnExclusionDropsAPathFromBothWalks holds the property that makes an exclusion usable. +// +// A section is walked twice, once as a type to decide what it declares and once as a value to decide what +// it states. An exclusion that reached one walk and not the other would leave a section declaring a key +// nothing answers, or answering a key it never declared, and the registry refuses both. So it has to reach +// both, and the only way to see that is through a resolution. +func TestAnExclusionDropsAPathFromBothWalks(t *testing.T) { + registry.Reset() + type leftOut struct { + Kept string `mapstructure:"kept"` + Dropped string `mapstructure:"dropped"` + } + registry.RegisterSectionExcluding("exclusion_both_walks", &leftOut{}, func(registry.Mode) any { + return leftOut{Kept: "a", Dropped: "b"} + }, "dropped") + + registered, ok := registry.Lookup("exclusion_both_walks") + if !ok { + t.Fatalf("not registered; Defects: %v", registry.Defects()) + } + if want := []string{"exclusion_both_walks.kept"}; !reflect.DeepEqual(registered.Keys, want) { + t.Errorf("declares %v, want %v", registered.Keys, want) + } + + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if _, answered := resolved.Values["exclusion_both_walks.dropped"]; answered { + t.Error("the excluded path is answered, so the value walk kept what the type walk dropped") + } + if got := resolved.Values["exclusion_both_walks.kept"]; got != "a" { + t.Errorf("the kept key answers %#v; excluding one path must not drop the others", got) + } +} + +// TestAnExclusionCoveringNothingIsRefused keeps a stale exclusion from reading as a deliberate omission. +// +// The field an exclusion names can be renamed or removed. Left alone, the exclusion then excludes nothing +// while still saying in the source that this section deliberately leaves a setting out. +func TestAnExclusionCoveringNothingIsRefused(t *testing.T) { + registry.Reset() + type present struct { + Kept string `mapstructure:"kept"` + } + registry.RegisterSectionExcluding("exclusion_covers_nothing", &present{}, func(registry.Mode) any { + return present{Kept: "a"} + }, "renamed-away") + + if _, ok := registry.Lookup("exclusion_covers_nothing"); ok { + t.Fatal("the section registered with an exclusion naming no field it carries") + } + var found bool + for _, d := range registry.Defects() { + if d.Section == "exclusion_covers_nothing" && strings.Contains(d.Err.Error(), "covers nothing") { + found = true + } + } + if !found { + t.Errorf("no defect says the exclusion covers nothing; Defects: %v", registry.Defects()) + } +} + +// TestAFieldCollectingUnmatchedKeysDeclaresNone covers the one tag option that has no name. +// +// A remaining field is where the decode puts what it matched no field for, so what lands in it is what an +// operator misspelled. No exclusion can reach it, because an exclusion names a key and this field has none. +func TestAFieldCollectingUnmatchedKeysDeclaresNone(t *testing.T) { + registry.Reset() + type collector struct { + Kept string `mapstructure:"kept"` + Other map[string]any `mapstructure:",remain"` + } + registry.RegisterSection("remaining_field", &collector{}, func(registry.Mode) any { + return collector{Kept: "a"} + }) + + registered, ok := registry.Lookup("remaining_field") + if !ok { + t.Fatalf("a struct carrying a remaining field was refused; Defects: %v", registry.Defects()) + } + if want := []string{"remaining_field.kept"}; !reflect.DeepEqual(registered.Keys, want) { + t.Errorf("declares %v, want %v; the collector itself is not a setting", registered.Keys, want) + } +} + +// TestANestedFileSourceIsRefusedRatherThanResolvedPast measures what a wrong shape used to cost. +// +// File is one flat map of whole dotted keys. Handed a decoded configuration file instead, every key an +// operator wrote sits a level below where anything matches it, so the resolution carried pure defaults +// and reported the table's name as the one key nothing declares. A caller installing that result writes +// the binary's own defaults over the file it was reading, which is how an empty value reaches a setting +// something outside the binary had patched. +// +// So the shape is refused. Asserted through what an operator loses rather than through the error, because +// an error nobody returns and a value nobody applies look the same from the outside. +func TestANestedFileSourceIsRefusedRatherThanResolvedPast(t *testing.T) { + registry.Reset() + type peers struct { + Persistent string `mapstructure:"persistent-peers"` + } + registry.RegisterSection("net", &peers{}, + func(registry.Mode) any { return peers{Persistent: ""} }) + requireNoDefects(t) + + const written = "node-id@10.0.0.1:26656" + nested := map[string]any{"net": map[string]any{"persistent-peers": written}} + if _, err := registry.Resolve(registry.ModeValidator, registry.Sources{File: nested}); err == nil { + t.Fatal("a nested file source resolved, so a caller installing the result would have written " + + "the declared empty value over the peer list the file carried") + } + + // The flat shape of the same file still resolves, so the refusal is of the shape and not of the key. + flat := map[string]any{"net.persistent-peers": written} + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{File: flat}) + if err != nil { + t.Fatalf("the flat shape was refused: %v", err) + } + if got := resolved.Values["net.persistent-peers"]; got != written { + t.Errorf("net.persistent-peers resolved to %v, want the written %q", got, written) + } + if !slices.Contains(resolved.Overrides, "net.persistent-peers") { + t.Errorf("Overrides is %v, so an install would skip the one key the file wrote", resolved.Overrides) + } +} diff --git a/config/tendermintbase/tendermintbase.go b/config/tendermintbase/tendermintbase.go new file mode 100644 index 0000000000..ae5dd40362 --- /dev/null +++ b/config/tendermintbase/tendermintbase.go @@ -0,0 +1,95 @@ +package tendermintbase + +import ( + "github.com/sei-protocol/sei-chain/app/params" + "github.com/sei-protocol/sei-chain/config/registry" + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" +) + +// The names these sections have in the configuration key space. +const ( + P2PSectionName = "p2p" + RPCSectionName = "rpc" +) + +// Registration puts these sections in the configuration registry. +// +// Neither the package that defines these settings nor the package that decides them can register them. The +// struct they are read into belongs to the node's own configuration package, and the rules that vary them +// by node kind live in the parameters package, which imports that struct. So the importing direction is +// already fixed and only a third package can see both. +// +// 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, + filledFromTheCommandLine, derivedFromTheConnectionLimit, readByNothing) + registry.RegisterSectionExcluding(RPCSectionName, &tmcfg.RPCConfig{}, rpcDefaults, + filledFromTheCommandLine) +} + +// 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 +// declares, then the mode rules the binary applies to them. So a declared value is what a generated file +// carries for every key but the ones in filledByTheGenerator, and a change to either half moves this +// with it. That command goes on to set those from inputs no mode carries, which is why they are named +// rather than described. +// +// The mode is written onto the configuration before the rules run, because the rules read it from there +// rather than taking it as an argument. The command reaches the same rules by a different route for an +// archive node, writing the full-node mode before it runs them, and the two agree only because the rules +// answer alike for both. A rule that stopped answering alike would leave this the more specific of the +// two answers, so the test holding what varies by node kind is what keeps them together. +func forMode(mode registry.Mode) *tmcfg.Config { + out := tmcfg.DefaultConfig() + out.Mode = string(mode) + params.SetTendermintConfigByMode(out) + return out +} + +// filledFromTheCommandLine is the path five of these sections carry and none declares. +// +// Each holds a root directory field tagged the same as the one 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 +// what these sections state for it is the empty string. Declaring it would hand a delivery 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. +const filledFromTheCommandLine = "home" + +// derivedFromTheConnectionLimit is the ceiling this section leaves out because unset is the setting. +// +// The field is a pointer the defaults leave unset, and unset is what selects the behaviour: the node +// derives a ceiling from the total connection limit instead. Declaring it would need a default, and any +// number written here would be this package inventing one that no generated file carries. +const derivedFromTheConnectionLimit = "max-outbound-connections" + +// readByNothing is the dial hook this section leaves out because no code reads it. +// +// The field is declared beside the peer-to-peer settings and its own comment calls it a testing parameter, +// but nothing in the tree reads it and the generated file does not write it. Declaring it would offer a +// key that changes nothing about how the node runs. +const readByNothing = "test-dial-fail" + +// filledByTheGenerator are keys the init command sets after the pipeline forMode mirrors, from an input a +// mode does not carry. +// +// Declared rather than excluded, because the generated file writes each one into every node's +// configuration, and a key this space refuses is a key an operator's own file reports as unknown. What +// they cost is the invariant on forMode: for these, a declared value is not what a generated file +// carries. The peer seeds come from the chain identifier, which is a runtime input and not a node kind, +// so no answer keyed on mode can be right. A writer takes the value from the generator instead. +var filledByTheGenerator = []string{P2PSectionName + ".bootstrap-peers"} + +// p2pDefaults is what a generated file carries for the peer-to-peer section. +// +// Answered per mode. Three of these settings follow from what kind of node is asking: a validator refuses +// duplicate addresses, a seed accepts them and raises its connection ceiling because serving peers is what +// it exists for, and a node that serves queries binds an address where a validator leaves the default. +func p2pDefaults(mode registry.Mode) any { return *forMode(mode).P2P } + +// rpcDefaults is what a generated file carries for the remote procedure call section. +// +// Answered per mode, for the listen address alone: a node that serves queries binds one and a validator +// does not. +func rpcDefaults(mode registry.Mode) any { return *forMode(mode).RPC } diff --git a/config/tendermintbase/tendermintbase_test.go b/config/tendermintbase/tendermintbase_test.go new file mode 100644 index 0000000000..1722340469 --- /dev/null +++ b/config/tendermintbase/tendermintbase_test.go @@ -0,0 +1,257 @@ +package tendermintbase + +import ( + "fmt" + "reflect" + "slices" + "sort" + "strings" + "testing" + + "github.com/sei-protocol/sei-chain/app/seeds" + "github.com/sei-protocol/sei-chain/config/registry" + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" +) + +// whatVariesByNodeKind is every key these sections answer differently depending on the kind of node. +// +// Held by name and value rather than described, because two of these decide what a node exposes to the +// network. A rule that stopped varying would leave a validator binding the address a query-serving node +// binds, and a comment saying otherwise cannot fail. +var whatVariesByNodeKind = map[string]map[registry.Mode]string{ + "p2p.laddr": { + registry.ModeValidator: "tcp://127.0.0.1:26656", + registry.ModeSeed: "tcp://127.0.0.1:26656", + registry.ModeFull: "tcp://0.0.0.0:26656", + registry.ModeArchive: "tcp://0.0.0.0:26656", + }, + "rpc.laddr": { + registry.ModeValidator: "tcp://127.0.0.1:26657", + registry.ModeSeed: "tcp://127.0.0.1:26657", + registry.ModeFull: "tcp://0.0.0.0:26657", + registry.ModeArchive: "tcp://0.0.0.0:26657", + }, + "p2p.max-connections": { + registry.ModeValidator: "100", + registry.ModeSeed: "1000", + registry.ModeFull: "100", + registry.ModeArchive: "100", + }, + "p2p.allow-duplicate-ip": { + registry.ModeValidator: "false", + registry.ModeSeed: "true", + registry.ModeFull: "false", + registry.ModeArchive: "false", + }, +} + +// TestWhatVariesByNodeKindIsTheRecordedSet measures the mode rules through the declared values. +// +// A key that starts varying fails and so does one that stops, which means changing a rule has to account +// for its row here. Two rows are the reason this is measured rather than stated: the listen addresses are +// what put a request surface on a node, and a validator holds a signing key. +func TestWhatVariesByNodeKindIsTheRecordedSet(t *testing.T) { + byMode := map[registry.Mode]map[string]any{} + for _, mode := range registry.Modes() { + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve(%s): %v", mode, err) + } + byMode[mode] = resolved.Values + } + + var measured []string + for key := range byMode[registry.ModeValidator] { + if !strings.HasPrefix(key, P2PSectionName+".") && !strings.HasPrefix(key, RPCSectionName+".") { + continue + } + seen := map[string]bool{} + for _, mode := range registry.Modes() { + seen[fmt.Sprint(byMode[mode][key])] = true + } + if len(seen) == 1 { + if _, recorded := whatVariesByNodeKind[key]; recorded { + t.Errorf("%s is recorded as varying by node kind and answers the same for every mode. "+ + "Take it off the record, so the record stays the set of keys a generated file writes "+ + "differently per kind of node", key) + } + continue + } + measured = append(measured, key) + want, recorded := whatVariesByNodeKind[key] + if !recorded { + t.Errorf("%s varies by node kind and nothing records it", key) + continue + } + for _, mode := range registry.Modes() { + if got := fmt.Sprint(byMode[mode][key]); got != want[mode] { + t.Errorf("%s for %s is %q, recorded as %q", key, mode, got, want[mode]) + } + } + } + + sort.Strings(measured) + if len(measured) != len(whatVariesByNodeKind) { + t.Errorf("measured %d keys varying by node kind and %d are recorded: %v", + len(measured), len(whatVariesByNodeKind), measured) + } +} + +// TestTheDeclaredKeysAreTheOnesTheReaderDecodes holds the declaration to the struct the node decodes into. +// +// Derived from that struct's own tags, so this asserts the count rather than the spelling: a renamed tag +// moves the reader and the declaration together, and there is no third statement to drift from. +func TestTheDeclaredKeysAreTheOnesTheReaderDecodes(t *testing.T) { + for _, tc := range []struct { + section string + proto any + exclude int + }{ + {P2PSectionName, &tmcfg.P2PConfig{}, 3}, + {RPCSectionName, &tmcfg.RPCConfig{}, 1}, + } { + registered, ok := registry.Lookup(tc.section) + if !ok { + t.Errorf("%s is not registered; Defects: %v", tc.section, registry.Defects()) + continue + } + tagged := taggedFields(reflect.TypeOf(tc.proto).Elem()) + if want := tagged - tc.exclude; len(registered.Keys) != want { + t.Errorf("%s declares %d keys and the struct carries %d tagged fields with %d excluded, "+ + "so %d were expected", tc.section, len(registered.Keys), tagged, tc.exclude, want) + } + if len(registered.Excluded) != tc.exclude { + t.Errorf("%s excludes %v and %d exclusions were expected", + tc.section, registered.Excluded, tc.exclude) + } + } +} + +// TestTheExcludedPathIsTheOneWithNoDefault names why the one exclusion is there. +// +// The outbound ceiling is a pointer the node's defaults leave unset, and unset is the setting: the node +// derives a ceiling from the total limit instead. A default here would be invented. If the node ever gives +// it one, this fails and the key should be declared rather than excluded. +func TestTheExcludedPathIsTheOneWithNoDefault(t *testing.T) { + registered, ok := registry.Lookup(P2PSectionName) + if !ok { + t.Fatalf("%s is not registered", P2PSectionName) + } + if !slices.Contains(registered.Excluded, P2PSectionName+".max-outbound-connections") { + t.Fatalf("excluded is %v and does not name the outbound ceiling", registered.Excluded) + } + if got := tmcfg.DefaultP2PConfig().MaxOutboundConnections; got != nil { + t.Errorf("the node now defaults the outbound ceiling to %v, so it states a value and belongs "+ + "declared rather than excluded", *got) + } +} + +// taggedFields counts the fields of a struct that carry a mapstructure name, following the same rules the +// registry derives keys by: a squashed field contributes its own, and a dash or a remaining field none. +func taggedFields(t reflect.Type) int { + n := 0 + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + tag, ok := f.Tag.Lookup("mapstructure") + if !ok || f.PkgPath != "" { + continue + } + parts := strings.Split(tag, ",") + opts := parts[1:] + if hasOpt(opts, "remain") || parts[0] == "-" { + continue + } + ft := f.Type + for ft.Kind() == reflect.Pointer { + ft = ft.Elem() + } + if hasOpt(opts, "squash") { + n += taggedFields(ft) + continue + } + if ft.Kind() == reflect.Struct && ft.String() != "time.Time" && ft.String() != "big.Int" { + n += taggedFields(ft) + continue + } + n++ + } + return n +} + +func hasOpt(opts []string, want string) bool { + for _, o := range opts { + if o == want { + return true + } + } + return false +} + +// TestNoSectionDeclaresTheRootDirectory covers a field five of these sections carry. +// +// Each holds a root directory tagged the same as the key at the top of the file, and the node fills every +// one from the command line after the file is read. So each states the empty string, and a delivery that +// wrote a declared value would blank the root a running node found its data, its genesis file and its +// signing key under. +// +// Checked across every registered section rather than the five, so a section added later that carries the +// same field fails here instead of shipping the same hole. +func TestNoSectionDeclaresTheRootDirectory(t *testing.T) { + for _, s := range registry.Sections() { + for _, key := range s.Keys { + if key == "home" || strings.HasSuffix(key, ".home") { + t.Errorf("%s declares %q, and the node fills that field from the command line after the "+ + "file is read, so what this section states for it is the empty string", s.Name, key) + } + } + } +} + +// TestWhatTheGeneratorFillsIsNotWhatTheDeclarationStates names the writer for each key on the list. +// +// A declared value is what a generated file carries, and these are the keys where that is not true +// because the init command sets them after the pipeline forMode mirrors. Held as a list with a test +// rather than a sentence, so a third key joining the class fails here instead of becoming a declared +// default nothing questions. +// +// Each row asserts both halves: the key is declared, so it is not quietly excluded, and what the +// generator supplies is something the declaration does not state. A key that stopped diverging fails +// too, which is what makes this a record of the class rather than a note about two keys. +func TestWhatTheGeneratorFillsIsNotWhatTheDeclarationStates(t *testing.T) { + // The generator's own source for each key, read from it rather than restated, so a change there + // moves this. A chain identifier is needed for the peer seeds because that is the input the mode + // rules do not carry, and the public networks are the ones an operator runs. + supplied := map[string]string{ + P2PSectionName + ".bootstrap-peers": seeds.BootstrapPeers("pacific-1"), + } + + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + for _, key := range filledByTheGenerator { + want, named := supplied[key] + if !named { + t.Errorf("%s is on the list and no writer is named for it, so nothing measures that the "+ + "generator fills it", key) + continue + } + declared, declares := resolved.Values[key] + if !declares { + t.Errorf("%s is on the list and no section declares it; a key the generated file carries "+ + "belongs in the key space", key) + continue + } + if want == "" { + t.Errorf("%s names a writer that supplies nothing, so the row records no divergence", key) + continue + } + if fmt.Sprint(declared) == want { + t.Errorf("%s is declared as %q and the generator supplies the same, so it no longer "+ + "diverges. Take it off the list, and off forMode's exception", declared, want) + } + } + if len(supplied) != len(filledByTheGenerator) { + t.Errorf("%d writers are named and %d keys are listed", len(supplied), len(filledByTheGenerator)) + } +}