From 1da0d72ff8cc75af4c5f28af38098f45bbe85ca4 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 20 Aug 2026 14:02:34 -0700 Subject: [PATCH 1/3] config: the first four sections enter the registry 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. --- admin/register.go | 19 ++++++++ admin/register_test.go | 37 ++++++++++++++++ config/registry/registry.go | 32 +++++++++----- config/registry/resolve.go | 7 ++- config/registry/spec_test.go | 55 ++++++++++++++++++++++-- giga/executor/config/register.go | 27 ++++++++++++ giga/executor/config/register_test.go | 41 ++++++++++++++++++ sei-db/config/receipt_register.go | 21 +++++++++ sei-db/config/receipt_register_test.go | 55 ++++++++++++++++++++++++ sei-wasmd/x/wasm/config_register.go | 48 +++++++++++++++++++++ sei-wasmd/x/wasm/config_register_test.go | 52 ++++++++++++++++++++++ 11 files changed, 379 insertions(+), 15 deletions(-) create mode 100644 admin/register.go create mode 100644 admin/register_test.go create mode 100644 giga/executor/config/register.go create mode 100644 giga/executor/config/register_test.go create mode 100644 sei-db/config/receipt_register.go create mode 100644 sei-db/config/receipt_register_test.go create mode 100644 sei-wasmd/x/wasm/config_register.go create mode 100644 sei-wasmd/x/wasm/config_register_test.go diff --git a/admin/register.go b/admin/register.go new file mode 100644 index 0000000000..3d4bfba2b6 --- /dev/null +++ b/admin/register.go @@ -0,0 +1,19 @@ +package admin + +import ( + "github.com/sei-protocol/sei-chain/config/registry" +) + +// SectionName is this section's name in the configuration key space. +const SectionName = "admin_server" + +// Registration puts this section in the configuration registry. +// +// The keys derive from the mapstructure tags, so they are admin_server.admin_enabled and +// admin_server.admin_address, which are the strings this package's reader already resolves. +func init() { + registry.RegisterSection(SectionName, &Config{}, defaults) +} + +// defaults is what this section resolves to for a node that has written nothing. +func defaults(registry.Mode) any { return DefaultConfig } diff --git a/admin/register_test.go b/admin/register_test.go new file mode 100644 index 0000000000..2085f6498b --- /dev/null +++ b/admin/register_test.go @@ -0,0 +1,37 @@ +package admin + +import ( + "reflect" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestTheDeclaredKeysAreTheKeysThisReaderResolves holds the declaration against the reader. +// +// This package names its keys only in its mapstructure tags, so the check is that the registry derives +// exactly the two the reader resolves and no third. +func TestTheDeclaredKeysAreTheKeysThisReaderResolves(t *testing.T) { + section, ok := registry.Lookup(SectionName) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) + } + + want := []string{"admin_server.admin_address", "admin_server.admin_enabled"} + if got := section.Keys; !reflect.DeepEqual(got, want) { + t.Errorf("declared keys are %v, want %v", got, want) + } +} + +// TestTheDefaultsAreWhatTheNodeAlreadyRuns keeps the section from restating the values by hand. +func TestTheDefaultsAreWhatTheNodeAlreadyRuns(t *testing.T) { + for _, mode := range registry.Modes() { + got, ok := defaults(mode).(Config) + if !ok { + t.Fatalf("mode %q: defaults returned %T, want Config", mode, defaults(mode)) + } + if got != DefaultConfig { + t.Errorf("mode %q resolves to %+v, want the package default %+v", mode, got, DefaultConfig) + } + } +} diff --git a/config/registry/registry.go b/config/registry/registry.go index 008738680c..ea15452d0b 100644 --- a/config/registry/registry.go +++ b/config/registry/registry.go @@ -238,10 +238,13 @@ func walk(t reflect.Type, prefix string, keys *[]string, open map[reflect.Type]b continue } - tag, squash, err := tagOf(f, prefix) + tag, squash, skip, err := tagOf(f, prefix) if err != nil { return err } + if skip { + continue + } ft := f.Type for ft.Kind() == reflect.Ptr { @@ -289,10 +292,10 @@ func walkSubtree(t reflect.Type, path, field string, keys *[]string, open map[re } // tagOf returns a field's mapstructure name, or reports that the field cannot be addressed. -func tagOf(f reflect.StructField, prefix string) (name string, squash bool, err error) { +func tagOf(f reflect.StructField, prefix string) (name string, squash, skip bool, err error) { tag, ok := f.Tag.Lookup("mapstructure") if !ok { - return "", false, fmt.Errorf("%s.%s has no mapstructure tag; a key derived from a field "+ + return "", false, false, fmt.Errorf("%s.%s has no mapstructure tag; a key derived from a field "+ "name is a key no operator writes, which is how ninety-two legacy keys became "+ "unreachable through their tags", prefix, f.Name) } @@ -306,25 +309,34 @@ func tagOf(f reflect.StructField, prefix string) (name string, squash bool, err } if squash { if name != "" { - return "", false, fmt.Errorf("%s.%s is squashed and also names %q; one or the other", + return "", false, false, fmt.Errorf("%s.%s is squashed and also names %q; one or the other", prefix, f.Name, name) } - return "", true, nil + return "", true, false, nil + } + if name == "-" { + // The tag mapstructure honours for a field configuration does not reach. Something else in the + // program assigns it: the receipt store's KeepRecent comes from the global min-retain-blocks + // flag at the app layer, and its ExternalPruning from whatever constructs the collector. + // + // So it declares no key rather than declaring one that resolves to a default. A declared key is + // written at override precedence, which would put the default over the value that code assigned. + return "", false, true, nil } - if name == "" || name == "-" { - return "", false, fmt.Errorf("%s.%s has an empty mapstructure name", prefix, f.Name) + if name == "" { + return "", false, false, fmt.Errorf("%s.%s has an empty mapstructure name", prefix, f.Name) } if bad, found := unaddressableChar(name); found { - return "", false, fmt.Errorf("%s.%s names %q, which carries %q. A dot makes the field claim a "+ + return "", false, false, fmt.Errorf("%s.%s names %q, which carries %q. A dot makes the field claim a "+ "subtree the struct does not have, and neither a dot nor a space survives a round trip "+ "through a configuration source", prefix, f.Name, name, bad) } if name != strings.ToLower(name) { - return "", false, fmt.Errorf("%s.%s names %q, which is not lower case; a configuration "+ + return "", false, false, fmt.Errorf("%s.%s names %q, which is not lower case; a configuration "+ "source enumerates lower-cased, so this key would never match a written one", prefix, f.Name, name) } - return name, false, nil + return name, false, false, nil } // unaddressableChar returns the first character in a key segment that no configuration source can diff --git a/config/registry/resolve.go b/config/registry/resolve.go index dfcca8f9e3..ebc44785b2 100644 --- a/config/registry/resolve.go +++ b/config/registry/resolve.go @@ -221,10 +221,15 @@ func walkValues(v reflect.Value, prefix string, out map[string]any) error { } continue } - tag, squash, err := tagOf(f, prefix) + tag, squash, skip, err := tagOf(f, prefix) if err != nil { return err } + if skip { + // Skipped on the type side too, so the declared keys and the rendered defaults describe the + // same set of fields and matchesDeclaration has nothing to disagree about. + continue + } fv := v.Field(i) for fv.Kind() == reflect.Ptr { diff --git a/config/registry/spec_test.go b/config/registry/spec_test.go index cddce89064..dd88b2693b 100644 --- a/config/registry/spec_test.go +++ b/config/registry/spec_test.go @@ -758,9 +758,6 @@ func TestEveryRefusalIsReportedAsADefect(t *testing.T) { type emptyName struct { N string `mapstructure:""` } - type dashName struct { - N string `mapstructure:"-"` - } type upperName struct { N string `mapstructure:"N"` } @@ -812,7 +809,6 @@ func TestEveryRefusalIsReportedAsADefect(t *testing.T) { }{ {"a squashed field that also names a segment", "s", &squashNamed{}, anyDefault, "one or the other"}, {"an empty mapstructure name", "s", &emptyName{}, anyDefault, "empty mapstructure name"}, - {"a dash mapstructure name", "s", &dashName{}, anyDefault, "empty mapstructure name"}, {"an upper-case key", "s", &upperName{}, anyDefault, "not lower case"}, {"a squashed scalar", "s", &squashScalar{}, anyDefault, "not a struct"}, {"a struct declaring nothing", "s", &noKeys{}, anyDefault, "declares no keys"}, @@ -1353,3 +1349,54 @@ func TestARefusalInsideASquashedBaseIsReported(t *testing.T) { t.Errorf("the refusal reads %q; a squashed field's path is the section's own", msg) } } + +// TestAFieldExcludedFromConfigDeclaresNoKey covers the tag that means "not from configuration". +// +// mapstructure reads "-" as skip this field, and a config struct uses it for a field something else in +// the program assigns: the receipt store's KeepRecent comes from the global min-retain-blocks flag at the +// app layer, and its ExternalPruning from whatever constructs the collector. +// +// Such a field declares no key. Declaring one that resolved to the default would be worse than refusing +// the section: a declared key is written at override precedence, so the default would land on top of the +// value that code assigned, and a node with min-retain-blocks set would silently keep nothing. +// +// An untagged field stays a defect. The two look alike and mean opposite things: one is a field the author +// excluded, the other is a field configuration cannot reach because nothing names it. +func TestAFieldExcludedFromConfigDeclaresNoKey(t *testing.T) { + type excluded struct { + Kept string `mapstructure:"kept"` + Assigned int `mapstructure:"-"` + } + + registry.Reset() + registry.RegisterSection("probe", &excluded{}, func(registry.Mode) any { + return &excluded{Kept: "x", Assigned: 42} + }) + for _, d := range registry.Defects() { + t.Fatalf("a field excluded from configuration was reported as a defect: %v", d.Err) + } + + if got, want := registry.Keys(), []string{"probe.kept"}; !reflect.DeepEqual(got, want) { + t.Fatalf("declared keys are %v, want %v. An excluded field declaring a key would have that key "+ + "written at override precedence over whatever assigned the field", got, want) + } + + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{}) + if err != nil { + t.Fatalf("resolving a section with an excluded field: %v", err) + } + if _, present := resolved.Values["probe.assigned"]; present { + t.Error("the excluded field resolved to a value, so installing it would overwrite what assigns it") + } + + // An untagged field means the opposite and stays a defect. + type untagged struct { + Kept string `mapstructure:"kept"` + Forgotten int + } + registry.Reset() + registry.RegisterSection("probe", &untagged{}, func(registry.Mode) any { return &untagged{} }) + if len(registry.Defects()) == 0 { + t.Error("a field with no tag at all registered cleanly, so a key nothing names reaches no field") + } +} diff --git a/giga/executor/config/register.go b/giga/executor/config/register.go new file mode 100644 index 0000000000..c22802ed11 --- /dev/null +++ b/giga/executor/config/register.go @@ -0,0 +1,27 @@ +package config + +import ( + "github.com/sei-protocol/sei-chain/config/registry" +) + +// SectionName is this section's name in the configuration key space. +// +// The same prefix the flag constants already use, so the derived keys are the keys this package's reader +// resolves rather than a second spelling of them. +const SectionName = "giga_executor" + +// Registration puts this section in the configuration registry. +// +// The owning package registers its own section, so the struct, the values and the keys come from one place +// and cannot drift apart. The dotted keys derive from the mapstructure tags, which is what makes the +// registry's spelling and this package's flag constants the same strings. +func init() { + registry.RegisterSection(SectionName, &Config{}, defaults) +} + +// defaults is what this section resolves to for a node that has written nothing. +// +// The same values for every mode, because that is what a node runs today. A mode-varying default would +// change what an archive node does, which is a decision about how the executor should behave rather than +// a consequence of describing it here. +func defaults(registry.Mode) any { return DefaultConfig } diff --git a/giga/executor/config/register_test.go b/giga/executor/config/register_test.go new file mode 100644 index 0000000000..d1e9e5203d --- /dev/null +++ b/giga/executor/config/register_test.go @@ -0,0 +1,41 @@ +package config + +import ( + "reflect" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestTheDeclaredKeysAreTheKeysThisReaderResolves holds the declaration against the reader. +// +// The registry derives a key from the section name and a mapstructure tag; ReadConfig asks for a flag +// constant. Those are two spellings of one key, and a section is only useful if they are the same string. +// Checked against the constants rather than against a written-out list, so a rename of either moves both +// or fails here. +func TestTheDeclaredKeysAreTheKeysThisReaderResolves(t *testing.T) { + section, ok := registry.Lookup(SectionName) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) + } + + want := []string{FlagEnabled, FlagOCCEnabled} + if got := section.Keys; !reflect.DeepEqual(got, want) { + t.Errorf("declared keys are %v, want the keys the reader asks for, %v", got, want) + } +} + +// TestTheDefaultsAreWhatTheNodeAlreadyRuns keeps the section from restating the values by hand. +func TestTheDefaultsAreWhatTheNodeAlreadyRuns(t *testing.T) { + for _, mode := range registry.Modes() { + got, ok := defaults(mode).(Config) + if !ok { + t.Fatalf("mode %q: defaults returned %T, want Config", mode, defaults(mode)) + } + if got != DefaultConfig { + t.Errorf("mode %q resolves to %+v, want the package default %+v. A section states what the "+ + "binary already runs; a different value here is a behaviour change nobody asked for", + mode, got, DefaultConfig) + } + } +} diff --git a/sei-db/config/receipt_register.go b/sei-db/config/receipt_register.go new file mode 100644 index 0000000000..945ae73780 --- /dev/null +++ b/sei-db/config/receipt_register.go @@ -0,0 +1,21 @@ +package config + +import ( + "github.com/sei-protocol/sei-chain/config/registry" +) + +// ReceiptStoreSectionName is this section's name in the configuration key space. +const ReceiptStoreSectionName = "receipt-store" + +// Registration puts this section in the configuration registry. +// +// Two of the struct's fields carry the tag that excludes a field from configuration, so they declare no +// key: KeepRecent is derived from the global min-retain-blocks flag at the app layer, and ExternalPruning +// is set by whatever constructs the garbage collector. Declaring a key for either would put a default over +// the value that code assigns. +func init() { + registry.RegisterSection(ReceiptStoreSectionName, &ReceiptStoreConfig{}, receiptStoreDefaults) +} + +// receiptStoreDefaults is what this section resolves to for a node that has written nothing. +func receiptStoreDefaults(registry.Mode) any { return DefaultReceiptStoreConfig() } diff --git a/sei-db/config/receipt_register_test.go b/sei-db/config/receipt_register_test.go new file mode 100644 index 0000000000..373ce6183b --- /dev/null +++ b/sei-db/config/receipt_register_test.go @@ -0,0 +1,55 @@ +package config + +import ( + "reflect" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestTheDeclaredKeysAreTheKeysThisReaderResolves holds the declaration against the reader. +// +// Two of the struct's fields are excluded from configuration and must declare nothing. KeepRecent is +// derived from the global min-retain-blocks flag at the app layer and ExternalPruning is set by whatever +// constructs the collector, so a key for either would be written at override precedence over the value +// that code assigns. +func TestTheDeclaredKeysAreTheKeysThisReaderResolves(t *testing.T) { + section, ok := registry.Lookup(ReceiptStoreSectionName) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", ReceiptStoreSectionName) + } + + want := []string{ + "receipt-store.async-write-buffer", + "receipt-store.db-directory", + "receipt-store.enable-read-write-metrics", + "receipt-store.log-filter-parallelism", + "receipt-store.prune-interval-seconds", + "receipt-store.rs-backend", + } + if got := section.Keys; !reflect.DeepEqual(got, want) { + t.Errorf("declared keys are\n %v\nwant\n %v", got, want) + } + for _, excluded := range []string{"receipt-store.keep-recent", "receipt-store.external-pruning"} { + for _, key := range section.Keys { + if key == excluded { + t.Errorf("%s is declared. Nothing sources it from configuration, so installing it would "+ + "put a default over the value the app layer assigns", excluded) + } + } + } +} + +// TestTheDefaultsAreWhatTheNodeAlreadyRuns keeps the section from restating the values by hand. +func TestTheDefaultsAreWhatTheNodeAlreadyRuns(t *testing.T) { + for _, mode := range registry.Modes() { + got, ok := receiptStoreDefaults(mode).(ReceiptStoreConfig) + if !ok { + t.Fatalf("mode %q: defaults returned %T, want ReceiptStoreConfig", mode, receiptStoreDefaults(mode)) + } + if !reflect.DeepEqual(got, DefaultReceiptStoreConfig()) { + t.Errorf("mode %q resolves to %+v, want the package default %+v", + mode, got, DefaultReceiptStoreConfig()) + } + } +} diff --git a/sei-wasmd/x/wasm/config_register.go b/sei-wasmd/x/wasm/config_register.go new file mode 100644 index 0000000000..0ca9482380 --- /dev/null +++ b/sei-wasmd/x/wasm/config_register.go @@ -0,0 +1,48 @@ +package wasm + +import ( + "strconv" + + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-wasmd/x/wasm/types" +) + +// SectionName is this section's name in the configuration key space. +const SectionName = "wasm" + +// wasmSchema names the keys this module's reader resolves. +// +// A schema rather than types.WasmConfig itself, which carries no mapstructure tags at all, so registering +// it would derive keys from field names and those are not the keys the reader asks for. The schema states +// the three the module reads, and states them once. +// +// SimulationGasLimit 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. A number cannot carry that, and +// the reader already parses this key from text. +type wasmSchema struct { + MemoryCacheSize uint32 `mapstructure:"memory_cache_size"` + QueryGasLimit uint64 `mapstructure:"query_gas_limit"` + SimulationGasLimit string `mapstructure:"simulation_gas_limit"` +} + +// Registration puts this section in the configuration registry. +// +// Three keys, matching the three flag constants above. Two settings of types.WasmConfig are deliberately +// absent: ContractDebugMode has no key any reader resolves, and lru_size is written into app.toml by the +// template and read by nothing, so declaring either would put a key in the space that reaches no field. +func init() { + registry.RegisterSection(SectionName, &wasmSchema{}, defaults) +} + +// defaults is what this section resolves to for a node that has written nothing. +func defaults(registry.Mode) any { + live := types.DefaultWasmConfig() + schema := wasmSchema{ + MemoryCacheSize: live.MemoryCacheSize, + QueryGasLimit: live.SmartQueryGasLimit, + } + if live.SimulationGasLimit != nil { + schema.SimulationGasLimit = strconv.FormatUint(*live.SimulationGasLimit, 10) + } + return schema +} diff --git a/sei-wasmd/x/wasm/config_register_test.go b/sei-wasmd/x/wasm/config_register_test.go new file mode 100644 index 0000000000..b0263a01d7 --- /dev/null +++ b/sei-wasmd/x/wasm/config_register_test.go @@ -0,0 +1,52 @@ +package wasm + +import ( + "reflect" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-wasmd/x/wasm/types" +) + +// TestTheDeclaredKeysAreTheFlagsThisModuleReads holds the schema against the module. +// +// The schema exists because types.WasmConfig carries no mapstructure tags, so the keys cannot be derived +// from it. That makes the schema a second statement of the same key set, and a second statement is only +// safe while something holds it against the first. These are the flag constants the module registers and +// reads. +func TestTheDeclaredKeysAreTheFlagsThisModuleReads(t *testing.T) { + section, ok := registry.Lookup(SectionName) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) + } + + want := []string{flagWasmMemoryCacheSize, flagWasmQueryGasLimit, flagWasmSimulationGasLimit} + if got := section.Keys; !reflect.DeepEqual(got, want) { + t.Errorf("declared keys are %v, want the flags this module reads, %v", got, want) + } +} + +// TestTheDefaultsCarryTheLiveWasmConfig keeps the schema's values from drifting from the real ones. +// +// The schema restates three settings of types.WasmConfig, so nothing stops those values diverging from +// what DefaultWasmConfig returns except this. +func TestTheDefaultsCarryTheLiveWasmConfig(t *testing.T) { + live := types.DefaultWasmConfig() + got, ok := defaults(registry.ModeValidator).(wasmSchema) + if !ok { + t.Fatalf("defaults returned %T, want wasmSchema", defaults(registry.ModeValidator)) + } + + if got.MemoryCacheSize != live.MemoryCacheSize { + t.Errorf("memory_cache_size resolves to %d, want the live %d", got.MemoryCacheSize, live.MemoryCacheSize) + } + if got.QueryGasLimit != live.SmartQueryGasLimit { + t.Errorf("query_gas_limit resolves to %d, want the live %d", got.QueryGasLimit, live.SmartQueryGasLimit) + } + // Absent is a meaning of its own here: unset means the consensus block gas limit applies, so an unset + // live value has to resolve to no text rather than to a zero. + if live.SimulationGasLimit == nil && got.SimulationGasLimit != "" { + t.Errorf("simulation_gas_limit resolves to %q where the live value is unset. A number here claims "+ + "a limit the node does not apply", got.SimulationGasLimit) + } +} From f7dc6bb69d2a30e8f69dcbd42ec9ffedefcd5887 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 20 Aug 2026 15:33:04 -0700 Subject: [PATCH 2/3] config: say why a field excluded from configuration declares no key 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. --- admin/config.go | 10 ++++- admin/register.go | 4 +- admin/register_test.go | 13 ++++-- cmd/seid/cmd/registry_sections_test.go | 51 ++++++++++++++++++++++++ config/registry/doc.go | 5 +++ config/registry/registry.go | 12 +++--- config/registry/spec_test.go | 15 ++++--- giga/executor/config/register_test.go | 7 ++++ sei-db/config/receipt_register.go | 17 ++++++-- sei-db/config/receipt_register_test.go | 38 +++++++++--------- sei-wasmd/x/wasm/config_register.go | 16 ++++++-- sei-wasmd/x/wasm/config_register_test.go | 28 +++++++++---- 12 files changed, 165 insertions(+), 51 deletions(-) create mode 100644 cmd/seid/cmd/registry_sections_test.go diff --git a/admin/config.go b/admin/config.go index d2d0be3c78..25c2545c3a 100644 --- a/admin/config.go +++ b/admin/config.go @@ -13,6 +13,12 @@ const ( DefaultAddress = "127.0.0.1:9095" ) +// The keys this package's reader resolves. +const ( + flagAdminEnabled = "admin_server.admin_enabled" + flagAdminAddress = "admin_server.admin_address" +) + // Config defines configuration for the admin gRPC server. type Config struct { // Enabled controls whether the admin gRPC server starts. @@ -29,10 +35,10 @@ var DefaultConfig = Config{ // ReadConfig reads admin config from app options (Viper-backed). func ReadConfig(opts servertypes.AppOptions) (Config, error) { cfg := DefaultConfig - if v := opts.Get("admin_server.admin_enabled"); v != nil { + if v := opts.Get(flagAdminEnabled); v != nil { cfg.Enabled = cast.ToBool(v) } - if v := opts.Get("admin_server.admin_address"); v != nil { + if v := opts.Get(flagAdminAddress); v != nil { if s := cast.ToString(v); s != "" { cfg.Address = s } diff --git a/admin/register.go b/admin/register.go index 3d4bfba2b6..08f138249f 100644 --- a/admin/register.go +++ b/admin/register.go @@ -9,8 +9,8 @@ const SectionName = "admin_server" // Registration puts this section in the configuration registry. // -// The keys derive from the mapstructure tags, so they are admin_server.admin_enabled and -// admin_server.admin_address, which are the strings this package's reader already resolves. +// The keys derive from the mapstructure tags, and the reader resolves the same strings through the +// constants beside it, so a rename moves one occurrence and the test holds the two together. func init() { registry.RegisterSection(SectionName, &Config{}, defaults) } diff --git a/admin/register_test.go b/admin/register_test.go index 2085f6498b..4120c38cd4 100644 --- a/admin/register_test.go +++ b/admin/register_test.go @@ -2,6 +2,7 @@ package admin import ( "reflect" + "sort" "testing" "github.com/sei-protocol/sei-chain/config/registry" @@ -9,15 +10,21 @@ import ( // TestTheDeclaredKeysAreTheKeysThisReaderResolves holds the declaration against the reader. // -// This package names its keys only in its mapstructure tags, so the check is that the registry derives -// exactly the two the reader resolves and no third. +// The tags derive the keys and the constants below are what the reader passes to Get, so this compares +// two statements that are edited for different reasons rather than one written out twice. func TestTheDeclaredKeysAreTheKeysThisReaderResolves(t *testing.T) { + for _, defect := range registry.Defects() { + if defect.Section == SectionName { + t.Fatalf("%s was refused: %v", SectionName, defect.Err) + } + } section, ok := registry.Lookup(SectionName) if !ok { t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) } - want := []string{"admin_server.admin_address", "admin_server.admin_enabled"} + want := []string{flagAdminAddress, flagAdminEnabled} + sort.Strings(want) if got := section.Keys; !reflect.DeepEqual(got, want) { t.Errorf("declared keys are %v, want %v", got, want) } diff --git a/cmd/seid/cmd/registry_sections_test.go b/cmd/seid/cmd/registry_sections_test.go new file mode 100644 index 0000000000..e0dc016ae4 --- /dev/null +++ b/cmd/seid/cmd/registry_sections_test.go @@ -0,0 +1,51 @@ +package cmd + +import ( + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestEverySectionThisBinaryDeclaresIsUsable is the check no single section can make. +// +// A section registers during its own package's initialisation, and a registration the registry cannot use +// is recorded rather than panicked, so a section that failed to register is absent rather than loud. Two +// of the refusals depend on what else has registered: two sections declaring one key, and two keys that +// collapse onto one environment variable. Neither is visible from inside either section, and the section +// that loses is dropped whole, with every key it declared. +// +// This package links every section a node's configuration reaches, so asking here is asking about the set +// a node actually gets. Nothing is enumerated, so a section added later is covered without this file +// changing. +func TestEverySectionThisBinaryDeclaresIsUsable(t *testing.T) { + for _, defect := range registry.Defects() { + t.Errorf("%s was refused, so none of its keys is declared: %v", defect.Section, defect.Err) + } + if len(registry.Sections()) == 0 { + t.Fatal("no section registered, so the checks above hold for an empty set. This package links " + + "the packages that register, and one of those imports has gone") + } +} + +// TestEveryDeclaredKeyResolvesForEveryMode covers the half of a registration a section's own test cannot. +// +// Registering validates the struct a section declares against. Whether its defaults can state one value +// for every key it declared is checked when something resolves them, and until now nothing did outside the +// registry's own tests. A default that arrives short is refused rather than filled, so the failure is an +// error here instead of a key resolving to a zero nobody chose. +func TestEveryDeclaredKeyResolvesForEveryMode(t *testing.T) { + for _, mode := range registry.Modes() { + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Errorf("mode %q does not resolve: %v", mode, err) + continue + } + for _, section := range registry.Sections() { + for _, key := range section.Keys { + if _, ok := resolved.Values[key]; !ok { + t.Errorf("mode %q: %s declares %s and it did not resolve", mode, section.Name, key) + } + } + } + } +} diff --git a/config/registry/doc.go b/config/registry/doc.go index d1955c08f3..ebf63e3d99 100644 --- a/config/registry/doc.go +++ b/config/registry/doc.go @@ -71,6 +71,11 @@ // tag, an unexported field carrying a tag, two fields declaring one path, a struct that declares no // key, a struct that contains itself, and two keys that collapse onto one environment variable. // +// A field tagged "-" is the deliberate opposite and is not a defect. That tag excludes a field from +// configuration, so the field declares no key at all rather than one resolving to a default. The +// distinction matters because a missing tag and a "-" tag look alike in a diff: one is a key nothing +// names reaching a field, and the other is a field nothing configures. +// // A key segment is also refused if it is upper-case, or if it carries a dot or a space. That rule // holds for the section name and for a field's tag alike, since both become segments of the same // dotted key and answer to the same sources. diff --git a/config/registry/registry.go b/config/registry/registry.go index ea15452d0b..4ec7796758 100644 --- a/config/registry/registry.go +++ b/config/registry/registry.go @@ -315,12 +315,14 @@ func tagOf(f reflect.StructField, prefix string) (name string, squash, skip bool return "", true, false, nil } if name == "-" { - // The tag mapstructure honours for a field configuration does not reach. Something else in the - // program assigns it: the receipt store's KeepRecent comes from the global min-retain-blocks - // flag at the app layer, and its ExternalPruning from whatever constructs the collector. + // The tag that excludes a field from configuration. Something else in the program assigns the + // field, so no reader resolves a key for it. // - // So it declares no key rather than declaring one that resolves to a default. A declared key is - // written at override precedence, which would put the default over the value that code assigned. + // It declares no key. Declaring one would put a key in the space that reaches no field, which an + // operator can write and nothing answers, and that is what every other refusal here exists to + // prevent. A field with no tag stays a defect for the same reason read from the other end: it + // would declare a key derived from a field name, which is a key no operator writes. The two look + // alike in a diff and mean opposite things. return "", false, true, nil } if name == "" { diff --git a/config/registry/spec_test.go b/config/registry/spec_test.go index dd88b2693b..cddb627238 100644 --- a/config/registry/spec_test.go +++ b/config/registry/spec_test.go @@ -1352,16 +1352,15 @@ func TestARefusalInsideASquashedBaseIsReported(t *testing.T) { // TestAFieldExcludedFromConfigDeclaresNoKey covers the tag that means "not from configuration". // -// mapstructure reads "-" as skip this field, and a config struct uses it for a field something else in -// the program assigns: the receipt store's KeepRecent comes from the global min-retain-blocks flag at the -// app layer, and its ExternalPruning from whatever constructs the collector. +// mapstructure reads "-" as skip this field, and a configuration struct uses it for a field something else +// in the program assigns. // -// Such a field declares no key. Declaring one that resolved to the default would be worse than refusing -// the section: a declared key is written at override precedence, so the default would land on top of the -// value that code assigned, and a node with min-retain-blocks set would silently keep nothing. +// Such a field declares no key. Declaring one that resolved to a default would put a key in the space that +// reaches no field: an operator could write it and the assignment would discard whatever they wrote. // -// An untagged field stays a defect. The two look alike and mean opposite things: one is a field the author -// excluded, the other is a field configuration cannot reach because nothing names it. +// An untagged field stays a defect. The two look alike in a diff and mean opposite things: one is a field +// the author excluded from configuration, the other is a field configuration cannot reach because nothing +// names it. func TestAFieldExcludedFromConfigDeclaresNoKey(t *testing.T) { type excluded struct { Kept string `mapstructure:"kept"` diff --git a/giga/executor/config/register_test.go b/giga/executor/config/register_test.go index d1e9e5203d..4a7aef2754 100644 --- a/giga/executor/config/register_test.go +++ b/giga/executor/config/register_test.go @@ -2,6 +2,7 @@ package config import ( "reflect" + "sort" "testing" "github.com/sei-protocol/sei-chain/config/registry" @@ -14,12 +15,18 @@ import ( // Checked against the constants rather than against a written-out list, so a rename of either moves both // or fails here. func TestTheDeclaredKeysAreTheKeysThisReaderResolves(t *testing.T) { + for _, defect := range registry.Defects() { + if defect.Section == SectionName { + t.Fatalf("%s was refused: %v", SectionName, defect.Err) + } + } section, ok := registry.Lookup(SectionName) if !ok { t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) } want := []string{FlagEnabled, FlagOCCEnabled} + sort.Strings(want) if got := section.Keys; !reflect.DeepEqual(got, want) { t.Errorf("declared keys are %v, want the keys the reader asks for, %v", got, want) } diff --git a/sei-db/config/receipt_register.go b/sei-db/config/receipt_register.go index 945ae73780..11fd6e25f2 100644 --- a/sei-db/config/receipt_register.go +++ b/sei-db/config/receipt_register.go @@ -10,12 +10,23 @@ const ReceiptStoreSectionName = "receipt-store" // Registration puts this section in the configuration registry. // // Two of the struct's fields carry the tag that excludes a field from configuration, so they declare no -// key: KeepRecent is derived from the global min-retain-blocks flag at the app layer, and ExternalPruning -// is set by whatever constructs the garbage collector. Declaring a key for either would put a default over -// the value that code assigns. +// key. KeepRecent is assigned from the global min-retain-blocks flag at the app layer, after this reader +// has returned, and ExternalPruning by whatever constructs the garbage collector. A key for either would +// be one an operator can write that the assignment then discards, which is a key reaching no field. +// +// The reader resolves one further key that this section does not declare: the retired spelling of the +// backend, which it answers by refusing to start. Declaring it would offer an operator a key whose only +// outcome is a stopped node. func init() { registry.RegisterSection(ReceiptStoreSectionName, &ReceiptStoreConfig{}, receiptStoreDefaults) } // receiptStoreDefaults is what this section resolves to for a node that has written nothing. +// +// The database directory resolves to an empty string, and the emptiness carries meaning rather than +// standing in for a path nobody chose. The app layer fills it only while it is empty, and what it fills +// it with depends on the host: a node that already holds the store at its former path keeps using that +// path, and any other node gets the current one. So a caller that renders this value into a file has to +// leave it empty. A path written there is one host's answer, and on a host whose store sits at the other +// path it names an empty directory. func receiptStoreDefaults(registry.Mode) any { return DefaultReceiptStoreConfig() } diff --git a/sei-db/config/receipt_register_test.go b/sei-db/config/receipt_register_test.go index 373ce6183b..7981fb0430 100644 --- a/sei-db/config/receipt_register_test.go +++ b/sei-db/config/receipt_register_test.go @@ -2,6 +2,7 @@ package config import ( "reflect" + "sort" "testing" "github.com/sei-protocol/sei-chain/config/registry" @@ -9,35 +10,36 @@ import ( // TestTheDeclaredKeysAreTheKeysThisReaderResolves holds the declaration against the reader. // -// Two of the struct's fields are excluded from configuration and must declare nothing. KeepRecent is -// derived from the global min-retain-blocks flag at the app layer and ExternalPruning is set by whatever -// constructs the collector, so a key for either would be written at override precedence over the value -// that code assigns. +// Six keys, which is every key the reader resolves and takes a value from. Two of the struct's fields are +// excluded from configuration and declare nothing, because the app layer assigns them after this reader +// has returned and a key for either is one an operator writes that the assignment discards. The reader +// resolves a seventh key, the retired spelling of the backend, only to refuse to start; a key whose one +// outcome is a stopped node is not one to offer. func TestTheDeclaredKeysAreTheKeysThisReaderResolves(t *testing.T) { + for _, defect := range registry.Defects() { + if defect.Section == ReceiptStoreSectionName { + t.Fatalf("%s was refused: %v", ReceiptStoreSectionName, defect.Err) + } + } section, ok := registry.Lookup(ReceiptStoreSectionName) if !ok { t.Fatalf("%s is not registered, so nothing resolves its keys", ReceiptStoreSectionName) } + // The constants this package's own reader passes to Get, rather than the same strings written again. + // A second list agrees with itself while the reader asks for something else. want := []string{ - "receipt-store.async-write-buffer", - "receipt-store.db-directory", - "receipt-store.enable-read-write-metrics", - "receipt-store.log-filter-parallelism", - "receipt-store.prune-interval-seconds", - "receipt-store.rs-backend", + flagRSAsyncWriteBuffer, + flagRSDBDirectory, + flagRSReadWriteMetrics, + flagRSLogFilterParallelism, + flagRSPruneIntervalSeconds, + flagRSBackend, } + sort.Strings(want) if got := section.Keys; !reflect.DeepEqual(got, want) { t.Errorf("declared keys are\n %v\nwant\n %v", got, want) } - for _, excluded := range []string{"receipt-store.keep-recent", "receipt-store.external-pruning"} { - for _, key := range section.Keys { - if key == excluded { - t.Errorf("%s is declared. Nothing sources it from configuration, so installing it would "+ - "put a default over the value the app layer assigns", excluded) - } - } - } } // TestTheDefaultsAreWhatTheNodeAlreadyRuns keeps the section from restating the values by hand. diff --git a/sei-wasmd/x/wasm/config_register.go b/sei-wasmd/x/wasm/config_register.go index 0ca9482380..515af57778 100644 --- a/sei-wasmd/x/wasm/config_register.go +++ b/sei-wasmd/x/wasm/config_register.go @@ -27,14 +27,24 @@ type wasmSchema struct { // Registration puts this section in the configuration registry. // -// Three keys, matching the three flag constants above. Two settings of types.WasmConfig are deliberately -// absent: ContractDebugMode has no key any reader resolves, and lru_size is written into app.toml by the -// template and read by nothing, so declaring either would put a key in the space that reaches no field. +// Three keys, matching the three flag constants the module declares. Two settings of types.WasmConfig are +// deliberately absent, for different reasons. The contract debug switch is read from the node-wide trace +// flag, so its key belongs to the root of the file rather than to this section and this section cannot +// declare it; a consequence worth knowing is that these three keys do not determine the whole +// configuration the module ends up with. The cache size written as lru_size is put into app.toml by the +// template and read by nothing, so declaring it would offer a key that reaches no field. func init() { registry.RegisterSection(SectionName, &wasmSchema{}, defaults) } // defaults is what this section resolves to for a node that has written nothing. +// +// The query gas limit is the one value here that the binary states twice. This is what a file with no +// wasm section resolves to, and the template writes a tenth of it into every file it generates, so a node +// provisioned by the binary runs the smaller number and a node whose file predates the section runs this +// one. Whoever renders declared values into a file has to decide which of the two survives, and the +// decision is not this section's to make: the limit bounds the work one smart query can ask of a node +// that serves queries to anyone. func defaults(registry.Mode) any { live := types.DefaultWasmConfig() schema := wasmSchema{ diff --git a/sei-wasmd/x/wasm/config_register_test.go b/sei-wasmd/x/wasm/config_register_test.go index b0263a01d7..4be6cc7299 100644 --- a/sei-wasmd/x/wasm/config_register_test.go +++ b/sei-wasmd/x/wasm/config_register_test.go @@ -2,6 +2,8 @@ package wasm import ( "reflect" + "sort" + "strconv" "testing" "github.com/sei-protocol/sei-chain/config/registry" @@ -15,22 +17,30 @@ import ( // safe while something holds it against the first. These are the flag constants the module registers and // reads. func TestTheDeclaredKeysAreTheFlagsThisModuleReads(t *testing.T) { + for _, defect := range registry.Defects() { + if defect.Section == SectionName { + t.Fatalf("%s was refused: %v", SectionName, defect.Err) + } + } section, ok := registry.Lookup(SectionName) if !ok { t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) } want := []string{flagWasmMemoryCacheSize, flagWasmQueryGasLimit, flagWasmSimulationGasLimit} + sort.Strings(want) if got := section.Keys; !reflect.DeepEqual(got, want) { t.Errorf("declared keys are %v, want the flags this module reads, %v", got, want) } } -// TestTheDefaultsCarryTheLiveWasmConfig keeps the schema's values from drifting from the real ones. +// TestTheDefaultsAreTheModuleDeclaredOnes keeps the schema's values from drifting from the struct's. // // The schema restates three settings of types.WasmConfig, so nothing stops those values diverging from -// what DefaultWasmConfig returns except this. -func TestTheDefaultsCarryTheLiveWasmConfig(t *testing.T) { +// what DefaultWasmConfig returns except this. It compares against that struct and against nothing else: +// the query gas limit the template writes into a generated file is a tenth of the one here, and this test +// is not the place that reconciles them. +func TestTheDefaultsAreTheModuleDeclaredOnes(t *testing.T) { live := types.DefaultWasmConfig() got, ok := defaults(registry.ModeValidator).(wasmSchema) if !ok { @@ -44,9 +54,13 @@ func TestTheDefaultsCarryTheLiveWasmConfig(t *testing.T) { t.Errorf("query_gas_limit resolves to %d, want the live %d", got.QueryGasLimit, live.SmartQueryGasLimit) } // Absent is a meaning of its own here: unset means the consensus block gas limit applies, so an unset - // live value has to resolve to no text rather than to a zero. - if live.SimulationGasLimit == nil && got.SimulationGasLimit != "" { - t.Errorf("simulation_gas_limit resolves to %q where the live value is unset. A number here claims "+ - "a limit the node does not apply", got.SimulationGasLimit) + // live value resolves to no text rather than to a zero, and a set one resolves to its digits. + want := "" + if live.SimulationGasLimit != nil { + want = strconv.FormatUint(*live.SimulationGasLimit, 10) + } + if got.SimulationGasLimit != want { + t.Errorf("simulation_gas_limit resolves to %q, want %q. Unset means the consensus block gas limit "+ + "applies, and a number here claims a limit the node does not apply", got.SimulationGasLimit, want) } } From 12fe6c06c7c01ab0c3565a6f5b0baaf36b6dd2a6 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Fri, 21 Aug 2026 07:27:45 -0700 Subject: [PATCH 3/3] config: name the untagged field, and say each thing once 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. --- config/registry/doc.go | 3 ++- config/registry/spec_test.go | 21 ++++++++++++++------- giga/executor/config/register.go | 6 +++--- sei-wasmd/x/wasm/config_register.go | 6 +++--- sei-wasmd/x/wasm/config_register_test.go | 10 ++++++++-- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/config/registry/doc.go b/config/registry/doc.go index ebf63e3d99..f49e13b6ea 100644 --- a/config/registry/doc.go +++ b/config/registry/doc.go @@ -74,7 +74,8 @@ // A field tagged "-" is the deliberate opposite and is not a defect. That tag excludes a field from // configuration, so the field declares no key at all rather than one resolving to a default. The // distinction matters because a missing tag and a "-" tag look alike in a diff: one is a key nothing -// names reaching a field, and the other is a field nothing configures. +// names reaching a field, and the other is a field nothing configures. It is meaningful only on an +// exported field, since an unexported one carrying any tag is refused before the tag is read. // // A key segment is also refused if it is upper-case, or if it carries a dot or a space. That rule // holds for the section name and for a field's tag alike, since both become segments of the same diff --git a/config/registry/spec_test.go b/config/registry/spec_test.go index cddb627238..9091b8a965 100644 --- a/config/registry/spec_test.go +++ b/config/registry/spec_test.go @@ -29,8 +29,9 @@ import ( // authoring check, and it reads its order from Source's declaration rather than from its caller's // argument order, so no caller can reorder its way to a different answer. -// gigaSection mirrors what the giga executor's own package would register. The struct under test -// is the real one, so the key comparison below measures the live reader rather than a copy of it. +// gigaSection re-registers the giga executor's section with a default that varies by mode, so the +// mode property below has something to measure. The struct is the real one, so the key comparison +// measures the live reader rather than a copy of it. const gigaSection = "giga_executor" func registerGiga(t *testing.T) { @@ -1376,8 +1377,8 @@ func TestAFieldExcludedFromConfigDeclaresNoKey(t *testing.T) { } if got, want := registry.Keys(), []string{"probe.kept"}; !reflect.DeepEqual(got, want) { - t.Fatalf("declared keys are %v, want %v. An excluded field declaring a key would have that key "+ - "written at override precedence over whatever assigned the field", got, want) + t.Fatalf("declared keys are %v, want %v. A key for an excluded field is one an operator can write "+ + "that whatever assigns the field then discards", got, want) } resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{}) @@ -1385,7 +1386,8 @@ func TestAFieldExcludedFromConfigDeclaresNoKey(t *testing.T) { t.Fatalf("resolving a section with an excluded field: %v", err) } if _, present := resolved.Values["probe.assigned"]; present { - t.Error("the excluded field resolved to a value, so installing it would overwrite what assigns it") + t.Error("the excluded field resolved to a value, so an operator could write a key that reaches no " + + "field") } // An untagged field means the opposite and stays a defect. @@ -1395,7 +1397,12 @@ func TestAFieldExcludedFromConfigDeclaresNoKey(t *testing.T) { } registry.Reset() registry.RegisterSection("probe", &untagged{}, func(registry.Mode) any { return &untagged{} }) - if len(registry.Defects()) == 0 { - t.Error("a field with no tag at all registered cleanly, so a key nothing names reaches no field") + defects := registry.Defects() + if len(defects) == 0 { + t.Fatal("a field with no tag at all registered cleanly, so a key nothing names reaches no field") + } + // Named, so this cannot pass on a refusal raised for some other reason. + if got := defects[0].Err.Error(); !strings.Contains(got, "Forgotten") { + t.Errorf("the refusal reads %q and does not name the untagged field", got) } } diff --git a/giga/executor/config/register.go b/giga/executor/config/register.go index c22802ed11..271a65805d 100644 --- a/giga/executor/config/register.go +++ b/giga/executor/config/register.go @@ -21,7 +21,7 @@ func init() { // defaults is what this section resolves to for a node that has written nothing. // -// The same values for every mode, because that is what a node runs today. A mode-varying default would -// change what an archive node does, which is a decision about how the executor should behave rather than -// a consequence of describing it here. +// The same values for every mode. Nothing in the binary makes either setting follow from what kind of +// node is asking, so a default that varied here would be this section inventing a rule rather than +// stating one. func defaults(registry.Mode) any { return DefaultConfig } diff --git a/sei-wasmd/x/wasm/config_register.go b/sei-wasmd/x/wasm/config_register.go index 515af57778..45be17d511 100644 --- a/sei-wasmd/x/wasm/config_register.go +++ b/sei-wasmd/x/wasm/config_register.go @@ -34,10 +34,10 @@ type wasmSchema struct { // configuration the module ends up with. The cache size written as lru_size is put into app.toml by the // template and read by nothing, so declaring it would offer a key that reaches no field. func init() { - registry.RegisterSection(SectionName, &wasmSchema{}, defaults) + registry.RegisterSection(SectionName, &wasmSchema{}, sectionDefaults) } -// defaults is what this section resolves to for a node that has written nothing. +// sectionDefaults is what this section resolves to for a node that has written nothing. // // The query gas limit is the one value here that the binary states twice. This is what a file with no // wasm section resolves to, and the template writes a tenth of it into every file it generates, so a node @@ -45,7 +45,7 @@ func init() { // one. Whoever renders declared values into a file has to decide which of the two survives, and the // decision is not this section's to make: the limit bounds the work one smart query can ask of a node // that serves queries to anyone. -func defaults(registry.Mode) any { +func sectionDefaults(registry.Mode) any { live := types.DefaultWasmConfig() schema := wasmSchema{ MemoryCacheSize: live.MemoryCacheSize, diff --git a/sei-wasmd/x/wasm/config_register_test.go b/sei-wasmd/x/wasm/config_register_test.go index 4be6cc7299..085d485aa3 100644 --- a/sei-wasmd/x/wasm/config_register_test.go +++ b/sei-wasmd/x/wasm/config_register_test.go @@ -42,9 +42,15 @@ func TestTheDeclaredKeysAreTheFlagsThisModuleReads(t *testing.T) { // is not the place that reconciles them. func TestTheDefaultsAreTheModuleDeclaredOnes(t *testing.T) { live := types.DefaultWasmConfig() - got, ok := defaults(registry.ModeValidator).(wasmSchema) + for _, mode := range registry.Modes() { + if got := sectionDefaults(mode); !reflect.DeepEqual(got, sectionDefaults(registry.ModeValidator)) { + t.Errorf("mode %q resolves differently from the others, and nothing in the module makes "+ + "either setting follow from what kind of node is asking", mode) + } + } + got, ok := sectionDefaults(registry.ModeValidator).(wasmSchema) if !ok { - t.Fatalf("defaults returned %T, want wasmSchema", defaults(registry.ModeValidator)) + t.Fatalf("defaults returned %T, want wasmSchema", sectionDefaults(registry.ModeValidator)) } if got.MemoryCacheSize != live.MemoryCacheSize {