From 25fa5f997bde6b2c34cc35db3becb43302cc3fd2 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Thu, 20 Aug 2026 14:38:22 -0700 Subject: [PATCH 1/2] config: the EVM sections enter the registry Four sections, each registered by the package that owns its struct. eth_blocktest 2 keys eth_replay 4 keys evm 57 keys evm_query 1 key All four register the struct their reader fills. None needs a schema, because in each of them the mapstructure tags already spell the keys the reader looks up, so the registry derives what a node reads and nothing restates a list of fifty-seven keys. Each package's test holds the derived keys against the reader's own constants, which are the second statement of the same set in the same file: a rename that moves one and not the other compiles. The EVM section resolves the same values for every mode. Nothing consults a node's kind while reading these keys, so a file missing them serves both interfaces whatever kind of node it is, and that is what these resolve to. A node seid init provisioned is the other case and needs nothing here, since that path writes the two interface toggles per mode and a written value is what resolves. Two of that section's values come from the machine rather than from a decision. The simulation call limit is the processor count and the worker pool is twice it, capped, so they describe whichever host resolved them. That is stated where they are declared, because a caller rendering them into a file carries one host's sizing to whatever reads that file next. Replay declares three of its four keys under the name the template writes and one under a different one. The template renders eth_replay_contract_state_checks and the reader looks up contract_state_checks, so every generated file already carries a name nothing resolves. The declared key is the one a value reaches a reader through, and a test refuses the other: declaring it would add a key an operator can set and no reader answers, which is worse than the mismatch, because the value would look as though it applied. The recorded configuration surface does not move, because nothing consumes the registry on a boot path yet. --- evmrpc/config/register.go | 30 +++++++++++ evmrpc/config/register_test.go | 92 ++++++++++++++++++++++++++++++++ x/evm/blocktest/register.go | 24 +++++++++ x/evm/blocktest/register_test.go | 47 ++++++++++++++++ x/evm/querier/register.go | 21 ++++++++ x/evm/querier/register_test.go | 40 ++++++++++++++ x/evm/replay/register.go | 26 +++++++++ x/evm/replay/register_test.go | 66 +++++++++++++++++++++++ 8 files changed, 346 insertions(+) create mode 100644 evmrpc/config/register.go create mode 100644 evmrpc/config/register_test.go create mode 100644 x/evm/blocktest/register.go create mode 100644 x/evm/blocktest/register_test.go create mode 100644 x/evm/querier/register.go create mode 100644 x/evm/querier/register_test.go create mode 100644 x/evm/replay/register.go create mode 100644 x/evm/replay/register_test.go diff --git a/evmrpc/config/register.go b/evmrpc/config/register.go new file mode 100644 index 0000000000..7cb5c2209c --- /dev/null +++ b/evmrpc/config/register.go @@ -0,0 +1,30 @@ +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, all fifty-seven of +// them, so the registry derives what a node reads rather than restating a list this long. +func init() { + registry.RegisterSection(SectionName, &Config{}, defaults) +} + +// defaults is what this section resolves to for a node that has written nothing. +// +// The declared defaults, unchanged by mode, because that is what such a node runs: nothing consults the +// node's kind while reading these keys, so a file missing them serves both interfaces whatever kind of +// node it is. +// +// A node seid init provisioned is a different case and needs no help from here. That path writes the two +// interface toggles per mode, closing them for a validator and a seed, so those nodes carry written values +// and a written value is what resolves. +// +// Two of these values come from the machine rather than from a decision: the simulation call limit is the +// processor count and the worker pool is twice it, capped. They describe the host that asked, so a caller +// that renders them into a file carries one host's sizing to whatever reads that file next. +func defaults(registry.Mode) any { return DefaultConfig } diff --git a/evmrpc/config/register_test.go b/evmrpc/config/register_test.go new file mode 100644 index 0000000000..a2269efa32 --- /dev/null +++ b/evmrpc/config/register_test.go @@ -0,0 +1,92 @@ +package config + +import ( + "reflect" + "runtime" + "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 fifty-seven 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) { + 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) + } + if !reflect.DeepEqual(section.Keys, want) { + t.Errorf("%s declares %d keys and its reader resolves %d.\ndeclared: %v\nresolved: %v", + SectionName, len(section.Keys), len(want), section.Keys, want) + } +} + +// TestDefaultsAreTheReaderOwnForEveryMode covers the value side of the same registration. +// +// Unchanged by mode, which is the decision worth pinning. seid init writes the two interface toggles per +// mode, so a validator it provisioned carries them as written values. These are what a node with nothing +// written runs, and no read of these keys consults the node's kind. +func TestDefaultsAreTheReaderOwnForEveryMode(t *testing.T) { + for _, mode := range registry.Modes() { + got, ok := defaults(mode).(Config) + if !ok { + t.Fatalf("mode %q: defaults returned %T, want the type its reader fills", mode, defaults(mode)) + } + if !reflect.DeepEqual(got, DefaultConfig) { + t.Errorf("mode %q resolves to a value other than the reader's own default", mode) + } + if !got.HTTPEnabled || !got.WSEnabled { + t.Errorf("mode %q resolves an interface closed. A node whose file lacks these keys serves "+ + "both, so resolving one closed would take an interface away from a running node", mode) + } + } +} + +// TestTheTwoHostDerivedValuesDescribeThisHost covers the two defaults that are measurements. +// +// Every other value here is a decision someone wrote down and is the same on any machine. These two are +// the processor count and twice it, so they describe whichever host resolved them. Nothing here can make +// that portable, and stating it is what keeps a caller from rendering them into a file as though it were. +func TestTheTwoHostDerivedValuesDescribeThisHost(t *testing.T) { + got, ok := defaults(registry.ModeValidator).(Config) + if !ok { + t.Fatalf("defaults returned %T, want the type its reader fills", defaults(registry.ModeValidator)) + } + if got.MaxConcurrentSimulationCalls != runtime.NumCPU() { + t.Errorf("%s resolves to %d and this host has %d processors", + flagMaxConcurrentSimulationCalls, got.MaxConcurrentSimulationCalls, runtime.NumCPU()) + } + if want := min(MaxWorkerPoolSize, runtime.NumCPU()*2); got.WorkerPoolSize != want { + t.Errorf("%s resolves to %d, want %d on a host with %d processors", + flagWorkerPoolSize, got.WorkerPoolSize, want, runtime.NumCPU()) + } +} diff --git a/x/evm/blocktest/register.go b/x/evm/blocktest/register.go new file mode 100644 index 0000000000..293cd89ea5 --- /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 this section resolves to for a node that has written nothing. +// +// 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..d5ef96ed60 --- /dev/null +++ b/x/evm/blocktest/register_test.go @@ -0,0 +1,47 @@ +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 and there is no second list to fall behind. What remains is the constants ReadConfig looks up, +// which state the same keys again a few lines away, and a rename that moves one and not the other +// compiles. +func TestDeclaredKeysAreTheOnesItsReaderResolves(t *testing.T) { + want := []string{flagEnabled, flagTestDataPath} + sort.Strings(want) + + section, ok := registry.Lookup(SectionName) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) + } + if !reflect.DeepEqual(section.Keys, want) { + t.Errorf("%s declares\n %v\nand its reader resolves\n %v", SectionName, section.Keys, want) + } +} + +// TestDefaultsAreTheReaderOwnForEveryMode covers the value side of the same registration. +// +// Off for every mode, which is the value worth pinning: a mode that resolved this on would have those +// nodes replay recorded data instead of serving the chain. +func TestDefaultsAreTheReaderOwnForEveryMode(t *testing.T) { + for _, mode := range registry.Modes() { + got, ok := defaults(mode).(Config) + if !ok { + t.Fatalf("mode %q: defaults returned %T, want the type its reader fills", mode, defaults(mode)) + } + if got != DefaultConfig { + t.Errorf("mode %q resolves to %+v, want the reader's own default %+v", mode, got, DefaultConfig) + } + if got.Enabled { + t.Errorf("mode %q resolves the block-test harness on", mode) + } + } +} diff --git a/x/evm/querier/register.go b/x/evm/querier/register.go new file mode 100644 index 0000000000..532f1f3530 --- /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 this section resolves to for a node that has written nothing. +// +// 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..42d2d799a0 --- /dev/null +++ b/x/evm/querier/register_test.go @@ -0,0 +1,40 @@ +package querier + +import ( + "reflect" + "sort" + "testing" + + "github.com/sei-protocol/sei-chain/config/registry" +) + +// TestDeclaredKeysAreTheOnesItsReaderResolves holds the derived keys 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 and there is no second list to fall behind. What remains is the constant ReadConfig looks up, 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) { + want := []string{flagGasLimit} + sort.Strings(want) + + section, ok := registry.Lookup(SectionName) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) + } + if !reflect.DeepEqual(section.Keys, want) { + t.Errorf("%s declares\n %v\nand its reader resolves\n %v", SectionName, section.Keys, want) + } +} + +// TestDefaultsAreTheReaderOwnForEveryMode covers the value side of the same registration. +func TestDefaultsAreTheReaderOwnForEveryMode(t *testing.T) { + for _, mode := range registry.Modes() { + got, ok := defaults(mode).(Config) + if !ok { + t.Fatalf("mode %q: defaults returned %T, want the type its reader fills", mode, defaults(mode)) + } + if got != DefaultConfig { + t.Errorf("mode %q resolves to %+v, want the reader's own default %+v", mode, got, DefaultConfig) + } + } +} diff --git a/x/evm/replay/register.go b/x/evm/replay/register.go new file mode 100644 index 0000000000..fa3e1a291b --- /dev/null +++ b/x/evm/replay/register.go @@ -0,0 +1,26 @@ +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 this section resolves to for a node that has written nothing. +// +// The same values for every mode, and replay off. Turning it on makes application construction dial the +// endpoint and fail when it cannot reach it, so a mode whose defaults turned it on would stop those nodes +// booting. The endpoint itself is a fixed third-party address, which is another reason no mode implies it. +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..49079caaba --- /dev/null +++ b/x/evm/replay/register_test.go @@ -0,0 +1,66 @@ +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. +// +// 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 a few lines away, and a rename that moves one and not the other +// compiles. +func TestDeclaredKeysAreTheOnesItsReaderResolves(t *testing.T) { + want := []string{flagEnabled, flagEthRPC, flagEthDataDir, flagContractStateChecks} + sort.Strings(want) + + section, ok := registry.Lookup(SectionName) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) + } + if !reflect.DeepEqual(section.Keys, want) { + t.Errorf("%s declares\n %v\nand its reader resolves\n %v", SectionName, section.Keys, want) + } +} + +// TestTheWrittenSpellingOfTheStateCheckIsNotDeclared covers a name that is written and never read. +// +// The app.toml template renders eth_replay_contract_state_checks and the reader looks up +// contract_state_checks, so every generated file carries a name nothing resolves. Declaring that name +// would add a key an operator can set and no reader answers, which is the one outcome worse than the +// mismatch itself: a value that looks as though it applied. +func TestTheWrittenSpellingOfTheStateCheckIsNotDeclared(t *testing.T) { + section, ok := registry.Lookup(SectionName) + if !ok { + t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) + } + for _, key := range section.Keys { + if key == SectionName+".eth_replay_contract_state_checks" { + t.Errorf("%s is declared and no reader looks it up", key) + } + } +} + +// TestDefaultsAreTheReaderOwnForEveryMode covers the value side of the same registration. +// +// Off for every mode, which is the value worth pinning: turning replay on makes application construction +// dial the endpoint, so a mode that resolved it on would stop those nodes booting. +func TestDefaultsAreTheReaderOwnForEveryMode(t *testing.T) { + for _, mode := range registry.Modes() { + got, ok := defaults(mode).(Config) + if !ok { + t.Fatalf("mode %q: defaults returned %T, want the type its reader fills", mode, defaults(mode)) + } + if got != DefaultConfig { + t.Errorf("mode %q resolves to %+v, want the reader's own default %+v", mode, got, DefaultConfig) + } + if got.Enabled { + t.Errorf("mode %q resolves replay on, which makes those nodes dial %q at construction", + mode, got.EthRPC) + } + } +} From dc4a458221b754b210d1b2ea051f308821f5df24 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Fri, 21 Aug 2026 08:39:18 -0700 Subject: [PATCH 2/2] config: hand out values nothing else holds, and answer the EVM interfaces per mode A resolved list was the section's own list. A section's default is usually a package-level variable, so a slice field handed out the array that variable holds, and one in-place write by a caller rewrote it for the whole process: every later resolution, and every reader that copies the same struct. Two of the five lists this reaches are deny lists, so the rewrite is silent and it is a security control. The registry already copies a section's keys for exactly this reason and said so in a comment; values now get the same guarantee, at the one function both walks pass through. Removing it fails a test that writes into a resolved list and asks the section's default what it holds. The EVM interfaces answer per kind of node. A full node and an archive node serve queries, which is what those interfaces are for. A validator and a seed serve none, and this section declared both open for every kind, which puts a public request surface on the node that holds a signing key. The rule already exists in this binary. It could not be read from here, because the package that owns the node mode imports this one, so the rule moved to the registry, which is a leaf both sides reach, and the node-mode type now delegates to it rather than stating it twice. Forgetting archive in that one statement now fails a test. Each section also holds its keys to the values their fields hold, not to its own defaults struct compared with itself. That comparison agreed with itself while two tags sat on the wrong fields: the key set stays identical and every field still holds the value it always did, so an endpoint and a directory, or a deny list and an origin list, change places unnoticed. Both of those swaps now fail. Each section reports its own refusal. A registration the registry cannot use is recorded rather than raised, and these tests inferred it from a lookup coming back empty, which threw away the sentence saying why. Two tests are gone because they restated checks that already exist a few files away, where the message is better. Two comments are corrected: enabling replay opens a client without reaching the endpoint, so an unreachable one surfaces during replay rather than at startup, and the two machine-derived values are not one case, because the worker pool re-measures when it is given a value that is not positive and the simulation limit reads zero as no limit at all. The registry now states what a resolved value's type depends on, because it resolves values and does not convert them: a default arrives as its field's type, a file as whatever the format decodes to, and an environment variable as one string. --- app/params/config.go | 6 +- config/registry/detach_test.go | 65 +++++++++++++++++++++ config/registry/registry.go | 11 ++++ config/registry/resolve.go | 43 +++++++++++++- evmrpc/config/register.go | 32 ++++++----- evmrpc/config/register_test.go | 97 +++++++++++++++++++++----------- x/evm/blocktest/register_test.go | 47 ++++++++++------ x/evm/querier/register_test.go | 36 +++++++----- x/evm/replay/register.go | 7 ++- x/evm/replay/register_test.go | 66 ++++++++++------------ 10 files changed, 290 insertions(+), 120 deletions(-) create mode 100644 config/registry/detach_test.go 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/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/registry.go b/config/registry/registry.go index 008738680c..cee2e7666c 100644 --- a/config/registry/registry.go +++ b/config/registry/registry.go @@ -26,6 +26,17 @@ 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. diff --git a/config/registry/resolve.go b/config/registry/resolve.go index dfcca8f9e3..b64a00b90d 100644 --- a/config/registry/resolve.go +++ b/config/registry/resolve.go @@ -10,6 +10,15 @@ 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. // @@ -259,11 +268,43 @@ func walkValues(v reflect.Value, prefix string, out map[string]any) error { } 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: diff --git a/evmrpc/config/register.go b/evmrpc/config/register.go index 7cb5c2209c..30d76881e6 100644 --- a/evmrpc/config/register.go +++ b/evmrpc/config/register.go @@ -8,23 +8,29 @@ 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, all fifty-seven of -// them, so the registry derives what a node reads rather than restating a list this long. +// 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 this section resolves to for a node that has written nothing. // -// The declared defaults, unchanged by mode, because that is what such a node runs: nothing consults the -// node's kind while reading these keys, so a file missing them serves both interfaces whatever kind of -// node it is. +// The two interface toggles answer per kind of node. 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. // -// A node seid init provisioned is a different case and needs no help from here. That path writes the two -// interface toggles per mode, closing them for a validator and a seed, so those nodes carry written values -// and a written value is what resolves. -// -// Two of these values come from the machine rather than from a decision: the simulation call limit is the -// processor count and the worker pool is twice it, capped. They describe the host that asked, so a caller -// that renders them into a file carries one host's sizing to whatever reads that file next. -func defaults(registry.Mode) any { return DefaultConfig } +// 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 index a2269efa32..23eab42ea5 100644 --- a/evmrpc/config/register_test.go +++ b/evmrpc/config/register_test.go @@ -2,7 +2,6 @@ package config import ( "reflect" - "runtime" "sort" "testing" @@ -13,12 +12,17 @@ import ( // // 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 fifty-seven keys again in the same file, and a rename that moves one and not the -// other compiles. +// 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, @@ -44,49 +48,74 @@ func TestDeclaredKeysAreTheOnesItsReaderResolves(t *testing.T) { if !ok { t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) } - if !reflect.DeepEqual(section.Keys, want) { - t.Errorf("%s declares %d keys and its reader resolves %d.\ndeclared: %v\nresolved: %v", - SectionName, len(section.Keys), len(want), section.Keys, want) + 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) } } -// TestDefaultsAreTheReaderOwnForEveryMode covers the value side of the same registration. +// TestEachKindOfNodeResolvesTheInterfacesItIsFor is the mode-varying part of this section. // -// Unchanged by mode, which is the decision worth pinning. seid init writes the two interface toggles per -// mode, so a validator it provisioned carries them as written values. These are what a node with nothing -// written runs, and no read of these keys consults the node's kind. -func TestDefaultsAreTheReaderOwnForEveryMode(t *testing.T) { +// 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() { - got, ok := defaults(mode).(Config) - if !ok { - t.Fatalf("mode %q: defaults returned %T, want the type its reader fills", mode, defaults(mode)) + 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) } - if !reflect.DeepEqual(got, DefaultConfig) { - t.Errorf("mode %q resolves to a value other than the reader's own default", mode) + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) } - if !got.HTTPEnabled || !got.WSEnabled { - t.Errorf("mode %q resolves an interface closed. A node whose file lacks these keys serves "+ - "both, so resolving one closed would take an interface away from a running node", mode) + 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) + } } } } -// TestTheTwoHostDerivedValuesDescribeThisHost covers the two defaults that are measurements. +// TestEachKeyResolvesToTheValueItsFieldHolds covers the binding a key set cannot show. // -// Every other value here is a decision someone wrote down and is the same on any machine. These two are -// the processor count and twice it, so they describe whichever host resolved them. Nothing here can make -// that portable, and stating it is what keeps a caller from rendering them into a file as though it were. -func TestTheTwoHostDerivedValuesDescribeThisHost(t *testing.T) { - got, ok := defaults(registry.ModeValidator).(Config) - if !ok { - t.Fatalf("defaults returned %T, want the type its reader fills", defaults(registry.ModeValidator)) +// 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) } - if got.MaxConcurrentSimulationCalls != runtime.NumCPU() { - t.Errorf("%s resolves to %d and this host has %d processors", - flagMaxConcurrentSimulationCalls, got.MaxConcurrentSimulationCalls, runtime.NumCPU()) - } - if want := min(MaxWorkerPoolSize, runtime.NumCPU()*2); got.WorkerPoolSize != want { - t.Errorf("%s resolves to %d, want %d on a host with %d processors", - flagWorkerPoolSize, got.WorkerPoolSize, want, runtime.NumCPU()) + 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/x/evm/blocktest/register_test.go b/x/evm/blocktest/register_test.go index d5ef96ed60..035a267866 100644 --- a/x/evm/blocktest/register_test.go +++ b/x/evm/blocktest/register_test.go @@ -11,37 +11,50 @@ import ( // 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 a few lines away, and a rename that moves one and not the other -// compiles. +// 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) { - want := []string{flagEnabled, flagTestDataPath} - sort.Strings(want) - + 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) } } -// TestDefaultsAreTheReaderOwnForEveryMode covers the value side of the same registration. +// TestEachKeyResolvesToTheValueItsFieldHolds covers the binding a key set cannot show. // -// Off for every mode, which is the value worth pinning: a mode that resolved this on would have those -// nodes replay recorded data instead of serving the chain. -func TestDefaultsAreTheReaderOwnForEveryMode(t *testing.T) { +// 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() { - got, ok := defaults(mode).(Config) - if !ok { - t.Fatalf("mode %q: defaults returned %T, want the type its reader fills", mode, defaults(mode)) + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) } - if got != DefaultConfig { - t.Errorf("mode %q resolves to %+v, want the reader's own default %+v", mode, got, DefaultConfig) + 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 got.Enabled { - t.Errorf("mode %q resolves the block-test harness on", mode) + 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_test.go b/x/evm/querier/register_test.go index 42d2d799a0..4721542400 100644 --- a/x/evm/querier/register_test.go +++ b/x/evm/querier/register_test.go @@ -8,33 +8,41 @@ import ( "github.com/sei-protocol/sei-chain/config/registry" ) -// TestDeclaredKeysAreTheOnesItsReaderResolves holds the derived keys against the reader's own constant. +// 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 and there is no second list to fall behind. What remains is the constant ReadConfig looks up, which -// states the same key again a few lines away, and a rename that moves one and not the other compiles. +// 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) { - want := []string{flagGasLimit} - sort.Strings(want) - + 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) } } -// TestDefaultsAreTheReaderOwnForEveryMode covers the value side of the same registration. -func TestDefaultsAreTheReaderOwnForEveryMode(t *testing.T) { +// 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() { - got, ok := defaults(mode).(Config) - if !ok { - t.Fatalf("mode %q: defaults returned %T, want the type its reader fills", mode, defaults(mode)) + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) } - if got != DefaultConfig { - t.Errorf("mode %q resolves to %+v, want the reader's own default %+v", mode, got, DefaultConfig) + 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 index fa3e1a291b..e6672b4ad2 100644 --- a/x/evm/replay/register.go +++ b/x/evm/replay/register.go @@ -20,7 +20,8 @@ func init() { // defaults is what this section resolves to for a node that has written nothing. // -// The same values for every mode, and replay off. Turning it on makes application construction dial the -// endpoint and fail when it cannot reach it, so a mode whose defaults turned it on would stop those nodes -// booting. The endpoint itself is a fixed third-party address, which is another reason no mode implies it. +// 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 index 49079caaba..2cb3231650 100644 --- a/x/evm/replay/register_test.go +++ b/x/evm/replay/register_test.go @@ -10,57 +10,49 @@ import ( // 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 a few lines away, and a rename that moves one and not the other -// compiles. +// 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) { - want := []string{flagEnabled, flagEthRPC, flagEthDataDir, flagContractStateChecks} - sort.Strings(want) - + 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) } } -// TestTheWrittenSpellingOfTheStateCheckIsNotDeclared covers a name that is written and never read. -// -// The app.toml template renders eth_replay_contract_state_checks and the reader looks up -// contract_state_checks, so every generated file carries a name nothing resolves. Declaring that name -// would add a key an operator can set and no reader answers, which is the one outcome worse than the -// mismatch itself: a value that looks as though it applied. -func TestTheWrittenSpellingOfTheStateCheckIsNotDeclared(t *testing.T) { - section, ok := registry.Lookup(SectionName) - if !ok { - t.Fatalf("%s is not registered, so nothing resolves its keys", SectionName) - } - for _, key := range section.Keys { - if key == SectionName+".eth_replay_contract_state_checks" { - t.Errorf("%s is declared and no reader looks it up", key) - } - } -} - -// TestDefaultsAreTheReaderOwnForEveryMode covers the value side of the same registration. +// TestEachKeyResolvesToTheValueItsFieldHolds covers the binding a key set cannot show. // -// Off for every mode, which is the value worth pinning: turning replay on makes application construction -// dial the endpoint, so a mode that resolved it on would stop those nodes booting. -func TestDefaultsAreTheReaderOwnForEveryMode(t *testing.T) { +// 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() { - got, ok := defaults(mode).(Config) - if !ok { - t.Fatalf("mode %q: defaults returned %T, want the type its reader fills", mode, defaults(mode)) + resolved, err := registry.Resolve(mode, registry.Sources{}) + if err != nil { + t.Fatalf("mode %q: %v", mode, err) } - if got != DefaultConfig { - t.Errorf("mode %q resolves to %+v, want the reader's own default %+v", mode, got, DefaultConfig) + 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 got.Enabled { - t.Errorf("mode %q resolves replay on, which makes those nodes dial %q at construction", - mode, got.EthRPC) + 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]) } } }