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 new file mode 100644 index 0000000000..a7be61ccba --- /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, 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) +} + +// defaults is what the seid init command writes for a node of this kind. +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..4120c38cd4 --- /dev/null +++ b/admin/register_test.go @@ -0,0 +1,44 @@ +package admin + +import ( + "reflect" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestTheDeclaredKeysAreTheKeysThisReaderResolves holds the declaration against the reader. +// +// 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{flagAdminAddress, flagAdminEnabled} + sort.Strings(want) + 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/app/config_register.go b/app/config_register.go new file mode 100644 index 0000000000..e7ab04ba8f --- /dev/null +++ b/app/config_register.go @@ -0,0 +1,194 @@ +package app + +import ( + "github.com/sei-protocol/sei-chain/app/params" + "github.com/sei-protocol/sei-chain/config/registry" + srvconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" + "github.com/sei-protocol/sei-chain/sei-db/config" +) + +// The names these sections have in the configuration key space. +const ( + LightInvarianceSectionName = "light_invariance" + GenesisSectionName = "genesis" + StateStoreSectionName = "state-store" + StateCommitSectionName = "state-commit" +) + +// Registration puts this package's configuration sections in the registry. +// +// The owning package registers its own sections, so the struct, the values and the keys come from one +// place and cannot drift apart. The keys derive from mapstructure tags, so a section's spelling and its +// reader's own constants stay the same strings. +func init() { + registry.RegisterSection(LightInvarianceSectionName, &LightInvarianceConfig{}, lightInvarianceDefaults) + registry.RegisterSection(GenesisSectionName, &genesisSchema{}, genesisDefaults) + registry.RegisterSection(StateStoreSectionName, &stateStoreSchema{}, stateStoreDefaults) + registry.RegisterSection(StateCommitSectionName, &stateCommitSchema{}, stateCommitDefaults) +} + +// lightInvarianceDefaults is what this section resolves to for a node that has written nothing. +// +// The same value for every mode, and on. What the check compares is a property of every node rather than +// of one kind, so a mode that resolved it off would stop those nodes noticing they had diverged. +func lightInvarianceDefaults(registry.Mode) any { return DefaultLightInvarianceConfig } + +// genesisSchema declares the keys the genesis import reader resolves. +// +// A schema and not a transport: nothing decodes into it. The type the reader fills is +// genesistypes.GenesisImportConfig, which carries no mapstructure tags at all, so no key can be derived +// from it. Declaring the spelling here is what lets the registry name the keys the reader looks up. +type genesisSchema struct { + StreamImport bool `mapstructure:"stream-import"` + ImportFile string `mapstructure:"import-file"` +} + +// genesisDefaults is what this section resolves to for a node that has written nothing. +// +// Read out of the reader's own default rather than written again here, so a changed default moves both at +// once and this states only which key carries which setting. The same values for every mode: streaming a +// genesis file is what an operator does to import a chain's existing state, and no node mode implies it. +func genesisDefaults(registry.Mode) any { + return genesisSchema{ + StreamImport: DefaultGenesisConfig.StreamGenesisImport, + ImportFile: DefaultGenesisConfig.GenesisStreamFile, + } +} + +// stateStoreSchema declares the keys parseSSConfigs resolves. +// +// A schema and not a transport: nothing decodes into it. config.StateStoreConfig carries mapstructure +// tags of its own and every one names something other than the key the reader looks up, so deriving from +// that type would declare a set of keys no operator writes. It also holds settings no key reaches, which +// stay at whatever the defaults struct holds; giving them keys would declare settings a written value +// could not change. +type stateStoreSchema struct { + Enable bool `mapstructure:"ss-enable"` + DBDirectory string `mapstructure:"ss-db-directory"` + Backend string `mapstructure:"ss-backend"` + AsyncWriteBuffer int `mapstructure:"ss-async-write-buffer"` + KeepRecent int `mapstructure:"ss-keep-recent"` + PruneIntervalSeconds int `mapstructure:"ss-prune-interval"` + ImportNumWorkers int `mapstructure:"ss-import-num-workers"` + EnableReadWriteMetrics bool `mapstructure:"ss-enable-read-write-metrics"` + SnapshotEnable bool `mapstructure:"ss-snapshot-enable"` + EVMDBDirectory string `mapstructure:"evm-ss-db-directory"` + SeparateEVMSubDBs bool `mapstructure:"evm-ss-separate-dbs"` + EVMSplit bool `mapstructure:"evm-ss-split"` +} + +// stateStoreDefaults is what the seid init command writes for a node of this kind, with one deliberate +// departure. +// +// Answered per mode, because two of these settings mean something different depending on what kind of node +// asks. An archive node exists to keep history, so it keeps every version; a validator and a seed serve no +// queries, so the store is off for them. Both come from the mode rules the binary already states rather +// than being written again here, so a change to those rules moves this too. +// +// The departure is the retention an archive node keeps. The mode rules set it to keep everything and the +// command does not write that, because the type it renders declares a state store field of its own and +// fills it from the mode-blind default, so the rule is applied and then discarded. PLT-955 records that, +// and records the decision: pin what a node resolves today and correct it here, in the versioned +// declaration, rather than at the point that loses it. So this states the rule and the command states the +// value the rule was overwritten by, and the test beside this holds both, because a departure nothing +// measures is indistinguishable from an oversight. +// +// The declared values are also not what this section's reader produces for a file missing the keys, which +// is a different comparison and measured separately. +func stateStoreDefaults(mode registry.Mode) any { + server := srvconfig.DefaultConfig() + params.SetAppConfigByMode(server, params.NodeMode(mode)) + live := server.StateStore + return stateStoreSchema{ + Enable: live.Enable, + DBDirectory: live.DBDirectory, + Backend: live.Backend, + AsyncWriteBuffer: live.AsyncWriteBuffer, + KeepRecent: live.KeepRecent, + PruneIntervalSeconds: live.PruneIntervalSeconds, + ImportNumWorkers: live.ImportNumWorkers, + EnableReadWriteMetrics: live.EnableReadWriteMetrics, + SnapshotEnable: live.SnapshotEnable, + EVMDBDirectory: live.EVMDBDirectory, + SeparateEVMSubDBs: live.SeparateEVMSubDBs, + EVMSplit: live.EVMSplit, + } +} + +// stateCommitFlatKVSchema declares the one flat key-value key this package's reader resolves. +// +// A nested segment, because the key is state-commit.flatkv.enable-read-write-metrics. Four further keys +// under that name are read by the Cosmos server's own configuration reader and not by this one, so they +// belong to whoever registers that reader's section rather than to this one. +type stateCommitFlatKVSchema struct { + EnableReadWriteMetrics bool `mapstructure:"enable-read-write-metrics"` +} + +// stateCommitSchema declares the keys parseSCConfigs resolves. +// +// A schema and not a transport: nothing decodes into it. config.StateCommitConfig nests its settings +// under MemIAVLConfig, FlatKVConfig and HashLogger, and the keys the reader looks up are flat names on the +// section itself, so no derivation from that type produces them. +// +// The write mode is a plain string rather than the reader's own named type, because the reader parses a +// written name into that type itself. Declaring the named type would have one key answer as a named string +// from these defaults and as a plain one from an operator's file, which is a difference a caller can trip +// over and nothing here needs. +type stateCommitSchema struct { + Enable bool `mapstructure:"sc-enable"` + Directory string `mapstructure:"sc-directory"` + AsyncCommitBuffer int `mapstructure:"sc-async-commit-buffer"` + SnapshotKeepRecent uint32 `mapstructure:"sc-keep-recent"` + SnapshotInterval uint32 `mapstructure:"sc-snapshot-interval"` + SnapshotMinTimeInterval uint32 `mapstructure:"sc-snapshot-min-time-interval"` + SnapshotWriterLimit int `mapstructure:"sc-snapshot-writer-limit"` + SnapshotPrefetchThreshold float64 `mapstructure:"sc-snapshot-prefetch-threshold"` + SnapshotWriteRateMBps int `mapstructure:"sc-snapshot-write-rate-mbps"` + HistoricalProofMaxInFlight int `mapstructure:"sc-historical-proof-max-inflight"` + HistoricalProofRateLimit float64 `mapstructure:"sc-historical-proof-rate-limit"` + HistoricalProofBurst int `mapstructure:"sc-historical-proof-burst"` + WriteMode string `mapstructure:"sc-write-mode"` + WriteModeEnableAuto bool `mapstructure:"sc-write-mode-enable-auto"` + HashLoggerEnable bool `mapstructure:"sc-hash-logger-enable"` + HashLoggerDirectory string `mapstructure:"sc-hash-logger-directory"` + HashLoggerBlocksToRetain uint `mapstructure:"sc-hash-logger-blocks-to-retain"` + HashLoggerTargetFileSize uint `mapstructure:"sc-hash-logger-target-file-size"` + HashLoggerMaxDiskSize uint `mapstructure:"sc-hash-logger-max-disk-size"` + FlatKV stateCommitFlatKVSchema `mapstructure:"flatkv"` +} + +// stateCommitDefaults is what this section resolves to for a node that has written nothing. +// +// The declared defaults. Two of them are not what this section's reader produces for a file missing the +// key, and a test names which two and what a node runs instead. +// +// The same values for every mode. How often a node snapshots and how much proof history it serves are +// decisions about disk and load that an operator writes down, and nothing in the binary makes either +// follow from what kind of node is asking. +func stateCommitDefaults(registry.Mode) any { + live := config.DefaultStateCommitConfig() + return stateCommitSchema{ + Enable: live.Enable, + Directory: live.Directory, + AsyncCommitBuffer: live.MemIAVLConfig.AsyncCommitBuffer, + SnapshotKeepRecent: live.MemIAVLConfig.SnapshotKeepRecent, + SnapshotInterval: live.MemIAVLConfig.SnapshotInterval, + SnapshotMinTimeInterval: live.MemIAVLConfig.SnapshotMinTimeInterval, + SnapshotWriterLimit: live.MemIAVLConfig.SnapshotWriterLimit, + SnapshotPrefetchThreshold: live.MemIAVLConfig.SnapshotPrefetchThreshold, + SnapshotWriteRateMBps: live.MemIAVLConfig.SnapshotWriteRateMBps, + HistoricalProofMaxInFlight: live.HistoricalProofMaxInFlight, + HistoricalProofRateLimit: live.HistoricalProofRateLimit, + HistoricalProofBurst: live.HistoricalProofBurst, + WriteMode: string(live.WriteMode), + WriteModeEnableAuto: live.WriteModeEnableAuto, + HashLoggerEnable: live.HashLogger.Enable, + HashLoggerDirectory: live.HashLogger.Directory, + HashLoggerBlocksToRetain: live.HashLogger.BlocksToRetain, + HashLoggerTargetFileSize: live.HashLogger.TargetFileSize, + HashLoggerMaxDiskSize: live.HashLogger.MaxDiskSize, + FlatKV: stateCommitFlatKVSchema{ + EnableReadWriteMetrics: live.FlatKVConfig.EnableReadWriteMetrics, + }, + } +} diff --git a/app/config_register_agreement_test.go b/app/config_register_agreement_test.go new file mode 100644 index 0000000000..ca84b68182 --- /dev/null +++ b/app/config_register_agreement_test.go @@ -0,0 +1,179 @@ +package app + +import ( + "fmt" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/testutil/configtest" +) + +// whatANodeRunsToday is what each diverging key resolves to for a file carrying no keys at all. +// +// Every entry is a read that takes no account of whether the key was present, or a value another key +// transforms afterwards. Held separately from the modes because the reader takes no mode: it produces one +// answer, and which modes disagree with it depends on what the section declares. +var whatANodeRunsToday = map[string]string{ + FlagSSEnable: "false", + FlagSSBackend: "", + FlagSSAsyncWriterBuffer: "0", + FlagSSKeepRecent: "0", + FlagSSPruneInterval: "0", + FlagSSImportNumWorkers: "0", + FlagSCEnable: "false", + FlagSCWriteMode: "auto", +} + +// whyItMatters says what a node gets today, for the keys where that is worth stating. +var whyItMatters = map[string]string{ + FlagSSPruneInterval: "pruning is off, in the store and in the write-ahead log, so installing the " + + "declared value starts deleting what the node was retaining", + FlagSSKeepRecent: "every version is kept, so for an archive node what is declared and what runs " + + "agree about keeping history and for the others they do not", + FlagSCEnable: "state commitment reads as disabled, and a node started that way stops, which is why " + + "no running node has this key missing", + FlagSCWriteMode: "another key transforms this one after it is read, so the mode a node commits " + + "through is derived rather than carried by this key", +} + +// theDivergences is which keys disagree with the reader, per mode. +// +// Per mode because the section answers per mode for two of these settings and the reader does not answer +// per mode at all. An archive node declares the retention the reader also produces, so that key agrees for +// archive and disagrees everywhere else; the store toggle is the reverse. +var theDivergences = map[registry.Mode][]string{ + registry.ModeValidator: {FlagSSBackend, FlagSSAsyncWriterBuffer, FlagSSKeepRecent, + FlagSSPruneInterval, FlagSSImportNumWorkers, FlagSCEnable, FlagSCWriteMode}, + registry.ModeSeed: {FlagSSBackend, FlagSSAsyncWriterBuffer, FlagSSKeepRecent, + FlagSSPruneInterval, FlagSSImportNumWorkers, FlagSCEnable, FlagSCWriteMode}, + registry.ModeFull: {FlagSSEnable, FlagSSBackend, FlagSSAsyncWriterBuffer, FlagSSKeepRecent, + FlagSSPruneInterval, FlagSSImportNumWorkers, FlagSCEnable, FlagSCWriteMode}, + registry.ModeArchive: {FlagSSEnable, FlagSSBackend, FlagSSAsyncWriterBuffer, + FlagSSPruneInterval, FlagSSImportNumWorkers, FlagSCEnable, FlagSCWriteMode}, +} + +// readerValues is what each section's reader produces for a file carrying no keys at all. +// +// Written as a map from key to the field that key fills, because that pairing is what the comparison +// needs and neither the reader nor the section states it: the reader takes a key and assigns a field, and +// the section declares a key and a value. +func readerValues(t *testing.T) map[string]string { + t.Helper() + ss := parseSSConfigs(configtest.AppOpts{}) + sc := parseSCConfigs(configtest.AppOpts{}) + return map[string]string{ + FlagSSEnable: fmt.Sprint(ss.Enable), + FlagSSDirectory: fmt.Sprint(ss.DBDirectory), + FlagSSBackend: fmt.Sprint(ss.Backend), + FlagSSAsyncWriterBuffer: fmt.Sprint(ss.AsyncWriteBuffer), + FlagSSKeepRecent: fmt.Sprint(ss.KeepRecent), + FlagSSPruneInterval: fmt.Sprint(ss.PruneIntervalSeconds), + FlagSSImportNumWorkers: fmt.Sprint(ss.ImportNumWorkers), + FlagSSReadWriteMetrics: fmt.Sprint(ss.EnableReadWriteMetrics), + FlagSSSnapshotEnable: fmt.Sprint(ss.SnapshotEnable), + FlagEVMSSDirectory: fmt.Sprint(ss.EVMDBDirectory), + FlagEVMSSSeparateDBs: fmt.Sprint(ss.SeparateEVMSubDBs), + FlagEVMSSSplit: fmt.Sprint(ss.EVMSplit), + FlagSCEnable: fmt.Sprint(sc.Enable), + FlagSCDirectory: fmt.Sprint(sc.Directory), + FlagSCAsyncCommitBuffer: fmt.Sprint(sc.MemIAVLConfig.AsyncCommitBuffer), + FlagSCSnapshotKeepRecent: fmt.Sprint(sc.MemIAVLConfig.SnapshotKeepRecent), + FlagSCSnapshotInterval: fmt.Sprint(sc.MemIAVLConfig.SnapshotInterval), + FlagSCSnapshotMinTimeInterval: fmt.Sprint(sc.MemIAVLConfig.SnapshotMinTimeInterval), + FlagSCSnapshotWriterLimit: fmt.Sprint(sc.MemIAVLConfig.SnapshotWriterLimit), + FlagSCSnapshotPrefetchThreshold: fmt.Sprint(sc.MemIAVLConfig.SnapshotPrefetchThreshold), + FlagSCSnapshotWriteRateMBps: fmt.Sprint(sc.MemIAVLConfig.SnapshotWriteRateMBps), + FlagSCHistoricalProofMaxInFlight: fmt.Sprint(sc.HistoricalProofMaxInFlight), + FlagSCHistoricalProofRateLimit: fmt.Sprint(sc.HistoricalProofRateLimit), + FlagSCHistoricalProofBurst: fmt.Sprint(sc.HistoricalProofBurst), + FlagSCWriteMode: fmt.Sprint(sc.WriteMode), + FlagSCWriteModeEnableAuto: fmt.Sprint(sc.WriteModeEnableAuto), + FlagSCHashLoggerEnable: fmt.Sprint(sc.HashLogger.Enable), + FlagSCHashLoggerDirectory: fmt.Sprint(sc.HashLogger.Directory), + FlagSCHashLoggerBlocksToRetain: fmt.Sprint(sc.HashLogger.BlocksToRetain), + FlagSCHashLoggerTargetFileSize: fmt.Sprint(sc.HashLogger.TargetFileSize), + FlagSCHashLoggerMaxDiskSize: fmt.Sprint(sc.HashLogger.MaxDiskSize), + FlagSCFlatKVReadWriteMetrics: fmt.Sprint(sc.FlatKVConfig.EnableReadWriteMetrics), + } +} + +// TestTheDivergencesFromTheReaderAreTheRecordedOnes measures what the doc comments describe. +// +// The two storage sections declare defaults their readers do not produce for a file missing the keys, +// because most of those reads take no account of whether the key was present. Prose describing which keys +// those are cannot fail when it is wrong, and it was: it named four of the six store settings and one +// commitment setting that does not in fact differ, and missed the setting that selects how a node commits. +// +// So the set is measured here rather than described. A key that starts diverging fails this test, and so +// does one that stops: guarding a read means deleting its row, which is what makes the reconciliation +// something a change has to account for rather than something a comment claims. +func TestTheDivergencesFromTheReaderAreTheRecordedOnes(t *testing.T) { + reader := readerValues(t) + for _, mode := range registry.Modes() { + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + recorded, named := theDivergences[mode] + if !named { + t.Fatalf("mode %q has no record here, so a mode was added and this was not revisited", mode) + } + listed := make(map[string]bool, len(recorded)) + for _, key := range recorded { + listed[key] = true + } + + var measured []string + for key, got := range reader { + declared, declares := resolved.Values[key] + if !declares { + t.Errorf("mode %q: %s is read by this package and no section declares it", mode, key) + continue + } + if fmt.Sprint(declared) == got { + if listed[key] { + t.Errorf("mode %q: %s no longer diverges, both sides being %v. Take it off that "+ + "mode's list, so the list stays the set of keys installing this section changes", + mode, key, declared) + } + continue + } + measured = append(measured, key) + if !listed[key] { + t.Errorf("mode %q: %s declares %v and its reader produces %q for a file with no keys, and "+ + "nothing records that. Installing this section changes what such a node runs. %s", + mode, key, declared, got, whyItMatters[key]) + } + if want, stated := whatANodeRunsToday[key]; stated && want != got { + t.Errorf("mode %q: %s is recorded as producing %q and produces %q", mode, key, want, got) + } + } + + sort.Strings(measured) + if len(measured) != len(recorded) { + t.Errorf("mode %q: measured %d divergences and %d are recorded: %v", + mode, len(measured), len(recorded), measured) + } + } +} + +// TestEveryKeyThisPackageDeclaresIsOneItsReadersFill holds the two lists against each other. +// +// The declared keys come from the schemas and the read keys from the map above, so a key on one side only +// is either a setting an operator writes that no reader fills, or one this package reads and nothing +// declares. +func TestEveryKeyThisPackageDeclaresIsOneItsReadersFill(t *testing.T) { + reader := readerValues(t) + for _, section := range []string{StateStoreSectionName, StateCommitSectionName} { + registered, ok := registry.Lookup(section) + if !ok { + t.Fatalf("%s is not registered", section) + } + for _, key := range registered.Keys { + if _, filled := reader[key]; !filled { + t.Errorf("%s declares %s and no field above is paired with it", section, key) + } + } + } +} diff --git a/app/config_register_test.go b/app/config_register_test.go new file mode 100644 index 0000000000..7fe25c5c86 --- /dev/null +++ b/app/config_register_test.go @@ -0,0 +1,273 @@ +package app + +import ( + "reflect" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-db/config" + "github.com/sei-protocol/sei-chain/testutil/configtest" +) + +// manifestKeys returns the keys a section's read-site record names, plus any named here. +// +// The record is this package's own statement of which keys each reader looks up, kept for another purpose +// and held against a golden file. Taking the key set from it means a section's declaration is compared +// against something maintained under a different discipline, rather than against a list written beside it +// by the same hand in the same commit. +func manifestKeys(specs []configtest.KeySpec, also ...string) []string { + out := make([]string, 0, len(specs)+len(also)) + for _, spec := range specs { + out = append(out, spec.Key) + } + out = append(out, also...) + sort.Strings(out) + return out +} + +// requireDeclares holds a section's declared keys against the record of what its reader looks up. +func requireDeclares(t *testing.T, section string, want []string) { + t.Helper() + for _, defect := range registry.Defects() { + if defect.Section == section { + t.Fatalf("%s was refused, so none of its keys is declared: %v", section, defect.Err) + } + } + registered, ok := registry.Lookup(section) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", section) + } + if !reflect.DeepEqual(registered.Keys, want) { + t.Errorf("%s declares\n %v\nand its read-site record names\n %v\nA key on one side only is either "+ + "a setting an operator writes that no reader fills, or one this package reads and nothing "+ + "declares", section, registered.Keys, want) + } +} + +// requireResolves holds a section's resolved values against what its reader's own defaults hold. +// +// Resolving renders every registered section, so a section elsewhere whose defaults cannot state a value +// for a key it declares fails here too. The registry names that section in the error, so the message +// points at the real one rather than at whichever test asked. +// +// Resolving is what to compare against rather than the registered struct, because the resolved map carries +// the key a tag produced and the value that tag's field held. A comparison of struct to struct agrees with +// itself while two tags sit on the wrong fields, since each field still holds the value the test names for +// it. The swap moves the value to the other key, and this notices. +func requireResolves(t *testing.T, mode registry.Mode, section string, want map[string]any) { + t.Helper() + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + for key, expected := range want { + if got := resolved.Values[key]; !reflect.DeepEqual(got, expected) { + t.Errorf("mode %q: %s resolves to %#v (%T), want %#v (%T)", + mode, key, got, got, expected, expected) + } + } +} + +// TestLightInvarianceDeclaresAndResolves covers the one section registered as the type its reader fills. +func TestLightInvarianceDeclaresAndResolves(t *testing.T) { + requireDeclares(t, LightInvarianceSectionName, manifestKeys(lightInvarianceKeys)) + for _, mode := range registry.Modes() { + requireResolves(t, mode, LightInvarianceSectionName, map[string]any{ + flagSupplyEnabled: DefaultLightInvarianceConfig.SupplyEnabled, + }) + } +} + +// TestGenesisDeclaresAndResolves holds the genesis schema against the record and the reader's defaults. +// +// The record names one of the two keys as a row and the other beside it, because that one is read as a type +// assertion rather than a guarded cast and a row would predict the wrong resolution. Both are this +// package's, so both are declared. +func TestGenesisDeclaresAndResolves(t *testing.T) { + requireDeclares(t, GenesisSectionName, manifestKeys(genesisKeys, flagGenesisImportFile)) + for _, mode := range registry.Modes() { + requireResolves(t, mode, GenesisSectionName, map[string]any{ + flagGenesisStreamImport: DefaultGenesisConfig.StreamGenesisImport, + flagGenesisImportFile: DefaultGenesisConfig.GenesisStreamFile, + }) + } +} + +// TestStateStoreDeclaresEveryKeyItsReaderResolves holds the schema against the read-site record. +func TestStateStoreDeclaresEveryKeyItsReaderResolves(t *testing.T) { + requireDeclares(t, StateStoreSectionName, manifestKeys(ssKeys)) +} + +// TestStateStoreResolvesWhatEachKindOfNodeNeeds is the mode-varying half of this section. +// +// Two of these settings mean something different depending on what kind of node asks, and the values are +// written out here rather than taken from the same rules the section reads. An archive node exists to keep +// history, so a retention that pruned it would be the one declaration here that destroys data, and it +// would do so with nothing to alert on, because pruning frees disk rather than filling it. +func TestStateStoreResolvesWhatEachKindOfNodeNeeds(t *testing.T) { + byMode := map[registry.Mode]struct { + enable bool + keepRecent int + }{ + registry.ModeValidator: {enable: false, keepRecent: 100000}, + registry.ModeSeed: {enable: false, keepRecent: 100000}, + registry.ModeFull: {enable: true, keepRecent: 100000}, + registry.ModeArchive: {enable: true, keepRecent: 0}, + } + for _, mode := range registry.Modes() { + want, named := byMode[mode] + if !named { + t.Fatalf("mode %q has no expectation here, so a mode was added and this was not revisited", mode) + } + requireResolves(t, mode, StateStoreSectionName, map[string]any{ + FlagSSEnable: want.enable, + FlagSSKeepRecent: want.keepRecent, + }) + } +} + +// TestStateStoreResolvesItsOtherValuesTheSameForEveryMode covers the ten settings a mode does not change. +func TestStateStoreResolvesItsOtherValuesTheSameForEveryMode(t *testing.T) { + live := config.DefaultStateStoreConfig() + for _, mode := range registry.Modes() { + requireResolves(t, mode, StateStoreSectionName, map[string]any{ + FlagSSDirectory: live.DBDirectory, + FlagSSBackend: live.Backend, + FlagSSAsyncWriterBuffer: live.AsyncWriteBuffer, + FlagSSPruneInterval: live.PruneIntervalSeconds, + FlagSSImportNumWorkers: live.ImportNumWorkers, + FlagSSReadWriteMetrics: live.EnableReadWriteMetrics, + FlagSSSnapshotEnable: live.SnapshotEnable, + FlagEVMSSDirectory: live.EVMDBDirectory, + FlagEVMSSSeparateDBs: live.SeparateEVMSubDBs, + FlagEVMSSSplit: live.EVMSplit, + }) + } +} + +// TestStateCommitDeclaresEveryKeyItsReaderResolves holds the schema against the read-site record. +// +// Twenty keys: the seventeen the record holds as rows, and three it names beside them because each has a +// target of its own. The four keys under this section's flat key-value name that only the Cosmos server's +// reader resolves are not among them, and are not this section's to declare. +func TestStateCommitDeclaresEveryKeyItsReaderResolves(t *testing.T) { + requireDeclares(t, StateCommitSectionName, manifestKeys(scKeys, + FlagSCWriteMode, FlagSCWriteModeEnableAuto, FlagSCHashLoggerTargetFileSize)) +} + +// TestStateCommitResolvesTheModuleDeclaredValues covers the value side of the same registration. +// +// The write mode is a plain string here because the reader parses a written name into its own type, and +// comparing values is what holds it to that: the named type carries the same text and is not the same +// value. +func TestStateCommitResolvesTheModuleDeclaredValues(t *testing.T) { + live := config.DefaultStateCommitConfig() + for _, mode := range registry.Modes() { + requireResolves(t, mode, StateCommitSectionName, map[string]any{ + FlagSCEnable: live.Enable, + FlagSCDirectory: live.Directory, + FlagSCAsyncCommitBuffer: live.MemIAVLConfig.AsyncCommitBuffer, + FlagSCSnapshotKeepRecent: live.MemIAVLConfig.SnapshotKeepRecent, + FlagSCSnapshotInterval: live.MemIAVLConfig.SnapshotInterval, + FlagSCSnapshotMinTimeInterval: live.MemIAVLConfig.SnapshotMinTimeInterval, + FlagSCSnapshotWriterLimit: live.MemIAVLConfig.SnapshotWriterLimit, + FlagSCSnapshotPrefetchThreshold: live.MemIAVLConfig.SnapshotPrefetchThreshold, + FlagSCSnapshotWriteRateMBps: live.MemIAVLConfig.SnapshotWriteRateMBps, + FlagSCHistoricalProofMaxInFlight: live.HistoricalProofMaxInFlight, + FlagSCHistoricalProofRateLimit: live.HistoricalProofRateLimit, + FlagSCHistoricalProofBurst: live.HistoricalProofBurst, + FlagSCWriteMode: string(live.WriteMode), + FlagSCWriteModeEnableAuto: live.WriteModeEnableAuto, + FlagSCHashLoggerEnable: live.HashLogger.Enable, + FlagSCHashLoggerDirectory: live.HashLogger.Directory, + FlagSCHashLoggerBlocksToRetain: live.HashLogger.BlocksToRetain, + FlagSCHashLoggerTargetFileSize: live.HashLogger.TargetFileSize, + FlagSCHashLoggerMaxDiskSize: live.HashLogger.MaxDiskSize, + FlagSCFlatKVReadWriteMetrics: live.FlatKVConfig.EnableReadWriteMetrics, + }) + } +} + +// TestStateCommitWriteModeDefaultIsOneTheReaderAccepts covers the one declared value that is parsed text. +// +// Every other declared value is used as it stands. This one is a name the reader turns into a mode, so a +// default nothing parses would put a value in a generated file that stops the node it was generated for. +func TestStateCommitWriteModeDefaultIsOneTheReaderAccepts(t *testing.T) { + for _, mode := range registry.Modes() { + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + declared, ok := resolved.Values[FlagSCWriteMode].(string) + if !ok { + t.Fatalf("mode %q: %s resolves to %T, and the reader parses text", + mode, FlagSCWriteMode, resolved.Values[FlagSCWriteMode]) + } + if _, err := config.ParseSCWriteMode(declared); err != nil { + t.Errorf("mode %q: %s resolves to %q, which this binary's own reader refuses: %v", + mode, FlagSCWriteMode, declared, err) + } + } +} + +// TestTheSectionsThisPackageRegistersAreUsable covers what the registry refuses. +// +// Scoped to the four names this file registers. The whole-registry sweep belongs where every section is +// linked, because a refusal that depends on what else registered is not this package's to answer for. +func TestTheSectionsThisPackageRegistersAreUsable(t *testing.T) { + mine := map[string]bool{ + LightInvarianceSectionName: true, + GenesisSectionName: true, + StateStoreSectionName: true, + StateCommitSectionName: true, + } + for _, defect := range registry.Defects() { + if mine[defect.Section] { + t.Errorf("%s was refused, so none of its keys is declared: %v", defect.Section, defect.Err) + } + } +} + +// TestTheArchiveRetentionDepartsFromWhatTheCommandWrites measures the one deliberate departure. +// +// A declared value is what the seid init command writes for a kind of node. This section departs from that +// in exactly one place: the retention an archive node keeps. The mode rules set it to keep everything, and +// the command does not write that, because the type it renders declares a state store field of its own and +// fills it from the mode-blind default, so the rule is applied and then thrown away. +// +// PLT-955 records the defect and the decision to correct it in the versioned declaration rather than at the +// point that loses it. So the departure is intended, and it is held here for two reasons. It fails if the +// command starts writing the rule, which is the day this departure should be deleted. And it fails if this +// section stops departing, which would put a retention on the one kind of node whose purpose is keeping +// what it would prune. +func TestTheArchiveRetentionDepartsFromWhatTheCommandWrites(t *testing.T) { + live := config.DefaultStateStoreConfig() + + // What the command renders for an archive node: the mode rules are applied to the server + // configuration, and then the type it renders fills its own state store field from the mode-blind + // default, which is what reaches the file. + written := live.KeepRecent + if written == 0 { + t.Fatalf("the mode-blind default retention is already zero, so this departure measures nothing " + + "and the comparison below holds for any declaration") + } + + resolved, err := registry.Resolve(registry.ModeArchive, registry.Sources{}) + if err != nil { + t.Fatalf("%v", err) + } + declared := resolved.Values[FlagSSKeepRecent] + + if declared == written { + t.Errorf("%s resolves to %v for an archive node, which is what the command writes. Either the "+ + "command now carries the mode rule, in which case this departure and its note should go, or "+ + "this section stopped departing and an archive node is declared to prune the history it "+ + "exists to keep", FlagSSKeepRecent, declared) + } + if declared != 0 { + t.Errorf("%s resolves to %v for an archive node, want zero. The mode rule keeps every version, "+ + "and departing from the command is only defensible while this states that rule", + FlagSSKeepRecent, declared) + } +} diff --git a/app/params/config.go b/app/params/config.go index 0fb57adb01..0b5fd2be40 100644 --- a/app/params/config.go +++ b/app/params/config.go @@ -1,6 +1,7 @@ package params import ( + "github.com/sei-protocol/sei-chain/config/registry" evmrpcconfig "github.com/sei-protocol/sei-chain/evmrpc/config" srvconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" "github.com/sei-protocol/sei-chain/sei-cosmos/types/address" @@ -94,8 +95,11 @@ const ( ) // IsFullnodeType returns true if the node is a fullnode-like node (full or archive) +// +// The rule itself lives in the configuration registry, because a section's own package needs the same +// fact and cannot import this one. func (m NodeMode) IsFullnodeType() bool { - return m == NodeModeFull || m == NodeModeArchive + return registry.IsFullnodeMode(registry.Mode(m)) } // setValidatorTypeTendermintConfig sets common Tendermint config for validator-like nodes diff --git a/cmd/seid/cmd/app_config.go b/cmd/seid/cmd/app_config.go index b984c12c2a..7a48229074 100644 --- a/cmd/seid/cmd/app_config.go +++ b/cmd/seid/cmd/app_config.go @@ -7,6 +7,7 @@ import ( gigaconfig "github.com/sei-protocol/sei-chain/giga/executor/config" srvconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" seidbconfig "github.com/sei-protocol/sei-chain/sei-db/config" + "github.com/sei-protocol/sei-chain/sei-wasmd/x/wasm" "github.com/sei-protocol/sei-chain/x/evm/blocktest" "github.com/sei-protocol/sei-chain/x/evm/querier" "github.com/sei-protocol/sei-chain/x/evm/replay" @@ -44,7 +45,7 @@ func NewCustomAppConfig(baseConfig *srvconfig.Config, evmConfig evmrpcconfig.Con StateStore: seidbconfig.DefaultStateStoreConfig(), ReceiptStore: seidbconfig.DefaultReceiptStoreConfig(), WASM: WASMConfig{ - QueryGasLimit: 300000, + QueryGasLimit: wasm.GeneratedQueryGasLimit, LruSize: 1, }, EVM: evmConfig, diff --git a/cmd/seid/cmd/boot_install_test.go b/cmd/seid/cmd/boot_install_test.go new file mode 100644 index 0000000000..8e12d33f73 --- /dev/null +++ b/cmd/seid/cmd/boot_install_test.go @@ -0,0 +1,267 @@ +package cmd + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "go.opentelemetry.io/otel/sdk/trace" + + "github.com/sei-protocol/sei-chain/cmd/seid/cmd/configmanager" + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + "github.com/sei-protocol/sei-chain/testutil/configtest" +) + +// Keys these tests measure through, and why each one. +// +// The first two are declared keys that nothing else in a booting node answers: no start flag carries them +// and the generated app.toml does not name them, so a value read back for one came from this install and +// from nowhere else. Of a hundred and fifty declared keys only eleven are like that, and the rest are +// reachable by a flag of the same name, whose registration default answers before the lookup comes back +// empty. Measuring through one of those would be reading the flag's default and calling it an install. +// +// The third is the opposite case on purpose: a key a start flag does carry, so it is the one that can show +// the flag channel reaching a declared key at all. +const ( + bootProbeKey = "evm.max_tx_pool_txs" + bootUntouchedKey = "state-commit.sc-snapshot-writer-limit" + bootFlagKey = "state-sync.snapshot-keep-recent" +) + +// bootWith runs a real boot against a sei.toml and returns the source a node would read. +// +// Flags are set through the command rather than handed to the install, because it is the flag being marked +// changed that the snapshot reads. A value poked in directly would hold even if the boot never looked at +// the command line. +func bootWith(t *testing.T, body string, typed map[string]string) *server.Context { + t.Helper() + home := configtest.NewHome(t) + if err := os.MkdirAll(filepath.Join(home.Root, "config"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + if body != "" { + path := filepath.Join(home.Root, "config", "sei.toml") + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + } + + cmd := server.StartCmd(nil, home.Root, []trace.TracerProviderOption{}) + if err := cmd.Flags().Set("home", home.Root); err != nil { + t.Fatalf("set --home: %v", err) + } + for name, value := range typed { + if err := cmd.Flags().Set(name, value); err != nil { + t.Fatalf("set --%s=%s: %v", name, value, err) + } + } + ctx, err := runManager(t, configmanager.SeiConfigManager{}, cmd) + if err != nil { + t.Fatalf("the boot was refused: %v", err) + } + return ctx +} + +// seiTomlWriting returns a file body that writes one key, wherever that key belongs. +// +// A key with no section goes above every table. Once a table heading is open every bare key after it +// belongs to that table, so a node-wide setting written after one would be read under the wrong name. +func seiTomlWriting(key, value string) string { + const header = "schema_version = 1\nnode_mode = \"validator\"\n" + if i := indexOf(key, '.'); i >= 0 { + return header + "\n[" + key[:i] + "]\n" + key[i+1:] + " = " + value + "\n" + } + return header + key + " = " + value + "\n" +} + +func indexOf(s string, c byte) int { + for i := 0; i < len(s); i++ { + if s[i] == c { + return i + } + } + return -1 +} + +// TestEachChannelWinsOverTheOneBelowIt drives the declared order through a real boot. +// +// Every channel that can carry a value has to reach the resolution. A channel that is not wired does not +// fail, it stops applying: a value an operator supplied through it loses to a lower layer and nothing +// reports it. So each one is supplied a value and the declared order has to hold. +func TestEachChannelWinsOverTheOneBelowIt(t *testing.T) { + t.Run("nothing written leaves the key as it was", func(t *testing.T) { + configtest.Isolate(t) + ctx := bootWith(t, "schema_version = 1\nnode_mode = \"validator\"\n", nil) + if got := ctx.Viper.Get(bootProbeKey); got != nil { + t.Errorf("%s reads %#v with nothing written. A file that supplies no value installs nothing, "+ + "so this key should read as it did before the manager ran", bootProbeKey, got) + } + }) + + t.Run("the file beats the default", func(t *testing.T) { + configtest.Isolate(t) + ctx := bootWith(t, seiTomlWriting(bootProbeKey, "111"), nil) + if got := ctx.Viper.Get(bootProbeKey); !sameSetting(got, int64(111)) { + t.Errorf("%s reads %#v with 111 written, want 111. A file channel that is not passed to the "+ + "resolution leaves the operator's value losing to the default", bootProbeKey, got) + } + }) + + t.Run("the environment beats the file", func(t *testing.T) { + configtest.Isolate(t) + t.Setenv(registry.EnvName(bootProbeKey), "222") + ctx := bootWith(t, seiTomlWriting(bootProbeKey, "111"), nil) + if got := ctx.Viper.Get(bootProbeKey); !sameSetting(got, "222") { + t.Errorf("%s reads %#v with 111 in the file and 222 in the environment, want 222", bootProbeKey, got) + } + }) + + t.Run("a typed flag beats both", func(t *testing.T) { + configtest.Isolate(t) + t.Setenv(registry.EnvName(bootFlagKey), "222") + ctx := bootWith(t, seiTomlWriting(bootFlagKey, "111"), map[string]string{bootFlagKey: "333"}) + if got := ctx.Viper.Get(bootFlagKey); !sameSetting(got, "333") { + t.Errorf("%s reads %#v with 111 in the file, 222 in the environment and --%s=333 typed, "+ + "want 333. An operator who types a flag to override a file has to win, and a flag "+ + "whose name never reaches the resolution loses to both", bootFlagKey, got, bootFlagKey) + } + }) +} + +// TestOnlyWhatASourceSuppliedIsInstalled is the property that makes this safe to enable. +// +// A resolution answers for every declared key. Installing all of it would write a default over whatever a +// node's app.toml holds for every key its sei.toml does not mention, so moving one setting would replace a +// hundred and fifty. This installs only what a source supplied, so a key reaches a node exactly when +// somebody asked for it. +// +// Measured as an absence rather than against a value read back from a second boot. A baseline taken through +// this same install would carry whatever the install wrote, so an install that wrote a default over every +// key would write the same one twice and the two runs would agree. The assertion is that the key is not +// there at all, which no install can satisfy by being wrong the same way twice. +func TestOnlyWhatASourceSuppliedIsInstalled(t *testing.T) { + configtest.Isolate(t) + + // The declared value is read out first, because a key whose declaration answers nothing would pass + // this whether the install was contained or not. + const untouched = bootUntouchedKey + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if resolved.Values[untouched] == nil { + t.Fatalf("%s declares no value, so an install that wrote every declared default would leave it "+ + "absent too and this would measure nothing", untouched) + } + + ctx := bootWith(t, seiTomlWriting(bootProbeKey, "111"), nil) + + if got := ctx.Viper.Get(untouched); got != nil { + t.Errorf("%s reads %#v after a file that never mentions it, and %s declares %#v. Installing a "+ + "declared default over a key nobody wrote replaces an operator's configuration rather than "+ + "moving one setting of it", untouched, got, untouched, resolved.Values[untouched]) + } + if got := ctx.Viper.Get(bootProbeKey); !sameSetting(got, int64(111)) { + t.Errorf("%s reads %#v, so nothing was installed at all and the check above holds for an install "+ + "that does nothing", bootProbeKey, got) + } +} + +// TestAppTomlDoesNotReachTheFlagChannel is the guard on where the flag snapshot is taken. +// +// The handler this manager re-enters copies configuration values into flags, so that a file can supply a +// flag's default: for every flag whose name its source knows a value for, it calls Set, and Set marks the +// flag changed. After that has run, a flag an operator typed and a key their app.toml holds cannot be told +// apart. +// +// A flag channel built from that state puts app.toml at the top of the order, above sei.toml, which is a +// worse inversion than the one the channel exists to prevent. Taking the snapshot at the entry to Apply is +// what keeps the two apart, and there is no later point where the truth survives. +func TestAppTomlDoesNotReachTheFlagChannel(t *testing.T) { + const key = "state-sync.snapshot-keep-recent" + if _, declared := declaredKey(key); !declared { + t.Skipf("%s is not declared, so this cannot happen through it", key) + } + configtest.Isolate(t) + + home := configtest.NewHome(t) + // app.toml holds one value and sei.toml another, and the operator typed no flag at all. + home.WriteAppTOML(t, []byte("[state-sync]\nsnapshot-keep-recent = 77\n")) + if err := os.MkdirAll(filepath.Join(home.Root, "config"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + body := "schema_version = 1\nnode_mode = \"validator\"\n\n[state-sync]\nsnapshot-keep-recent = 111\n" + if err := os.WriteFile(filepath.Join(home.Root, "config", "sei.toml"), []byte(body), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + + cmd := server.StartCmd(nil, home.Root, []trace.TracerProviderOption{}) + if err := cmd.Flags().Set("home", home.Root); err != nil { + t.Fatalf("set --home: %v", err) + } + ctx, err := runManager(t, configmanager.SeiConfigManager{}, cmd) + if err != nil { + t.Fatalf("the boot was refused: %v", err) + } + + if got := ctx.Viper.Get(key); !sameSetting(got, int64(111)) { + t.Errorf("%s reads %#v with 77 in app.toml, 111 in sei.toml and no flag typed, want 111.\n\n"+ + "A value of 77 means app.toml arrived through the flag channel, because the handler marked "+ + "the flag changed on its behalf. The snapshot has to be taken before the handler runs", key, got) + } +} + +// TestAFileThisBinaryCannotUseLeavesTheNodeAsItWas is the promise that makes the switch safe. +// +// Selecting this manager is a switch rather than a configuration change, so a file it cannot use installs +// nothing and the node reads what it always read. Refusing instead would turn a mistyped line in a +// hand-editable file into an outage on the next restart. +// +// Every case writes a value for a declared key, so a file that was wrongly accepted would install one and +// the assertion would see it. A case supplying nothing would read as unusable whether it was refused or +// accepted, which measures the absence of a value rather than the refusal. +func TestAFileThisBinaryCannotUseLeavesTheNodeAsItWas(t *testing.T) { + supplies := "\n[evm]\nmax_tx_pool_txs = 111\n" + for name, body := range map[string]string{ + "no file at all": "", + "a mode nothing knows": "schema_version = 1\nnode_mode = \"sentry\"\n" + supplies, + "no mode at all": "schema_version = 1\n" + supplies, + "not parseable": "schema_version = 1\nnode_mode = \"validator\"\n[evm\n" + supplies, + } { + t.Run(name, func(t *testing.T) { + configtest.Isolate(t) + ctx := bootWith(t, body, nil) + if got := ctx.Viper.Get(bootProbeKey); got != nil { + t.Errorf("%s reads %#v, so a value was installed from a file this binary cannot use. "+ + "A node whose file names a mode this binary does not know would run one mode's "+ + "answers while being configured as another", bootProbeKey, got) + } + }) + } +} + +// sameSetting compares two resolved values without caring which shape carried them. +// +// A value reaches a source as its own Go type from a default, as whatever the file format decoded to from +// a file, and as one string from a variable. A comparison that insisted on the type would be asserting +// which channel answered rather than what the node reads. +func sameSetting(a, b any) bool { + if a == nil || b == nil { + return a == nil && b == nil + } + return fmt.Sprint(a) == fmt.Sprint(b) +} + +// declaredKey reports whether the registry declares a key. +func declaredKey(key string) (string, bool) { + for _, section := range registry.Sections() { + for _, k := range section.Keys { + if k == key { + return section.Name, true + } + } + } + return "", false +} diff --git a/cmd/seid/cmd/configmanager/check.go b/cmd/seid/cmd/configmanager/check.go new file mode 100644 index 0000000000..1168a50a3c --- /dev/null +++ b/cmd/seid/cmd/configmanager/check.go @@ -0,0 +1,141 @@ +package configmanager + +import ( + "fmt" + "io" + "os" + "sort" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + + "github.com/sei-protocol/sei-chain/config/registry" + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" +) + +// CheckCmd answers, without starting a node, whether this binary can use a sei.toml. +// +// A boot may not refuse a file. A node that stopped because one line was mistyped is worse than a node +// running the value it ran yesterday, so every failure at boot is a report and the node keeps going. That +// makes the report the only signal, and a fleet rolling a configuration change forward reads it after the +// change is already on every node. +// +// The same questions have exact answers before then. The file, the binary and the environment are all the +// input, so a refusal is deterministic: the same file against the same binary gives the same answer here as +// it will at boot. This asks them where an answer costs a failed check rather than a restart. +func CheckCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "check", + Short: "Report whether this binary can use the node's sei.toml", + Long: "Resolves the node's sei.toml the way a boot resolves it and reports every value this " + + "binary would refuse, without starting anything. Exits non-zero if there is one.\n\n" + + "A boot cannot refuse a file, so it applies what it can and reports the rest. Running this " + + "first is how a mistyped value costs a failed check rather than a restart.", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + problems, found, err := checkSeiToml(cmd) + if err != nil { + return err + } + if !found { + report(cmd.OutOrStdout(), "this node has no sei.toml, so every key reads as it always "+ + "has and there is nothing here to be wrong") + return nil + } + out := cmd.OutOrStdout() + for _, line := range problems { + report(out, line) + } + if len(problems) > 0 { + return fmt.Errorf("%d problem(s); a boot would apply what it could and report the rest", + len(problems)) + } + report(out, "every value this file supplies is one this binary can use") + return nil + }, + } + return cmd +} + +// report writes one line of the answer. +// +// A failed write is dropped rather than returned. Where this runs the answer is the exit status, and a +// caller that cannot read the report still gets that. +func report(out io.Writer, line string) { _, _ = fmt.Fprintln(out, line) } + +// checkSeiToml resolves the node's file and returns what a boot would refuse, in the order it would. +// +// The absence of a file is not a problem to report. A node without one reads exactly as it always has, so +// there is nothing here that could be wrong. +func checkSeiToml(cmd *cobra.Command) (problems []string, found bool, err error) { + home, err := resolveHomeDir(cmd) + if err != nil { + return nil, false, fmt.Errorf("resolve the home directory: %w", err) + } + file, ok := readSeiTomlAt(home) + if !ok { + return nil, false, nil + } + mode, err := file.Mode() + if err != nil { + return []string{fmt.Sprintf("sei.toml records no usable node mode: %v", err)}, true, nil + } + written, err := file.Values() + if err != nil { + return []string{fmt.Sprintf("sei.toml cannot be read: %v", err)}, true, nil + } + + resolved, err := registry.Resolve(registry.Mode(mode), registry.Sources{ + File: written, + LookupEnv: os.LookupEnv, + Flags: flagValues(TypedFlags(cmd)), + }) + if err != nil { + return []string{fmt.Sprintf("this node's configuration cannot be resolved: %v", err)}, true, nil + } + + for _, key := range resolved.Unknown { + problems = append(problems, fmt.Sprintf("%s: sei.toml writes this and no section declares it, "+ + "so it has no effect", key)) + } + problems = append(problems, whatADecodeWouldRefuse(resolved)...) + return problems, true, nil +} + +// whatADecodeWouldRefuse rehearses each decoded section the way the boot's delivery does. +// +// Rehearsed against a fresh configuration rather than a running node's, because there is no node here. That +// is a weaker target than the delivery uses, and the difference is the point: a value this accepts may still +// be refused at boot if the field it lands on holds something this cannot see. It is why this reports what +// it can answer and the boot still reports what it finds. +func whatADecodeWouldRefuse(resolved registry.Resolved) []string { + bySection := registry.SuppliedByDecodedSection(resolved) + var problems []string + for _, name := range sortedSectionNames(bySection) { + values := bySection[name] + base := tmcfg.DefaultConfig() + + if bad := refuseWhatDecodesToSomethingElse(base, values); len(bad) > 0 { + problems = append(problems, fmt.Sprintf("[%s]: %s is a length of time written as a plain "+ + "number, which reads as nanoseconds", name, strings.Join(bad, "; "))) + continue + } + + source := viper.New() + for key, value := range values { + source.Set(key, value) + } + candidate, err := copyNodeConfig(base) + if err != nil { + problems = append(problems, fmt.Sprintf("[%s]: cannot be rehearsed: %v", name, err)) + continue + } + if err := source.Unmarshal(candidate); err != nil { + problems = append(problems, fmt.Sprintf("[%s]: %v, so none of this section would apply "+ + "(keys: %s)", name, err, strings.Join(sortedKeys(values), ","))) + } + } + sort.Strings(problems) + return problems +} diff --git a/cmd/seid/cmd/configmanager/check_test.go b/cmd/seid/cmd/configmanager/check_test.go new file mode 100644 index 0000000000..ffde32ee2a --- /dev/null +++ b/cmd/seid/cmd/configmanager/check_test.go @@ -0,0 +1,169 @@ +package configmanager + +import ( + "bytes" + "context" + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/sei-protocol/sei-chain/sei-cosmos/client/flags" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + serverconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" + "github.com/sei-protocol/sei-chain/testutil/configtest" + "go.opentelemetry.io/otel/sdk/trace" +) + +// runCheck runs the command against a home holding the given sei.toml, and returns what it printed and +// whether it failed. +func runCheck(t *testing.T, body string) (string, error) { + t.Helper() + home := t.TempDir() + if err := os.MkdirAll(filepath.Join(home, "config"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + if body != "" { + if err := os.WriteFile(filepath.Join(home, "config", seiTomlName), []byte(body), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + } + + cmd := CheckCmd() + cmd.Flags().String(flags.FlagHome, home, "") + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + err := cmd.Execute() + return out.String(), err +} + +// TestTheCheckFailsOnWhatABootWouldRefuse is the point of the command. +// +// A boot may not refuse a file, so every value it cannot use is a report on a node that has already +// restarted. The same questions have exact answers beforehand, and this is where an answer costs a failed +// check instead. +func TestTheCheckFailsOnWhatABootWouldRefuse(t *testing.T) { + const header = "schema_version = 1\nnode_mode = \"validator\"\n" + + t.Run("a file this binary can use passes", func(t *testing.T) { + out, err := runCheck(t, header+"\n[mempool]\nttl-duration = \"60s\"\nsize = 4321\n") + if err != nil { + t.Errorf("a usable file was refused: %v\n%s", err, out) + } + }) + + t.Run("no file at all is not a problem", func(t *testing.T) { + out, err := runCheck(t, "") + if err != nil { + t.Errorf("a node with no sei.toml was refused: %v", err) + } + if !strings.Contains(out, "no sei.toml") { + t.Errorf("the report does not say the file is absent, so a missing file reads as a clean "+ + "one:\n%s", out) + } + }) + + t.Run("a length of time written as a plain number fails", func(t *testing.T) { + out, err := runCheck(t, header+"\n[mempool]\nttl-duration = 60\n") + if err == nil { + t.Errorf("a plain number in a length of time passed:\n%s", out) + } + if !strings.Contains(out, "nanoseconds") { + t.Errorf("the report does not say what is wrong with it:\n%s", out) + } + }) + + t.Run("a value the decode refuses fails", func(t *testing.T) { + out, err := runCheck(t, header+"\n[instrumentation]\nmax-open-connections = \"not a number\"\n") + if err == nil { + t.Errorf("a value no decode accepts passed:\n%s", out) + } + }) + + t.Run("a key no section declares is reported", func(t *testing.T) { + out, err := runCheck(t, header+"\n[mempool]\nnot-a-key = 1\n") + if err == nil { + t.Errorf("a key nothing declares passed:\n%s", out) + } + if !strings.Contains(out, "no effect") { + t.Errorf("the report does not say the key has no effect:\n%s", out) + } + }) + + t.Run("a mode this binary does not know fails", func(t *testing.T) { + out, err := runCheck(t, "schema_version = 1\nnode_mode = \"sentry\"\n") + if err == nil { + t.Errorf("a mode nothing declares passed:\n%s", out) + } + }) +} + +// TestADisagreementAboutTheKindOfNodeIsFound covers a fact two files state under different names. +// +// sei.toml records the kind of node at its top and every value resolved through this manager is the answer +// for that kind. The node's own configuration file states it again in a key of its own, and that one is what +// the node runs as. Nothing here declares the second on purpose, so the two can be written to disagree, and +// a node that resolves a validator's values while running as a full node reads correctly in every report +// about it. +func TestADisagreementAboutTheKindOfNodeIsFound(t *testing.T) { + for _, tc := range []struct { + recorded, running string + disagree bool + why string + }{ + {"validator", "validator", false, "the same kind is not a disagreement"}, + {"validator", "full", true, "a validator that runs as a query-serving node serves queries"}, + {"full", "validator", true, "a node resolved for queries that runs as a validator holds a key"}, + {"seed", "full", true, "a seed exists to serve peers and would be serving queries"}, + {"archive", "full", false, "the kind that keeps every version has no name of its own in that " + + "file, so the command that writes it writes this one"}, + {"archive", "validator", true, "an archive that runs as a validator is a disagreement"}, + } { + if got := modesDisagree(tc.recorded, tc.running); got != tc.disagree { + t.Errorf("sei.toml %q against a node running %q reports disagree=%v, want %v: %s", + tc.recorded, tc.running, got, tc.disagree, tc.why) + } + } +} + +// TestApplyReportsADisagreementAboutTheKindOfNode drives the real Apply, so the wiring is what is asserted. +// +// The test beside this one holds the decision, which a comparison never reached would still pass. This one +// gives the two files different kinds of node and looks for the report, so removing the call fails here. +func TestApplyReportsADisagreementAboutTheKindOfNode(t *testing.T) { + configtest.Isolate(t) + root := writeMinimalHome(t, "mode = \"full\"\n", "") + if err := os.WriteFile(filepath.Join(root, "config", seiTomlName), + []byte("schema_version = 1\nnode_mode = \"validator\"\n"), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + + cmd := server.StartCmd(nil, "/foobar", []trace.TracerProviderOption{}) + if err := cmd.Flags().Set(flags.FlagHome, root); err != nil { + t.Fatalf("set --home: %v", err) + } + serverCtx := &server.Context{} + cmd.SetContext(context.WithValue(context.Background(), server.ServerContextKey, serverCtx)) + + capture := &capturingHandler{} + mgr := SeiConfigManager{logger: slog.New(capture)} + if err := mgr.Apply(cmd, serverconfig.DefaultConfigTemplate, serverconfig.DefaultConfig()); err != nil { + t.Fatalf("the fixture is meant to boot, so this is the fixture: %v", err) + } + + var found bool + for _, r := range capture.records { + if strings.Contains(r.Message, "one kind of node") { + found = true + } + } + if !found { + t.Error("sei.toml said validator, the node's own file said full, and nothing reported it. A " + + "node resolving a validator's values while running as a query-serving node reads correctly " + + "in every other report about it") + } +} diff --git a/cmd/seid/cmd/configmanager/configmanager.go b/cmd/seid/cmd/configmanager/configmanager.go index c6e6eec499..cd1b62376b 100644 --- a/cmd/seid/cmd/configmanager/configmanager.go +++ b/cmd/seid/cmd/configmanager/configmanager.go @@ -21,6 +21,13 @@ import ( var logger = seilog.NewLogger("cmd", "seid", "configmanager") +// loggerName is the name the logger above is registered under, and ownReportingFloor is the level its +// reports are held at. +const ( + loggerName = "cmd/seid/configmanager" + ownReportingFloor = slog.LevelInfo +) + // EnvVar gates which configuration manager seid uses. const EnvVar = "SEI_CONFIG_MANAGER" @@ -55,6 +62,27 @@ type SeiConfigManager struct { } // log returns the logger to report through, and never returns nil. +// keepOwnReportingVisible holds this package's own logger at a level its reports survive. +// +// Called after anything that may have set a level, and it is called more than once for that reason: the +// handler sets one, and a level this manager resolves sets another. Both set every logger in the process, so +// a floor applied before either is simply overwritten. +// +// The handler this manager re-enters sets one level across every logger in the process, from a key an +// operator writes, and a fleet that runs its nodes quiet sets it above the level these reports use. Every +// outcome here is a report: what was applied, what moved, what was refused and what had no effect. Silenced, +// the manager becomes a component that changes what a node runs and says nothing about it, and the file +// stops being something an operator can reason about from the node itself. +// +// So this one logger keeps a floor, and only this one. Raising the level for the rest of the process is +// still the operator's to choose. +func keepOwnReportingVisible() { + if seilog.SetLevel(loggerName, ownReportingFloor) == 0 { + // Nothing to hold, which happens when a caller supplied a logger of its own. + return + } +} + func (m SeiConfigManager) log() *slog.Logger { if m.logger != nil { return m.logger @@ -80,10 +108,23 @@ func (m SeiConfigManager) log() *slog.Logger { // handler and return nil, turning a boot the legacy path aborts into a successful one. // TestApplyPropagatesALegacyHandlerPanic fails on that combination. func (m SeiConfigManager) Apply(cmd *cobra.Command, customAppConfigTemplate string, customAppConfig any) error { + // Before the handler, because the handler copies configuration values into flags and marks them + // changed. Afterwards there is no way to tell a flag an operator typed from a key their app.toml + // holds, and treating the second as the first would put app.toml above sei.toml. + typed := TypedFlags(cmd) + out := validateAdvisory(cmd) err := server.InterceptConfigsPreRunHandler(cmd, customAppConfigTemplate, customAppConfig) + keepOwnReportingVisible() reportAdvisory(m.log(), out) - return err + if err != nil { + return err + } + + // After the handler, because the source it builds is the one the resolved values go into and it does + // not exist before. Nothing this does can refuse the boot. + installResolved(cmd, typed, m.log()) + return nil } // reportAdvisory logs an advisory outcome, containing a panic from the logging itself. diff --git a/cmd/seid/cmd/configmanager/install.go b/cmd/seid/cmd/configmanager/install.go new file mode 100644 index 0000000000..8639a8a270 --- /dev/null +++ b/cmd/seid/cmd/configmanager/install.go @@ -0,0 +1,315 @@ +package configmanager + +import ( + "context" + "log/slog" + "os" + "path/filepath" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" + + "github.com/sei-protocol/sei-chain/config/appopts" + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/config/seitoml" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + + // The sections whose keys belong to the upstream server, which nothing else imports. A section + // reaches the registry through its owning package's initialisation, so a section nothing imports is + // absent from what this installs and absent silently, since an undeclared key is left to whatever + // answered it before. + _ "github.com/sei-protocol/sei-chain/config/cosmosbase" + + // The sections whose keys belong to the node's own configuration file, which nothing else imports + // either. These are the sections the delivery beside this one decodes rather than installs. + _ "github.com/sei-protocol/sei-chain/config/tendermintbase" +) + +// seiTomlName is the file this manager reads. +const seiTomlName = "sei.toml" + +// installResolved puts the values sei.toml supplies into the source the boot has just built. +// +// Nothing here can stop a node starting. A node with no sei.toml, an unreadable one, or one recording a +// mode this binary does not know installs nothing and reads exactly as it always has, so selecting this +// manager is a switch rather than a configuration change. Refusing instead would turn a mistyped line in +// a hand-editable file into an outage on the next restart. +func installResolved(cmd *cobra.Command, typed map[string]string, log *slog.Logger) { + ctx := server.GetServerContextFromCmd(cmd) + if ctx == nil || ctx.Viper == nil { + log.Warn("no configuration source to install into; every key reads as it always has") + return + } + + file, ok := readSeiToml(cmd, log) + if !ok { + return + } + mode, ok := recordedMode(file, log) + if !ok { + return + } + written, err := file.Values() + if err != nil { + log.Warn("cannot read the values sei.toml writes; every key reads as it always has", "err", err) + return + } + + // Every channel an operator can use. Omitting one installs a lower layer over the top of what they + // chose, which is a value silently ignored rather than a value overridden. The flag channel matters + // most: an installed value sits above a bound flag, so a declared key a flag also delivers would + // resolve without ever seeing the command line and then bury it. + resolved, err := registry.Resolve(registry.Mode(mode), registry.Sources{ + File: written, + LookupEnv: os.LookupEnv, + Flags: flagValues(typed), + }) + if err != nil { + log.Warn("cannot resolve this node's configuration; every key reads as it always has", + "mode", mode, "err", err) + return + } + // First, because every report below is a log line and a refusal is reported at a level an operator + // may have raised the threshold above. Doing this after would mean the one setting somebody changes + // in order to see a refusal is the setting a refusal suppresses. + applyResolvedLogLevel(resolved, typed, log) + + // After the level, so a file that raises it can report its own mistakes. A key nothing declares is the + // most common thing an operator gets wrong and the only signal they have for it. + reportWhatTheFileDidNotReach(resolved, log) + + reportWhatTheFileSaysTheNodeIs(ctx, mode, log) + + // The sections a reader looks up key by key, and the sections a reader decodes whole. Two deliveries, + // because putting a value into the source is no delivery at all for the second kind: their file is + // read into a struct before this runs and nothing consults the source for them afterwards. + deliverDecodedSections(ctx, resolved, log) + + supplied := onlyWhatALookupSourceSupplied(resolved) + if len(supplied.Values) == 0 { + log.Info("sei.toml supplies no declared value; every key reads as it always has", "mode", mode) + return + } + report, err := appopts.Install(ctx.Viper, supplied) + if err != nil { + log.Warn("cannot install the values sei.toml supplies; every key reads as it always has", + "err", err) + return + } + log.Info("configuration installed", "mode", mode, + "installed", strings.Join(report.Installed, ",")) +} + +// onlyWhatALookupSourceSupplied narrows a resolution to the keys something other than the defaults +// answered, for the sections whose readers look a key up rather than decoding one. +// +// This is the whole difference between moving a setting and replacing a file. A resolution answers for +// every declared key, so installing all of it would write a default over whatever an operator's app.toml +// holds for every key their sei.toml does not mention: a hundred and fifty settings replaced because they +// moved one. Installing only what a source supplied means a key reaches the node exactly when somebody +// asked for it, and every other key reads as it always has. +// +// It also means a declared default never reaches a running node, which is what lets a default state what +// the provisioning command writes rather than having to state what each node already runs. +func onlyWhatALookupSourceSupplied(resolved registry.Resolved) registry.Resolved { + decoded := registry.DecodedSections() + owning := map[string]bool{} + for _, section := range registry.Sections() { + if _, ok := decoded[section.Name]; !ok { + continue + } + for _, key := range section.Keys { + owning[key] = true + } + } + + out := registry.Resolved{Values: make(map[string]any, len(resolved.Overrides))} + for _, key := range resolved.Overrides { + // A key both deliveries carried would be installed into the source as well as decoded, and the + // install refuses a key its own contract does not cover, which would take the whole install down + // and with it every key of every other section. + if owning[key] { + continue + } + out.Values[key] = resolved.Values[key] + } + return out +} + +// reportWhatTheFileDidNotReach says what an operator asked for that had no effect. +// +// Two things, and neither is visible anywhere else. A key no section declares is one this file cannot +// deliver, so it reads as a setting and changes nothing. And a variable set for a key no environment +// variable can carry is ignored on purpose, with the reason recorded where the key is declared. +// +// Reported once each rather than per key, because a node resolves over a hundred declared keys and a line +// each would bury the two or three that matter in the noise it creates. +func reportWhatTheFileDidNotReach(resolved registry.Resolved, log *slog.Logger) { + if len(resolved.Unknown) > 0 { + log.Warn("sei.toml writes keys no section declares; they have no effect", + "count", len(resolved.Unknown), "keys", strings.Join(resolved.Unknown, ",")) + } + if len(resolved.Ignored) == 0 { + return + } + cannot := registry.EnvCannotDeliver() + for _, key := range resolved.Ignored { + log.Warn("an environment variable is set for a key the environment cannot supply; it has no "+ + "effect and the file's value applies", "key", key, "variable", registry.EnvName(key), + "why", cannot[key]) + } +} + +// reportWhatTheFileSaysTheNodeIs names a disagreement about what kind of node this is. +// +// Two files state that, under different names. sei.toml records it at the top, and every value resolved +// through this manager is the answer for that kind of node. The node's own configuration file states it +// again in a key of its own, and that one is what the node runs as. +// +// This manager does not declare the second, on purpose: two keys for one fact can be written to disagree, +// and then a resolution answers for one while the node is the other. Not declaring it means nothing here +// can change it, which leaves the disagreement possible and unreported. A node whose file says validator +// while it runs as a full node resolves a validator's values and serves queries, and every report about it +// reads correctly. +// +// So it is compared and reported. Reported rather than corrected, because what kind of node this is gets +// decided when it is provisioned, and a configuration manager is not the thing that should change it. +func reportWhatTheFileSaysTheNodeIs(ctx *server.Context, mode string, log *slog.Logger) { + if ctx == nil || ctx.Config == nil || ctx.Config.Mode == "" { + return + } + running := ctx.Config.Mode + if !modesDisagree(mode, running) { + return + } + log.Error("sei.toml says this is one kind of node and the node's own configuration file says another; "+ + "every value resolved here is the answer for the first and the node runs as the second", + "sei.toml", mode, "running", running) +} + +// modesDisagree reports whether the kind of node sei.toml records and the kind the node runs as are +// different kinds. +// +// One pairing is not a disagreement. The kind that keeps every version of history has no name of its own in +// the node's own configuration file, so the command that writes that file writes the query-serving name +// instead, and the difference between them lives in settings the node's own file does not carry. +func modesDisagree(recorded, running string) bool { + if recorded == running { + return false + } + return recorded != string(registry.ModeArchive) || running != string(registry.ModeFull) +} + +// OwnReportingEnabledForTest reports whether this package's logger would emit at the level its reports use. +// +// Exported for the test that holds the floor, because the thing under test is a level and not a message. +func OwnReportingEnabledForTest() bool { + return logger.Enabled(context.Background(), ownReportingFloor) +} + +// readSeiToml loads the node's sei.toml, reporting the ordinary absence quietly. +// +// A node that has not generated one is the expected state while sections are still moving, so that is not +// a warning. A file that exists and will not parse is, because somebody wrote it and it is not doing what +// they think. +func readSeiToml(cmd *cobra.Command, log *slog.Logger) (*seitoml.File, bool) { + home, err := resolveHomeDir(cmd) + if err != nil { + log.Warn("cannot resolve the home directory; every key reads as it always has", "err", err) + return nil, false + } + file, ok := readSeiTomlAt(home) + if !ok { + log.Debug("no readable sei.toml; every key reads as it always has", "home", home) + } + return file, ok +} + +// readSeiTomlAt loads the sei.toml under a home directory. +// +// Separate from the reporting, so the check command can ask the same question without a logger and get the +// same answer for the same file. +func readSeiTomlAt(home string) (*seitoml.File, bool) { + file, err := seitoml.Load(filepath.Join(home, "config", seiTomlName)) + if err != nil { + return nil, false + } + return file, true +} + +// recordedMode reads the node mode the file records. +// +// Every value a node reads through the registry is the resolution for one mode, so a file that does not +// say which cannot be used at all. Reported rather than guessed: guessing picks one mode's answers for a +// node configured as another. +// +// Whether the mode is one this binary knows is not checked here. The resolution refuses a mode no section +// declares defaults for, and it names the modes there are, so a check here would be the same guard a +// second time and a worse message. +func recordedMode(file *seitoml.File, log *slog.Logger) (string, bool) { + mode, err := file.Mode() + if err != nil { + log.Warn("sei.toml records no usable node mode; every key reads as it always has", "err", err) + return "", false + } + return mode, true +} + +// TypedFlags records which flags this invocation carried, and has to run before anything else touches +// them. +// +// A flag reports itself changed when something called Set on it, and the handler this manager re-enters +// calls Set on every flag whose name its configuration knows a value for, so that a file can supply a +// flag's default. After that has run, a flag an operator typed and a key their app.toml holds are +// indistinguishable, and a flag channel built from that state would put app.toml above sei.toml. That is +// a worse inversion than the one the channel exists to prevent: the file an operator is being migrated +// onto would lose to the file they are being migrated off. +// +// So the snapshot is taken at the one point before that happens, which is the entry to Apply. Taking it +// there rather than inside the install is the difference between an invariant and a convention, because +// there is no later point at which the truth is still available. +func TypedFlags(cmd *cobra.Command) map[string]string { + out := map[string]string{} + cmd.Flags().VisitAll(func(f *pflag.Flag) { + if f.Changed { + out[strings.ToLower(f.Name)] = f.Value.String() + } + }) + return out +} + +// flagValues renders a snapshot of typed flags as a configuration source, under the keys the sections +// declare. +// +// A flag's name and the key it carries are not always spelled the same. The node's own flags separate words +// with an underscore where the tag they decode through uses a hyphen, so a flag named for a declared key is +// not equal to it, and comparing the two by string leaves an operator's typed flag looking like a name +// nothing declares. It is then dropped, and the file wins over the command line: the one channel somebody +// reaches for during an incident is the one that loses. +// +// Matched through the environment spelling, where a dot and a hyphen and an underscore are all the same +// character. That is an equivalence the registry already refuses to let two declared keys share, so a flag +// matches at most one key and no ambiguity is possible here. +// +// A flag matching no declared key is left under its own name. Most of the flags a node starts with were +// never configuration keys, and the resolution reports the unmatched ones from the file alone. +func flagValues(typed map[string]string) map[string]any { + if len(typed) == 0 { + return nil + } + byEnvName := map[string]string{} + for _, key := range registry.Keys() { + byEnvName[registry.EnvName(key)] = key + } + + out := make(map[string]any, len(typed)) + for name, value := range typed { + key := name + if declared, ok := byEnvName[registry.EnvName(name)]; ok { + key = declared + } + out[key] = value + } + return out +} diff --git a/cmd/seid/cmd/configmanager/tendermint.go b/cmd/seid/cmd/configmanager/tendermint.go new file mode 100644 index 0000000000..f606e5f629 --- /dev/null +++ b/cmd/seid/cmd/configmanager/tendermint.go @@ -0,0 +1,228 @@ +package configmanager + +import ( + "cmp" + "fmt" + "log/slog" + "os" + "sort" + "strings" + + "github.com/spf13/viper" + + "github.com/sei-protocol/seilog" + + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" +) + +// deliverDecodedSections puts the resolved values of the decoded sections into the node's own +// configuration. +// +// Putting a value into the source a node reads is the whole delivery for a section whose reader looks its +// keys up one at a time. It is no delivery at all for the sections this covers, which the boot's handler +// reads once into a struct before this runs. Those values are decoded into that struct instead, which is +// the same mechanism the handler used and therefore the same casts, the same tags and the same hooks. +// +// Nothing here can stop a node starting, which is the one promise this manager makes. +func deliverDecodedSections(ctx *server.Context, resolved registry.Resolved, log *slog.Logger) { + bySection := registry.SuppliedByDecodedSection(resolved) + if len(bySection) == 0 { + return + } + if ctx == nil || ctx.Config == nil { + log.Error("no node configuration to deliver into; every one of these keys reads as it always has", + "sections", len(bySection)) + return + } + + // One section at a time. A decode is all or nothing for whatever it is handed, so a single value a + // decoder refuses would otherwise cost every key in the file rather than the keys of the section it + // appeared in. An operator who fixes one setting and mistypes another has to end up with the first + // one applied. + for _, name := range sortedSectionNames(bySection) { + deliverOneSection(ctx, name, bySection[name], log) + } +} + +// deliverOneSection decodes one section's resolved values into the node's configuration. +// +// Decoded into a copy of that configuration first, and published by replacing it. A decoder gathers errors +// and keeps going, so a value it refuses partway leaves its target holding some of the new values and some +// of the old, with nothing to compare against and no way back. Rehearsing into a copy of the configuration +// the node already has, rather than into a fresh one, is what makes the rehearsal answer the same question: +// what a decoder writes can depend on what the target already holds, and only a copy holds the same things. +func deliverOneSection(ctx *server.Context, name string, values map[string]any, log *slog.Logger) { + keys := sortedKeys(values) + + source := viper.New() + for key, value := range values { + source.Set(key, value) + } + + // Refused before the decode, because a plain number where a length of time belongs decodes cleanly + // and means nanoseconds. Nothing after this can tell that apart from a value somebody meant. + if bad := refuseWhatDecodesToSomethingElse(ctx.Config, values); len(bad) > 0 { + log.Error("a length of time in this section is written as a plain number, which reads as "+ + "nanoseconds; none of the section is applied and every one of its keys reads as it always has", + "section", name, "written", strings.Join(bad, "; ")) + return + } + + candidate, err := copyNodeConfig(ctx.Config) + if err != nil { + log.Error("cannot copy this node's configuration, so nothing can be delivered into it without "+ + "risking a half-written one; these keys read as they always have", + "section", name, "keys", strings.Join(keys, ","), "err", err) + return + } + before, readErr := describe(ctx.Config, keys) + + if err := source.Unmarshal(candidate); err != nil { + log.Error("a written value in this section was refused, so none of the section is applied and "+ + "every one of its keys reads as it always has", + "section", name, "keys", strings.Join(keys, ","), "err", err) + return + } + + *ctx.Config = *candidate + after, afterErr := describe(ctx.Config, keys) + if readErr != nil || afterErr != nil { + // Reported rather than compared. Two unreadable sides look identical, so comparing them would + // say every value matched, which is a statement about nothing produced by reading nothing. + log.Error("this section was applied and what moved cannot be read, so nothing here says which "+ + "settings now differ from the node's own file", "section", name, + "keys", strings.Join(keys, ","), "err", cmp.Or(readErr, afterErr)) + return + } + reportWhatMoved(name, keys, before, after, log) +} + +// copyNodeConfig returns a configuration that holds what this one holds and shares nothing with it. +// +// A shallow copy would share every section, so a decode into the copy would write through to the original +// and a refused value would leave exactly the half-written configuration the copy exists to prevent. This +// copies the top level and every section under it. +// +// Written against the type rather than field by field, so a section added to it is copied without this +// function changing. A field this cannot copy is an error rather than a silent share. +func copyNodeConfig(from *tmcfg.Config) (*tmcfg.Config, error) { + if from == nil { + return nil, fmt.Errorf("no configuration to copy") + } + out := *from + if err := detachSections(&out, from); err != nil { + return nil, err + } + return &out, nil +} + +// reportWhatMoved names every key whose value the delivery changed, and what it changed from. +// +// The node's own configuration file still says what it said, and every tool an operator reaches for reads +// that file: a patch command, a validator, an audit, somebody reading it over their shoulder at three in +// the morning. None of them describes the running node after this. This log line is the only place the two +// can be told apart, so it names the key, what the file gave it and what the node now runs. +// +// Keys that did not move are not reported. An operator who writes the value their file already held has +// changed nothing, and a line saying so buries the ones that did. +func reportWhatMoved(name string, keys []string, before, after map[string]string, log *slog.Logger) { + var moved []string + for _, key := range keys { + if before[key] != after[key] { + moved = append(moved, fmt.Sprintf("%s: %s -> %s", key, before[key], after[key])) + } + } + if len(moved) == 0 { + log.Info("this section's written values match what the node's own file already gave it", + "section", name, "keys", len(keys)) + return + } + log.Info("this section's settings now differ from what the node's own configuration file says", + "section", name, "changed", strings.Join(moved, "; ")) +} + +// sortedKeys returns a map's keys in a fixed order, so a log line does not vary between runs. +func sortedKeys(values map[string]any) []string { + out := make([]string, 0, len(values)) + for key := range values { + out = append(out, key) + } + sort.Strings(out) + return out +} + +// sortedSectionNames returns the sections to deliver in a fixed order. +func sortedSectionNames(bySection map[string]map[string]any) []string { + out := make([]string, 0, len(bySection)) + for name := range bySection { + out = append(out, name) + } + sort.Strings(out) + return out +} + +// logLevelKey is the one delivered setting the struct is not the end of. +const logLevelKey = "log-level" + +// loggerOwnVariable is the environment variable the logger itself reads when it starts. +// +// Not the variable this key answers to in the resolution, which carries the binary's own prefix. Two names +// for one setting, and the older one is read before any of this runs. +const loggerOwnVariable = "SEI_LOG_LEVEL" + +// applyResolvedLogLevel hands a resolved log level to the logger, which the struct alone does not reach. +// +// The boot's handler reads the level off the struct and sets it before any of this runs, so a value that +// only reaches the struct moves a field and changes no logging. A setting that appears to take and does not +// is what this key space exists to remove. +// +// Applied from the resolution rather than after the decode, and before it. Every failure this manager can +// have is a log line, and a refusal is reported at a level an operator may have raised the threshold above. +// Waiting for a successful decode would mean the one setting somebody changes in order to see a refusal is +// the setting a refusal suppresses. +// +// Which value arrives is already decided: the resolution ranks a flag over the environment over the file. +// A level that cannot be read is reported and skipped, and the node keeps the level it had. +func applyResolvedLogLevel(resolved registry.Resolved, typed map[string]string, log *slog.Logger) { + supplied := false + for _, key := range resolved.Overrides { + if key == logLevelKey { + supplied = true + } + } + if !supplied { + return + } + + // The logger reads a variable of its own at start-up, under a name that is not the one this key + // answers to, and the boot's own handler steps aside when it is set: a flag beats it and a file does + // not. Applying here regardless would put the file above it, so an operator who exported a level and + // then adopted this file would find the level they exported ignored. A typed flag still wins, which is + // the order that was already there. + if _, fromFlag := flagValues(typed)[logLevelKey]; !fromFlag { + if os.Getenv(loggerOwnVariable) != "" { + log.Info("a log level is set in the environment under the logger's own variable, which the "+ + "node already applied; the level this file supplies is not used", + "variable", loggerOwnVariable, "ignored", resolved.Values[logLevelKey]) + return + } + } + text, isText := resolved.Values[logLevelKey].(string) + if !isText { + log.Error("the resolved log level is not text; the node keeps the level it already had", + "value", resolved.Values[logLevelKey]) + return + } + var level slog.Level + if err := level.UnmarshalText([]byte(text)); err != nil { + log.Error("the resolved log level cannot be read; the node keeps the level it already had", + "level", text, "err", err) + return + } + seilog.SetDefaultLevel(level, true) + // That set every logger in the process, this one included, so the floor goes back on. + keepOwnReportingVisible() + log.Info("resolved log level applied", "level", text) +} diff --git a/cmd/seid/cmd/configmanager/tendermint_copy.go b/cmd/seid/cmd/configmanager/tendermint_copy.go new file mode 100644 index 0000000000..e878e80649 --- /dev/null +++ b/cmd/seid/cmd/configmanager/tendermint_copy.go @@ -0,0 +1,335 @@ +package configmanager + +import ( + "fmt" + "reflect" + "sort" + "strings" + "time" + + "github.com/go-viper/mapstructure/v2" + + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" +) + +// detachSections makes a copy hold what the original holds without sharing anything a decode can write +// through. +// +// A copy of the struct alone shares every section, every list and every map it points at. A decoder writes +// a list into the array its target already holds, so a shared one means the rehearsal edits the original +// and a refused value leaves exactly the half-written configuration the copy exists to prevent. +// +// Walked over the type rather than field by field, so a section or a list added to the node's configuration +// is detached without this changing. A field it cannot detach is an error rather than a silent share, and +// the test beside this holds every reference in the type against that promise. +func detachSections(out, from *tmcfg.Config) error { + if out == nil || from == nil { + return fmt.Errorf("no configuration to detach") + } + return detachValue(reflect.ValueOf(out).Elem(), "") +} + +// detachValue replaces every reference under v with one nothing else holds. +// +// An unexported field is skipped rather than refused. The copy this walks was made by assigning the struct, +// which copies unexported fields by value, and a decoder cannot write to one either. +func detachValue(v reflect.Value, path string) error { + switch v.Kind() { + case reflect.Pointer: + if v.IsNil() || !v.CanSet() { + return nil + } + fresh := reflect.New(v.Type().Elem()) + fresh.Elem().Set(v.Elem()) + if err := detachValue(fresh.Elem(), path); err != nil { + return err + } + v.Set(fresh) + + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + f := v.Type().Field(i) + if !v.Field(i).CanSet() { + continue + } + if err := detachValue(v.Field(i), join(path, f.Name)); err != nil { + return err + } + } + + case reflect.Slice: + if v.IsNil() || !v.CanSet() { + return nil + } + fresh := reflect.MakeSlice(v.Type(), v.Len(), v.Len()) + reflect.Copy(fresh, v) + for i := 0; i < fresh.Len(); i++ { + if err := detachValue(fresh.Index(i), path); err != nil { + return err + } + } + v.Set(fresh) + + case reflect.Map: + if v.IsNil() || !v.CanSet() { + return nil + } + fresh := reflect.MakeMapWithSize(v.Type(), v.Len()) + for _, key := range v.MapKeys() { + elem := reflect.New(v.Type().Elem()).Elem() + elem.Set(v.MapIndex(key)) + if err := detachValue(elem, path); err != nil { + return err + } + fresh.SetMapIndex(key, elem) + } + v.Set(fresh) + + case reflect.Interface: + if v.IsNil() || !v.CanSet() { + return nil + } + inner := v.Elem() + fresh := reflect.New(inner.Type()).Elem() + fresh.Set(inner) + if err := detachValue(fresh, path); err != nil { + return err + } + v.Set(fresh) + + case reflect.Chan, reflect.Func, reflect.UnsafePointer: + return fmt.Errorf("%s is a %s, which cannot be copied", path, v.Kind()) + } + return nil +} + +// join builds a field path for a message. +func join(path, field string) string { + if path == "" { + return field + } + return path + "." + field +} + +// describe reads the value the node's configuration currently holds for each key, as text. +// +// Read through the same tags the decode writes through, so a key names the same field in both directions. +// Held as text because what a report needs is whether two values differ and what they are, and comparing +// the shapes a decode produced against the shapes a struct holds would answer a different question. +func describe(cfg *tmcfg.Config, keys []string) (map[string]string, error) { + out := map[string]string{} + if cfg == nil { + return out, fmt.Errorf("no configuration to read") + } + var nested map[string]any + if err := mapstructure.Decode(cfg, &nested); err != nil { + return out, err + } + flat := map[string]any{} + flatten("", nested, flat) + for _, key := range keys { + if v, ok := flat[key]; ok { + out[key] = fmt.Sprint(v) + } + } + return out, nil +} + +// flatten turns a nested map into one keyed by dotted path. +func flatten(prefix string, in map[string]any, out map[string]any) { + for name, value := range in { + path := name + if prefix != "" { + path = prefix + "." + name + } + if inner, nested := value.(map[string]any); nested { + flatten(path, inner, out) + continue + } + out[path] = value + } +} + +// DescribeForTest reads what a node's configuration holds for each key, as text. +// +// Exported for the test that measures the two generators against each other, which lives beside the boot +// because only a boot produces a generated file. +func DescribeForTest(cfg *tmcfg.Config, keys []string) map[string]string { + out, _ := describe(cfg, keys) + return out +} + +// refuseWhatDecodesToSomethingElse reports written values the decoder accepts and turns into something the +// operator did not mean, with what they should have written. +// +// Two shapes, and both decode cleanly, which is why nothing later objects. +// +// A length of time has no form of its own in the file, so it is written as text with a unit. A plain number +// is read as nanoseconds, the shortest unit there is, so sixty means sixty billionths of a second. Zero is +// the exception and is allowed: nanoseconds and seconds are the same at zero, and zero is the documented way +// to turn several of these settings off. +// +// A negative number written where the field cannot hold one wraps to the largest value that field has. So +// minus one, which is how an operator says "no limit" in most software they have used, becomes a limit of +// eighteen million million million: the ceiling on connected peers stops bounding anything, and a window +// measured in seconds becomes six centuries. +// +// This is the one place either can be caught. The resolution sees a number and a key; only the struct says +// what the key is. +func refuseWhatDecodesToSomethingElse(cfg *tmcfg.Config, values map[string]any) []string { + t := reflect.TypeOf(*cfg) + durations := durationKeys(t, "") + unsigned := unsignedKeys(t, "") + + var bad []string + for key, value := range values { + n, numeric := asNumber(value) + if !numeric { + continue + } + switch { + case durations[key] && n != 0: + bad = append(bad, fmt.Sprintf("%s = %v is a length of time, so write a unit, as %q", + key, value, fmt.Sprintf("%vs", value))) + case unsigned[key] && n < 0: + bad = append(bad, fmt.Sprintf("%s = %v cannot be negative, and decodes to the largest value "+ + "this setting can hold rather than to no limit", key, value)) + } + } + sort.Strings(bad) + return bad +} + +// asNumber reports whether a written value arrived as a number, and what it was. +// +// Held as a float because what the checks above ask is whether it is zero and whether it is negative, and +// every numeric shape a file, a variable or a flag can carry answers both. +func asNumber(value any) (float64, bool) { + switch v := value.(type) { + case int: + return float64(v), true + case int8: + return float64(v), true + case int16: + return float64(v), true + case int32: + return float64(v), true + case int64: + return float64(v), true + case uint: + return float64(v), true + case uint8: + return float64(v), true + case uint16: + return float64(v), true + case uint32: + return float64(v), true + case uint64: + return float64(v), true + case float32: + return float64(v), true + case float64: + return v, true + } + return 0, false +} + +// unsignedKeys returns the dotted keys whose field cannot hold a negative number. +func unsignedKeys(t reflect.Type, prefix string) map[string]bool { + return keysWhoseFieldIs(t, prefix, func(ft reflect.Type) bool { + switch ft.Kind() { + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return true + } + return false + }) +} + +// durationKeys returns the dotted keys whose field is a length of time. +// +// Matched by conversion rather than by identity, so a named type over the same underlying number is a length +// of time too. +func durationKeys(t reflect.Type, prefix string) map[string]bool { + durationType := reflect.TypeOf(time.Duration(0)) + return keysWhoseFieldIs(t, prefix, func(ft reflect.Type) bool { + return ft.Kind() == reflect.Int64 && ft.ConvertibleTo(durationType) && ft != reflect.TypeOf(int64(0)) + }) +} + +// keysWhoseFieldIs returns the dotted keys whose field answers a question about its type. +// +// One walk for every such question, over the same tag rules the declaration derives keys by, so a key found +// here is a key that can be written. +func keysWhoseFieldIs(t reflect.Type, prefix string, is func(reflect.Type) bool) map[string]bool { + out := map[string]bool{} + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + if f.PkgPath != "" { + continue + } + tag, ok := f.Tag.Lookup("mapstructure") + if !ok { + continue + } + name := strings.Split(tag, ",")[0] + squash := strings.Contains(tag, ",squash") + ft := f.Type + for ft.Kind() == reflect.Pointer { + ft = ft.Elem() + } + path := name + if prefix != "" && name != "" { + path = prefix + "." + name + } + if squash { + for key := range keysWhoseFieldIs(ft, prefix, is) { + out[key] = true + } + continue + } + if is(ft) { + out[path] = true + continue + } + if ft.Kind() == reflect.Struct { + for key := range keysWhoseFieldIs(ft, path, is) { + out[key] = true + } + } + } + return out +} + +// referencePathsIn returns every path in a type that a copy has to detach, for the test that holds +// detachSections to the type it copies. +func referencePathsIn(t reflect.Type, path string, seen map[reflect.Type]bool) []string { + if seen[t] { + return nil + } + seen[t] = true + defer delete(seen, t) + + var out []string + switch t.Kind() { + case reflect.Pointer, reflect.Slice, reflect.Map, reflect.Interface: + if path != "" { + out = append(out, path) + } + if t.Kind() != reflect.Interface { + out = append(out, referencePathsIn(t.Elem(), path, seen)...) + } + case reflect.Struct: + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + if f.PkgPath != "" { + continue + } + out = append(out, referencePathsIn(f.Type, join(path, f.Name), seen)...) + } + } + sort.Strings(out) + return out +} + +// samePath reports whether two paths name the same field, ignoring repeats a pointer produces. +func samePath(a, b string) bool { return strings.TrimSuffix(a, ".") == strings.TrimSuffix(b, ".") } diff --git a/cmd/seid/cmd/configmanager/tendermint_copy_test.go b/cmd/seid/cmd/configmanager/tendermint_copy_test.go new file mode 100644 index 0000000000..65c8220a1f --- /dev/null +++ b/cmd/seid/cmd/configmanager/tendermint_copy_test.go @@ -0,0 +1,115 @@ +package configmanager + +import ( + "reflect" + "testing" + + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" +) + +// TestTheCopyShareNothingWithWhatItCopied is the property the rollback rests on. +// +// The delivery decodes into a copy and publishes by replacing, so a refused value leaves the node's +// configuration untouched. That holds only if the copy shares nothing the decode can write through, and a +// decoder writes a list into the array its target already holds. One shared section, list or map and the +// rehearsal edits the original. +// +// Walked over the whole type rather than the fields anyone thought of, so a reference added to the node's +// configuration fails here rather than quietly sharing. +func TestTheCopyShareNothingWithWhatItCopied(t *testing.T) { + from := tmcfg.DefaultConfig() + // Give every list something in it, so a shared backing array is observable. + from.RPC.CORSAllowedOrigins = []string{"a", "b", "c"} + from.StateSync.RPCServers = []string{"one:1", "two:2"} + from.TxIndex.Indexer = []string{"kv"} + from.Other = map[string]any{"left": "over"} + + out, err := copyNodeConfig(from) + if err != nil { + t.Fatalf("copyNodeConfig: %v", err) + } + + for _, path := range referencePathsIn(reflect.TypeOf(tmcfg.Config{}), "", map[reflect.Type]bool{}) { + a, okA := fieldByPath(reflect.ValueOf(from).Elem(), path) + b, okB := fieldByPath(reflect.ValueOf(out).Elem(), path) + if !okA || !okB { + continue + } + if shares(a, b) { + t.Errorf("%s is shared between the node's configuration and the copy, so a decode into the "+ + "copy writes through to the node and a refused value cannot be rolled back", path) + } + } +} + +// TestTheCopyHoldsWhatItCopied is the other half: detaching must not lose a value. +// +// A copy that shares nothing and holds nothing would pass the test above and deliver a configuration of +// zeroes over a running node. +func TestTheCopyHoldsWhatItCopied(t *testing.T) { + from := tmcfg.DefaultConfig() + from.RPC.CORSAllowedOrigins = []string{"a", "b", "c"} + from.Mempool.Size = 4321 + from.Instrumentation.Prometheus = true + from.Other = map[string]any{"left": "over"} + + out, err := copyNodeConfig(from) + if err != nil { + t.Fatalf("copyNodeConfig: %v", err) + } + if !reflect.DeepEqual(from, out) { + t.Error("the copy does not hold what it copied; a delivery would publish a configuration that " + + "differs from the node's in ways nobody wrote") + } +} + +// fieldByPath walks a dotted field path, following pointers. +func fieldByPath(v reflect.Value, path string) (reflect.Value, bool) { + for _, name := range splitPath(path) { + for v.Kind() == reflect.Pointer { + if v.IsNil() { + return reflect.Value{}, false + } + v = v.Elem() + } + if v.Kind() != reflect.Struct { + return reflect.Value{}, false + } + f := v.FieldByName(name) + if !f.IsValid() { + return reflect.Value{}, false + } + v = f + } + return v, true +} + +// splitPath breaks a dotted field path into its names. +func splitPath(path string) []string { + if path == "" { + return nil + } + var out []string + start := 0 + for i := 0; i < len(path); i++ { + if path[i] == '.' { + out = append(out, path[start:i]) + start = i + 1 + } + } + return append(out, path[start:]) +} + +// shares reports whether two values point at the same memory. +func shares(a, b reflect.Value) bool { + if a.Kind() != b.Kind() { + return false + } + switch a.Kind() { + case reflect.Pointer, reflect.Map: + return !a.IsNil() && !b.IsNil() && a.Pointer() == b.Pointer() + case reflect.Slice: + return a.Len() > 0 && b.Len() > 0 && a.Pointer() == b.Pointer() + } + return false +} diff --git a/cmd/seid/cmd/node_agreement_test.go b/cmd/seid/cmd/node_agreement_test.go new file mode 100644 index 0000000000..58245a13db --- /dev/null +++ b/cmd/seid/cmd/node_agreement_test.go @@ -0,0 +1,125 @@ +package cmd + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "testing" + + "go.opentelemetry.io/otel/sdk/trace" + + "github.com/sei-protocol/sei-chain/cmd/seid/cmd/configmanager" + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + "github.com/sei-protocol/sei-chain/testutil/configtest" +) + +// bootGeneratedDefaults is what each diverging key resolves to for a node that has no configuration file of +// its own and lets the boot generate one. +// +// A declared value is what the init command writes for a kind of node. That command is not the only thing +// in this binary that writes this file: a node started without one gets it generated by the boot instead, +// and the two do not agree. These are the keys where they differ, with what the second one produces. +// +// Held as text because the two sides carry different Go types for the same key often enough that comparing +// values would be comparing shapes. What matters is which keys disagree and what a node gets instead. +var bootGeneratedDefaults = map[string]string{ + "p2p.recv-rate": "5120000", + "p2p.send-rate": "5120000", + "proxy-app": "", + "tx-index.indexer": "[kv]", +} + +// reasoning says what a node gets, and it is why each row is measured rather than described. +var reasoning = map[string]string{ + "p2p.recv-rate": "the ceiling on what one connection may pull, four times lower than the declared " + + "value, so a node adopting a file generated by the other writer would have it raised", + "p2p.send-rate": "the same ceiling in the other direction", + "proxy-app": "a setting no reader reads, whose declared value comes from the node's own defaults and " + + "which a bound flag's empty default overwrites during the boot's own decode", + "tx-index.indexer": "whether the node indexes transactions. The boot's writer produces a file for a " + + "node that serves queries, because the kind it defaults to is that one, so this row is what a " + + "resolution for a validator states against a file generated for something else. The pair in that " + + "file agrees with itself; what disagrees is the kind of node each side is describing", +} + +// TestTheDivergencesFromAGeneratedFileAreTheRecordedOnes measures what a comment would only claim. +// +// The init command and the boot both generate this file and they disagree, so a declared value is what one +// of them writes and not simply what a generated file carries. Which keys those are is measured here rather +// than described, because a key that starts diverging fails and so does one that stops. +// +// Driven through a real boot with no configuration file of its own and no sei.toml, so nothing is delivered +// and what the node holds is purely what the boot generated. +func TestTheDivergencesFromAGeneratedFileAreTheRecordedOnes(t *testing.T) { + configtest.Isolate(t) + generated := whatTheBootGenerates(t) + + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + var measured []string + for key, got := range generated { + declared, declares := resolved.Values[key] + if !declares { + continue + } + if fmt.Sprint(declared) == got { + if _, listed := bootGeneratedDefaults[key]; listed { + t.Errorf("%s no longer diverges, both sides being %v. Take it off the record, so the "+ + "record stays the set of keys the two generators state differently", key, declared) + } + continue + } + measured = append(measured, key) + want, listed := bootGeneratedDefaults[key] + switch { + case !listed: + t.Errorf("%s is declared as %v and a node that let the boot generate its file runs %q, and "+ + "nothing records that. %s", key, declared, got, reasoning[key]) + case want != got: + t.Errorf("%s is recorded as running %q and runs %q", key, want, got) + } + } + + sort.Strings(measured) + if len(measured) != len(bootGeneratedDefaults) { + t.Errorf("measured %d divergences and %d are recorded: %v", + len(measured), len(bootGeneratedDefaults), measured) + } +} + +// whatTheBootGenerates returns what a node holds for every declared key of the decoded sections, having +// started with no configuration file of its own. +func whatTheBootGenerates(t *testing.T) map[string]string { + t.Helper() + home := configtest.NewHome(t) + if err := os.MkdirAll(filepath.Join(home.Root, "config"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + + cmd := server.StartCmd(nil, home.Root, []trace.TracerProviderOption{}) + if err := cmd.Flags().Set("home", home.Root); err != nil { + t.Fatalf("set --home: %v", err) + } + ctx, err := runManager(t, configmanager.SeiConfigManager{}, cmd) + if err != nil { + t.Fatalf("the boot was refused: %v", err) + } + if ctx.Config == nil { + t.Fatal("the boot produced no node configuration") + } + + var keys []string + for name := range registry.DecodedSections() { + section, ok := registry.Lookup(name) + if !ok { + continue + } + keys = append(keys, section.Keys...) + } + return configmanager.DescribeForTest(ctx.Config, keys) +} diff --git a/cmd/seid/cmd/node_delivery_test.go b/cmd/seid/cmd/node_delivery_test.go new file mode 100644 index 0000000000..abae1b238c --- /dev/null +++ b/cmd/seid/cmd/node_delivery_test.go @@ -0,0 +1,380 @@ +package cmd + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "testing" + "time" + + "go.opentelemetry.io/otel/sdk/trace" + + "github.com/sei-protocol/sei-chain/cmd/seid/cmd/configmanager" + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" + "github.com/sei-protocol/sei-chain/testutil/configtest" +) + +// The second delivery, driven the way an operator reaches it. +// +// A section whose reader looks its keys up one at a time is delivered by putting the value into the source. +// The node's own configuration file is read once into a struct before any of that, so a value put into the +// source reaches nothing and has to be decoded into the struct instead. These read the setting the node +// runs rather than the source it was resolved into, because a key can be correct in the source and absent +// from the struct. + +// bootWithNodeFile runs a real boot against a sei.toml and a generated node configuration file. +func bootWithNodeFile(t *testing.T, seiToml string, edit func(*tmcfg.Config)) *server.Context { + t.Helper() + home := configtest.NewHome(t) + dir := filepath.Join(home.Root, "config") + if err := os.MkdirAll(dir, 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + + // The node's own file, generated the way the node generates it, so what the delivery writes over is + // what an operator would actually have. + live := tmcfg.DefaultConfig() + if edit != nil { + edit(live) + } + if err := tmcfg.WriteConfigFile(home.Root, live); err != nil { + t.Fatalf("render the node's configuration file: %v", err) + } + if seiToml != "" { + if err := os.WriteFile(filepath.Join(dir, "sei.toml"), []byte(seiToml), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + } + + cmd := server.StartCmd(nil, home.Root, []trace.TracerProviderOption{}) + if err := cmd.Flags().Set("home", home.Root); err != nil { + t.Fatalf("set --home: %v", err) + } + ctx, err := runManager(t, configmanager.SeiConfigManager{}, cmd) + if err != nil { + t.Fatalf("the boot was refused: %v", err) + } + if ctx.Config == nil { + t.Fatal("the boot produced no node configuration") + } + return ctx +} + +const nodeFileHeader = "schema_version = 1\nnode_mode = \"validator\"\n" + +// TestAWrittenValueReachesTheNodesOwnConfiguration is the property the whole thing rests on. +func TestAWrittenValueReachesTheNodesOwnConfiguration(t *testing.T) { + configtest.Isolate(t) + ctx := bootWithNodeFile(t, nodeFileHeader+ + "\n[instrumentation]\nprometheus = true\nmax-open-connections = 41\n", nil) + + if !ctx.Config.Instrumentation.Prometheus { + t.Error("sei.toml turned the metrics listener on and the node runs with it off. The value was " + + "resolved and put into a source that nothing reading this file ever consults") + } + if got := ctx.Config.Instrumentation.MaxOpenConnections; got != 41 { + t.Errorf("sei.toml set max-open-connections to 41 and the node runs %d", got) + } +} + +// TestAnUnwrittenKeyKeepsWhatTheNodesOwnFileSaid separates delivering a value from overwriting one. +// +// A section read by a lookup can be delivered whole, because its reader has nowhere else to get a value +// from. A section read by a decode already holds what its own file said, put there before this ran. So a +// key the operator's sei.toml does not mention has to arrive at whatever that file gave it, and delivering +// a default instead replaces their file with one nobody chose, on every boot. +// +// The fixture turns the key on in the node's own file, where the default is off, so the two disagree. +// Without that they agree and the overwrite is invisible. +func TestAnUnwrittenKeyKeepsWhatTheNodesOwnFileSaid(t *testing.T) { + configtest.Isolate(t) + ctx := bootWithNodeFile(t, nodeFileHeader+"\n[mempool]\nsize = 4321\n", func(live *tmcfg.Config) { + live.Instrumentation.Prometheus = true + }) + + if got := ctx.Config.Mempool.Size; got != 4321 { + t.Fatalf("the written key arrived as %d, so this test cannot tell the two cases apart", got) + } + if !ctx.Config.Instrumentation.Prometheus { + t.Error("the node's own file turned the metrics listener on, sei.toml said nothing about it, and " + + "the node runs with it off. A default was delivered over the operator's own file, which " + + "happens on every boot for every key their sei.toml does not mention") + } +} + +// TestARefusedValueLeavesItsSectionAlone is the promise that makes this safe to enable. +// +// A decoder gathers errors and keeps going, so a value it refuses partway leaves its target holding some +// new values and some old, with nothing to compare against. The delivery decodes into a copy and publishes +// by replacing, so a refused value leaves the section exactly as the node had it. +func TestARefusedValueLeavesItsSectionAlone(t *testing.T) { + configtest.Isolate(t) + ctx := bootWithNodeFile(t, nodeFileHeader+ + "\n[instrumentation]\nprometheus = true\nmax-open-connections = \"not a number\"\n", nil) + + if got := ctx.Config.Instrumentation.MaxOpenConnections; got != 3 { + t.Errorf("max-open-connections is %d after a refused value, want the 3 the node had. A "+ + "partly applied decode leaves settings nobody chose", got) + } + if ctx.Config.Instrumentation.Prometheus { + t.Error("the value beside the refused one was applied, so a partial decode was published. " + + "Either all of a section's values arrive or none do") + } +} + +// TestARefusedValueCostsOnlyItsOwnSection is why the delivery is per section. +// +// One decode for the whole file would mean an operator who fixed one setting and mistyped another boots +// with neither applied. The mistyped section is lost; the one beside it is not. +func TestARefusedValueCostsOnlyItsOwnSection(t *testing.T) { + configtest.Isolate(t) + ctx := bootWithNodeFile(t, nodeFileHeader+ + "\n[instrumentation]\nmax-open-connections = \"not a number\"\n"+ + "\n[mempool]\nsize = 4321\n", nil) + + if got := ctx.Config.Instrumentation.MaxOpenConnections; got != 3 { + t.Errorf("the refused section was applied anyway, reading %d", got) + } + if got := ctx.Config.Mempool.Size; got != 4321 { + t.Errorf("mempool.size is %d and sei.toml set it to 4321. A value refused in one section took "+ + "another section's settings down with it", got) + } +} + +// TestEachChannelWinsForADecodedKeyToo is precedence, asserted where a decoded value lands. +func TestEachChannelWinsForADecodedKeyToo(t *testing.T) { + const key = "rpc.max-open-connections" + body := nodeFileHeader + "\n[rpc]\nmax-open-connections = 111\n" + + t.Run("the file beats what the node's own file said", func(t *testing.T) { + configtest.Isolate(t) + ctx := bootWithNodeFile(t, body, nil) + if got := ctx.Config.RPC.MaxOpenConnections; got != 111 { + t.Errorf("the node runs %d with 111 in sei.toml; the value resolved and never reached the "+ + "struct the node reads", got) + } + }) + + t.Run("the environment beats the file", func(t *testing.T) { + configtest.Isolate(t) + t.Setenv(registry.EnvName(key), "222") + ctx := bootWithNodeFile(t, body, nil) + if got := ctx.Config.RPC.MaxOpenConnections; got != 222 { + t.Errorf("the node runs %d with 222 in the environment and 111 in the file", got) + } + }) +} + +// TestTheDeliveryLeavesTheRootDirectoryAlone is what the root-directory exclusions buy. +func TestTheDeliveryLeavesTheRootDirectoryAlone(t *testing.T) { + configtest.Isolate(t) + ctx := bootWithNodeFile(t, nodeFileHeader+"\n[instrumentation]\nprometheus = true\n", nil) + + if !ctx.Config.Instrumentation.Prometheus { + t.Fatal("the delivery did not run, so this test would pass with the root directory declared") + } + if ctx.Config.RootDir == "" { + t.Error("the node's root directory is empty after the delivery") + } + if ctx.Config.PrivValidator.RootDir == "" { + t.Error("the signing key's root directory is empty after the delivery. A node that cannot find " + + "its key does not sign") + } +} + +// TestATypedFlagReachesTheKeyItCarries covers the one channel an operator reaches for under pressure. +// +// A flag's name and the key it carries are not always spelled the same: the node's own flags separate words +// with an underscore where the tag they decode through uses a hyphen. Compared as strings such a flag looks +// like a name nothing declares, so it is dropped, and the file wins over the command line. +// +// Driven with the file and the flag disagreeing, and read off the struct the node runs from, because this +// key belongs to a section delivered by a decode. +func TestATypedFlagReachesTheKeyItCarries(t *testing.T) { + const key = "p2p.unconditional-peer-ids" + const flag = "p2p.unconditional_peer_ids" + configtest.Isolate(t) + + home := configtest.NewHome(t) + dir := filepath.Join(home.Root, "config") + if err := os.MkdirAll(dir, 0o750); err != nil { + t.Fatal(err) + } + if err := tmcfg.WriteConfigFile(home.Root, tmcfg.DefaultConfig()); err != nil { + t.Fatalf("render the node's configuration file: %v", err) + } + body := nodeFileHeader + "\n[p2p]\nunconditional-peer-ids = \"from-the-file\"\n" + if err := os.WriteFile(filepath.Join(dir, "sei.toml"), []byte(body), 0o600); err != nil { + t.Fatalf("write sei.toml: %v", err) + } + + cmd := server.StartCmd(nil, home.Root, []trace.TracerProviderOption{}) + if err := cmd.Flags().Set("home", home.Root); err != nil { + t.Fatal(err) + } + if err := cmd.Flags().Set(flag, "from-the-command-line"); err != nil { + t.Skipf("--%s is not on this command, so nothing here can carry the key: %v", flag, err) + } + ctx, err := runManager(t, configmanager.SeiConfigManager{}, cmd) + if err != nil { + t.Fatalf("the boot was refused: %v", err) + } + + if got := ctx.Config.P2P.UnconditionalPeerIDs; got != "from-the-command-line" { + t.Errorf("the node runs %q with --%s typed and a different value in the file, want the typed "+ + "one. The flag's name and the key it carries are spelled differently, so comparing them as "+ + "strings drops the flag and the file wins over the command line", got, flag) + } +} + +// TestALengthOfTimeWrittenAsAPlainNumberIsRefused covers a value that decodes cleanly and is wrong by a +// factor of a billion. +// +// The file format has no way to say how long something is, so a length of time is written as text with a +// unit. A plain number decodes as nanoseconds, the shortest unit there is, so sixty means sixty billionths +// of a second and the node starts. Nothing later objects, because nothing later can tell. +func TestALengthOfTimeWrittenAsAPlainNumberIsRefused(t *testing.T) { + configtest.Isolate(t) + was := tmcfg.DefaultConfig().Mempool.TTLDuration + + t.Run("a plain number is refused and the section is left alone", func(t *testing.T) { + ctx := bootWithNodeFile(t, nodeFileHeader+"\n[mempool]\nttl-duration = 60\nsize = 4321\n", nil) + if got := ctx.Config.Mempool.TTLDuration; got != was { + t.Errorf("the node runs a time-to-live of %v after a plain 60 was written, want the %v it "+ + "had. Sixty read as nanoseconds is sixty billionths of a second", got, was) + } + if got := ctx.Config.Mempool.Size; got == 4321 { + t.Error("the value beside the refused one was applied, so the section was published in part") + } + }) + + t.Run("zero is applied, because zero is the same in every unit", func(t *testing.T) { + // Several of these settings document zero as the way to turn them off, and three declare it as + // their value, so an operator writing it is doing the ordinary thing. Refusing it would cost them + // every other key in the section. + ctx := bootWithNodeFile(t, nodeFileHeader+ + "\n[rpc]\ntimeout-read-header = 0\nmax-open-connections = 41\n", nil) + if got := ctx.Config.RPC.TimeoutReadHeader; got != 0 { + t.Errorf("the node runs a read-header timeout of %v with 0 written, want 0", got) + } + if got := ctx.Config.RPC.MaxOpenConnections; got != 41 { + t.Errorf("max-open-connections is %d, so writing a zero length of time cost the section. "+ + "Zero nanoseconds and zero seconds are the same value, so there is nothing to refuse", got) + } + }) + + t.Run("the same number with a unit is applied", func(t *testing.T) { + ctx := bootWithNodeFile(t, nodeFileHeader+"\n[mempool]\nttl-duration = \"60s\"\n", nil) + if got := ctx.Config.Mempool.TTLDuration; got != 60*time.Second { + t.Errorf("the node runs %v with \"60s\" written, want 60s. Refusing a plain number must not "+ + "refuse the written form an operator is being asked for", got) + } + }) +} + +// TestTheReportSurvivesAQuietNode is what a fleet running its nodes quiet needs. +// +// One log level covers every logger in the process and an operator writes it. A fleet that sets it above the +// level these reports use turns this manager into a component that changes what a node runs and says nothing +// about it, and the report is the only place the node's own file and the running settings can be told apart. +// +// The level is what is asserted rather than a message, because a message can be absent for reasons that have +// nothing to do with whether it would have been printed. +func TestTheReportSurvivesAQuietNode(t *testing.T) { + configtest.Isolate(t) + + ctx := bootWithNodeFile(t, nodeFileHeader+"log-level = \"error\"\n\n[mempool]\nsize = 4321\n", nil) + if got := ctx.Config.Mempool.Size; got != 4321 { + t.Fatalf("the value was not delivered (%d), so this test cannot show a report being kept", got) + } + if !configmanager.OwnReportingEnabledForTest() { + t.Error("a node whose file sets the level to error delivered a value and this manager's own " + + "reporting is switched off. The report is the only signal it has, and the node's own file " + + "and its running settings can be told apart nowhere else") + } +} + +// TestANegativeNumberWhereTheSettingCannotHoldOneIsRefused covers the habit of writing minus one for +// "no limit". +// +// Most software an operator has used takes minus one that way. Here the field cannot hold a negative number, +// so the decoder wraps it to the largest value the field has: the ceiling on connected peers stops bounding +// anything, and a window measured in seconds becomes centuries. The value decodes cleanly, so nothing later +// objects. +func TestANegativeNumberWhereTheSettingCannotHoldOneIsRefused(t *testing.T) { + configtest.Isolate(t) + was := tmcfg.DefaultConfig().P2P.MaxConnections + + ctx := bootWithNodeFile(t, nodeFileHeader+ + "\n[p2p]\nmax-connections = -1\nsend-rate = 1234567\n", nil) + + if got := ctx.Config.P2P.MaxConnections; got != was { + t.Errorf("the node allows %d connected peers after minus one was written, want the %d it had. "+ + "Minus one wraps to the largest value this setting can hold, which is no bound at all", got, was) + } + if got := ctx.Config.P2P.SendRate; got == 1234567 { + t.Error("the value beside the refused one was applied, so the section was published in part") + } +} + +// TestNoDeliveryCarriesADeclaredDefault is the one rule both deliveries depend on, named. +// +// A resolution answers for every declared key, and a declared value is what a provisioning command writes +// for a kind of node rather than what any particular node runs. Delivering one would replace a setting an +// operator never mentioned, on every boot, for every key their file omits. Both deliveries avoid that by +// narrowing to the keys a source supplied, and each does it in its own function. +// +// That makes it a rule three call sites remember rather than one a single function enforces, which is the +// shape this repository's own guidance says to guard. Until the narrowing has one home, this is the guard: +// it boots with a file that supplies one key and asserts that nothing else moved anywhere, across both +// deliveries and every mode. +func TestNoDeliveryCarriesADeclaredDefault(t *testing.T) { + for _, mode := range registry.Modes() { + t.Run(string(mode), func(t *testing.T) { + configtest.Isolate(t) + + // What the node holds before any file supplies anything. + bare := bootWithNodeFile(t, "schema_version = 1\nnode_mode = \""+string(mode)+"\"\n", nil) + keys := everyDeclaredKey() + before := configmanager.DescribeForTest(bare.Config, keys) + beforeSource := map[string]string{} + for _, key := range keys { + beforeSource[key] = fmt.Sprint(bare.Viper.Get(key)) + } + + // The same node, with a file supplying exactly one key. + after := bootWithNodeFile(t, "schema_version = 1\nnode_mode = \""+string(mode)+"\"\n"+ + "\n[mempool]\nsize = 4321\n", nil) + if got := after.Config.Mempool.Size; got != 4321 { + t.Fatalf("the one supplied key arrived as %d, so nothing was delivered and this test "+ + "would pass for a delivery that does nothing", got) + } + + afterDescribed := configmanager.DescribeForTest(after.Config, keys) + for _, key := range keys { + if key == "mempool.size" { + continue + } + if afterDescribed[key] != before[key] { + t.Errorf("%s reads %q after a file that supplies only mempool.size, and %q before. A "+ + "declared default was delivered over a setting nobody wrote", + key, afterDescribed[key], before[key]) + } + if got := fmt.Sprint(after.Viper.Get(key)); got != beforeSource[key] { + t.Errorf("%s reads %q in the source and %q before it. A declared default was "+ + "installed for a key nobody wrote", key, got, beforeSource[key]) + } + } + }) + } +} + +// everyDeclaredKey returns every key any registered section declares, sorted. +func everyDeclaredKey() []string { + keys := registry.Keys() + sort.Strings(keys) + return keys +} 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/cmd/seid/cmd/root.go b/cmd/seid/cmd/root.go index b4cfacac90..cd3e95b2e7 100644 --- a/cmd/seid/cmd/root.go +++ b/cmd/seid/cmd/root.go @@ -145,6 +145,7 @@ func initRootCmd( tmcli.NewCompletionCmd(rootCmd, true), debugCmd, config.Cmd(), + seiConfigCmd(), tools.ToolCmd(), SnapshotCmd(), LogLevelCmd(), @@ -477,3 +478,16 @@ supply_enabled = {{ .LightInvariance.SupplyEnabled }} return customAppTemplate, customAppConfig } + +// seiConfigCmd groups the commands that answer questions about a node's sei.toml. +// +// Its own group rather than a subcommand of the existing configuration command, which reads and writes the +// files this one is about rather than the file that replaces them. +func seiConfigCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "sei-config", + Short: "Inspect the node's sei.toml", + } + cmd.AddCommand(configmanager.CheckCmd()) + return cmd +} diff --git a/config/cosmosbase/agreement_test.go b/config/cosmosbase/agreement_test.go new file mode 100644 index 0000000000..a13982ecca --- /dev/null +++ b/config/cosmosbase/agreement_test.go @@ -0,0 +1,200 @@ +package cosmosbase + +import ( + "fmt" + "sort" + "testing" + + "github.com/spf13/viper" + + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + srvconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" +) + +// whatANodeRunsToday is what each diverging key resolves to for a configuration carrying no keys. +// +// A declared value is what seid init writes for a kind of node. That is not what a node with nothing +// written resolves, and these are the keys where the two differ. Most are reads that take no account of +// whether the key was present, so an absent key casts to a zero and the default beside it is lost. +// +// Held as text because the two sides carry different Go types for the same key often enough that comparing +// values would be comparing shapes. What matters here is which keys disagree and what a node gets instead. +var whatANodeRunsToday = map[string]string{ + "api.address": "", + "api.max-open-connections": "0", + "api.rpc-max-body-bytes": "0", + "api.rpc-read-timeout": "0", + "api.swagger": "false", + "grpc.enable": "true", + "minimum-gas-prices": "", + "occ-enabled": "false", + "pruning": "default", + "pruning-keep-every": "", + "telemetry.enabled": "false", + "telemetry.prometheus-retention-time": "0", +} + +// whyItMatters says what a node gets today, for the keys where that is worth stating. +var whyItMatters = map[string]string{ + "pruning": "a command flag of this name carries the standard schedule below the file, so a node with " + + "nothing written prunes on that schedule where a generated file would have said keep everything", + "grpc.enable": "a command flag of this name defaults the interface on, so a validator with nothing " + + "written serves gRPC where a generated file would have written it off. This is the one interface " + + "toggle of the two that diverges; the REST one agrees", + "api.max-open-connections": "zero is unlimited, so the ceiling a generated file states is simply " + + "absent from a node that never wrote it, and the same holds for the body-size ceiling beside it", + "minimum-gas-prices": "an empty price refuses to start, so this key is one no running node can " + + "actually have unwritten", + "occ-enabled": "the transaction execution path, and no command flag carries it, so an absent key " + + "reads as off where a generated file says on", +} + +// readerValues is what a node resolves for a configuration carrying none of these keys. +// +// Driven through the reader rather than reasoned about, because the reader is the authority on what an +// absent key resolves to and its answer differs per key: some reads check that the key was present, most +// do not, and two are rescued by a clamp that does nothing for an absent value. +// +// The start command's flags are bound first, the way a booting node binds them, and that is what makes +// this the answer a node gets rather than the answer the reader gives in isolation. Seventeen of these keys +// are also command flags, so a flag's registration default is what an absent key reaches before the lookup +// comes back empty. Without the binding, a key like the gRPC toggle reads as its type's zero and the +// comparison would report agreement where a node disagrees. +// +// One key has to be supplied. The metric label set is the first thing the reader asks for and it refuses a +// configuration without it, so a reader handed nothing at all answers for no key at all. +func readerValues(t *testing.T) map[string]string { + t.Helper() + v := viper.New() + start := server.StartCmd(nil, t.TempDir(), nil) + if err := v.BindPFlags(start.Flags()); err != nil { + t.Fatalf("bind the start flags: %v", err) + } + v.Set(globalLabelsKey, []any{}) + cfg, err := srvconfig.GetConfig(v) + if err != nil { + t.Fatalf("the reader refused a configuration carrying only the label set: %v", err) + } + + return map[string]string{ + "minimum-gas-prices": fmt.Sprint(cfg.MinGasPrices), + "pruning": fmt.Sprint(cfg.Pruning), + "pruning-keep-recent": fmt.Sprint(cfg.PruningKeepRecent), + "pruning-keep-every": fmt.Sprint(cfg.PruningKeepEvery), + "pruning-interval": fmt.Sprint(cfg.PruningInterval), + "halt-height": fmt.Sprint(cfg.HaltHeight), + "halt-time": fmt.Sprint(cfg.HaltTime), + "freeze-height": fmt.Sprint(cfg.FreezeHeight), + "min-retain-blocks": fmt.Sprint(cfg.MinRetainBlocks), + "inter-block-cache": fmt.Sprint(cfg.InterBlockCache), + "compaction-interval": fmt.Sprint(cfg.CompactionInterval), + "concurrency-workers": fmt.Sprint(cfg.ConcurrencyWorkers), + "occ-enabled": fmt.Sprint(cfg.OccEnabled), + "api.enable": fmt.Sprint(cfg.API.Enable), + "api.swagger": fmt.Sprint(cfg.API.Swagger), + "api.address": fmt.Sprint(cfg.API.Address), + "api.enabled-unsafe-cors": fmt.Sprint(cfg.API.EnableUnsafeCORS), + "api.max-open-connections": fmt.Sprint(cfg.API.MaxOpenConnections), + "api.rpc-read-timeout": fmt.Sprint(cfg.API.RPCReadTimeout), + "api.rpc-write-timeout": fmt.Sprint(cfg.API.RPCWriteTimeout), + "api.rpc-max-body-bytes": fmt.Sprint(cfg.API.RPCMaxBodyBytes), + "grpc.enable": fmt.Sprint(cfg.GRPC.Enable), + "grpc.address": fmt.Sprint(cfg.GRPC.Address), + "grpc.max-recv-msg-size": fmt.Sprint(cfg.GRPC.MaxRecvMsgSize), + "grpc.max-open-connections": fmt.Sprint(cfg.GRPC.MaxOpenConnections), + "grpc.max-connection-idle": fmt.Sprint(cfg.GRPC.MaxConnectionIdle), + "grpc.max-connection-age": fmt.Sprint(cfg.GRPC.MaxConnectionAge), + "grpc.max-connection-age-grace": fmt.Sprint(cfg.GRPC.MaxConnectionAgeGrace), + "grpc.keepalive-time": fmt.Sprint(cfg.GRPC.KeepaliveTime), + "grpc.keepalive-timeout": fmt.Sprint(cfg.GRPC.KeepaliveTimeout), + "grpc.keepalive-min-time": fmt.Sprint(cfg.GRPC.KeepaliveMinTime), + "grpc.keepalive-permit-without-stream": fmt.Sprint(cfg.GRPC.KeepalivePermitWithoutStream), + "telemetry.service-name": fmt.Sprint(cfg.Telemetry.ServiceName), + "telemetry.enabled": fmt.Sprint(cfg.Telemetry.Enabled), + "telemetry.enable-hostname": fmt.Sprint(cfg.Telemetry.EnableHostname), + "telemetry.enable-hostname-label": fmt.Sprint(cfg.Telemetry.EnableHostnameLabel), + "telemetry.enable-service-label": fmt.Sprint(cfg.Telemetry.EnableServiceLabel), + "telemetry.prometheus-retention-time": fmt.Sprint(cfg.Telemetry.PrometheusRetentionTime), + "state-sync.snapshot-interval": fmt.Sprint(cfg.StateSync.SnapshotInterval), + "state-sync.snapshot-keep-recent": fmt.Sprint(cfg.StateSync.SnapshotKeepRecent), + "state-sync.snapshot-directory": fmt.Sprint(cfg.StateSync.SnapshotDirectory), + "index-events": fmt.Sprint(cfg.IndexEvents), + globalLabelsKey: fmt.Sprint(cfg.Telemetry.GlobalLabels), + } +} + +// TestTheDivergencesFromTheReaderAreTheRecordedOnes measures what a comment used to count. +// +// A declared value is what seid init writes for a kind of node, and for a good number of these keys that is +// not what a node with nothing written resolves. Which keys those are was carried in prose, in four +// paragraphs, and one of the counts was wrong. Prose cannot fail when it is wrong. +// +// So the set is measured. A key that starts diverging fails, and so does one that stops, which means +// guarding a read has to account for its row rather than quietly making a sentence stale. +// +// Run for the mode whose declared values match the reader's own mode-blind answer most closely, because +// the reader takes no mode and comparing every mode against it would report the mode rules as divergences. +// The mode-varying keys are held by name in the test beside this one. +func TestTheDivergencesFromTheReaderAreTheRecordedOnes(t *testing.T) { + reader := readerValues(t) + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + var measured []string + for key, got := range reader { + declared, declares := resolved.Values[key] + if !declares { + t.Errorf("%s is read by the upstream reader and no section here declares it", key) + continue + } + if fmt.Sprint(declared) == got { + if _, listed := whatANodeRunsToday[key]; listed { + t.Errorf("%s no longer diverges, both sides being %v. Take it off the record, so the "+ + "record stays the set of keys a generated file states differently from a node that "+ + "never wrote them", key, declared) + } + continue + } + measured = append(measured, key) + want, listed := whatANodeRunsToday[key] + switch { + case !listed: + t.Errorf("%s is declared as %v and a node with nothing written resolves %q, and nothing "+ + "records that. %s", key, declared, got, whyItMatters[key]) + case want != got: + t.Errorf("%s is recorded as resolving %q and resolves %q", key, want, got) + } + } + + sort.Strings(measured) + if len(measured) != len(whatANodeRunsToday) { + t.Errorf("measured %d divergences and %d are recorded: %v", + len(measured), len(whatANodeRunsToday), measured) + } +} + +// TestEveryKeyTheseSectionsDeclareIsOneTheReaderResolves holds the two lists against each other. +// +// The reader's side is written out above, which is a second statement of the same key set. It is the only +// statement available: this reader looks its keys up as inline strings rather than through constants, so +// there is nothing to compare a tag against. A key on one side only is either a setting an operator writes +// that no reader fills, or one the reader fills that no section here declares. +func TestEveryKeyTheseSectionsDeclareIsOneTheReaderResolves(t *testing.T) { + reader := readerValues(t) + for _, section := range []string{ + BaseSectionName, APISectionName, GRPCSectionName, TelemetrySectionName, StateSyncSectionName, + } { + registered, ok := registry.Lookup(section) + if !ok { + t.Fatalf("%s is not registered", section) + } + for _, key := range registered.Keys { + if _, filled := reader[key]; !filled { + t.Errorf("%s declares %s and no field above is paired with it", section, key) + } + } + } +} diff --git a/config/cosmosbase/cosmosbase.go b/config/cosmosbase/cosmosbase.go new file mode 100644 index 0000000000..cc38c75030 --- /dev/null +++ b/config/cosmosbase/cosmosbase.go @@ -0,0 +1,157 @@ +// 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. +// 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. +// +// A section belongs here only when its keys are upstream's and that edge is in the way. Everything else +// registers in the package that owns its struct, so the struct, the values and the keys stay together. +package cosmosbase + +import ( + "github.com/sei-protocol/sei-chain/app/params" + "github.com/sei-protocol/sei-chain/config/registry" + srvconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" +) + +// The names these sections have in the configuration key space. +// +// BaseSectionName names a section whose keys carry no prefix at all. The name is for lookups and reports +// and is not part of any key, because giving those settings a section would rename every one of them. +const ( + BaseSectionName = "base" + APISectionName = "api" + GRPCSectionName = "grpc" + TelemetrySectionName = "telemetry" + StateSyncSectionName = "state-sync" +) + +// globalLabelsKey is the metric label set, which is the one key here no environment variable can supply. +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 +// 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(TelemetrySectionName, &telemetrySchema{}, telemetryDefaults) + registry.RegisterSection(StateSyncSectionName, &srvconfig.StateSyncConfig{}, stateSyncDefaults) + + registry.RefuseFromEnvironment(TelemetrySectionName, globalLabelsKey, + "the metric label set is a list of name and value rows, and its reader takes that exact shape "+ + "rather than casting what it finds, so no single environment string can supply it. Write it "+ + "in the configuration file instead") +} + +// forMode is the server configuration the seid init command writes for a node of this kind. +// +// The upstream defaults with the binary's own mode rules applied, which is the pipeline that command +// builds and renders through the template. So a declared value here is what that file would have held, and +// a caller writing a configuration file writes what that command would have written. +// +// Named by the command, because this binary generates a file two ways and they do not agree. A node +// starting without one gets a file from a second pipeline that applies no mode rules at all and carries +// overrides of its own, so it writes the standard pruning strategy where this writes keeping everything, +// a metric retention of sixty where this writes seven thousand two hundred, the REST interface on for a +// validator where this writes it off, and a pruning interval drawn at random each time it runs. This +// follows the command an operator runs to provision a node, not the file a node writes for itself. +// +// That is what a declared value states, and it is deliberately not what a node with nothing written +// resolves. Those differ for a good number of these keys, because most are read with no check that the key +// was present and several are bound to a command flag carrying its own default below the file. The set is +// measured rather than counted, in the agreement test beside this one. +// +// Three settings differ by mode today and each of them matters in a different direction. A node that +// serves queries needs the interfaces that serve them; a validator is meant to expose as little as it can; +// and how many blocks a node retains is a decision about its disk. +func forMode(mode registry.Mode) *srvconfig.Config { + out := srvconfig.DefaultConfig() + params.SetAppConfigByMode(out, params.NodeMode(mode)) + return out +} + +// baseDefaults is what the node-wide settings resolve to for a node of this kind. +// +// One of these keys answers per mode: how many blocks a node retains, which is a hundred thousand for a +// full node and everything for the rest. The other two mode-varying keys in this package are the interface +// toggles, which belong to the sections that own them. +// +// Every one of these keys is read with a casting getter and no check that the key was present, so an +// absent key casts to a zero and clobbers the default beside it. Which keys those are, and what a node +// resolves for each instead, belongs in a measurement rather than in a count here. +// +// A caller resolving for a running node has to supply that node's flag values, and only the ones an +// operator actually set. A flag nobody typed still reports a default, and this resolution ranks flags above +// the file, so passing defaults would put every one of them over an operator's own value. +func baseDefaults(mode registry.Mode) any { return forMode(mode).BaseConfig } + +// apiDefaults is what the REST interface 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. Serving queries is what the +// first two are for, and the second two are meant to expose as little as they can. +func apiDefaults(mode registry.Mode) any { return forMode(mode).API } + +// grpcDefaults is what the gRPC 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 same rule the REST +// interface follows and for the same reason. The upstream default is on for every kind, so declaring that +// would state an open interface on the nodes meant to expose the least. +// +// Six of these eleven keys are read only when the key is present. Two more are durations read through a +// clamp that rescues a negative value and does nothing for an absent one, so those two are unguarded and +// their clobber leaves no trace. The durations are declared as durations and written into a file as text, +// which is the shape the reader parses back. +func grpcDefaults(mode registry.Mode) any { return forMode(mode).GRPC } + +// 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 +// inverts: it is declared as keeping two snapshots and an absent key casts to zero, which the file format +// documents as keeping every snapshot. +func stateSyncDefaults(mode registry.Mode) any { return forMode(mode).StateSync } + +// telemetrySchema declares the keys the metric settings reader resolves. +// +// A schema rather than the upstream type, and the only one of these five that needs one. The difference is +// a single field's type. The upstream struct declares the label set as a list of string pairs, and the +// reader takes a list of untyped rows: it asserts that exact shape rather than casting what it finds, and +// the struct's own type does not satisfy it, including that type's empty value. Registering the upstream +// type would resolve a default the reader refuses, and it refuses by returning an error that is the first +// statement of the whole server configuration, so the node stops. Every node, not only one that wrote the +// key. +// +// Every other field matches the upstream type, so this is one field's shape and not the section's. +type telemetrySchema struct { + ServiceName string `mapstructure:"service-name"` + Enabled bool `mapstructure:"enabled"` + EnableHostname bool `mapstructure:"enable-hostname"` + EnableHostnameLabel bool `mapstructure:"enable-hostname-label"` + EnableServiceLabel bool `mapstructure:"enable-service-label"` + PrometheusRetentionTime int64 `mapstructure:"prometheus-retention-time"` + GlobalLabels []any `mapstructure:"global-labels"` +} + +// telemetryDefaults is what the metric settings resolve to for a node of this kind. +// +// Read out of the upstream defaults rather than written again here, so a changed default moves both at +// once and this states only which key carries which setting. +// +// The label set is empty, which is what the upstream default holds, so there is nothing to convert into +// the untyped rows the reader takes. A test holds that emptiness, because a default that gained rows would +// need converting and would otherwise reach the reader as the shape it refuses. +func telemetryDefaults(mode registry.Mode) any { + live := forMode(mode).Telemetry + return telemetrySchema{ + ServiceName: live.ServiceName, + Enabled: live.Enabled, + EnableHostname: live.EnableHostname, + EnableHostnameLabel: live.EnableHostnameLabel, + EnableServiceLabel: live.EnableServiceLabel, + PrometheusRetentionTime: live.PrometheusRetentionTime, + GlobalLabels: []any{}, + } +} diff --git a/config/cosmosbase/cosmosbase_test.go b/config/cosmosbase/cosmosbase_test.go new file mode 100644 index 0000000000..e038e15435 --- /dev/null +++ b/config/cosmosbase/cosmosbase_test.go @@ -0,0 +1,296 @@ +package cosmosbase + +import ( + "reflect" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/app/params" + "github.com/sei-protocol/sei-chain/config/registry" + "github.com/sei-protocol/sei-chain/sei-cosmos/baseapp" + "github.com/sei-protocol/sei-chain/sei-cosmos/server" + srvconfig "github.com/sei-protocol/sei-chain/sei-cosmos/server/config" + "github.com/sei-protocol/sei-chain/sei-cosmos/telemetry" +) + +// requireDeclares holds one section's declared keys against the keys named for it. +func requireDeclares(t *testing.T, section string, reads []string) registry.Section { + t.Helper() + registered, ok := registry.Lookup(section) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", section) + } + want := append([]string(nil), reads...) + sort.Strings(want) + if !reflect.DeepEqual(registered.Keys, want) { + t.Errorf("%s declares\n %v\nand its reader resolves\n %v", section, registered.Keys, want) + } + return registered +} + +// TestTheNodeWideKeysAreTheOnesTheirReaderResolves holds the root section against the server's constants. +// +// Fourteen keys and not one of them carries a segment in front. The reader looks these up by the constants +// below, so a prefix here would declare fourteen keys no operator writes and leave the real ones +// undeclared. +func TestTheNodeWideKeysAreTheOnesTheirReaderResolves(t *testing.T) { + section := requireDeclares(t, BaseSectionName, []string{ + server.FlagMinGasPrices, server.FlagPruning, server.FlagPruningKeepRecent, + server.FlagPruningKeepEvery, server.FlagPruningInterval, server.FlagHaltHeight, + server.FlagFreezeHeight, server.FlagHaltTime, server.FlagMinRetainBlocks, + server.FlagInterBlockCache, server.FlagIndexEvents, server.FlagCompactionInterval, + server.FlagConcurrencyWorkers, baseapp.FlagOccEnabled, + }) + if section.Prefix != "" { + t.Errorf("the section carries prefix %q, and one here renames every key it declares", section.Prefix) + } +} + +// TestTheSnapshotKeysAreTheOnesTheirReaderResolves holds the snapshot section against the server's +// constants. +func TestTheSnapshotKeysAreTheOnesTheirReaderResolves(t *testing.T) { + requireDeclares(t, StateSyncSectionName, []string{ + server.FlagStateSyncSnapshotInterval, + server.FlagStateSyncSnapshotKeepRecent, + server.FlagStateSyncSnapshotDir, + }) +} + +// TestTheRESTKeysAreTheOnesItsReaderResolves holds the REST section against the keys its reader looks up. +// +// Written out rather than taken from constants, because this reader has none: it looks each key up as a +// literal string where it reads it. That is the whole reason a comparison is worth making here. +func TestTheRESTKeysAreTheOnesItsReaderResolves(t *testing.T) { + requireDeclares(t, APISectionName, []string{ + "api.enable", "api.swagger", "api.enabled-unsafe-cors", "api.address", + "api.max-open-connections", "api.rpc-read-timeout", "api.rpc-write-timeout", + "api.rpc-max-body-bytes", + }) +} + +// TestTheGRPCKeysAreTheOnesItsReaderResolves holds the gRPC section against the keys its reader looks up. +// +// Written out for the same reason as the REST section: the reader has no constants for these. +func TestTheGRPCKeysAreTheOnesItsReaderResolves(t *testing.T) { + requireDeclares(t, GRPCSectionName, []string{ + "grpc.enable", "grpc.address", "grpc.max-recv-msg-size", "grpc.max-open-connections", + "grpc.max-connection-idle", "grpc.max-connection-age", "grpc.max-connection-age-grace", + "grpc.keepalive-time", "grpc.keepalive-timeout", "grpc.keepalive-min-time", + "grpc.keepalive-permit-without-stream", + }) +} + +// TestTheMetricKeysAreTheOnesItsReaderResolves holds the metric section against the keys its reader looks +// up, the label set among them. +func TestTheMetricKeysAreTheOnesItsReaderResolves(t *testing.T) { + requireDeclares(t, TelemetrySectionName, []string{ + "telemetry.service-name", "telemetry.enabled", "telemetry.enable-hostname", + "telemetry.enable-hostname-label", "telemetry.enable-service-label", + "telemetry.prometheus-retention-time", globalLabelsKey, + }) +} + +// TestTheMetricSchemaRestatesTheUpstreamTypeExactlyOnceOver is what a schema costs. +// +// The schema exists for one field's shape, so every other field has to be the upstream field: same name, +// same tag, same type. A field that drifted would declare a key under a spelling the reader does not look +// up, or resolve a value of a type it cannot take, and the section would go on registering cleanly either +// way. +func TestTheMetricSchemaRestatesTheUpstreamTypeExactlyOnceOver(t *testing.T) { + upstream := reflect.TypeOf(telemetry.Config{}) + schema := reflect.TypeOf(telemetrySchema{}) + if schema.NumField() != upstream.NumField() { + t.Fatalf("the schema has %d fields and the upstream type has %d; a field on one side only is "+ + "either a key nothing reads or a setting nothing declares", + schema.NumField(), upstream.NumField()) + } + + differing := 0 + for i := range schema.NumField() { + got, want := schema.Field(i), upstream.Field(i) + if got.Name != want.Name { + t.Errorf("field %d is %s here and %s upstream", i, got.Name, want.Name) + continue + } + if got.Tag != want.Tag { + t.Errorf("%s is tagged %q here and %q upstream, so it declares a key the reader does not "+ + "look up", got.Name, got.Tag, want.Tag) + } + if got.Type == want.Type { + continue + } + differing++ + if got.Name != "GlobalLabels" { + t.Errorf("%s is %s here and %s upstream. The label set is the only field whose shape this "+ + "schema changes, so a second one is a divergence nothing decided", + got.Name, got.Type, want.Type) + } + } + if differing != 1 { + t.Errorf("%d fields differ in type, want exactly one. If the upstream type came to match, this "+ + "schema is a restatement with nothing left to justify it", differing) + } +} + +// TestTheUpstreamDefaultCarriesNoLabels holds the assumption the declared label set is built on. +// +// The declared default is an empty list of rows, which is right only while the upstream default holds no +// labels. A default that gained a pair would need converting into the untyped rows the reader takes, and +// without that it reaches the reader as the shape it refuses. +func TestTheUpstreamDefaultCarriesNoLabels(t *testing.T) { + if got := srvconfig.DefaultConfig().Telemetry.GlobalLabels; len(got) != 0 { + t.Errorf("the upstream default carries %d label rows: %v. They need converting into untyped rows "+ + "here, because the reader asserts that shape rather than casting what it finds", len(got), got) + } +} + +// TestTheLabelSetIsRefusedFromTheEnvironment covers the one key no variable here can supply. +// +// Its reader asserts a list of untyped rows and an environment carries one string, so resolving the +// variable installs a value the reader refuses, and it refuses in the first statement of the whole server +// configuration. The node stops. Leaving the channel out means the file's value applies and the node runs. +func TestTheLabelSetIsRefusedFromTheEnvironment(t *testing.T) { + reason, refused := registry.EnvCannotDeliver()[globalLabelsKey] + if !refused { + t.Fatalf("%s is not refused from the environment, so a variable naming it resolves to a string "+ + "and installing that stops the node", globalLabelsKey) + } + if reason == "" { + t.Error("the refusal carries no reason, so an operator whose variable is ignored cannot be told why") + } + + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{ + LookupEnv: func(name string) (string, bool) { + if name == registry.EnvName(globalLabelsKey) { + return "chain_id=pacific-1", true + } + return "", false + }, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got := resolved.Values[globalLabelsKey]; !reflect.DeepEqual(got, []any{}) { + t.Errorf("%s resolved to %#v (%T), want the declared default it was left to", + globalLabelsKey, got, got) + } + for _, key := range resolved.Overrides { + if key == globalLabelsKey { + t.Errorf("%s is reported as a value an operator supplied, and the variable did nothing", + globalLabelsKey) + } + } +} + +// TestEachKindOfNodeResolvesTheInterfacesItIsFor is the mode-varying part of these sections. +// +// Three settings differ by kind of node, and the values are written out here rather than taken from the +// same rules the sections read, so a change to those rules fails this and gets looked at. Each matters in a +// different direction. A node that serves queries needs the two interfaces that serve them, and declaring +// them closed would take a service away from one. A validator is meant to expose as little as it can, and +// declaring gRPC open would state the opposite of that on every validator. And how many blocks a node +// keeps is a decision about its disk. +func TestEachKindOfNodeResolvesTheInterfacesItIsFor(t *testing.T) { + byMode := map[registry.Mode]struct { + api, grpc bool + retain uint64 + }{ + registry.ModeValidator: {api: false, grpc: false, retain: 0}, + registry.ModeSeed: {api: false, grpc: false, retain: 0}, + registry.ModeFull: {api: true, grpc: true, retain: 100000}, + registry.ModeArchive: {api: true, grpc: true, retain: 0}, + } + for _, mode := range registry.Modes() { + want, named := byMode[mode] + if !named { + t.Fatalf("mode %q has no expectation here, so a mode was added and this was not revisited", mode) + } + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + for key, expected := range map[string]any{ + "api.enable": want.api, + "grpc.enable": want.grpc, + "min-retain-blocks": want.retain, + } { + if got := resolved.Values[key]; !reflect.DeepEqual(got, expected) { + t.Errorf("mode %q: %s resolves to %#v, want %#v", mode, key, got, expected) + } + } + } +} + +// TestDefaultsAreTheUpstreamOnesApartFromTheModeRules covers everything a mode does not change. +// +// Compared against the upstream defaults with the same mode rules applied, so this holds the sections to +// carrying the whole of that configuration rather than a subset of it, and the three settings the rules +// touch are pinned by name above. +func TestDefaultsAreTheUpstreamOnesApartFromTheModeRules(t *testing.T) { + for _, mode := range registry.Modes() { + live := srvconfig.DefaultConfig() + params.SetAppConfigByMode(live, params.NodeMode(mode)) + for _, c := range []struct { + section string + got any + want any + }{ + {BaseSectionName, baseDefaults(mode), live.BaseConfig}, + {APISectionName, apiDefaults(mode), live.API}, + {GRPCSectionName, grpcDefaults(mode), live.GRPC}, + {StateSyncSectionName, stateSyncDefaults(mode), live.StateSync}, + } { + if !reflect.DeepEqual(c.got, c.want) { + t.Errorf("mode %q: %s resolves to something other than that mode's upstream configuration", + mode, c.section) + } + } + + if _, ok := telemetryDefaults(mode).(telemetrySchema); !ok { + t.Fatalf("mode %q: the metric defaults returned %T, want the schema", mode, telemetryDefaults(mode)) + } + // Every field the schema copies by hand, held against the upstream value, and held as the + // resolved key rather than as a struct field. The section that has to restate its values is the + // one where a field can be assigned from the wrong neighbour, and a struct comparison would not + // see it: each field still holds a value, and the count still matches. + requireResolvesTelemetry(t, mode, live.Telemetry) + } +} + +// requireResolvesTelemetry holds every key the metric schema declares against the upstream value. +func requireResolvesTelemetry(t *testing.T, mode registry.Mode, live telemetry.Config) { + t.Helper() + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + for key, want := range map[string]any{ + "telemetry.service-name": live.ServiceName, + "telemetry.enabled": live.Enabled, + "telemetry.enable-hostname": live.EnableHostname, + "telemetry.enable-hostname-label": live.EnableHostnameLabel, + "telemetry.enable-service-label": live.EnableServiceLabel, + "telemetry.prometheus-retention-time": live.PrometheusRetentionTime, + globalLabelsKey: []any{}, + } { + if got := resolved.Values[key]; !reflect.DeepEqual(got, want) { + t.Errorf("mode %q: %s resolves to %#v (%T), want %#v (%T)", mode, key, got, got, want, want) + } + } +} + +// TestTheSectionsThisPackageRegistersAreUsable covers what the registry refuses. +// +// Scoped to the five names this file registers. A refusal that depends on what else has registered is +// not this package's to answer for, and the sweep that covers it belongs where every section is linked. +func TestTheSectionsThisPackageRegistersAreUsable(t *testing.T) { + mine := map[string]bool{ + BaseSectionName: true, APISectionName: true, GRPCSectionName: true, + TelemetrySectionName: true, StateSyncSectionName: true, + } + for _, defect := range registry.Defects() { + if mine[defect.Section] { + t.Errorf("%s was refused, so none of its keys is declared: %v", defect.Section, defect.Err) + } + } +} diff --git a/config/registry/delivery.go b/config/registry/delivery.go new file mode 100644 index 0000000000..5967511b62 --- /dev/null +++ b/config/registry/delivery.go @@ -0,0 +1,113 @@ +package registry + +import ( + "fmt" + "sort" +) + +// decodedNotLookedUp holds the sections whose values reach their reader by a decode, and why. +var decodedNotLookedUp = map[string]string{} + +// DeclareDecodedNotLookedUp records that a section's values reach their reader by being decoded into a +// struct, rather than by a lookup in the source a node reads. +// +// Almost every section is read the other way: a reader asks for a key by name, so putting the resolved +// value into that source is the whole delivery. The sections this names are read once, by decoding a file +// into a struct before any of that happens, and a value put into the source afterwards reaches nothing. +// They need delivering a second way. +// +// The reason is required and names the struct the values are decoded into, which is what a reader has to +// check the claim against. A section declared with no reason is recorded as a defect rather than accepted, +// because the claim is the whole basis for delivering its keys differently. +func DeclareDecodedNotLookedUp(section, why string) { + mu.Lock() + defer mu.Unlock() + if why == "" { + defects = append(defects, Defect{Section: section, Err: fmt.Errorf( + "declared as decoded rather than looked up with no reason; the reason names the struct its " + + "values are decoded into, which is what a reader checks the claim against")}) + return + } + decodedNotLookedUp[section] = why +} + +// DecodedSections returns the sections whose values reach their reader by a decode, with the reason each +// gave, so a caller can report what it is about to do and to what. +func DecodedSections() map[string]string { + mu.RLock() + defer mu.RUnlock() + out := make(map[string]string, len(decodedNotLookedUp)) + for name, why := range decodedNotLookedUp { + out[name] = why + } + return out +} + +// SuppliedByDecodedSection splits a resolution into the values each decoded section has to deliver +// itself, keyed by section name and then by dotted key. +// +// Split per section rather than pooled, because a decode is all or nothing for whatever it is handed. One +// value a decoder refuses would otherwise cost every key in the file rather than the keys of the one +// section it appeared in, and an operator who fixed one setting and mistyped another would boot with +// neither applied and no way to tell which. +// +// The defaults are deliberately left out, and this is the difference between delivering a value and +// overwriting one. A section read by a lookup can be delivered whole, because its reader has nowhere else +// to get a value from. A section read by a decode already holds what its own file said, put there before +// any of this ran. Delivering a default over that replaces the operator's file with one nobody chose, on +// every boot, for every key their file does not mention. +// +// So a key that took its default is skipped and a key any other layer answered is delivered. That includes +// an operator writing the default value explicitly, because what is recorded is which layer answered and +// not whether the answer differs from the default: writing false where the file says true has to arrive. +func SuppliedByDecodedSection(resolved Resolved) map[string]map[string]any { + owning := DecodedSections() + + supplied := make(map[string]bool, len(resolved.Overrides)) + for _, key := range resolved.Overrides { + supplied[key] = true + } + + out := map[string]map[string]any{} + for _, section := range Sections() { + if _, owned := owning[section.Name]; !owned { + continue + } + for _, key := range section.Keys { + if !supplied[key] { + continue + } + if out[section.Name] == nil { + out[section.Name] = map[string]any{} + } + out[section.Name][key] = resolved.Values[key] + } + } + return out +} + +// UndeliveredSections returns the registered sections that named no delivery, sorted. +// +// A section reaches its reader one of two ways and the registry cannot tell which, so the answer is +// declared. A section that declares nothing is treated as read by a lookup, which is right for almost all +// of them and silently wrong for the rest: its keys resolve, install into the source, and change nothing +// the node runs. That is the failure this package exists to remove, so the set is reported and a caller +// linking every section holds it to what it expects. +func UndeliveredSections(expectDecoded map[string]bool) []string { + var out []string + for _, section := range Sections() { + if expectDecoded[section.Name] != DecodedNotLookedUp(section.Name) { + out = append(out, section.Name) + } + } + sort.Strings(out) + return out +} + +// DecodedNotLookedUp reports whether a section's values reach their reader by a decode. +func DecodedNotLookedUp(section string) bool { + mu.RLock() + defer mu.RUnlock() + _, ok := decodedNotLookedUp[section] + return ok +} diff --git a/config/registry/detach_test.go b/config/registry/detach_test.go new file mode 100644 index 0000000000..1e3a8205c6 --- /dev/null +++ b/config/registry/detach_test.go @@ -0,0 +1,65 @@ +package registry_test + +import ( + "reflect" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// listBearing is a probe whose default is a package-level variable, which is the usual shape. +type listBearing struct { + Allowed []string `mapstructure:"allowed"` + Labels map[string]string `mapstructure:"labels"` + Absent []string `mapstructure:"absent"` +} + +var listBearingDefault = listBearing{ + Allowed: []string{"callTracer", "prestateTracer"}, + Labels: map[string]string{"chain": "pacific-1"}, +} + +// TestAResolvedListIsTheCallersToWriteInto covers what a caller may do with a resolved value. +// +// A section's default is usually a package-level variable, so handing out its slice hands out the array +// that variable holds. A caller sorting or de-duplicating a resolved list in place, which is what a caller +// producing deterministic output does, would rewrite that variable for the whole process: every later +// resolution and every reader that copies the same struct. Two of the lists this reaches in practice are +// deny lists, so the rewrite is silent and it is a security control. +func TestAResolvedListIsTheCallersToWriteInto(t *testing.T) { + registry.Reset() + registry.RegisterSection("probe", &listBearing{}, func(registry.Mode) any { return listBearingDefault }) + for _, d := range registry.Defects() { + t.Fatalf("the probe was refused: %v", d.Err) + } + + resolved, err := registry.Resolve(registry.ModeFull, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + resolved.Values["probe.allowed"].([]string)[0] = "written-by-the-caller" + resolved.Values["probe.labels"].(map[string]string)["chain"] = "written-by-the-caller" + + if got := listBearingDefault.Allowed[0]; got != "callTracer" { + t.Errorf("writing into the resolved list changed the section's own default to %q, so every later "+ + "resolution and every reader copying that struct carries the caller's value", got) + } + if got := listBearingDefault.Labels["chain"]; got != "pacific-1" { + t.Errorf("writing into the resolved map changed the section's own default to %q", got) + } + + again, err := registry.Resolve(registry.ModeFull, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got := again.Values["probe.allowed"]; !reflect.DeepEqual(got, []string{"callTracer", "prestateTracer"}) { + t.Errorf("a later resolution carries %v, so one caller's edit reached another's answer", got) + } + + // A nil list stays nil rather than becoming an empty one, because absent and empty are different + // answers to a reader that checks length. + if got := again.Values["probe.absent"]; got == nil || !reflect.ValueOf(got).IsNil() { + t.Errorf("an unset list resolved to %#v, want a nil slice of its own type", got) + } +} diff --git a/config/registry/doc.go b/config/registry/doc.go index d1955c08f3..2610487898 100644 --- a/config/registry/doc.go +++ b/config/registry/doc.go @@ -36,7 +36,13 @@ // serve. // // The third argument answers per node mode, because a validator and a seed node do not default -// alike. +// alike. A mode this package does not declare is refused rather than answered for: what a section +// does with an argument it cannot match is not a decision anybody made. +// +// Some settings are node-wide and are written at the top of a file rather than inside a table. +// RegisterRootKeys declares those: the name it takes is what a lookup and a report are keyed by and is +// not part of any key, so the keys are the tags alone. Giving such a section a segment would rename +// every key it declares, and a renamed key is one an operator's existing file no longer reaches. // // # Defaults // @@ -52,10 +58,25 @@ // no exported value repeats it. // // Alongside the values it reports which keys something other than the defaults supplied, and which -// keys a source carried that no section declares. The first is what a diff renders, since a written +// keys the file carried that no section declares. The first is what a diff renders, since a written // value and a default are otherwise indistinguishable once merged. The second is why a typo in an // operator's file is visible rather than silently dropped. // +// The file and not every source, because an undeclared name means something different in each. A file +// exists to carry declared keys, so one that is not is a typo. The environment layer looks up only +// names a section declares, so it cannot produce one. The command line is a namespace this package does +// not own: most of the flags a node starts with were never configuration keys, and a misspelled one is +// refused by the command before any of this runs, so reporting those would bury the file's one typo +// under forty names working exactly as intended. +// +// One channel has a per-key hole. A reader that takes its value's exact type cannot be handed the one +// string an environment carries, so a section may refuse that channel for such a key, and the file's +// value applies instead of a value that would stop the node. The variable is still read and its value +// still discarded, and the key is reported as ignored, because a channel that quietly does nothing is +// the failure this package exists to remove. A refusal carries the reason an operator is owed, and one +// naming a key no section declares is refused in turn: it would cover nothing while reading as though +// it covered something. +// // Resolve either answers for every declared key or returns an error naming what it could not answer // for. A caller is never handed a resolution with a hole in it. // @@ -71,6 +92,21 @@ // 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. It is meaningful only on an +// exported field, since an unexported one carrying any tag is refused before the tag is read. +// +// One more becomes possible once a key can sit at the top of a file, and it could not happen while every +// key carried its section's name: two sections declaring one key, where one default renders over the +// other and which one depends on the order the sections are walked. The environment check refuses it, +// because two identical keys answer to one variable. +// +// Refusing the environment for a key is itself refused when it carries no reason. An operator told +// their variable does nothing has to be told why, and a refusal with nothing to print is worse than +// resolving the variable or leaving it alone. +// // 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. @@ -82,10 +118,17 @@ // - Not a file format. Nothing here reads or writes a configuration file. // - Not a validator. A section may state rules about its own values; this package invents none. // - Not wired. No section is registered by this package and no reader is migrated onto it. +// - Not a guard against a key and a table sharing one name. A key at the top of the file that is also +// a section's name cannot be written at all, because no file holds both a value for that name and a +// table under it, so one of the two settings is unreachable and nothing says which. One section +// declares keys at the top of the file today and none of its names is a section's, so the collision +// has no instance; a second such section is where it becomes reachable. // // # Adding a Section // -// 1. Give the section a name, and use it as the first segment of every key it declares. +// 1. Give the section a name, and use it as the first segment of every key it declares. A section +// whose settings sit at the top of the file instead declares root keys, and its name is then a +// handle for lookups and reports rather than part of any key. // 2. Register the struct the reader already uses, with a per-mode default. // 3. Assert the registration produced no Defect. // 4. Hold the derived key names against the reader, so a key that reaches nothing fails. diff --git a/config/registry/environment.go b/config/registry/environment.go new file mode 100644 index 0000000000..d3f440b178 --- /dev/null +++ b/config/registry/environment.go @@ -0,0 +1,46 @@ +package registry + +import "fmt" + +// envCannotDeliver holds the keys an environment variable cannot supply, with the reason. +var envCannotDeliver = map[string]string{} + +// RefuseFromEnvironment records that an environment variable cannot supply a key. +// +// An environment carries one string per name. Most readers cast that string into whatever the setting +// needs, so the environment works for them. A reader that takes its value's exact type instead cannot be +// handed a string at all, and no spelling of the variable would satisfy it. +// +// Resolving such a key from the environment puts an unusable value at the top of the order, and installing +// it stops the node. Leaving the channel out means the file's value applies and the node runs. That is +// deliberately not what the machinery this replaces does, which resolves the variable and refuses to +// start, so the difference is recorded rather than assumed. A value silently doing nothing is the failure +// this whole surface exists to remove, which is why the reason is required and not optional. +// +// section is the section that declares the key, so a refused key is attributable to a registration the +// way every other defect is. Whether the key is one that section declares is answered when something +// resolves, because a refusal may be recorded before the registration it belongs to. +// +// Called from the owning package, beside its registration, so the reason sits with the code that knows it. +func RefuseFromEnvironment(section, key, reason string) { + mu.Lock() + defer mu.Unlock() + if reason == "" { + defects = append(defects, Defect{Section: section, Err: fmt.Errorf( + "refusing %q from the environment with no reason; an operator whose variable is ignored has "+ + "to be told why", key)}) + return + } + envCannotDeliver[key] = reason +} + +// EnvCannotDeliver returns the keys an environment variable cannot supply, and why. +func EnvCannotDeliver() map[string]string { + mu.RLock() + defer mu.RUnlock() + out := make(map[string]string, len(envCannotDeliver)) + for key, reason := range envCannotDeliver { + out[key] = reason + } + return out +} diff --git a/config/registry/registry.go b/config/registry/registry.go index 008738680c..b34f4095de 100644 --- a/config/registry/registry.go +++ b/config/registry/registry.go @@ -26,12 +26,38 @@ const ( // Modes returns every mode a default is asked for, in a fixed order. func Modes() []Mode { return []Mode{ModeValidator, ModeFull, ModeSeed, ModeArchive} } +// IsFullnodeMode reports whether a node of this kind serves queries to callers other than itself. +// +// Stated here because more than one package needs it and they sit on opposite sides of an import edge. +// The package that owns the node a binary was started as also owns the type that describes it, and a +// section's own package needs the same fact to state a default that varies on it while being imported by +// that package rather than importing it. +// +// An archive node counts. It serves queries, which is the property this names, and it is the mode most +// easily forgotten when the rule is written out by hand. +func IsFullnodeMode(mode Mode) bool { return mode == ModeFull || mode == ModeArchive } + // Section is one registered configuration section. type Section struct { - // Name is the section's own segment, and the first segment of every key it declares. + // Name identifies the section. A lookup, a report and a defect are keyed by it, and for most + // sections it is also the first segment of every key. Name string + // Prefix is the first segment of every key this section declares, and is empty for a section whose + // keys sit at the root of the file with no section of their own. + // + // Separate from Name because the two do different jobs. A node-wide setting such as the pruning + // strategy is written at the top of app.toml and read as "pruning", so it has no segment to take a + // name from, and it still needs one to be looked up and reported under. + 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 } @@ -68,7 +94,50 @@ 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) { - keys, err := deriveKeys(name, prototype) + 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. +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. +// +// name identifies the section for lookups and reports and is not part of any key. Everything else matches +// RegisterSection: the keys come from the mapstructure tags, and the tags are the only spelling. +// +// 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, 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, excluding []string) { + found, err := deriveKeys(name, prefix, prototype) + keys, excluded := found.keys, []string(nil) + if err == nil { + keys, excluded, err = withoutExcluded(prefix, keys, excluding) + } + if err == nil { + err = refuseDeclaredInterfaces(keys, found.interfaces) + } mu.Lock() defer mu.Unlock() @@ -86,8 +155,79 @@ func RegisterSection(name string, prototype any, defaults func(Mode) any) { defects = append(defects, Defect{Section: name, Err: err}) return } - sections[name] = Section{Name: name, Keys: keys, Defaults: defaults} + sections[name] = Section{ + Name: name, Prefix: prefix, Keys: keys, Excluded: excluded, Defaults: defaults, + } + } +} + +// refuseDeclaredInterfaces refuses a declared path whose field holds an interface. +// +// What a decoder writes into an interface depends on what the field already holds rather than on the +// field's type, so two structs of one type can accept and refuse the same written value. A caller that +// rehearses a decode into a copy to learn whether the real one will succeed gets an answer about the copy, +// and the two differ exactly where their existing values do. +// +// Checked against the declared paths and not the struct's fields, because a section may exclude such a +// field. An excluded path is not declared, and how a path nobody can write decodes is not a property worth +// refusing. +func refuseDeclaredInterfaces(keys, interfaces []string) error { + if len(interfaces) == 0 { + return nil + } + declared := make(map[string]bool, len(keys)) + for _, key := range keys { + declared[key] = true + } + var bad []string + for _, key := range interfaces { + if declared[key] { + bad = append(bad, key) + } } + if len(bad) > 0 { + return fmt.Errorf("%v name fields holding an interface, so what a written value decodes to "+ + "depends on what the field already holds and not on the field's type", bad) + } + return nil +} + +// 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 + } + 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. @@ -106,7 +246,17 @@ func envNamesAreDistinct(adding []string) error { } for _, key := range adding { env := EnvName(key) - if other, taken := spellings[env]; taken { + other, taken := spellings[env] + switch { + case taken && other == key: + // Two sections declaring one key, which a prefix made impossible and a key at the root of the + // file does not. One section's default renders over the other's and which one depends on the + // order the sections are walked, so the value a node runs is decided by nothing an operator + // or a reviewer can see. Named as the one key it is, because the spelling reason below is not + // the reason here. + return fmt.Errorf("%q is declared by two sections; one default renders over the other and "+ + "which one wins depends on the order the sections are walked", key) + case taken: return fmt.Errorf("%q and %q both answer to %s, because a dot and a hyphen are the same "+ "character to the environment, so one of them can never be set from it", other, key, env) } @@ -166,49 +316,63 @@ func Keys() []string { // outside state-commit.flatkv.*. Ninety-two operator-facing keys reach their field only through a // spelling the tags do not produce, and a silent fallback is what made that invisible. Refusing to // guess is what keeps the tag authoritative. -func deriveKeys(section string, prototype any) ([]string, error) { - if section == "" { - return nil, fmt.Errorf("section name is empty") +func deriveKeys(name, prefix string, prototype any) (derived, error) { + if name == "" { + return derived{}, fmt.Errorf("section name is empty") } - if section != strings.ToLower(section) { - return nil, fmt.Errorf("section name %q is not lower case; configuration sources "+ - "enumerate lower-cased, so a key under it would never match a written one", section) + if name != strings.ToLower(name) { + return derived{}, fmt.Errorf("section name %q is not lower case; configuration sources "+ + "enumerate lower-cased, so a key under it would never match a written one", name) } - if bad, found := unaddressableChar(section); found { - return nil, fmt.Errorf("section name %q carries %q, and a section is one segment. A dotted name "+ + if bad, found := unaddressableChar(name); found { + return derived{}, fmt.Errorf("section name %q carries %q, and a section is one segment. A dotted name "+ "declares keys inside another section's subtree, where the two sections' defaults land in "+ "one map and whichever renders last silently wins; a space cannot be written in an "+ - "environment variable name at all", section, bad) + "environment variable name at all", name, bad) } if prototype == nil { - return nil, fmt.Errorf("no struct") + return derived{}, fmt.Errorf("no struct") } t := reflect.TypeOf(prototype) for t.Kind() == reflect.Ptr { t = t.Elem() } if t.Kind() != reflect.Struct { - return nil, fmt.Errorf("%s is not a struct", t.Kind()) + return derived{}, fmt.Errorf("%s is not a struct", t.Kind()) } - var keys []string - if err := walk(t, section, &keys, map[reflect.Type]bool{}); err != nil { - return nil, err + var found derived + if err := walk(t, prefix, &found, map[reflect.Type]bool{}); err != nil { + return derived{}, err } + keys := found.keys if len(keys) == 0 { - return nil, fmt.Errorf("declares no keys") + return derived{}, fmt.Errorf("declares no keys") } sort.Strings(keys) + sort.Strings(found.interfaces) // A path two fields both produce leaves one of them unreachable, and which one is not // observable: the value walk writes them into one map. That is the unaddressable-key failure // this package exists to refuse, so it cannot be allowed to arrive through the package itself. for i := 1; i < len(keys); i++ { if keys[i] == keys[i-1] { - return nil, fmt.Errorf("two fields both declare %q, so one of them is unreachable and "+ + return derived{}, fmt.Errorf("two fields both declare %q, so one of them is unreachable and "+ "which one is not observable", keys[i]) } } - return keys, nil + found.keys = keys + return found, nil +} + +// derived is what one walk of a section's type collects. +// +// Two lists rather than one, because a path whose field holds an interface is not refused where it is +// found. A section may exclude it, and an excluded path is not declared, so nothing about how it decodes +// matters. The refusal belongs after the exclusions are known. +type derived struct { + keys []string + // interfaces are paths whose field holds an interface, sorted with the keys they appear among. + interfaces []string } // walk appends the dotted keys a struct declares under prefix. @@ -216,7 +380,7 @@ func deriveKeys(section string, prototype any) ([]string, error) { // open carries the struct types on the current path, so a self-referential one is refused rather than // recursed into. A stack overflow cannot be recovered into a Defect, so this is the one refusal that // has to happen before the recursion rather than after it. -func walk(t reflect.Type, prefix string, keys *[]string, open map[reflect.Type]bool) error { +func walk(t reflect.Type, prefix string, found *derived, open map[reflect.Type]bool) error { if open[t] { return fmt.Errorf("%s is %s, which contains itself; a key space derived from it has no end", prefix, t) @@ -238,10 +402,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 { @@ -254,77 +421,127 @@ func walk(t reflect.Type, prefix string, keys *[]string, open map[reflect.Type]b if ft.Kind() != reflect.Struct { return fmt.Errorf("%s.%s is squashed but is a %s, not a struct", prefix, f.Name, ft.Kind()) } - if err := walkSubtree(ft, prefix, prefix+"."+f.Name, keys, open); err != nil { + if err := walkSubtree(ft, prefix, join(prefix, f.Name), found, open); err != nil { return err } continue } - path := prefix + "." + tag + path := join(prefix, tag) if ft.Kind() == reflect.Struct && !isLeaf(ft) { - if err := walkSubtree(ft, path, prefix+"."+f.Name, keys, open); err != nil { + if err := walkSubtree(ft, path, join(prefix, f.Name), found, open); err != nil { return err } continue } - *keys = append(*keys, path) + found.keys = append(found.keys, path) + // Recorded rather than refused. An excluded path is not declared, so how it decodes never + // matters; record decides, once the exclusions are known. + if ft.Kind() == reflect.Interface { + found.interfaces = append(found.interfaces, path) + } } return nil } +// join appends a key segment to a prefix, and returns the segment alone when there is no prefix. +func join(prefix, segment string) string { + if prefix == "" { + return segment + } + return prefix + "." + segment +} + // walkSubtree appends the keys a struct-typed field declares, and refuses one that declares none. // // A struct configuration cannot reach is a setting an operator writes into nothing. A defined type // over a leaf, an empty struct, and a struct whose every field is unexported all arrive here having // contributed nothing, and both walks agree about it, so no later check can see the loss. -func walkSubtree(t reflect.Type, path, field string, keys *[]string, open map[reflect.Type]bool) error { - before := len(*keys) - if err := walk(t, path, keys, open); err != nil { +func walkSubtree(t reflect.Type, path, field string, found *derived, open map[reflect.Type]bool) error { + before := len(found.keys) + if err := walk(t, path, found, open); err != nil { return err } - if len(*keys) == before { + if len(found.keys) == before { return fmt.Errorf("%s is a %s that declares no key, so configuration cannot reach it", field, t) } return nil } // 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) } - parts := strings.Split(tag, ",") - name = parts[0] - for _, opt := range parts[1:] { - if opt == "squash" { - squash = true - } + name, squash, remain := parseTag(tag) + if remain { + return "", false, true, nil } 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 assignedOutsideConfiguration(name) { + 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 "+ + if neverMatchesAWrittenKey(name) { + 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 +} + +// 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:] { + switch opt { + case "squash": + squash = true + case "remain": + remain = true + } + } + return parts[0], squash, remain +} + +// assignedOutsideConfiguration reports whether a tag excludes its field from configuration. +// +// Something else in the program assigns such a field, so no reader resolves a key for it, and declaring +// one would put a key in the space that reaches no field. An untagged field is refused 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. +func assignedOutsideConfiguration(name string) bool { + return name == "-" +} + +// neverMatchesAWrittenKey reports whether a key segment is spelled so that no written value can reach it. +// +// A configuration source enumerates its keys lower-cased, so a segment carrying an upper-case letter is +// one an operator can write and nothing answers. +func neverMatchesAWrittenKey(name string) bool { + return name != strings.ToLower(name) } // unaddressableChar returns the first character in a key segment that no configuration source can @@ -359,9 +576,11 @@ func isLeaf(t reflect.Type) bool { // another's declared set. func Reset() { mu.Lock() + decodedNotLookedUp = map[string]string{} defer mu.Unlock() sections = map[string]Section{} defects = nil + envCannotDeliver = map[string]string{} } // envPrefix is the environment namespace for every derived key. diff --git a/config/registry/resolve.go b/config/registry/resolve.go index dfcca8f9e3..4ebe29671a 100644 --- a/config/registry/resolve.go +++ b/config/registry/resolve.go @@ -10,13 +10,35 @@ import ( // Resolved is every declared key's value, plus what a caller has to be told about how it got there. type Resolved struct { // Values carries one value per declared key. + // + // A key's Go type depends on which source answered it, and a caller that type-asserts has to expect + // all three. A default arrives as the field's own type, so a duration is a duration and a list is a + // list. A file arrives as whatever the file format decodes to, so the same duration is text and the + // same list is a list of untyped elements. An environment variable arrives as one string, always. This + // resolves values and does not convert them, so the reader that owns a key remains the thing that + // turns any of the three into what that key means. + // + // A value is the caller's to write into. Nothing here shares storage with a section's own default. Values map[string]any // Overrides are the declared keys something other than this node's defaults supplied, sorted. // // The keys an operator has taken responsibility for, as distinct from the ones tracking the // binary's judgement. This is what a diff renders. Overrides []string - // Unknown are keys a source carried that no section declares, sorted. + // Ignored are declared keys an environment variable was set for and could not supply, sorted. + // + // Separate from Unknown because the two are different mistakes. An unknown key is one nothing reads. + // An ignored one is read, and the operator reached for the one channel that cannot carry it, so the + // value they wrote elsewhere is what applies. EnvCannotDeliver says why, per key. + Ignored []string + // Unknown are keys the file carried that no section declares, sorted. + // + // The file only, and not every source, because an undeclared name means something different in each. + // A file exists to carry declared keys, so one that is not is a typo. The environment layer looks up + // only names a section declares, so it cannot produce one at all. The command line is a namespace + // this package does not own: most of the flags a node starts with are not configuration keys, and a + // misspelled one is refused by the command before any of this runs, so reporting those would bury the + // file's one typo under forty names that are working exactly as intended. // // Reported rather than an error, because what to do about one is the caller's decision: a // generate path may want to refuse, while a boot on an operator's existing file must not. @@ -37,6 +59,16 @@ type Sources struct { Flags map[string]any } +// known reports whether this package declares defaults for a mode. +func known(mode Mode) bool { + for _, m := range Modes() { + if m == mode { + return true + } + } + return false +} + // Resolve reduces a node's configuration sources to one value per declared key. // // The precedence is stated once, in this function, and a caller cannot reorder its way to a different @@ -54,6 +86,16 @@ type Sources struct { func Resolve(mode Mode, from Sources) (Resolved, error) { var out Resolved + // Refused before anything is resolved, because a section's defaults answer per mode and a mode this + // package does not know reaches whatever each section does with an argument it cannot match. What that + // is varies by section and none of them is a decision anyone made: the upstream mode rules answer for + // an unrecognised mode as though it were a full node, so an empty string, a capitalised name or one + // with a trailing space resolves the interfaces a full node serves onto whatever asked. + if !known(mode) { + return out, fmt.Errorf("%q is not a mode this binary declares defaults for; the modes are %v", + mode, Modes()) + } + // 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. @@ -63,6 +105,17 @@ func Resolve(mode Mode, from Sources) (Resolved, error) { return out, err } declared := declaredKeys(registered) + undeliverable := EnvCannotDeliver() + // A refusal is recorded by a key, and a key that no section declares is one the environment layer + // would never have offered anyway, so the refusal protects nothing and reads as though it did. Held + // here because a refusal may be recorded before the section that declares its key registers, so this + // is the first point both sets exist. + for key := range undeliverable { + if !declared[key] { + return out, fmt.Errorf("%q is refused from the environment and no section declares it, so the "+ + "refusal covers nothing", key) + } + } out.Values = make(map[string]any, len(declared)) for key, v := range defaults { @@ -73,16 +126,25 @@ func Resolve(mode Mode, from Sources) (Resolved, error) { unknown := map[string]bool{} // Lowest precedence first, so a later source overwrites an earlier one. The one statement of the // order, which is why nothing exports it. - for _, values := range []map[string]any{ - fileValues(from.File), - envValues(declared, from.LookupEnv), - from.Flags, + fromEnv, ignored := envValues(declared, undeliverable, from.LookupEnv) + out.Ignored = ignored + for _, layer := range []struct { + values map[string]any + // namesAreAllKeys says every name in this layer is meant to be a declared key, so one that is + // not gets reported. True of the file alone; Unknown records why. + namesAreAllKeys bool + }{ + {values: fileValues(from.File), namesAreAllKeys: true}, + {values: fromEnv}, + {values: from.Flags}, } { - for key, v := range values { + for key, v := range layer.values { if !declared[key] { - // A key nothing declares cannot be resolved into anything, and silently dropping it is - // how an operator's typo becomes invisible. - unknown[key] = true + // A key nothing declares cannot be resolved into anything, and silently dropping one the + // operator meant as a setting is how a typo becomes invisible. + if layer.namesAreAllKeys { + unknown[key] = true + } continue } out.Values[key] = v @@ -128,10 +190,15 @@ func declaredKeys(registered []Section) map[string]bool { func defaultValues(mode Mode, registered []Section) (map[string]any, error) { out := map[string]any{} for _, s := range registered { - values, err := sectionValues(s.Name, s.Defaults(mode)) + values, err := sectionValues(s.Prefix, s.Defaults(mode)) 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) } @@ -221,10 +288,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 { @@ -252,18 +324,50 @@ func walkValues(v reflect.Value, prefix string, out map[string]any) error { } continue } - path := prefix + "." + tag + path := join(prefix, tag) if fv.Kind() == reflect.Struct && !isLeaf(fv.Type()) { if err := walkValues(fv, path, out); err != nil { return err } continue } - out[path] = fv.Interface() + out[path] = detach(fv) } return nil } +// detach returns a field's value with nothing shared with the struct it came from. +// +// A section's default is usually a package-level variable, so a slice or a map field hands out the +// backing array that variable holds. A caller sorting or de-duplicating a resolved list in place, which is +// what a caller producing deterministic output does, would rewrite that variable for the whole process: +// every later resolution, and every reader that copies the same struct. Two of the lists that reach here +// are deny lists, so the rewrite is silent and it is a security control. +// +// Lookup already copies a section's keys for this reason. This is the same guarantee for its values. +func detach(v reflect.Value) any { + switch v.Kind() { + case reflect.Slice: + if v.IsNil() { + return v.Interface() + } + out := reflect.MakeSlice(v.Type(), v.Len(), v.Len()) + reflect.Copy(out, v) + return out.Interface() + case reflect.Map: + if v.IsNil() { + return v.Interface() + } + out := reflect.MakeMapWithSize(v.Type(), v.Len()) + for _, key := range v.MapKeys() { + out.SetMapIndex(key, v.MapIndex(key)) + } + return out.Interface() + default: + return v.Interface() + } +} + // envValues reads the keys an environment supplies, from the caller's declared set. // // Driven by the declared set rather than by the environment, which is also what makes it complete: @@ -272,12 +376,27 @@ func walkValues(v reflect.Value, prefix string, out map[string]any) error { // declared is passed in rather than read here, so this shares Resolve's snapshot. Reading the registry // again would ask for a key the caller's declared set does not hold, and the answer would come back // only to be reported as one no section declares. -func envValues(declared map[string]bool, lookup func(string) (string, bool)) map[string]any { +func envValues(declared map[string]bool, undeliverable map[string]string, + lookup func(string) (string, bool)) (map[string]any, []string) { if lookup == nil { - return nil + return nil, nil } out := map[string]any{} + var ignored []string for key := range declared { + // A key no variable can carry is left to the sources that can. Resolving it would put a string + // at the top of the order for a reader that takes the exact type, and installing that stops the + // node. What an operator loses is the channel; what they keep is a node that boots. + // + // The variable is still read, and the value still discarded. Asking is what turns this from a + // silent skip into something a caller can report: a reason nothing can attach to an operator's + // own action is a reason nobody is ever told. + if _, refused := undeliverable[key]; refused { + if v, set := lookup(EnvName(key)); set && v != "" { + ignored = append(ignored, key) + } + continue + } // An empty value is treated as unset. A variable exported empty is far more often a shell // artefact than a deliberate empty string, and the two are indistinguishable here. The cost is // that clearing a key by exporting it empty reads as touching nothing, and Overrides will not @@ -286,7 +405,8 @@ func envValues(declared map[string]bool, lookup func(string) (string, bool)) map out[key] = v } } - return out + sort.Strings(ignored) + return out, ignored } // fileValues normalises a configuration file's keys to lower case. diff --git a/config/registry/rootkeys_test.go b/config/registry/rootkeys_test.go new file mode 100644 index 0000000000..d0a698d135 --- /dev/null +++ b/config/registry/rootkeys_test.go @@ -0,0 +1,275 @@ +package registry_test + +import ( + "reflect" + "sort" + "strings" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// nodeWide is a probe for the settings written at the top of a file rather than inside a table. +type nodeWide struct { + Pruning string `mapstructure:"pruning"` + HaltHeight uint64 `mapstructure:"halt-height"` + Concurrency int `mapstructure:"concurrency-workers"` +} + +// TestARootSectionDeclaresKeysWithNoPrefix is the whole of what registering root keys adds. +// +// Some settings are node-wide and are written at the top of a file. Giving them a section would rename +// them, and a renamed key is one an operator's existing file no longer reaches. +func TestARootSectionDeclaresKeysWithNoPrefix(t *testing.T) { + registry.Reset() + registry.RegisterRootKeys("base", &nodeWide{}, func(registry.Mode) any { + return nodeWide{Pruning: "nothing", Concurrency: 4} + }) + for _, d := range registry.Defects() { + t.Fatalf("registering root keys was refused: %v", d.Err) + } + + section, ok := registry.Lookup("base") + if !ok { + t.Fatal("the section did not register under its name, so nothing can look it up or report on it") + } + if section.Prefix != "" { + t.Errorf("the section carries prefix %q, and one here renames every key it declares", section.Prefix) + } + if got := strings.Join(section.Keys, ","); got != "concurrency-workers,halt-height,pruning" { + t.Errorf("derived %q, want the three keys with no prefix. A leading segment is a key no operator "+ + "writes", got) + } + + // The default has to render under the same prefix-free names, or a declared key states no value and + // the resolution is refused rather than short. + resolved, err := registry.Resolve(registry.ModeFull, registry.Sources{}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got := resolved.Values["pruning"]; got != "nothing" { + t.Errorf("pruning resolved to %#v, want %q", got, "nothing") + } +} + +// TestTwoSectionsCannotDeclareTheSameKey was impossible while every key carried its section's name. +// +// Two prefixes cannot collide. Two root sections can, and the default rendered for such a key would be +// whichever section the walk reached last. Refused by the environment check, which two identical keys +// reach by answering to one variable, and named as the one key it is rather than as two spellings. +func TestTwoSectionsCannotDeclareTheSameKey(t *testing.T) { + registry.Reset() + same := func(name string) { + registry.RegisterRootKeys(name, &struct { + Pruning string `mapstructure:"pruning"` + }{}, func(registry.Mode) any { + return struct { + Pruning string `mapstructure:"pruning"` + }{Pruning: "nothing"} + }) + } + same("base") + same("other") + + if _, ok := registry.Lookup("other"); ok { + t.Fatal("both sections declared the same key. One default renders over the other and which one " + + "wins depends on the order the sections are walked, so the value a node runs is not decided " + + "by anything an operator or a reviewer can see") + } + defects := registry.Defects() + if len(defects) != 1 { + t.Fatalf("recorded %d defects, want one", len(defects)) + } + // Named as one key two sections declare rather than as two spellings of one variable, which is the + // reason the same check gives for the collision it was written for. + if got := defects[0].Err.Error(); !strings.Contains(got, "is declared by two sections") { + t.Errorf("the refusal reads %q, and an identical key is not an environment spelling collision", got) + } +} + +// TestAKeyTheEnvironmentCannotDeliverIsLeftToTheOtherSources is what refusing a channel buys. +// +// The environment carries one string per name. A reader taking its value's exact type cannot be handed +// one, so resolving the variable installs a value that stops the node. Skipping it means the file's value +// applies and the node runs. +func TestAKeyTheEnvironmentCannotDeliverIsLeftToTheOtherSources(t *testing.T) { + registry.Reset() + registry.RegisterSection("probe", &struct { + Rows []any `mapstructure:"rows"` + Plain string `mapstructure:"plain"` + }{}, func(registry.Mode) any { + return struct { + Rows []any `mapstructure:"rows"` + Plain string `mapstructure:"plain"` + }{Rows: []any{}, Plain: "from the default"} + }) + registry.RefuseFromEnvironment("probe", "probe.rows", "its reader takes the exact type rather than casting") + for _, d := range registry.Defects() { + t.Fatalf("the registration was refused: %v", d.Err) + } + + // Both variables are set. Only the one the environment can carry is allowed to answer. + resolved, err := registry.Resolve(registry.ModeFull, registry.Sources{ + LookupEnv: func(name string) (string, bool) { + switch name { + case "SEID_PROBE_ROWS": + return "chain_id=pacific-1", true + case "SEID_PROBE_PLAIN": + return "from the environment", true + } + return "", false + }, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + if got := resolved.Values["probe.rows"]; !reflect.DeepEqual(got, []any{}) { + t.Errorf("probe.rows resolved to %#v (%T), want the default it was left to. Its reader takes a "+ + "list of rows, so installing the environment's string stops the node", got, got) + } + if got := resolved.Values["probe.plain"]; got != "from the environment" { + t.Errorf("probe.plain resolved to %#v; refusing one key's channel closed another's", got) + } + sort.Strings(resolved.Overrides) + if got := strings.Join(resolved.Overrides, ","); got != "probe.plain" { + t.Errorf("overrides are %q, want only probe.plain. A key nothing supplied is not one an operator "+ + "has taken responsibility for", got) + } +} + +// TestRefusingAChannelWithoutAReasonIsItselfRefused keeps the exemption from being unexplainable. +// +// A key left out of the environment layer is one whose variable does nothing, and an operator told that +// has to be told why. A refusal with no reason gives a diagnostic nothing to print. +func TestRefusingAChannelWithoutAReasonIsItselfRefused(t *testing.T) { + registry.Reset() + registry.RefuseFromEnvironment("probe", "probe.rows", "") + if len(registry.Defects()) != 1 { + t.Fatalf("recorded %d defects, want one naming the key with no reason", len(registry.Defects())) + } + if _, refused := registry.EnvCannotDeliver()["probe.rows"]; refused { + t.Error("the key was refused from the environment anyway. Its variable would then be ignored " + + "with nothing able to say why, which is worse than either resolving it or not") + } + + registry.Reset() + registry.RefuseFromEnvironment("probe", "probe.rows", "its reader takes the exact type") + if _, refused := registry.EnvCannotDeliver()["probe.rows"]; !refused { + t.Error("a refusal carrying a reason was not recorded") + } +} + +// TestAModeThisBinaryDoesNotDeclareIsRefused closes a resolution that answered for anything. +// +// A section's defaults answer per mode, and a mode this package does not know reaches whatever each +// section does with an argument it cannot match. Nothing about that is a decision anyone made: the mode +// rules these sections read answer for an unrecognised mode as though it were a full node, so an empty +// string, a capitalised name or one with a trailing space resolved the interfaces a full node serves onto +// whichever node asked. +func TestAModeThisBinaryDoesNotDeclareIsRefused(t *testing.T) { + registry.Reset() + registry.RegisterSection("probe", &struct { + Serves bool `mapstructure:"serves"` + }{}, func(mode registry.Mode) any { + return struct { + Serves bool `mapstructure:"serves"` + }{Serves: mode == registry.ModeFull || mode == registry.ModeArchive} + }) + for _, d := range registry.Defects() { + t.Fatalf("the probe was refused: %v", d.Err) + } + + for _, mode := range registry.Modes() { + if _, err := registry.Resolve(mode, registry.Sources{}); err != nil { + t.Errorf("mode %q is declared and did not resolve: %v", mode, err) + } + } + for _, mode := range []registry.Mode{"", "Validator", "validator ", "VALIDATOR", "sentry"} { + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err == nil { + t.Errorf("mode %q resolved, to serves=%v. A mode nothing declares has no answer, and the one "+ + "it reached is whatever the rules do with an argument they cannot match", + mode, resolved.Values["probe.serves"]) + } + } +} + +// TestARefusalNamingAKeyNothingDeclaresIsRefused keeps a refusal from covering nothing. +// +// A refusal is recorded by a key, so a slip in the spelling names a key no section declares. The +// environment layer would never have offered that key, so the refusal protects nothing while reading as +// though it did, and the key it was meant to cover resolves from the environment as before. +// +// Answered when something resolves rather than when the refusal is recorded, because a refusal may be +// recorded before the section declaring its key registers. Resolving is the first point both sets exist. +func TestARefusalNamingAKeyNothingDeclaresIsRefused(t *testing.T) { + registry.Reset() + registry.RegisterSection("probe", &struct { + Rows []any `mapstructure:"rows"` + }{}, func(registry.Mode) any { + return struct { + Rows []any `mapstructure:"rows"` + }{Rows: []any{}} + }) + registry.RefuseFromEnvironment("probe", "probe.rowz", "a slip in the spelling") + + if _, err := registry.Resolve(registry.ModeFull, registry.Sources{}); err == nil { + t.Error("a refusal naming a key nothing declares was accepted, so it covers nothing and the key " + + "it was written for still resolves from the environment") + } +} + +// TestAVariableSetForARefusedKeyIsReported is what makes the required reason worth requiring. +// +// The channel is skipped and the value discarded, which is the point. But an operator who set the variable +// believes otherwise, and a reason nothing can attach to their own action is a reason nobody is told. So +// the variable is still read, and the key comes back named. +func TestAVariableSetForARefusedKeyIsReported(t *testing.T) { + registry.Reset() + registry.RegisterSection("probe", &struct { + Rows []any `mapstructure:"rows"` + Plain string `mapstructure:"plain"` + }{}, func(registry.Mode) any { + return struct { + Rows []any `mapstructure:"rows"` + Plain string `mapstructure:"plain"` + }{Rows: []any{}, Plain: "from the default"} + }) + registry.RefuseFromEnvironment("probe", "probe.rows", "its reader takes the exact type") + for _, d := range registry.Defects() { + t.Fatalf("the probe was refused: %v", d.Err) + } + + resolved, err := registry.Resolve(registry.ModeFull, registry.Sources{ + LookupEnv: func(name string) (string, bool) { + if name == registry.EnvName("probe.rows") { + return "chain_id=pacific-1", true + } + return "", false + }, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + if got := strings.Join(resolved.Ignored, ","); got != "probe.rows" { + t.Errorf("the ignored variables are %q, want probe.rows. An operator set it and nothing here can "+ + "tell them it did nothing", got) + } + if !reflect.DeepEqual(resolved.Values["probe.rows"], []any{}) { + t.Errorf("probe.rows resolved to %#v, and the channel was supposed to be skipped", + resolved.Values["probe.rows"]) + } + + // A refused key nobody set is not news, so it is not reported. + quiet, err := registry.Resolve(registry.ModeFull, registry.Sources{ + LookupEnv: func(string) (string, bool) { return "", false }, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if len(quiet.Ignored) != 0 { + t.Errorf("a refused key nobody set is reported as ignored: %v", quiet.Ignored) + } +} diff --git a/config/registry/spec_test.go b/config/registry/spec_test.go index cddce89064..df3290c32b 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) { @@ -758,9 +759,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 +810,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 +1350,239 @@ 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 configuration struct uses it for a field something else +// in the program assigns. +// +// 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 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"` + 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. 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{}) + 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 an operator could write a key that reaches no " + + "field") + } + + // 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{} }) + 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) + } +} + +// 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) { + 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) { + 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) { + 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) + } +} + +// TestOnlyTheFileReportsAKeyNoSectionDeclares holds the one distinction between the three layers. +// +// An undeclared name means something different in each. In a file it is a typo, and reporting it is the +// only way an operator learns their setting does nothing. On the command line it is the ordinary case: +// most of the flags a node starts with are not configuration keys, so reporting them would produce a +// warning naming forty flags that work on every boot, with the file's one real typo somewhere inside it. +func TestOnlyTheFileReportsAKeyNoSectionDeclares(t *testing.T) { + type layers struct { + Kept string `mapstructure:"kept"` + } + registry.RegisterSection("layers_undeclared_names", &layers{}, func(registry.Mode) any { + return layers{Kept: "declared"} + }) + + resolved, err := registry.Resolve(registry.ModeValidator, registry.Sources{ + File: map[string]any{ + "layers_undeclared_names.kept": "from-file", + "layers_undeclared_names.typo": "x", + }, + Flags: map[string]any{"home": "/tmp", "log_level": "info"}, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + + for _, key := range resolved.Unknown { + if key == "home" || key == "log_level" { + t.Errorf("%q is reported as a key no section declares, and it is a flag name. Every boot "+ + "carries flags that were never configuration keys, so this fires always and the file's "+ + "one real typo is somewhere inside a list of forty", key) + } + } + if !contains(resolved.Unknown, "layers_undeclared_names.typo") { + t.Errorf("Unknown is %v and does not name the file's misspelled key. An operator learns their "+ + "setting does nothing only from this", resolved.Unknown) + } + if got := resolved.Values["layers_undeclared_names.kept"]; got != "from-file" { + t.Errorf("layers_undeclared_names.kept is %#v, want the file's value; not reporting a layer's "+ + "undeclared names must not stop its declared ones applying", got) + } +} + +// contains reports whether a sorted key list holds a key. +func contains(keys []string, want string) bool { + for _, key := range keys { + if key == want { + return true + } + } + return false +} + +// TestADeclaredInterfaceFieldIsRefusedAndAnExcludedOneIsNot holds the one property a rehearsed decode +// rests on. +// +// What a decoder writes into an interface depends on what the field already holds, so two structs of the +// same type can accept and refuse the same written value. A caller that decodes into a copy first to learn +// whether the real decode will succeed gets an answer about the copy, and the two differ exactly where +// their existing values do. +// +// Both directions matter. A section that declares such a field is refused, because nothing downstream can +// reason about it. A section that excludes it registers, because a path nobody can write has no decode to +// reason about, and the fields that carry this shape in practice are settings a reader has removed. +func TestADeclaredInterfaceFieldIsRefusedAndAnExcludedOneIsNot(t *testing.T) { + type holdsAny struct { + Kept string `mapstructure:"kept"` + Removed *any `mapstructure:"removed"` + } + + registry.RegisterSection("interface_declared", &holdsAny{}, func(registry.Mode) any { + return holdsAny{Kept: "a"} + }) + if _, ok := registry.Lookup("interface_declared"); ok { + t.Error("a section declaring a field that holds an interface registered") + } + var named bool + for _, d := range registry.Defects() { + if d.Section == "interface_declared" && strings.Contains(d.Err.Error(), "holding an interface") { + named = true + } + } + if !named { + t.Errorf("no defect says the field holds an interface; Defects: %v", registry.Defects()) + } + + registry.RegisterSectionExcluding("interface_excluded", &holdsAny{}, func(registry.Mode) any { + return holdsAny{Kept: "a"} + }, "removed") + registered, ok := registry.Lookup("interface_excluded") + if !ok { + t.Fatalf("excluding the field did not make the section usable; Defects: %v", registry.Defects()) + } + if want := []string{"interface_excluded.kept"}; !reflect.DeepEqual(registered.Keys, want) { + t.Errorf("declares %v, want %v", registered.Keys, want) + } +} diff --git a/config/tendermintbase/tendermintbase.go b/config/tendermintbase/tendermintbase.go new file mode 100644 index 0000000000..545e2ee254 --- /dev/null +++ b/config/tendermintbase/tendermintbase.go @@ -0,0 +1,277 @@ +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" + ConsensusSectionName = "consensus" + MempoolSectionName = "mempool" + + StateSyncSectionName = "statesync" + TxIndexSectionName = "tx-index" + InstrumentationSectionName = "instrumentation" + PrivValidatorSectionName = "priv-validator" + SelfRemediationSectionName = "self-remediation" + + // RootSectionName identifies the keys that sit at the top of the file with no table of their own. The + // name is for lookups and reports and is not part of any key. + RootSectionName = "node_base" +) + +// notWritableInThisFile are root paths this section does not declare. +// +// Neither is a setting an operator can usefully write here. The home directory is where this file is found, +// so a value inside it would be the file naming its own location, and the command line already carries it. +// The node mode is the same fact the file states at the top under its own name, and declaring a second +// spelling would let the two disagree, with the resolution answering for one and the node reading the +// other. +var notWritableInThisFile = []string{"home", "mode"} + +// nodeRootSchema declares the keys that sit at the root of the node's configuration file. +// +// The node's own top-level type carries these and the nine tables both, so declaring against it directly +// would declare every table's keys a second time. This squashes the same base group that type squashes, so +// fourteen spellings still come from the node's own tags, and restates only the two fields it holds beside +// that group. A test holds those two against it. +type nodeRootSchema struct { + tmcfg.BaseConfig `mapstructure:",squash"` + + AutobahnConfigFile string `mapstructure:"autobahn-config-file"` + HashVaultDisabledUnsafe bool `mapstructure:"hash-vault-disabled-unsafe"` +} + +// removedSettings are the consensus paths this section does not declare. +// +// Every one is a setting the node removed, and the struct marks each field deprecated. The fields are kept +// so a decode can tell that an operator set one, and declaring any of them would offer a key that changes +// nothing about how the node runs. +// +// The reader has a check that names the removed settings an operator wrote, and it reaches eight of these +// fifteen. Six are durations or booleans, where a written zero and an unwritten field are the same value, +// so no check can tell them apart. One more the check simply omits. Nothing calls the check in any case, so +// leaving these out of the file is what an operator actually gets. +var removedSettings = []string{ + "unsafe-overrides-enabled", + "unsafe-propose-timeout-override", + "unsafe-propose-timeout-delta-override", + "unsafe-vote-timeout-override", + "unsafe-vote-timeout-delta-override", + "unsafe-commit-timeout-override", + "unsafe-bypass-commit-timeout-override", + "timeout-propose", + "timeout-propose-delta", + "timeout-prevote", + "timeout-prevote-delta", + "timeout-precommit", + "timeout-precommit-delta", + "timeout-commit", + "skip-timeout-commit", +} + +// 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, + notDeclaredBy(P2PSectionName, filledFromTheCommandLine, "max-outbound-connections")...) + registry.RegisterSectionExcluding(RPCSectionName, &tmcfg.RPCConfig{}, rpcDefaults, + filledFromTheCommandLine) + registry.RegisterSectionExcluding(ConsensusSectionName, &tmcfg.ConsensusConfig{}, consensusDefaults, + append([]string{filledFromTheCommandLine}, removedSettings...)...) + registry.RegisterSectionExcluding(MempoolSectionName, &tmcfg.MempoolConfig{}, mempoolDefaults, + filledFromTheCommandLine) + registry.RegisterSectionExcluding(StateSyncSectionName, &tmcfg.StateSyncConfig{}, stateSyncDefaults, + notDeclaredBy(StateSyncSectionName, "rpc-servers")...) + registry.RegisterSection(TxIndexSectionName, &tmcfg.TxIndexConfig{}, txIndexDefaults) + registry.RegisterSection(InstrumentationSectionName, &tmcfg.InstrumentationConfig{}, + instrumentationDefaults) + registry.RegisterSectionExcluding(PrivValidatorSectionName, &tmcfg.PrivValidatorConfig{}, + privValidatorDefaults, filledFromTheCommandLine) + registry.RegisterSection(SelfRemediationSectionName, &tmcfg.SelfRemediationConfig{}, + selfRemediationDefaults) + registry.RegisterRootKeysExcluding(RootSectionName, &nodeRootSchema{}, rootDefaults, + notDeclaredBy(RootSectionName, append(notWritableInThisFile, noLongerHasAnyEffect...)...)...) + + // Each of these reaches its reader by a decode rather than a lookup, so the boot delivers them a + // second way. Declared in the same loop that names them, so a section registered above and forgotten + // here would be a section this package does not list at all. + for _, name := range declaredSectionNames() { + registry.DeclareDecodedNotLookedUp(name, + "decoded into the node's own configuration struct by the boot's handler, which reads that "+ + "file once; nothing looks these keys up afterwards") + } +} + +// declaredSectionNames are the sections this package registers. +// +// One list, read by the registration that marks them all as decoded and by the test that holds the two +// against each other, so a section can not be registered without being delivered. +func declaredSectionNames() []string { + return []string{ + P2PSectionName, RPCSectionName, ConsensusSectionName, MempoolSectionName, StateSyncSectionName, + TxIndexSectionName, InstrumentationSectionName, PrivValidatorSectionName, + SelfRemediationSectionName, RootSectionName, + } +} + +// notDeclaredBy gathers every path one section leaves out, from the reasons that apply to it. +// +// Gathered rather than written out per registration, because a path left out for a reason that covers +// several sections is easy to add to one and forget in the others. +func notDeclaredBy(section string, also ...string) []string { + out := append([]string{}, also...) + out = append(out, writtenBySomethingOutsideTheBinary[section]...) + out = append(out, forTestsOnly[section]...) + return out +} + +// 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. A declared value is therefore what a +// generated file carries, and a change to either half moves this with it. +// +// 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. +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" + +// writtenBySomethingOutsideTheBinary are paths a node's own file receives from elsewhere at boot. +// +// The rule that keeps these out is not that the binary fills them in, which is what the root directory +// does. It is that something else does, and this file cannot see it. The cluster's node controller resolves +// a peer set from live discovery and patches the addresses in; a node computes a trust height and hash from +// the chain tip each time it starts; a moniker is stamped per instance. A value declared here would be +// decoded over whichever of those already ran, and the file it came from would keep saying otherwise. +// +// The moniker has a second reason on its own. Its default is the host name of whatever machine resolved it, +// so no two machines agree on what this key declares. +var writtenBySomethingOutsideTheBinary = map[string][]string{ + P2PSectionName: {"external-address", "persistent-peers"}, + StateSyncSectionName: {"trust-height", "trust-hash"}, + RootSectionName: {"moniker"}, +} + +// noLongerHasAnyEffect are paths a reader keeps and ignores. +// +// The out-of-process application interface was removed, and the flag that carries this key is marked +// deprecated where it is declared, saying the flag is ignored. A declared key whose only effect is nothing +// is a setting an operator can spend an afternoon on. +var noLongerHasAnyEffect = []string{"abci"} + +// forTestsOnly are paths that exist to make a node misbehave. +// +// One makes every dial fail and one runs the node against a stub application. Neither has a use on a real +// network, and both are reachable from a file an operator edits by hand, on a node whose request surface +// faces the outside. +var forTestsOnly = map[string][]string{ + P2PSectionName: {"test-dial-fail"}, + RootSectionName: {"mock-app"}, +} + +// The other path the peer-to-peer section does not declare. +// +// The outbound connection ceiling 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. + +// 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 } + +// consensusDefaults is what a generated file carries for the consensus section. +// +// The same values for every mode. How long a node waits at each step of a round has to agree across the +// validator set for the set to reach a decision, so a value that followed from the kind of node asking +// would be this package proposing that they disagree. +func consensusDefaults(mode registry.Mode) any { return *forMode(mode).Consensus } + +// mempoolDefaults is what a generated file carries for the mempool section. +// +// The same values for every mode. What a node holds before a transaction is decided is a limit on its own +// memory and bandwidth, and nothing in the binary makes one follow from what kind of node is asking. +func mempoolDefaults(mode registry.Mode) any { return *forMode(mode).Mempool } + +// The one path the state sync section does not declare. +// +// The list of servers to fetch a snapshot from has no default and cannot have one: the addresses are the +// operator's own peers. An empty list is not a value they can inherit, and any address written here would +// name a host this binary does not know exists. + +// stateSyncDefaults is what a generated file carries for the state sync section. +// +// The same values for every mode. Whether a node starts from a snapshot is a decision about how it is being +// brought up rather than about what it will be, and every kind of node can be brought up either way. +func stateSyncDefaults(mode registry.Mode) any { return *forMode(mode).StateSync } + +// txIndexDefaults is what a generated file carries for the transaction index section. +// +// Answered per mode, for the indexer alone. A node that serves queries indexes transactions so it can +// answer them, and a validator and a seed serve none, so they index nothing and keep the write. +func txIndexDefaults(mode registry.Mode) any { return *forMode(mode).TxIndex } + +// instrumentationDefaults is what a generated file carries for the instrumentation section. +// +// The same values for every mode. What a node measures about itself is a decision about how it is operated, +// and an operator who collects metrics collects them from every kind of node they run. +func instrumentationDefaults(mode registry.Mode) any { return *forMode(mode).Instrumentation } + +// privValidatorDefaults is what a generated file carries for the signing key section. +// +// The same values for every mode. These are paths and an address for reaching a signer, and a node that +// does not sign simply does not use them, so varying them by kind would state a difference the binary does +// not make. +func privValidatorDefaults(mode registry.Mode) any { return *forMode(mode).PrivValidator } + +// rootDefaults is what a generated file carries at the top of the node's configuration file. +// +// The same values for every mode. These name where a node keeps its data and how it logs, and nothing in +// the binary makes either follow from what kind of node is asking. +func rootDefaults(mode registry.Mode) any { + live := forMode(mode) + return nodeRootSchema{ + BaseConfig: live.BaseConfig, + AutobahnConfigFile: live.AutobahnConfigFile, + HashVaultDisabledUnsafe: live.HashVaultDisabledUnsafe, + } +} + +// selfRemediationDefaults is what a generated file carries for the self remediation section. +// +// The same values for every mode. These are the thresholds at which a node restarts itself, and each one +// describes a node that has stopped making progress, which is the same condition whatever the node is for. +func selfRemediationDefaults(mode registry.Mode) any { return *forMode(mode).SelfRemediation } diff --git a/config/tendermintbase/tendermintbase_test.go b/config/tendermintbase/tendermintbase_test.go new file mode 100644 index 0000000000..0cbc91d749 --- /dev/null +++ b/config/tendermintbase/tendermintbase_test.go @@ -0,0 +1,594 @@ +package tendermintbase + +import ( + "fmt" + "reflect" + "slices" + "sort" + "strings" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" + tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config" + "github.com/spf13/viper" +) + +// 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", + }, + "tx-index.indexer": { + registry.ModeValidator: "[null]", + registry.ModeSeed: "[null]", + registry.ModeFull: "[kv]", + registry.ModeArchive: "[kv]", + }, +} + +// declaredSections are the sections this package registers, so a test walks the set rather than a list that +// has to be extended alongside it. +func declaredSections() []string { + return []string{ + P2PSectionName, RPCSectionName, ConsensusSectionName, MempoolSectionName, + StateSyncSectionName, TxIndexSectionName, InstrumentationSectionName, + PrivValidatorSectionName, SelfRemediationSectionName, + } +} + +// ours reports whether a key belongs to a section this package registers. +func ours(key string) bool { + for _, name := range declaredSections() { + if strings.HasPrefix(key, name+".") { + return true + } + } + return 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 !ours(key) { + 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{}, 5}, + {RPCSectionName, &tmcfg.RPCConfig{}, 1}, + {ConsensusSectionName, &tmcfg.ConsensusConfig{}, len(removedSettings) + 1}, + {MempoolSectionName, &tmcfg.MempoolConfig{}, 1}, + {StateSyncSectionName, &tmcfg.StateSyncConfig{}, 3}, + {TxIndexSectionName, &tmcfg.TxIndexConfig{}, 0}, + {InstrumentationSectionName, &tmcfg.InstrumentationConfig{}, 0}, + {PrivValidatorSectionName, &tmcfg.PrivValidatorConfig{}, 1}, + {SelfRemediationSectionName, &tmcfg.SelfRemediationConfig{}, 0}, + } { + 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 +} + +// warningCannotName are the removed settings the reader's own deprecation check does not report. +// +// Six are durations or booleans, where a written zero and an unwritten field hold the same value, so the +// check has nothing to test. The seventh is a pointer the check could name and does not. Recorded so that +// making the check complete fails here rather than leaving a sentence quietly stale. +var warningCannotName = map[string]bool{ + "unsafe-overrides-enabled": true, + "unsafe-propose-timeout-override": true, + "unsafe-propose-timeout-delta-override": true, + "unsafe-vote-timeout-override": true, + "unsafe-vote-timeout-delta-override": true, + "unsafe-commit-timeout-override": true, + "unsafe-bypass-commit-timeout-override": true, +} + +// TestTheExcludedConsensusPathsAreTheRemovedOnes ties the exclusion list to the struct's own marking. +// +// Each excluded path has to name a field the struct itself marks as deprecated, and no declared path may. +// The struct is the authority rather than the deprecation warning, because that warning is incomplete: it +// names eight of these fifteen. +// +// Nothing in the binary calls the warning either, so an operator who still has one of these in their file +// gets no error and no warning and the value is quietly ignored. That is what the exclusion is carrying. It +// keeps the key out of the new format rather than relying on a diagnostic that never runs. +func TestTheExcludedConsensusPathsAreTheRemovedOnes(t *testing.T) { + registered, ok := registry.Lookup(ConsensusSectionName) + if !ok { + t.Fatalf("%s is not registered; Defects: %v", ConsensusSectionName, registry.Defects()) + } + marked := deprecatedPaths(reflect.TypeOf(tmcfg.ConsensusConfig{})) + + excluded := map[string]bool{} + for _, key := range registered.Excluded { + excluded[key] = true + } + if want := len(removedSettings) + 1; len(excluded) != want { + t.Errorf("the section excludes %d paths and %d were expected, being the removed settings and the "+ + "root directory", len(excluded), want) + } + for _, rel := range removedSettings { + key := ConsensusSectionName + "." + rel + if !excluded[key] { + t.Errorf("%s is listed as removed and the section does not exclude it", key) + } + if !marked[rel] { + t.Errorf("%s is excluded as a removed setting and the struct does not mark its field "+ + "deprecated, so it is a setting an operator can use and belongs declared", key) + } + delete(marked, rel) + } + for rel := range marked { + t.Errorf("%s.%s names a field the struct marks deprecated and the section declares it", + ConsensusSectionName, rel) + } + + for _, key := range registered.Keys { + rel := strings.TrimPrefix(key, ConsensusSectionName+".") + if err := writtenThenChecked(t, rel); err != nil { + t.Errorf("%s is declared and the deprecation warning names it: %v", key, err) + } + } +} + +// TestTheDeprecationWarningReachesTheRecordedSubset measures the gap in the reader's own check. +// +// Eight of the fifteen removed settings make the warning name them and seven cannot, so an operator who +// wrote one of those seven would get nothing back even from a caller that ran the check. Held so that +// making the check complete shows up as a failure rather than as a sentence going quietly stale. +func TestTheDeprecationWarningReachesTheRecordedSubset(t *testing.T) { + for _, rel := range removedSettings { + err := writtenThenChecked(t, rel) + switch { + case warningCannotName[rel] && err != nil: + t.Errorf("the warning now names %s, so it reaches one more removed setting and the row "+ + "should go", rel) + case !warningCannotName[rel] && err == nil: + t.Errorf("the warning no longer names %s, so one more removed setting is now silent", rel) + } + } +} + +// deprecatedPaths returns the mapstructure names of the fields a struct marks deprecated. +func deprecatedPaths(t reflect.Type) map[string]bool { + out := map[string]bool{} + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + if !strings.HasPrefix(f.Name, "Deprecated") { + continue + } + if tag, ok := f.Tag.Lookup("mapstructure"); ok { + out[strings.Split(tag, ",")[0]] = true + } + } + return out +} + +// writtenThenChecked writes one consensus path into a configuration and returns what the reader's +// deprecation warning says about it. +// +// Written and decoded rather than assigned, because a removed setting is detected by its field being +// non-nil after a decode, and assigning it directly would skip the step under test. +func writtenThenChecked(t *testing.T, rel string) error { + t.Helper() + conf := tmcfg.DefaultConfig() + v := viper.New() + v.SetConfigType("toml") + body := "[consensus]\n" + rel + " = " + probeValueFor(rel) + "\n" + if err := v.ReadConfig(strings.NewReader(body)); err != nil { + t.Fatalf("compose a file setting %s: %v", rel, err) + } + if err := v.Unmarshal(conf); err != nil { + t.Skipf("%s does not decode from the probe value: %v", rel, err) + } + return conf.DeprecatedFieldWarning() +} + +// probeValueFor returns a written value of the right shape for a consensus path. +// +// Three shapes appear: a duration written as a string, a boolean, and a whole number. +func probeValueFor(rel string) string { + switch { + case strings.HasPrefix(rel, "skip-") || strings.HasPrefix(rel, "unsafe-") || + strings.HasPrefix(rel, "double-sign-") || strings.HasSuffix(rel, "-enabled"): + return "true" + case strings.Contains(rel, "timeout") || strings.Contains(rel, "-delta") || + strings.Contains(rel, "interval") || strings.Contains(rel, "period"): + return "\"1s\"" + default: + return "1" + } +} + +// TestTheStateSyncExclusionIsThePathWithNoDefault names why that section leaves one path out. +// +// The servers to fetch a snapshot from are the operator's own peers, so there is no value to inherit. An +// empty list is not a default an operator can start from, and an address written here would name a host +// this binary cannot know about. If the node ever ships one, this fails and the key should be declared. +func TestTheStateSyncExclusionIsThePathWithNoDefault(t *testing.T) { + registered, ok := registry.Lookup(StateSyncSectionName) + if !ok { + t.Fatalf("%s is not registered; Defects: %v", StateSyncSectionName, registry.Defects()) + } + if !slices.Contains(registered.Excluded, StateSyncSectionName+".rpc-servers") { + t.Fatalf("excluded is %v and does not name the snapshot servers", registered.Excluded) + } + if got := tmcfg.DefaultStateSyncConfig().RPCServers; len(got) != 0 { + t.Errorf("the node now defaults the snapshot servers to %v, so it states a value and the key "+ + "belongs declared rather than excluded", got) + } +} + +// TestEverySectionThisPackageRegistersIsUsable is the check no single section here can make. +// +// A registration the registry cannot use is recorded rather than panicked, so a section that failed to +// register is absent rather than loud, and two of the refusals depend on what else has registered. Nothing +// is enumerated beyond the section names this package owns, so adding one is covered by adding it there. +func TestEverySectionThisPackageRegistersIsUsable(t *testing.T) { + for _, name := range declaredSections() { + registered, ok := registry.Lookup(name) + if !ok { + t.Errorf("%s is not registered; Defects: %v", name, registry.Defects()) + continue + } + if len(registered.Keys) == 0 { + t.Errorf("%s registered and declares no key", name) + } + } + for _, d := range registry.Defects() { + t.Errorf("the registry refused %s: %v", d.Section, d.Err) + } +} + +// TestTheRootSchemaCarriesWhatTheNodesOwnTypeCarries closes the one place a spelling is restated. +// +// The root section declares against a schema rather than the node's top-level type, because that type +// carries the nine tables as well and declaring against it would declare their keys twice. The schema +// squashes the same base group, so fourteen keys still derive from the node's own tags, and it restates two +// fields by hand. This holds those two to the type they came from: name, tag and type each, and the count of +// non-table fields, so a third one appearing there fails here rather than going undeclared. +func TestTheRootSchemaCarriesWhatTheNodesOwnTypeCarries(t *testing.T) { + live := reflect.TypeOf(tmcfg.Config{}) + schema := reflect.TypeOf(nodeRootSchema{}) + + var restated int + for i := 0; i < live.NumField(); i++ { + f := live.Field(i) + tag, ok := f.Tag.Lookup("mapstructure") + if !ok { + continue + } + // The squashed base group and the nine tables are not restated; everything else is. + if strings.Contains(tag, "squash") { + continue + } + if f.Type.Kind() == reflect.Pointer && f.Type.Elem().Kind() == reflect.Struct { + continue + } + restated++ + + got, found := schema.FieldByName(f.Name) + if !found { + t.Errorf("the node's type carries %s (%s) at its root and the schema does not, so the key is "+ + "not declared at all", f.Name, tag) + continue + } + if want := got.Tag.Get("mapstructure"); want != tag { + t.Errorf("%s is tagged %q on the node's type and %q here, so the declared key is not the one "+ + "the reader decodes", f.Name, tag, want) + } + if got.Type != f.Type { + t.Errorf("%s is a %s on the node's type and a %s here, so the declared value has a shape the "+ + "reader does not read", f.Name, f.Type, got.Type) + } + } + + // The schema holds the squashed group plus exactly the restated fields, so a field added here that the + // node's type does not carry fails too. + if want := restated + 1; schema.NumField() != want { + t.Errorf("the schema carries %d fields and the node's type has %d root fields beside the squashed "+ + "group, so %d were expected", schema.NumField(), restated, want) + } +} + +// TestTheRootPathsLeftOutAreTheOnesTheFileAlreadyStates names why two root paths are not declared. +// +// The home directory is where this file is found, so a value inside it would be the file naming its own +// location, and the command line already carries it. The node mode is the fact the file states at the top +// under its own name, and a second spelling would let the two disagree: the resolution answers for one and +// the node reads the other. +// +// Written out here rather than read from the list the registration uses. Comparing that list against itself +// agrees however it changes, so a path dropped from it would leave this passing while the key became +// declared. +func TestTheRootPathsLeftOutAreTheOnesTheFileAlreadyStates(t *testing.T) { + registered, ok := registry.Lookup(RootSectionName) + if !ok { + t.Fatalf("%s is not registered; Defects: %v", RootSectionName, registry.Defects()) + } + declared := map[string]bool{} + for _, key := range registered.Keys { + declared[key] = true + } + for key, why := range map[string]string{ + "home": "the file's own location, which the command line carries", + "mode": "the fact the file states at the top under its own name", + } { + if declared[key] { + t.Errorf("%q is declared at the root and it is %s, so an operator can write a second value "+ + "for something already settled", key, why) + } + } + for _, key := range []string{"abci", "mock-app", "moniker"} { + if declared[key] { + t.Errorf("%q is declared at the root and it is left out for a reason of its own", key) + } + } +} + +// TestNoRootKeyCollidesWithAnotherSectionsName covers the collision the registry does not refuse. +// +// A key at the top of the file that is also a section's name cannot be written: no file holds both a value +// for that name and a table under it, so one of the two settings is unreachable and nothing says which. The +// registry does not catch it, and this package is the first to declare root keys beside another package's, +// so the check belongs here until it moves. +func TestNoRootKeyCollidesWithAnotherSectionsName(t *testing.T) { + sections := map[string]bool{} + for _, s := range registry.Sections() { + if s.Prefix != "" { + sections[s.Prefix] = true + } + } + for _, s := range registry.Sections() { + if s.Prefix != "" { + continue + } + for _, key := range s.Keys { + if sections[key] { + t.Errorf("%s declares %q at the top of the file and a section is named %q, so one of the "+ + "two cannot be written and nothing reports which", s.Name, key, key) + } + } + } +} + +// 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) + } + } + } +} + +// TestEverySectionThisPackageRegistersIsDeliveredByADecode is the partition, held from this side. +// +// A section reaches its reader one of two ways and the registry cannot tell which, so it is declared. A +// section that declares nothing is treated as read by a lookup, which is right for almost every section +// elsewhere and silently wrong for every one of these: its keys would resolve, install into the source a +// node reads, and change nothing the node runs. That is the exact failure this key space exists to remove. +// +// So the set is held both ways. Every section this package registers has to be declared decoded, and no +// section it does not register may be, because a section marked decoded whose values nothing decodes is +// undelivered in the other direction. +func TestEverySectionThisPackageRegistersIsDeliveredByADecode(t *testing.T) { + mine := map[string]bool{} + for _, name := range declaredSectionNames() { + mine[name] = true + if !registry.DecodedNotLookedUp(name) { + t.Errorf("%s is registered here and is not declared as delivered by a decode, so its keys "+ + "would be installed into a source nothing reads them from", name) + } + } + for name, why := range registry.DecodedSections() { + if !mine[name] { + continue + } + if why == "" { + t.Errorf("%s is declared decoded with no reason naming what decodes it", name) + } + } + if wrong := registry.UndeliveredSections(mine); len(wrong) > 0 { + t.Errorf("these sections disagree with what this package expects of them: %v. A section is "+ + "either read by a lookup or read by a decode, and one delivered the other way changes "+ + "nothing a node runs", wrong) + } +} + +// TestThePathsWrittenFromOutsideTheBinaryAreNotDeclared covers the exclusions with no local cause. +// +// Nothing in this repository fills these in, which is why declaring them looks harmless from here. A +// cluster controller resolves a peer set from live discovery and patches the addresses into the node's own +// file; a node computes a trust height and hash from the chain tip each time it starts; a moniker is +// stamped per instance. A declared value would be decoded over whichever of those already ran, and the +// only record of the change would be in memory. +func TestThePathsWrittenFromOutsideTheBinaryAreNotDeclared(t *testing.T) { + for section, paths := range writtenBySomethingOutsideTheBinary { + registered, ok := registry.Lookup(section) + if !ok { + t.Errorf("%s is not registered; Defects: %v", section, registry.Defects()) + continue + } + declared := map[string]bool{} + for _, key := range registered.Keys { + declared[key] = true + } + for _, rel := range paths { + key := rel + if registered.Prefix != "" { + key = registered.Prefix + "." + rel + } + if declared[key] { + t.Errorf("%s is declared, and something outside this binary writes it into the node's "+ + "own file at boot. A value from here would be applied over that, in memory only", key) + } + if !slices.Contains(registered.Excluded, key) { + t.Errorf("%s is neither declared nor excluded, so the exclusion naming it covers nothing "+ + "and the registration should have been refused", key) + } + } + } +} diff --git a/evmrpc/config/register.go b/evmrpc/config/register.go new file mode 100644 index 0000000000..3e0790895b --- /dev/null +++ b/evmrpc/config/register.go @@ -0,0 +1,40 @@ +package config + +import "github.com/sei-protocol/sei-chain/config/registry" + +// SectionName is this section's name in the configuration key space. +const SectionName = "evm" + +// Registration puts this package's configuration section in the registry. +// +// The owning package registers its own section, so the struct, the values and the keys come from one +// place. This section's mapstructure tags already spell the keys its reader resolves, so the registry +// derives what a node reads rather than restating them. +func init() { + registry.RegisterSection(SectionName, &Config{}, defaults) +} + +// defaults is what the seid init command writes for a node of this kind. +// +// That command applies the same mode rule to this section's own defaults and renders the result, and what +// it renders is passed through rather than refilled from a mode-blind copy, so a declared value here is the +// value that reaches the file. +// +// The two interface toggles are what the rule changes. A full node and an archive node serve queries, which +// is what these interfaces are for; a validator and a seed serve none, and leaving them open would put a +// public request surface on the node that holds a signing key. The rule is read from the registry rather +// than restated, because the package that owns the node mode imports this one and cannot be imported back. +// +// Two values come from the machine rather than from a decision, and they are not one case. The worker pool +// has a portable answer: the pool re-measures whenever the value it is given is not positive, so a file +// carrying zero lets every node size itself, and a caller rendering into a file should write that rather +// than this. The simulation call limit has no portable answer, because zero there is not a request to +// measure but the absence of a limit, and the limit is the only bound on how many simulations a node runs +// at once. Both describe the host that resolved them, so neither travels. +func defaults(mode registry.Mode) any { + cfg := DefaultConfig + serves := registry.IsFullnodeMode(mode) + cfg.HTTPEnabled = serves + cfg.WSEnabled = serves + return cfg +} diff --git a/evmrpc/config/register_test.go b/evmrpc/config/register_test.go new file mode 100644 index 0000000000..23eab42ea5 --- /dev/null +++ b/evmrpc/config/register_test.go @@ -0,0 +1,121 @@ +package config + +import ( + "reflect" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestDeclaredKeysAreTheOnesItsReaderResolves holds the derived keys against the reader's own constants. +// +// The section registers the struct its reader fills, so a mapstructure tag is the only spelling of these +// keys and there is no second list to fall behind. What remains is the constants ReadConfig looks up, +// which state the same keys again in the same file, and a rename that moves one and not the other +// compiles. +// +// Written out rather than derived from the struct, because a list derived from the same tags would agree +// with itself whatever those tags said. +func TestDeclaredKeysAreTheOnesItsReaderResolves(t *testing.T) { + for _, defect := range registry.Defects() { + if defect.Section == SectionName { + t.Fatalf("%s was refused, so none of its keys is declared: %v", SectionName, defect.Err) + } + } + want := []string{ + flagHTTPEnabled, flagHTTPPort, flagWSEnabled, flagWSPort, + flagReadTimeout, flagReadHeaderTimeout, flagWriteTimeout, flagIdleTimeout, + flagSimulationGasLimit, flagSimulationEVMTimeout, flagCORSOrigins, flagWSOrigins, + flagFilterTimeout, flagMaxTxPoolTxs, flagCheckTxTimeout, flagSlow, + flagEnableSimulation, flagDenyList, flagMaxLogNoBlock, flagMaxLogBytes, + flagMaxBlocksForLog, flagMaxEstimateGasCalls, flagMaxStateOverrideAccounts, + flagMaxStateOverrideSlots, flagMaxSubscriptionsNewHead, flagMaxSubscriptionsLogs, + flagEnableTestAPI, flagMaxConcurrentTraceCalls, flagMaxConcurrentSimulationCalls, + flagMaxTraceLookbackBlocks, flagTraceTimeout, flagMaxTraceStructLogBytes, + flagTraceAllowedTracers, flagTraceAllowJSTracers, flagEnableParallelizedBlockTrace, + flagRPCStatsInterval, flagWorkerPoolSize, flagWorkerQueueSize, flagEVMLegacySeiApis, + flagTraceBakeEnabled, flagTraceBakeWorkers, flagTraceBakeQueueSize, flagTraceBakeTracers, + flagTraceBakeWindowBlocks, flagTraceBakeUseSnapshot, flagTraceBakeSnapshotWindow, + flagIPRateLimitRPS, flagIPRateLimitBurst, flagRateLimitingEnabled, flagTrustedProxyCIDRs, + flagBatchRequestLimit, flagBatchResponseMaxSize, flagMaxRequestBodyBytes, + flagMaxConcurrentRequestBytes, flagWSAdmissionTimeout, flagMaxOpenConnections, + flagBodyReadIdleTimeout, + } + sort.Strings(want) + + section, ok := registry.Lookup(SectionName) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) + } + declared := map[string]bool{} + for _, key := range section.Keys { + declared[key] = true + } + for _, key := range want { + if !declared[key] { + t.Errorf("the reader resolves %s and no tag declares it", key) + } + delete(declared, key) + } + for key := range declared { + t.Errorf("%s is declared and no constant in this file resolves it", key) + } +} + +// TestEachKindOfNodeResolvesTheInterfacesItIsFor is the mode-varying part of this section. +// +// A full node and an archive node serve queries, which is what these two interfaces are for. A validator +// and a seed serve none, and an open interface on the node that holds a signing key is a public request +// surface on the one node meant to expose the least. The values are written out here rather than taken +// from the same rule the section reads, so a change to that rule fails this and gets looked at. +func TestEachKindOfNodeResolvesTheInterfacesItIsFor(t *testing.T) { + serving := map[registry.Mode]bool{ + registry.ModeValidator: false, + registry.ModeSeed: false, + registry.ModeFull: true, + registry.ModeArchive: true, + } + for _, mode := range registry.Modes() { + want, named := serving[mode] + if !named { + t.Fatalf("mode %q has no expectation here, so a mode was added and this was not revisited", mode) + } + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + for _, key := range []string{flagHTTPEnabled, flagWSEnabled} { + if got := resolved.Values[key]; got != want { + t.Errorf("mode %q: %s resolves to %v, want %v", mode, key, got, want) + } + } + } +} + +// TestEachKeyResolvesToTheValueItsFieldHolds covers the binding a key set cannot show. +// +// Resolving carries the key a tag produced together with the value that tag's field held. Comparing the +// defaults struct against itself does not: two tags on each other's fields leave the key set identical and +// every field still holding the value it always did, so a list and a URL change places unnoticed. +func TestEachKeyResolvesToTheValueItsFieldHolds(t *testing.T) { + resolved, err := registry.Resolve(registry.ModeFull, registry.Sources{}) + if err != nil { + t.Fatalf("%v", err) + } + for key, want := range map[string]any{ + flagCORSOrigins: DefaultConfig.CORSOrigins, + flagDenyList: DefaultConfig.DenyList, + flagTraceAllowedTracers: DefaultConfig.TraceAllowedTracers, + flagEVMLegacySeiApis: DefaultConfig.EnabledLegacySeiApis, + flagTrustedProxyCIDRs: DefaultConfig.TrustedProxyCIDRs, + flagReadTimeout: DefaultConfig.ReadTimeout, + flagHTTPPort: DefaultConfig.HTTPPort, + flagIPRateLimitRPS: DefaultConfig.IPRateLimitRPS, + flagMaxLogBytes: DefaultConfig.MaxLogBytes, + } { + if got := resolved.Values[key]; !reflect.DeepEqual(got, want) { + t.Errorf("%s resolves to %#v (%T), want %#v (%T)", key, got, got, want, want) + } + } +} diff --git a/giga/executor/config/register.go b/giga/executor/config/register.go new file mode 100644 index 0000000000..4d82e70356 --- /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 the seid init command writes for a node of this kind. +// +// 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/giga/executor/config/register_test.go b/giga/executor/config/register_test.go new file mode 100644 index 0000000000..4a7aef2754 --- /dev/null +++ b/giga/executor/config/register_test.go @@ -0,0 +1,48 @@ +package config + +import ( + "reflect" + "sort" + "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) { + 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) + } +} + +// 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..60ce5fc7f8 --- /dev/null +++ b/sei-db/config/receipt_register.go @@ -0,0 +1,32 @@ +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 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 the seid init command writes for a node of this kind. +// +// 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 new file mode 100644 index 0000000000..7981fb0430 --- /dev/null +++ b/sei-db/config/receipt_register_test.go @@ -0,0 +1,57 @@ +package config + +import ( + "reflect" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestTheDeclaredKeysAreTheKeysThisReaderResolves holds the declaration against the reader. +// +// 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{ + 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) + } +} + +// 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..646f219300 --- /dev/null +++ b/sei-wasmd/x/wasm/config_register.go @@ -0,0 +1,65 @@ +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" + +// GeneratedQueryGasLimit is the smart-query gas limit a generated app.toml carries. +// +// A tenth of what this module's own default holds, and it is the value every node provisioned by the +// binary runs. Declared here, beside the section, and read by the command that renders the file, so the +// number that reaches an operator and the number this section states are one statement. +const GeneratedQueryGasLimit uint64 = 300_000 + +// 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 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{}, sectionDefaults) +} + +// sectionDefaults is what the seid init command writes for a node of this kind. +// +// The query gas limit is a tenth of what this module's own default holds, and that is deliberate: it is +// the number the binary writes into every file it generates, so it is what every provisioned node runs. +// Declaring the module's larger default instead would have a caller rendering a file that loosens the +// only bound on the work one smart query can ask of a node serving queries to anyone. The module's +// default is still what a node whose file has no wasm section resolves, which is a different question and +// recorded as one. +func sectionDefaults(registry.Mode) any { + live := types.DefaultWasmConfig() + schema := wasmSchema{ + MemoryCacheSize: live.MemoryCacheSize, + QueryGasLimit: GeneratedQueryGasLimit, + } + 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..0a2655c11b --- /dev/null +++ b/sei-wasmd/x/wasm/config_register_test.go @@ -0,0 +1,82 @@ +package wasm + +import ( + "reflect" + "sort" + "strconv" + "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) { + 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) + } +} + +// TestTheDefaultsAreWhatTheCommandWrites keeps the schema's values from drifting from what a file carries. +// +// The schema restates three settings of types.WasmConfig, so nothing stops those values diverging except +// this. Two of them come from that struct. The third, the query gas limit, comes from what the command +// renders, which is a tenth of what the struct holds, and both halves of that are held below. +func TestTheDefaultsAreWhatTheCommandWrites(t *testing.T) { + live := types.DefaultWasmConfig() + 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", sectionDefaults(registry.ModeValidator)) + } + + if got.MemoryCacheSize != live.MemoryCacheSize { + t.Errorf("memory_cache_size resolves to %d, want the live %d", got.MemoryCacheSize, live.MemoryCacheSize) + } + // The limit the command writes, not the module's own default. The two differ by a factor of ten and + // both facts are held: declaring the larger one would have a caller rendering a file that loosens the + // only bound on what one smart query can ask of a node, and the larger one is still what a node whose + // file carries no wasm section resolves. + if got.QueryGasLimit != GeneratedQueryGasLimit { + t.Errorf("query_gas_limit resolves to %d, want the %d the command writes", + got.QueryGasLimit, GeneratedQueryGasLimit) + } + if GeneratedQueryGasLimit >= live.SmartQueryGasLimit { + t.Errorf("the limit the command writes, %d, is no longer below this module's own default, %d. "+ + "If they have converged the distinction here is spurious and should go; if the command's "+ + "limit has grown past the module's, a generated file now loosens the bound rather than "+ + "tightening it", GeneratedQueryGasLimit, live.SmartQueryGasLimit) + } + // Absent is a meaning of its own here: unset means the consensus block gas limit applies, so an unset + // 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) + } +} diff --git a/x/evm/blocktest/register.go b/x/evm/blocktest/register.go new file mode 100644 index 0000000000..14e3f87ba6 --- /dev/null +++ b/x/evm/blocktest/register.go @@ -0,0 +1,24 @@ +package blocktest + +import "github.com/sei-protocol/sei-chain/config/registry" + +// SectionName is this section's name in the configuration key space. +const SectionName = "eth_blocktest" + +// Registration puts this package's configuration section in the registry. +// +// The owning package registers its own section, so the struct, the values and the keys come from one +// place. This section's mapstructure tags already spell the keys its reader resolves, so the registry +// derives what a node reads rather than restating them. +func init() { + registry.RegisterSection(SectionName, &Config{}, defaults) +} + +// defaults is what the seid init command writes for a node of this kind. +// +// The same values for every mode. This section drives a harness against recorded block data, which is +// not something any kind of node does while serving a chain. +// +// The data path is a tilde path, and it resolves as written. Whoever opens it expands the tilde, so a +// caller that renders this value into a file writes the same text an operator would. +func defaults(registry.Mode) any { return DefaultConfig } diff --git a/x/evm/blocktest/register_test.go b/x/evm/blocktest/register_test.go new file mode 100644 index 0000000000..035a267866 --- /dev/null +++ b/x/evm/blocktest/register_test.go @@ -0,0 +1,60 @@ +package blocktest + +import ( + "reflect" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestDeclaredKeysAreTheOnesItsReaderResolves holds the derived keys against the reader's own constants. +// +// The section registers the struct its reader fills, so a mapstructure tag is the only spelling of these +// keys. What remains is the constants ReadConfig passes to Get, which state the same keys again in the same +// file, and a rename that moves one and not the other compiles. +// +// The section name is passed to the registry rather than derived, which is what keeps this section reachable +// at all: the struct that carries it in the generated file is tagged with a different spelling, and a +// registry that took the section name from a tag would declare a section no operator writes. +func TestDeclaredKeysAreTheOnesItsReaderResolves(t *testing.T) { + for _, defect := range registry.Defects() { + if defect.Section == SectionName { + t.Fatalf("%s was refused, so none of its keys is declared: %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, flagTestDataPath} + sort.Strings(want) + if !reflect.DeepEqual(section.Keys, want) { + t.Errorf("%s declares\n %v\nand its reader resolves\n %v", SectionName, section.Keys, want) + } +} + +// TestEachKeyResolvesToTheValueItsFieldHolds covers the binding a key set cannot show. +// +// These two fields carry different types, so a tag on the wrong field changes what a key resolves to +// without changing the key set at all. +func TestEachKeyResolvesToTheValueItsFieldHolds(t *testing.T) { + for _, mode := range registry.Modes() { + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + for key, want := range map[string]any{ + flagEnabled: DefaultConfig.Enabled, + flagTestDataPath: DefaultConfig.TestDataPath, + } { + if got := resolved.Values[key]; !reflect.DeepEqual(got, want) { + t.Errorf("mode %q: %s resolves to %#v (%T), want %#v (%T)", mode, key, got, got, want, want) + } + } + if resolved.Values[flagEnabled] == true { + t.Errorf("mode %q resolves the block-test harness on, which replays recorded data instead of "+ + "following the chain", mode) + } + } +} diff --git a/x/evm/querier/register.go b/x/evm/querier/register.go new file mode 100644 index 0000000000..243d701f98 --- /dev/null +++ b/x/evm/querier/register.go @@ -0,0 +1,21 @@ +package querier + +import "github.com/sei-protocol/sei-chain/config/registry" + +// SectionName is this section's name in the configuration key space. +const SectionName = "evm_query" + +// Registration puts this package's configuration section in the registry. +// +// The owning package registers its own section, so the struct, the values and the keys come from one +// place. This section's mapstructure tags already spell the key its reader resolves, so the registry +// derives what a node reads rather than restating it. +func init() { + registry.RegisterSection(SectionName, &Config{}, defaults) +} + +// defaults is what the seid init command writes for a node of this kind. +// +// The same value for every mode. The limit bounds the work a contract can ask the EVM to do inside a +// query, and every node answers the same queries. +func defaults(registry.Mode) any { return DefaultConfig } diff --git a/x/evm/querier/register_test.go b/x/evm/querier/register_test.go new file mode 100644 index 0000000000..4721542400 --- /dev/null +++ b/x/evm/querier/register_test.go @@ -0,0 +1,48 @@ +package querier + +import ( + "reflect" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestDeclaredKeysAreTheOnesItsReaderResolves holds the derived key against the reader's own constant. +// +// The section registers the struct its reader fills, so a mapstructure tag is the only spelling of its key. +// What remains is the constant ReadConfig passes to Get, which states the same key again a few lines away, +// and a rename that moves one and not the other compiles. +func TestDeclaredKeysAreTheOnesItsReaderResolves(t *testing.T) { + for _, defect := range registry.Defects() { + if defect.Section == SectionName { + t.Fatalf("%s was refused, so none of its keys is declared: %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{flagGasLimit} + sort.Strings(want) + if !reflect.DeepEqual(section.Keys, want) { + t.Errorf("%s declares\n %v\nand its reader resolves\n %v", SectionName, section.Keys, want) + } +} + +// TestEachKeyResolvesToTheValueItsFieldHolds covers the binding a key set cannot show. +// +// Resolving carries the key a tag produced together with the value that tag's field held, so this notices a +// tag sitting on the wrong field. Comparing the defaults struct against itself does not: the key set stays +// the same and every field still holds the value it always did. +func TestEachKeyResolvesToTheValueItsFieldHolds(t *testing.T) { + for _, mode := range registry.Modes() { + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + if got, want := resolved.Values[flagGasLimit], DefaultConfig.GasLimit; !reflect.DeepEqual(got, want) { + t.Errorf("mode %q: %s resolves to %#v (%T), want %#v (%T)", mode, flagGasLimit, got, got, want, want) + } + } +} diff --git a/x/evm/replay/register.go b/x/evm/replay/register.go new file mode 100644 index 0000000000..3aacb732b6 --- /dev/null +++ b/x/evm/replay/register.go @@ -0,0 +1,27 @@ +package replay + +import "github.com/sei-protocol/sei-chain/config/registry" + +// SectionName is this section's name in the configuration key space. +const SectionName = "eth_replay" + +// Registration puts this package's configuration section in the registry. +// +// The owning package registers its own section, so the struct, the values and the keys come from one +// place. This section's mapstructure tags already spell the keys its reader resolves, so the registry +// derives what a node reads rather than restating them. +// +// One of the four keys is written into app.toml under a name nothing reads. The template renders +// eth_replay_contract_state_checks and the reader looks up contract_state_checks, so the declared key is +// the one a value reaches a reader through. +func init() { + registry.RegisterSection(SectionName, &Config{}, defaults) +} + +// defaults is what the seid init command writes for a node of this kind. +// +// The same values for every mode, and replay off. Turning it on makes a node replay recorded chain data +// from an endpoint instead of following the chain, and the endpoint is a fixed third-party address, so no +// kind of node implies it. Construction opens a client for that address without reaching it, which is why +// an unreachable endpoint surfaces during replay rather than at startup. +func defaults(registry.Mode) any { return DefaultConfig } diff --git a/x/evm/replay/register_test.go b/x/evm/replay/register_test.go new file mode 100644 index 0000000000..2cb3231650 --- /dev/null +++ b/x/evm/replay/register_test.go @@ -0,0 +1,58 @@ +package replay + +import ( + "reflect" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestDeclaredKeysAreTheOnesItsReaderResolves holds the derived keys against the reader's own constants. +// +// Three of the four keys carry the name the template writes and one does not: the template renders +// eth_replay_contract_state_checks and the reader looks up contract_state_checks. The declared key is the +// one a value reaches a reader through, and the exact comparison below is what keeps the other out. +func TestDeclaredKeysAreTheOnesItsReaderResolves(t *testing.T) { + for _, defect := range registry.Defects() { + if defect.Section == SectionName { + t.Fatalf("%s was refused, so none of its keys is declared: %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, flagEthRPC, flagEthDataDir, flagContractStateChecks} + sort.Strings(want) + if !reflect.DeepEqual(section.Keys, want) { + t.Errorf("%s declares\n %v\nand its reader resolves\n %v", SectionName, section.Keys, want) + } +} + +// TestEachKeyResolvesToTheValueItsFieldHolds covers the binding a key set cannot show. +// +// Two of these fields are strings holding an endpoint and a directory. A tag on the wrong field leaves the +// key set identical and resolves a filesystem path where a reader expects a URL. +func TestEachKeyResolvesToTheValueItsFieldHolds(t *testing.T) { + for _, mode := range registry.Modes() { + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) + } + for key, want := range map[string]any{ + flagEnabled: DefaultConfig.Enabled, + flagEthRPC: DefaultConfig.EthRPC, + flagEthDataDir: DefaultConfig.EthDataDir, + flagContractStateChecks: DefaultConfig.ContractStateChecks, + } { + if got := resolved.Values[key]; !reflect.DeepEqual(got, want) { + t.Errorf("mode %q: %s resolves to %#v (%T), want %#v (%T)", mode, key, got, got, want, want) + } + } + if resolved.Values[flagEnabled] == true { + t.Errorf("mode %q resolves replay on, so those nodes would replay recorded data from %v instead "+ + "of following the chain", mode, resolved.Values[flagEthRPC]) + } + } +}