From aaaae2ac9c5e344f4908940200b91cf09243ab71 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 3 Sep 2026 14:40:28 -0700 Subject: [PATCH] feat(config): declare the gRPC-web settings, and record every key nothing declares A node's own configuration files state 276 keys and this binary declared 281, and 22 of the file's keys were not among them. An undeclared key goes on resolving from the file that states it, which is the one thing sei.toml cannot reach: the resolution has nothing to say about it, so writing that file changes nothing for it and neither does writing it again on every start. A node enrolled in automatic configuration management was held to its kind's defaults for every key but those 22. Four of them are used. The gRPC-web settings are read by the upstream server, and one of them is assigned per kind of node by the same mode rules the rest of this package follows, so a rule already targeted a key the declaration could not express. Declared now, which adds two rows to the agreement record: the interface is open on a generated file and declared closed for a validator and a seed, the way the REST interface and gRPC already are. Twelve are not used, and each now carries its reason. The query settings are the find: the reader that builds the server configuration from the source fills that whole section from its own defaults and never asks the source for any of its five keys. Measured, a written page ceiling of 500 arrives as the default 1000 and a written allowlist arrives empty, while a gRPC-web address written beside it arrives. The file's own comment tells an operator to set one of them. Rosetta's six are an unsupported feature, pending removal rather than declared. The remaining six were already decided, and those rows name the constant holding each reason rather than restating it. The measurement is what keeps this closed. A key written into a template with no section to declare it now fails rather than answering from app.toml quietly, and a key recorded here that the binary starts declaring fails as a record of a hole that is closed. Verified by mutation: a dropped row, a row naming a declared key, and the gRPC-web registration removed, which reports all four of its keys and is the gap this started from. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/seid/cmd/node_agreement_test.go | 8 +- cmd/seid/cmd/undeclared_keys_test.go | 128 +++++++++++++++++++++++++++ config/cosmosbase/cosmosbase.go | 17 +++- 3 files changed, 148 insertions(+), 5 deletions(-) create mode 100644 cmd/seid/cmd/undeclared_keys_test.go diff --git a/cmd/seid/cmd/node_agreement_test.go b/cmd/seid/cmd/node_agreement_test.go index 0da3f12eef..dabc958204 100644 --- a/cmd/seid/cmd/node_agreement_test.go +++ b/cmd/seid/cmd/node_agreement_test.go @@ -39,9 +39,9 @@ import ( // value is four times what a node runs, so a sparse file raises both. // connection policy p2p.max-connections is ten times higher declared than a seed runs, and // p2p.allow-duplicate-ip is declared on where a seed has it off. -// service switches api.enable, grpc.enable, evm.http_enabled, evm.ws_enabled and -// state-store.ss-enable are open on the node and declared closed for a validator -// and a seed, which serve no queries. A sparse file closes them. +// service switches api.enable, grpc.enable, grpc-web.enable, evm.http_enabled, evm.ws_enabled +// and state-store.ss-enable are open on the node and declared closed for a +// validator and a seed, which serve no queries. A sparse file closes them. // retention pruning is the flag-default row, in every kind: the flag prunes and the // declaration keeps all state history. min-retain-blocks and // state-store.ss-keep-recent are the block and state halves of the same choice, @@ -58,6 +58,7 @@ var divergences = map[registry.Mode]map[string]string{ "api.enable": "true", "evm.http_enabled": "true", "evm.ws_enabled": "true", + "grpc-web.enable": "true", "grpc.enable": "true", "p2p.recv-rate": "5120000", "p2p.send-rate": "5120000", @@ -79,6 +80,7 @@ var divergences = map[registry.Mode]map[string]string{ "api.enable": "true", "evm.http_enabled": "true", "evm.ws_enabled": "true", + "grpc-web.enable": "true", "grpc.enable": "true", "p2p.allow-duplicate-ip": "false", "p2p.max-connections": "100", diff --git a/cmd/seid/cmd/undeclared_keys_test.go b/cmd/seid/cmd/undeclared_keys_test.go new file mode 100644 index 0000000000..e1f86e2eac --- /dev/null +++ b/cmd/seid/cmd/undeclared_keys_test.go @@ -0,0 +1,128 @@ +package cmd + +import ( + "path/filepath" + "sort" + "testing" + + "github.com/spf13/viper" + + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/testutil/configtest" +) + +// keysAGeneratedFileStatesThatNothingDeclares is every key this binary writes into a node's own +// configuration files and does not declare, with the reason it is left out. +// +// An undeclared key goes on resolving from the file that states it. Under the versioned manager that is +// the one thing sei.toml cannot reach: the resolution has nothing to say about the key, so writing that +// file changes nothing for it and neither does writing it again on every start. A node enrolled in +// automatic configuration management is held to its kind's defaults for every key but these. +// +// So each one is a decision, and the decision is recorded here rather than left as the absence of a +// registration. A key added to a template without a section to declare it fails the measurement below +// instead of quietly answering from app.toml. +// +// Where the reason belongs to a registered section, it lives with that section and the row names the +// constant holding it. Restating it here would let the two drift. +var keysAGeneratedFileStatesThatNothingDeclares = map[string]string{ + // Not a supported feature of this chain. The upstream server still starts one when the key is on, and + // it prints that it is a beta feature not to be used in production. Pending removal. + "rosetta.enable": "rosetta is not a supported feature", + "rosetta.address": "rosetta is not a supported feature", + "rosetta.blockchain": "rosetta is not a supported feature", + "rosetta.network": "rosetta is not a supported feature", + "rosetta.offline": "rosetta is not a supported feature", + "rosetta.retries": "rosetta is not a supported feature", + + // The reader that builds the server configuration from the source fills this whole section from its + // own defaults and never asks the source for any of these keys. Measured: a written page ceiling of + // 500 arrives as the default 1000, and a written allowlist arrives empty. Declaring one would offer a + // setting that changes nothing, and the file's own comment already tells an operator to set one of + // them. + "query.disable-limits": "the server configuration reader never asks the source for this section", + "query.trusted-cidrs": "the server configuration reader never asks the source for this section", + "query.max-limit": "the server configuration reader never asks the source for this section", + "query.max-offset": "the server configuration reader never asks the source for this section", + "query.max-iterations": "the server configuration reader never asks the source for this section", + + // No reader resolves it. The struct the command renders sets one value for this key and the template + // beside it renders another as a bare literal, which is held by a test of its own. + "wasm.lru_size": "no reader resolves it", + + // The template renders this name and the reader looks up contract_state_checks, which is the key this + // binary declares. A value written under this name reaches nothing. + "eth_replay.eth_replay_contract_state_checks": "the reader looks up contract_state_checks", + + // Recorded where the sections that would carry them are registered. + "mode": "config/tendermintbase, statedAtTheTopOfTheFile", + "mempool.max-batch-bytes": "config/tendermintbase, unreadAndUnmarked", + "mempool.pending-ttl-duration": "config/tendermintbase, neverReachTheMempool", + "mempool.pending-ttl-num-blocks": "config/tendermintbase, neverReachTheMempool", + "self-remediation.p2p-no-peers-available-window-seconds": "config/tendermintbase, reachesNoReactor", +} + +// TestEveryKeyAGeneratedFileStatesIsDeclaredOrRecorded closes the gap a missing registration leaves. +// +// Two sides of the same key space, and neither one is checked by anything else. A key this binary writes +// into a file and does not declare resolves from that file forever, and a key recorded here that the +// binary has started declaring makes the record claim a hole that is closed. +// +// The sections a registration excludes are covered too, because an excluded path is still written into +// the file. Excluding a path and declaring the section it sits in are different statements, and only the +// first leaves the key resolving from app.toml. +func TestEveryKeyAGeneratedFileStatesIsDeclaredOrRecorded(t *testing.T) { + configtest.Isolate(t) + home := aNodeRunningAs(t, registry.ModeValidator) + + stated := whatTheFilesState(t, home) + declared := make(map[string]bool, len(registry.Keys())) + for _, key := range registry.Keys() { + declared[key] = true + } + + var undeclared []string + for _, key := range stated { + if declared[key] { + continue + } + undeclared = append(undeclared, key) + if _, recorded := keysAGeneratedFileStatesThatNothingDeclares[key]; !recorded { + t.Errorf("this binary writes %s into a node's own files and does not declare it, and nothing "+ + "records why. It resolves from that file whatever a sei.toml says, so a node enrolled in "+ + "automatic configuration management is not held to its kind's defaults for it", key) + } + } + + for key := range keysAGeneratedFileStatesThatNothingDeclares { + if declared[key] { + t.Errorf("%s is recorded as undeclared and this binary declares it, so the record names a "+ + "hole that is closed", key) + } + } + + sort.Strings(undeclared) + if len(undeclared) != len(keysAGeneratedFileStatesThatNothingDeclares) { + t.Errorf("measured %d undeclared keys and %d are recorded: %v", + len(undeclared), len(keysAGeneratedFileStatesThatNothingDeclares), undeclared) + } +} + +// whatTheFilesState returns every key the node's own configuration files state, by dotted name. +// +// Read the way a lookup reads them, so the names are the ones a reader resolves rather than the shape the +// files nest them in. +func whatTheFilesState(t *testing.T, home string) []string { + t.Helper() + var all []string + for _, name := range []string{"app.toml", "config.toml"} { + v := viper.New() + v.SetConfigFile(filepath.Join(home, "config", name)) + if err := v.ReadInConfig(); err != nil { + t.Fatalf("read %s: %v", name, err) + } + all = append(all, v.AllKeys()...) + } + sort.Strings(all) + return all +} diff --git a/config/cosmosbase/cosmosbase.go b/config/cosmosbase/cosmosbase.go index ba37836a12..01091f0d6c 100644 --- a/config/cosmosbase/cosmosbase.go +++ b/config/cosmosbase/cosmosbase.go @@ -1,6 +1,6 @@ // Package cosmosbase registers the configuration sections whose keys belong to the Cosmos server. // -// These five register here rather than beside the structs they describe, and the reason is an import edge. +// These six register here rather than beside the structs they describe, and the reason is an import edge. // The mode rules their defaults answer through live in app/params, which imports the upstream server // configuration, so that package cannot ask for them without a cycle. A vendored tree is not itself the // obstacle: other sections do register inside one. @@ -25,6 +25,7 @@ const ( GRPCSectionName = "grpc" TelemetrySectionName = "telemetry" StateSyncSectionName = "state-sync" + GRPCWebSectionName = "grpc-web" ) // globalLabelsKey is the metric label set, which is the one key here no environment variable can supply. @@ -32,12 +33,13 @@ const globalLabelsKey = TelemetrySectionName + ".global-labels" // Registration puts the upstream server's configuration sections in the registry. // -// Four of the five register the upstream struct directly, because their mapstructure tags already name the +// Five of the six register the upstream struct directly, because their mapstructure tags already name the // keys their reader resolves. func init() { registry.RegisterRootKeys(BaseSectionName, &srvconfig.BaseConfig{}, baseDefaults) registry.RegisterSection(APISectionName, &srvconfig.APIConfig{}, apiDefaults) registry.RegisterSection(GRPCSectionName, &srvconfig.GRPCConfig{}, grpcDefaults) + registry.RegisterSection(GRPCWebSectionName, &srvconfig.GRPCWebConfig{}, grpcWebDefaults) registry.RegisterSection(TelemetrySectionName, &telemetrySchema{}, telemetryDefaults) registry.RegisterSection(StateSyncSectionName, &srvconfig.StateSyncConfig{}, stateSyncDefaults) } @@ -102,6 +104,17 @@ func apiDefaults(mode registry.Mode) any { return forMode(mode).API } // which is the shape the reader parses back. func grpcDefaults(mode registry.Mode) any { return forMode(mode).GRPC } +// grpcWebDefaults is what the gRPC-web settings resolve to for a node of this kind. +// +// On for a full node and an archive node, off for a validator and a seed, which is the rule the REST +// interface and gRPC follow and for the same reason. The upstream default is on for every kind. +// +// Three of these four keys are read with a casting getter and no check that the key was present, and the +// fourth, the connection ceiling, is read only when the key is set. The three unguarded ones are safe in +// one direction only: whether the interface starts at all is one of them, so an absent key casts to false +// and the interface does not come up rather than coming up on an address cast from nothing. +func grpcWebDefaults(mode registry.Mode) any { return forMode(mode).GRPCWeb } + // stateSyncDefaults is what the snapshot settings resolve to for a node of this kind. // // All three keys are read with a casting getter and no presence check, and the retention is the one that