From d1be70fcf1f58266d6eb12a6ebc44ae9e7ae6e4f Mon Sep 17 00:00:00 2001
From: bdchatham <bdchatham@gmail.com>
Date: Fri, 31 Jul 2026 17:14:54 -0700
Subject: [PATCH 1/8] test(config): pin the section-level env shadow, and make
 the parity comparison deterministic (PLT-775)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two changes to the configuration characterization surface, both ahead of the
experimental-namespace work so that neither rides on it.

Pin the section-level environment shadow. A non-empty SEID_<SECTION> variable
makes every key beneath that section resolve to nothing, so the operator's
written value in app.toml is discarded and the reader falls back to its in-code
default. This is not the environment overriding the file, which is the intended
precedence: the variable's own value is never used, and setting it to "false"
gives the same answer as setting it to "true". viper's isPathShadowedInAutoEnv
walks every proper prefix of a dotted key and returns before the config file is
consulted, so it cannot tell "SEID_GIGA_EXECUTOR names a scalar, so
giga_executor.enabled cannot exist" from "that variable is unrelated to the key
I was asked for".

The rows record the behavior and leave it alone. The legacy path has shipped,
and changing how configuration resolves could silently break an operator who
has come to depend on the current answer. SeiConfigManager corrects it when it
owns resolution, and the divergence is then ratified against these rows rather
than discovered against a production node.

Make the legacy-vs-v2 settings comparison deterministic. The differential
compared Viper.AllSettings() in six places, and that comparison can fail on
identical input: AllSettings re-nests the flat key space by splitting on ".",
so when one key is a dotted prefix of another, whether the scalar or the
sub-tree survives depends on map iteration order. Measured at 43/157 across 200
reads of one file. The comparison is premise three of the boot-parity argument,
so a flake there quietly removes the safety net rather than failing loudly.

configtest.Settings keys on AllKeys and reads each key through Get, which is
stable because Get tries longest prefixes first. It returns a flat map rather
than a rendered string for two reasons: values keep their concrete Go type, so
int64(8) and "8" do not compare equal, and a key is a map key rather than a line
in a newline-joined document, so a key containing a newline — legal TOML, and
reachable from the parity fuzz target — cannot make two different key sets
compare equal. DumpViper remains the right tool for a readable failure message.

The shape is reachable today from any hand-written app.toml section, without
the experimental namespace existing, which is why this lands separately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../cmd/configmanager_differential_test.go    |  26 +--
 cmd/seid/cmd/envshadow_config_fuzz_test.go    | 174 ++++++++++++++++++
 testutil/configtest/viper.go                  |  31 ++++
 testutil/configtest/viper_settings_test.go    | 104 +++++++++++
 4 files changed, 322 insertions(+), 13 deletions(-)
 create mode 100644 cmd/seid/cmd/envshadow_config_fuzz_test.go
 create mode 100644 testutil/configtest/viper_settings_test.go

diff --git a/cmd/seid/cmd/configmanager_differential_test.go b/cmd/seid/cmd/configmanager_differential_test.go
index 85cfb38b52..4ee43f7e3e 100644
--- a/cmd/seid/cmd/configmanager_differential_test.go
+++ b/cmd/seid/cmd/configmanager_differential_test.go
@@ -220,7 +220,7 @@ func configCorpus() []corpusCase {
 //
 // It compares parsed semantics:
 //   - serverCtx.Config (the *tmcfg.Config the node runs on), and
-//   - serverCtx.Viper.AllSettings() (the AppOptions every Sei section reads via
+//   - configtest.Settings(serverCtx.Viper) (the AppOptions every Sei section reads via
 //     appOpts.Get), both at end-of-PersistentPreRunE and after the start.go
 //     chain-id mutation.
 func TestConfigManagerLegacyVsV2Differential(t *testing.T) {
@@ -233,7 +233,7 @@ func TestConfigManagerLegacyVsV2Differential(t *testing.T) {
 
 	require.Equal(t, legacyCtx.Config, v2Ctx.Config,
 		"serverCtx.Config differs between legacy and v2")
-	require.Equal(t, legacyCtx.Viper.AllSettings(), v2Ctx.Viper.AllSettings(),
+	require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper),
 		"serverCtx.Viper settings differ between legacy and v2")
 
 	// The start.go chain-id mutation is identical on both vipers; assert parity
@@ -241,7 +241,7 @@ func TestConfigManagerLegacyVsV2Differential(t *testing.T) {
 	const chainID = "differential-test-1"
 	legacyCtx.Viper.Set(flags.FlagChainID, chainID)
 	v2Ctx.Viper.Set(flags.FlagChainID, chainID)
-	require.Equal(t, legacyCtx.Viper.AllSettings(), v2Ctx.Viper.AllSettings(),
+	require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper),
 		"settings diverge after the start.go chain-id mutation")
 }
 
@@ -278,7 +278,7 @@ func TestConfigManagerLegacyVsV2Differential_EnvHome(t *testing.T) {
 
 	require.Equal(t, legacyCtx.Config, v2Ctx.Config,
 		"serverCtx.Config differs between legacy and v2 on the env-home path")
-	require.Equal(t, legacyCtx.Viper.AllSettings(), v2Ctx.Viper.AllSettings(),
+	require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper),
 		"serverCtx.Viper settings differ between legacy and v2 on the env-home path")
 }
 
@@ -300,7 +300,7 @@ func TestConfigManagerLegacyVsV2Differential_Corpus(t *testing.T) {
 
 			require.Equal(t, legacyCtx.Config, v2Ctx.Config,
 				"serverCtx.Config differs between legacy and v2 (%s)", tc.name)
-			require.Equal(t, legacyCtx.Viper.AllSettings(), v2Ctx.Viper.AllSettings(),
+			require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper),
 				"serverCtx.Viper settings differ between legacy and v2 (%s)", tc.name)
 		})
 	}
@@ -319,7 +319,7 @@ func TestConfigManagerV2AdvisoryNeverRefusesBoot(t *testing.T) {
 
 	legacyCtx := runConfigManager(t, configmanager.LegacyConfigManager{}, home)
 	require.Equal(t, legacyCtx.Config, v2Ctx.Config)
-	require.Equal(t, legacyCtx.Viper.AllSettings(), v2Ctx.Viper.AllSettings())
+	require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper))
 }
 
 // TestConfigManagerV2FreshHomeBoots exercises the fresh-home first-boot path: v2's
@@ -419,13 +419,13 @@ func TestConfigManagerV2WritesNothing(t *testing.T) {
 			"leaves the resolved channels alone is invisible to every other assertion here")
 }
 
-// FuzzConfigManagerEnvOnlyKeyParity closes the one class the AllSettings comparison
+// FuzzConfigManagerEnvOnlyKeyParity closes the one class the settings comparison
 // cannot reach.
 //
-// AllSettings enumerates only what viper knows structurally, from the files it read,
-// its defaults, overrides and bound flags. A value carried solely by the environment
-// for a key absent from app.toml has no enumerable existence, so it appears in neither
-// AllSettings nor AllKeys, and every comparison above is blind to it. It is not
+// AllKeys enumerates only what viper knows structurally, from the files it read, its
+// defaults, overrides and bound flags. A value carried solely by the environment for a
+// key absent from app.toml has no enumerable existence, so it appears in neither
+// AllKeys nor anything built on it, and every comparison above is blind to it. It is not
 // invisible to the node: app.New reads through appOpts.Get, and AutomaticEnv resolves
 // at Get time, so such a value does reach running code.
 //
@@ -513,7 +513,7 @@ func FuzzConfigManagerEnvOnlyKeyParity(f *testing.F) {
 
 		require.Equal(t, legacyGot, v2Got,
 			"env-only key %q resolves differently between legacy and v2 (env %s=%q). This is "+
-				"invisible to the AllSettings comparison, and app.New reads it through "+
+				"invisible to the settings comparison, and app.New reads it through "+
 				"appOpts.Get, so it reaches the running node", key, envKey, value)
 	})
 }
@@ -559,6 +559,6 @@ func FuzzConfigManagerLegacyVsV2Parity(f *testing.F) {
 			return
 		}
 		require.Equal(t, legacyCtx.Config, v2Ctx.Config, "Config diverges (case %q, suffix %q)", tc.name, appTOMLSuffix)
-		require.Equal(t, legacyCtx.Viper.AllSettings(), v2Ctx.Viper.AllSettings(), "settings diverge (case %q, suffix %q)", tc.name, appTOMLSuffix)
+		require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper), "settings diverge (case %q, suffix %q)", tc.name, appTOMLSuffix)
 	})
 }
diff --git a/cmd/seid/cmd/envshadow_config_fuzz_test.go b/cmd/seid/cmd/envshadow_config_fuzz_test.go
new file mode 100644
index 0000000000..99e28e5fd8
--- /dev/null
+++ b/cmd/seid/cmd/envshadow_config_fuzz_test.go
@@ -0,0 +1,174 @@
+package cmd
+
+import (
+	"fmt"
+	"slices"
+	"strings"
+	"testing"
+
+	"github.com/sei-protocol/sei-chain/testutil/configtest"
+	"github.com/stretchr/testify/require"
+)
+
+// Environment-variable shadowing of a whole config section.
+//
+// A non-empty SEID_<SECTION> variable makes every key under that section resolve to
+// nothing, so the operator's written value in app.toml is discarded and the reader
+// falls back to its in-code default. The variable's own value is never used: setting
+// the variable to "false" produces the same result as setting it to "true" or to any
+// other string. So this is not the environment overriding the file, which is the
+// documented and intended precedence — it is the file's value being dropped and a
+// third value, belonging to no layer, taking effect.
+//
+// The mechanism is viper's isPathShadowedInAutoEnv, which walks every proper prefix of
+// a dotted key and, if any prefixed variable is set, returns before the config file is
+// consulted. It cannot distinguish "SEID_GIGA_EXECUTOR names a scalar, so
+// giga_executor.enabled cannot exist" from "SEID_GIGA_EXECUTOR is unrelated to the key
+// I was asked for".
+//
+// These rows pin the behavior; they do not endorse it. The legacy path has shipped, and
+// changing how configuration resolves could silently break an operator who has come to
+// depend on the current answer, so the behavior is recorded here and left alone.
+// SeiConfigManager corrects it when it owns resolution, and the divergence is ratified
+// against these rows rather than discovered against a production node.
+//
+// The consequence is worth stating where the pin lives, because it is what makes the
+// pin worth having rather than trivia. giga_executor.enabled defaults to true
+// (giga/executor/config/config.go), its reader is presence-guarded so a nil read keeps
+// that default, and app.New feeds it to tmtypes.SkipLastResultsHashValidation.Store —
+// which gates whether the node compares block.LastResultsHash against
+// state.LastResultsHash (sei-tendermint/internal/state/validation.go). An operator who
+// writes `enabled = false` and has any non-empty SEID_GIGA_EXECUTOR in the process
+// environment therefore gets giga enabled and that consensus comparison relaxed, with
+// nothing in any log saying so.
+
+// sectionEnvVar returns the environment variable that shadows the given dotted path,
+// derived the way the server viper derives it rather than built by hand.
+//
+// Built by hand it is easy to get wrong in a way that reads as "the defect does not
+// exist": the replacer runs over the whole prefixed name, so folding only the key and
+// then joining with an underscore yields SEID_GIGA.EXECUTOR, which matches nothing and
+// shadows nothing.
+func sectionEnvVar(t *testing.T, path string) string {
+	t.Helper()
+	prefix, err := configtest.ServerEnvPrefix()
+	if err != nil {
+		t.Fatalf("resolve server env prefix: %v", err)
+	}
+	return configtest.ServerEnvKey(prefix, path)
+}
+
+// bootWithAppTOML boots one fixture home carrying the given app.toml body through the
+// legacy manager and returns the resolved viper every appOpts.Get() call site reads.
+func bootWithAppTOML(t *testing.T, body string) *configtest.Home {
+	t.Helper()
+	home := configtest.NewHome(t)
+	home.WriteAppTOML(t, []byte(body))
+	return home
+}
+
+// TestSectionEnvVarShadowsItsWholeSection records the shadow itself: the section's keys
+// stop resolving, a sibling section is untouched, and enumeration keeps listing the
+// shadowed keys.
+func TestSectionEnvVarShadowsItsWholeSection(t *testing.T) {
+	configtest.Isolate(t)
+
+	const body = `minimum-gas-prices = "0.1usei"
+
+[giga_executor]
+enabled = false
+occ_enabled = false
+
+[evm]
+max_log_bytes = 100
+`
+	home := bootWithAppTOML(t, body)
+	shadowVar := sectionEnvVar(t, "giga_executor")
+
+	// Baseline: no shadowing variable, so the operator's written values win.
+	base := applyLegacy(t, home, nil)
+	require.NoError(t, base.err)
+	require.Equal(t, false, base.ctx.Viper.Get("giga_executor.enabled"),
+		"without a shadowing variable the operator's written value must win")
+
+	for _, value := range []string{"true", "false", "1", "0", "anything-at-all"} {
+		t.Run("value="+value, func(t *testing.T) {
+			t.Setenv(shadowVar, value)
+			got := applyLegacy(t, home, nil)
+			require.NoError(t, got.err, "shadowing must not refuse the boot")
+			v := got.ctx.Viper
+
+			// Every key under the shadowed section resolves to nothing, whatever the
+			// variable's value. That is what distinguishes this from an override.
+			require.Nil(t, v.Get("giga_executor.enabled"),
+				"%s=%q must make giga_executor.enabled resolve to nothing", shadowVar, value)
+			require.Nil(t, v.Get("giga_executor.occ_enabled"))
+			require.False(t, v.IsSet("giga_executor.enabled"))
+
+			// A sibling section is untouched: the shadow is scoped to the prefix the
+			// variable names, not to the whole file.
+			require.Equal(t, int64(100), v.Get("evm.max_log_bytes"),
+				"the shadow must not reach a section the variable does not name")
+
+			// Enumeration still reports the shadowed keys as present, which is why a
+			// reader cannot infer "in effect" from "enumerated".
+			require.True(t, slices.Contains(v.AllKeys(), "giga_executor.enabled"),
+				"AllKeys must still list a shadowed key")
+		})
+	}
+}
+
+// TestEmptySectionEnvVarDoesNotShadow pins the boundary: an empty value is not a
+// shadow. viper's env lookup treats an empty value as absent unless AllowEmptyEnv is
+// set, and seid does not set it. Recorded because the empty and non-empty cases behave
+// differently, and only the pair distinguishes the real rule from "the variable
+// exists".
+func TestEmptySectionEnvVarDoesNotShadow(t *testing.T) {
+	configtest.Isolate(t)
+
+	home := bootWithAppTOML(t, "[giga_executor]\nenabled = false\n")
+	t.Setenv(sectionEnvVar(t, "giga_executor"), "")
+
+	got := applyLegacy(t, home, nil)
+	require.NoError(t, got.err)
+	require.Equal(t, false, got.ctx.Viper.Get("giga_executor.enabled"),
+		"an empty shadowing variable must not shadow; the operator's value must still win")
+	require.True(t, got.ctx.Viper.IsSet("giga_executor.enabled"))
+}
+
+// TestFullKeyEnvVarDeliversRatherThanShadows pins the other boundary, and it is the row
+// that shows the shadow is not precedence. A variable naming the key's FULL path
+// delivers its value, which is the intended env-over-file precedence. A variable naming
+// a proper prefix of that path delivers nothing and discards the file's value. Same
+// key, same environment mechanism, opposite outcomes.
+func TestFullKeyEnvVarDeliversRatherThanShadows(t *testing.T) {
+	configtest.Isolate(t)
+
+	home := bootWithAppTOML(t, "[giga_executor]\nocc_enabled = false\n")
+
+	t.Setenv(sectionEnvVar(t, "giga_executor.occ_enabled"), "true")
+	delivered := applyLegacy(t, home, nil)
+	require.NoError(t, delivered.err)
+	require.Equal(t, "true", fmt.Sprint(delivered.ctx.Viper.Get("giga_executor.occ_enabled")),
+		"a variable naming the full key path must deliver its value (env over file)")
+}
+
+// TestShadowFoldsSectionPunctuation records that the shadow does not care how the
+// section is punctuated, because the replacer folds ".", "-" and "_" to "_" before the
+// lookup. A dashed section is shadowed by the same variable-name shape as an
+// underscored one — which is also why two differently-punctuated names cannot be
+// distinguished by environment delivery.
+func TestShadowFoldsSectionPunctuation(t *testing.T) {
+	configtest.Isolate(t)
+
+	home := bootWithAppTOML(t, "[state-commit]\nsc-enable = true\n")
+	shadowVar := sectionEnvVar(t, "state-commit")
+	require.True(t, strings.Contains(shadowVar, "STATE_COMMIT"),
+		"the replacer must fold the dash to an underscore: got %q", shadowVar)
+
+	t.Setenv(shadowVar, "x")
+	got := applyLegacy(t, home, nil)
+	require.NoError(t, got.err)
+	require.Nil(t, got.ctx.Viper.Get("state-commit.sc-enable"),
+		"%s must shadow a dashed section the same way it shadows an underscored one", shadowVar)
+}
diff --git a/testutil/configtest/viper.go b/testutil/configtest/viper.go
index 5e787a2925..bb6b7a21b7 100644
--- a/testutil/configtest/viper.go
+++ b/testutil/configtest/viper.go
@@ -34,3 +34,34 @@ func DumpViper(v *viper.Viper) string {
 	}
 	return strings.Join(lines, "\n")
 }
+
+// Settings renders a viper instance as a flat map from dotted key to resolved
+// value: one entry per AllKeys entry, each value as Get returns it.
+//
+// Use this, not AllSettings, to compare two vipers. AllSettings re-nests the flat
+// key space by splitting each key on "." and merging the pieces into a tree, and
+// when one key is a dotted prefix of another — "giga" alongside "giga.x" — whether
+// the scalar or the sub-tree survives depends on map iteration order. Two
+// AllSettings calls over one file then disagree, so an equality assertion between
+// two of them can fail on identical input. Get is unaffected: it tries longest
+// prefixes first, so both keys resolve. Settings therefore keys on AllKeys and
+// reads each key through Get, which is stable by construction.
+//
+// A flat map rather than DumpViper's rendered string, for two reasons. Values keep
+// their concrete Go type, so int64(8) and "8" do not compare equal. And a key is a
+// map key rather than a line in a newline-joined document, so a key containing a
+// newline — legal in TOML, and reachable from a fuzz target that appends arbitrary
+// bytes — cannot make two different key sets render identically. DumpViper stays
+// the right tool for a readable failure message; this is the right tool for the
+// assertion itself.
+func Settings(v *viper.Viper) map[string]any {
+	if v == nil {
+		return nil
+	}
+	keys := v.AllKeys()
+	settings := make(map[string]any, len(keys))
+	for _, k := range keys {
+		settings[k] = v.Get(k)
+	}
+	return settings
+}
diff --git a/testutil/configtest/viper_settings_test.go b/testutil/configtest/viper_settings_test.go
new file mode 100644
index 0000000000..acb8517a2c
--- /dev/null
+++ b/testutil/configtest/viper_settings_test.go
@@ -0,0 +1,104 @@
+package configtest_test
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/sei-protocol/sei-chain/testutil/configtest"
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/require"
+)
+
+// newViperOver returns a viper reading the given app.toml body, configured the way the
+// server viper is: SEID prefix, AutomaticEnv, and the replacer that folds ".", "-" and
+// "_" to "_".
+func newViperOver(t *testing.T, body string) *viper.Viper {
+	t.Helper()
+	path := filepath.Join(t.TempDir(), "app.toml")
+	if err := os.WriteFile(path, []byte(body), 0o600); err != nil {
+		t.Fatalf("write app.toml: %v", err)
+	}
+	v := viper.New()
+	v.SetConfigFile(path)
+	if err := v.ReadInConfig(); err != nil {
+		t.Fatalf("read app.toml: %v", err)
+	}
+	return v
+}
+
+// TestSettingsIsStableWhereAllSettingsIsNot is the reason Settings exists, and it is
+// written as a comparison so the defect and the fix are visible in one place.
+//
+// The shape that breaks AllSettings is one key being a dotted prefix of another. That
+// makes "giga" ambiguous — a value, or the parent of "giga.x" — and AllSettings resolves
+// the ambiguity by map iteration order, which Go randomizes per run. Settings never
+// re-nests, so there is no ambiguity to resolve.
+//
+// The assertion is deliberately asymmetric: it requires Settings to be stable, and it
+// only *reports* whether AllSettings varied on this run rather than requiring that it
+// does. Requiring instability would make this test depend on hitting both orderings
+// within the sample, which is the same coin-flip it is documenting.
+func TestSettingsIsStableWhereAllSettingsIsNot(t *testing.T) {
+	const body = `
+[section]
+"giga" = 1
+"giga.x" = 2
+`
+	const runs = 200
+
+	settingsShapes := map[string]int{}
+	allSettingsShapes := map[string]int{}
+	for i := 0; i < runs; i++ {
+		v := newViperOver(t, body)
+		settingsShapes[fmt.Sprint(configtest.Settings(v))]++
+		allSettingsShapes[fmt.Sprint(v.AllSettings())]++
+	}
+
+	require.Len(t, settingsShapes, 1,
+		"Settings must return one shape across %d reads of one file; got %d: %v",
+		runs, len(settingsShapes), settingsShapes)
+
+	if len(allSettingsShapes) > 1 {
+		t.Logf("AllSettings returned %d distinct shapes across %d reads, as expected: %v",
+			len(allSettingsShapes), runs, allSettingsShapes)
+	} else {
+		t.Logf("AllSettings happened to return one shape across %d reads this time; the "+
+			"instability is order-dependent, so a single stable sample does not clear it",
+			runs)
+	}
+}
+
+// TestSettingsPreservesConcreteTypes pins the property that makes Settings usable for a
+// parity assertion: a TOML integer and a TOML string are not interchangeable, so a
+// comparison built on Settings catches a manager that resolves one as the other.
+func TestSettingsPreservesConcreteTypes(t *testing.T) {
+	v := newViperOver(t, "[s]\nnum = 8\nstr = \"8\"\n")
+	got := configtest.Settings(v)
+
+	require.Equal(t, int64(8), got["s.num"], "a TOML integer must stay an integer")
+	require.Equal(t, "8", got["s.str"], "a TOML string must stay a string")
+	require.NotEqual(t, got["s.num"], got["s.str"],
+		"a parity comparison must be able to tell 8 from \"8\"")
+}
+
+// TestSettingsKeysCannotCollideThroughRendering pins the second reason Settings is a map
+// rather than a rendered document. A key may legally contain a newline, and the parity
+// fuzz target appends arbitrary bytes to app.toml, so a newline-joined rendering can make
+// two different key sets produce one identical string. Map keys cannot collide that way.
+func TestSettingsKeysCannotCollideThroughRendering(t *testing.T) {
+	two := configtest.Settings(newViperOver(t, "[s]\n\"a\" = 1\n\"b\" = 2\n"))
+	one := configtest.Settings(newViperOver(t, "[s]\n\"a = 1\\nb\" = 2\n"))
+
+	require.NotEqual(t, two, one,
+		"two keys and one newline-containing key must not compare equal")
+	require.Len(t, two, 2)
+	require.Len(t, one, 1)
+}
+
+// TestSettingsOnNilViper pins the nil case, since the differential builds a
+// server.Context whose Viper may be unset before Apply runs.
+func TestSettingsOnNilViper(t *testing.T) {
+	require.Nil(t, configtest.Settings(nil))
+}

From d0cb8463375f191a785e1930ee86ed0137a91dfd Mon Sep 17 00:00:00 2001
From: bdchatham <bdchatham@gmail.com>
Date: Sat, 1 Aug 2026 08:49:58 -0700
Subject: [PATCH 2/8] test(config): address review feedback on the
 characterization pins
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Five review items, all non-blocking, one a real regression this branch
introduced.

Settings no longer tolerates a nil viper. (*viper.Viper)(nil).AllSettings()
panics — the dereference is v.aliases inside AllKeys — so before this branch an
unpopulated serverCtx.Viper failed loudly. Guarding it turned that into
require.Equal(nil, nil), which passes: the differential's central premise
reporting success on a boot that never happened. The nil branch is gone, and
TestSettingsOnNilViper became TestSettingsOnNilViperPanics so the loud failure
is pinned rather than merely restored. DumpViper keeps its nil tolerance,
because describing a broken state is its job; Settings is the assertion, and
tolerance there is what converts a broken premise into a pass.

The six comparisons now route through requireSameSettings, which asserts both
contexts carry a viper before comparing. A helper rather than twelve inline
lines for the same reason the regression happened: an assertion a seventh call
site can forget is the shape of defect being fixed. Verified by handing it two
unpopulated contexts and confirming it fails naming which premise broke.

Corrected the newViperOver doc comment, which claimed a SEID prefix,
AutomaticEnv and the replacer that the helper never configured. The helper is
deliberately file-only; the env behavior is pinned against the real server viper
in the sibling file, and wiring a second weaker copy here would make four
hermetic tests answerable to the developer's shell.

Dropped fmt.Sprint from the full-key delivery assertion. It erased the property
this change exists to preserve: the file layer resolves that key to bool(false)
and the env layer to string("true"), and asserting the string records the
asymmetry the coercion hides.

A fresh fixture home per row, per testutil/configtest/AGENTS.md. The shared home
was safe — app.toml is written only when absent — but the first boot creates
config.toml, so the baseline row and the shadowed rows differed in two variables
rather than one.

Renamed envshadow_config_fuzz_test.go to envshadow_config_test.go. It carries no
Fuzz target while all seven other *_config_fuzz_test.go files in the package do,
so the suffix keeps meaning what it says. No fuzz target was added to justify the
old name: isPathShadowedInAutoEnv tests only presence-and-non-emptiness, so the
value space collapses to a bit that two existing rows already pin.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../cmd/configmanager_differential_test.go    | 29 +++++++++++++++----
 ..._fuzz_test.go => envshadow_config_test.go} | 17 +++++++----
 testutil/configtest/viper.go                  |  8 +++--
 testutil/configtest/viper_settings_test.go    | 17 ++++++-----
 4 files changed, 49 insertions(+), 22 deletions(-)
 rename cmd/seid/cmd/{envshadow_config_fuzz_test.go => envshadow_config_test.go} (91%)

diff --git a/cmd/seid/cmd/configmanager_differential_test.go b/cmd/seid/cmd/configmanager_differential_test.go
index 4ee43f7e3e..6766b6dda7 100644
--- a/cmd/seid/cmd/configmanager_differential_test.go
+++ b/cmd/seid/cmd/configmanager_differential_test.go
@@ -134,6 +134,23 @@ func runManager(t *testing.T, mgr configmanager.ConfigManager, cmd *cobra.Comman
 	return serverCtx, applyErr
 }
 
+// requireSameSettings asserts legacy and v2 resolved the same flat settings map, having
+// first asserted that both contexts have a viper at all.
+//
+// The guard is why this is a helper rather than a bare require.Equal. Apply is what
+// populates serverCtx.Viper, so a context it never reached carries nil, and comparing
+// two of those is this file's central premise reporting a pass on a boot that did not
+// happen. Settings panics on a nil viper, so the guard is the redundant half of the
+// pair — it is the half that names the broken premise instead of surfacing a nil
+// dereference from inside viper. Every settings comparison in this file goes through
+// here, so the next one cannot be written without it.
+func requireSameSettings(t *testing.T, legacy, v2 *server.Context, msgAndArgs ...any) {
+	t.Helper()
+	require.NotNil(t, legacy.Viper, "legacy Apply left serverCtx.Viper nil: there is no resolved config to compare")
+	require.NotNil(t, v2.Viper, "v2 Apply left serverCtx.Viper nil: there is no resolved config to compare")
+	require.Equal(t, configtest.Settings(legacy.Viper), configtest.Settings(v2.Viper), msgAndArgs...)
+}
+
 // seedDefaultConfig returns a home carrying a complete, realistic config (all Sei
 // sections), generated by letting the legacy creator write into a fresh home.
 func seedDefaultConfig(t *testing.T) *configtest.Home {
@@ -233,7 +250,7 @@ func TestConfigManagerLegacyVsV2Differential(t *testing.T) {
 
 	require.Equal(t, legacyCtx.Config, v2Ctx.Config,
 		"serverCtx.Config differs between legacy and v2")
-	require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper),
+	requireSameSettings(t, legacyCtx, v2Ctx,
 		"serverCtx.Viper settings differ between legacy and v2")
 
 	// The start.go chain-id mutation is identical on both vipers; assert parity
@@ -241,7 +258,7 @@ func TestConfigManagerLegacyVsV2Differential(t *testing.T) {
 	const chainID = "differential-test-1"
 	legacyCtx.Viper.Set(flags.FlagChainID, chainID)
 	v2Ctx.Viper.Set(flags.FlagChainID, chainID)
-	require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper),
+	requireSameSettings(t, legacyCtx, v2Ctx,
 		"settings diverge after the start.go chain-id mutation")
 }
 
@@ -278,7 +295,7 @@ func TestConfigManagerLegacyVsV2Differential_EnvHome(t *testing.T) {
 
 	require.Equal(t, legacyCtx.Config, v2Ctx.Config,
 		"serverCtx.Config differs between legacy and v2 on the env-home path")
-	require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper),
+	requireSameSettings(t, legacyCtx, v2Ctx,
 		"serverCtx.Viper settings differ between legacy and v2 on the env-home path")
 }
 
@@ -300,7 +317,7 @@ func TestConfigManagerLegacyVsV2Differential_Corpus(t *testing.T) {
 
 			require.Equal(t, legacyCtx.Config, v2Ctx.Config,
 				"serverCtx.Config differs between legacy and v2 (%s)", tc.name)
-			require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper),
+			requireSameSettings(t, legacyCtx, v2Ctx,
 				"serverCtx.Viper settings differ between legacy and v2 (%s)", tc.name)
 		})
 	}
@@ -319,7 +336,7 @@ func TestConfigManagerV2AdvisoryNeverRefusesBoot(t *testing.T) {
 
 	legacyCtx := runConfigManager(t, configmanager.LegacyConfigManager{}, home)
 	require.Equal(t, legacyCtx.Config, v2Ctx.Config)
-	require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper))
+	requireSameSettings(t, legacyCtx, v2Ctx)
 }
 
 // TestConfigManagerV2FreshHomeBoots exercises the fresh-home first-boot path: v2's
@@ -559,6 +576,6 @@ func FuzzConfigManagerLegacyVsV2Parity(f *testing.F) {
 			return
 		}
 		require.Equal(t, legacyCtx.Config, v2Ctx.Config, "Config diverges (case %q, suffix %q)", tc.name, appTOMLSuffix)
-		require.Equal(t, configtest.Settings(legacyCtx.Viper), configtest.Settings(v2Ctx.Viper), "settings diverge (case %q, suffix %q)", tc.name, appTOMLSuffix)
+		requireSameSettings(t, legacyCtx, v2Ctx, "settings diverge (case %q, suffix %q)", tc.name, appTOMLSuffix)
 	})
 }
diff --git a/cmd/seid/cmd/envshadow_config_fuzz_test.go b/cmd/seid/cmd/envshadow_config_test.go
similarity index 91%
rename from cmd/seid/cmd/envshadow_config_fuzz_test.go
rename to cmd/seid/cmd/envshadow_config_test.go
index 99e28e5fd8..8cd6c1d3d1 100644
--- a/cmd/seid/cmd/envshadow_config_fuzz_test.go
+++ b/cmd/seid/cmd/envshadow_config_test.go
@@ -1,7 +1,6 @@
 package cmd
 
 import (
-	"fmt"
 	"slices"
 	"strings"
 	"testing"
@@ -70,6 +69,11 @@ func bootWithAppTOML(t *testing.T, body string) *configtest.Home {
 // TestSectionEnvVarShadowsItsWholeSection records the shadow itself: the section's keys
 // stop resolving, a sibling section is untouched, and enumeration keeps listing the
 // shadowed keys.
+//
+// Every row boots its own fixture home, the baseline included: the legacy path treats a
+// node directory as read-write while it reads it and creates config.toml on first boot,
+// so one home shared across the rows would vary a second input alongside the variable
+// under test (testutil/configtest/AGENTS.md).
 func TestSectionEnvVarShadowsItsWholeSection(t *testing.T) {
 	configtest.Isolate(t)
 
@@ -82,11 +86,10 @@ occ_enabled = false
 [evm]
 max_log_bytes = 100
 `
-	home := bootWithAppTOML(t, body)
 	shadowVar := sectionEnvVar(t, "giga_executor")
 
 	// Baseline: no shadowing variable, so the operator's written values win.
-	base := applyLegacy(t, home, nil)
+	base := applyLegacy(t, bootWithAppTOML(t, body), nil)
 	require.NoError(t, base.err)
 	require.Equal(t, false, base.ctx.Viper.Get("giga_executor.enabled"),
 		"without a shadowing variable the operator's written value must win")
@@ -94,7 +97,7 @@ max_log_bytes = 100
 	for _, value := range []string{"true", "false", "1", "0", "anything-at-all"} {
 		t.Run("value="+value, func(t *testing.T) {
 			t.Setenv(shadowVar, value)
-			got := applyLegacy(t, home, nil)
+			got := applyLegacy(t, bootWithAppTOML(t, body), nil)
 			require.NoError(t, got.err, "shadowing must not refuse the boot")
 			v := got.ctx.Viper
 
@@ -149,8 +152,10 @@ func TestFullKeyEnvVarDeliversRatherThanShadows(t *testing.T) {
 	t.Setenv(sectionEnvVar(t, "giga_executor.occ_enabled"), "true")
 	delivered := applyLegacy(t, home, nil)
 	require.NoError(t, delivered.err)
-	require.Equal(t, "true", fmt.Sprint(delivered.ctx.Viper.Get("giga_executor.occ_enabled")),
-		"a variable naming the full key path must deliver its value (env over file)")
+	require.Equal(t, "true", delivered.ctx.Viper.Get("giga_executor.occ_enabled"),
+		"a variable naming the full key path must deliver its value (env over file), and it "+
+			"arrives as the untyped string it was written as — the same key read from the file "+
+			"layer resolves to a bool, so which layer a value came from changes its Go type")
 }
 
 // TestShadowFoldsSectionPunctuation records that the shadow does not care how the
diff --git a/testutil/configtest/viper.go b/testutil/configtest/viper.go
index bb6b7a21b7..acefc1e801 100644
--- a/testutil/configtest/viper.go
+++ b/testutil/configtest/viper.go
@@ -54,10 +54,12 @@ func DumpViper(v *viper.Viper) string {
 // bytes — cannot make two different key sets render identically. DumpViper stays
 // the right tool for a readable failure message; this is the right tool for the
 // assertion itself.
+//
+// A nil viper is left to panic on AllKeys rather than guarded into an empty result,
+// which is the same distinction: DumpViper renders <nil-viper> because describing a
+// broken state is its job, while an empty result here would let two contexts nobody
+// populated compare equal and report a parity that was never established.
 func Settings(v *viper.Viper) map[string]any {
-	if v == nil {
-		return nil
-	}
 	keys := v.AllKeys()
 	settings := make(map[string]any, len(keys))
 	for _, k := range keys {
diff --git a/testutil/configtest/viper_settings_test.go b/testutil/configtest/viper_settings_test.go
index acb8517a2c..e2290857f4 100644
--- a/testutil/configtest/viper_settings_test.go
+++ b/testutil/configtest/viper_settings_test.go
@@ -11,9 +11,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// newViperOver returns a viper reading the given app.toml body, configured the way the
-// server viper is: SEID prefix, AutomaticEnv, and the replacer that folds ".", "-" and
-// "_" to "_".
+// newViperOver returns a viper whose only layer is the given app.toml body: no prefix,
+// no AutomaticEnv, no replacer. Settings's interaction with the environment is pinned
+// against the real server viper in cmd/seid/cmd, so nothing here needs an env layer.
 func newViperOver(t *testing.T, body string) *viper.Viper {
 	t.Helper()
 	path := filepath.Join(t.TempDir(), "app.toml")
@@ -97,8 +97,11 @@ func TestSettingsKeysCannotCollideThroughRendering(t *testing.T) {
 	require.Len(t, one, 1)
 }
 
-// TestSettingsOnNilViper pins the nil case, since the differential builds a
-// server.Context whose Viper may be unset before Apply runs.
-func TestSettingsOnNilViper(t *testing.T) {
-	require.Nil(t, configtest.Settings(nil))
+// TestSettingsOnNilViperPanics pins the nil case as a failure rather than a tolerance,
+// since the differential builds a server.Context whose Viper stays nil until Apply
+// populates it. An empty map would let two such contexts compare equal, so the
+// differential's strongest premise would report success on a boot that never ran. The
+// panic is what makes the guard at those call sites redundant rather than load-bearing.
+func TestSettingsOnNilViperPanics(t *testing.T) {
+	require.Panics(t, func() { configtest.Settings(nil) })
 }

From 90fa8bbddac3999be012fdc96a95d388c43838c8 Mon Sep 17 00:00:00 2001
From: bdchatham <bdchatham@gmail.com>
Date: Sat, 1 Aug 2026 09:36:37 -0700
Subject: [PATCH 3/8] test(config): strengthen the characterization pins after
 the second review cycle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Six items, two of which make the tests stronger than they were written.

The AllSettings instability is now asserted rather than logged. Requiring the
instability itself would depend on hitting both iteration orders within a sample,
which is the coin flip being documented — but lossiness is unconditional: both
orderings destroy a value, and only which one varies. Verified over 5000 reads:
exactly two shapes, leaf count always 1, AllKeys always 2. Each read now asserts
that flattening AllSettings yields one fewer leaf than AllKeys has keys, with a
guard that the fixture still presents two colliding keys, since a degenerate
fixture would satisfy a bare inequality.

The consensus consequence in the shadow test's header is now pinned at the step
it rests on. giga's own suite pins nil-keeps-default against a synthetic
AppOptions, and app/consensus_config_fuzz_test.go pins enabled through to the
atomic, but nothing asserted the join: a real boot, a real shadowing variable,
the real reader. Each shadow row now calls gigaconfig.ReadConfig on the resolved
viper and asserts Enabled is true against an app.toml that says false. The
literal true is asserted separately from the DefaultConfig comparison, because a
comparison against the default alone passes for whatever the default happens to
be — the trap the harness guide describes for goldens.

Settings' doc now gives both reasons it exists. Beyond flattening, viper's
getSettings skips keys whose Get returns nil, which is exactly the shadow this
branch pins, so a key one side enumerates and resolves to nothing is
indistinguishable from a key the other side never enumerated. Only the flat form
separates them.

requireSameSettings reports through DumpViper on mismatch while keeping the typed
map as the authority, since a rendering can collide where the maps differ. Note
this does not shrink the failure — testify already produced a sorted, typed,
per-key diff, and rendering grows the output about 25% — but the diff line a
human reads is in the harness's own notation.

bootWithAppTOML renamed to homeWithAppTOML: it built a home and returned one, and
booting is applyLegacy's job.

A sweep for the stale-comment class found two more, both introduced on this
branch. The env-name hazard example was impossible as written — there is no dot
in giga_executor, and the real hazard is the prefix, which under go test is
cmd.test. And a claim that the replacer folds ".", "-" and "_" invented a third
rule; there are two. That invention had already propagated from a comment dropped
in the previous commit, which is why the sweep was worth running.

One temp directory instead of 200: the loop exercises map iteration, not parsing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../cmd/configmanager_differential_test.go    | 21 +++++-
 cmd/seid/cmd/envshadow_config_test.go         | 48 +++++++++----
 testutil/configtest/viper.go                  | 28 +++++---
 testutil/configtest/viper_settings_test.go    | 68 +++++++++++++++----
 4 files changed, 131 insertions(+), 34 deletions(-)

diff --git a/cmd/seid/cmd/configmanager_differential_test.go b/cmd/seid/cmd/configmanager_differential_test.go
index 6766b6dda7..324290d293 100644
--- a/cmd/seid/cmd/configmanager_differential_test.go
+++ b/cmd/seid/cmd/configmanager_differential_test.go
@@ -14,6 +14,7 @@ import (
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/otel/sdk/trace"
 
@@ -144,11 +145,29 @@ func runManager(t *testing.T, mgr configmanager.ConfigManager, cmd *cobra.Comman
 // pair — it is the half that names the broken premise instead of surfacing a nil
 // dereference from inside viper. Every settings comparison in this file goes through
 // here, so the next one cannot be written without it.
+//
+// Compared as typed maps, reported as sorted dumps. The comparison has to be the typed
+// one, because the whole point of Settings is that int64(8) and "8" differ. The report
+// goes through DumpViper because that is the notation the rest of this harness fails in:
+// a differing key reads as `evm.max_log_bytes = int64(67108864)` rather than as spew's
+// `(string) (len=17) "evm.max_log_bytes": (int64) 67108864`. Both forms diff per key,
+// since testify sorts a map before diffing it, so this buys legibility rather than a
+// shorter failure. Both dumps are built only after a difference is known, since this
+// runs per fuzz execution.
 func requireSameSettings(t *testing.T, legacy, v2 *server.Context, msgAndArgs ...any) {
 	t.Helper()
 	require.NotNil(t, legacy.Viper, "legacy Apply left serverCtx.Viper nil: there is no resolved config to compare")
 	require.NotNil(t, v2.Viper, "v2 Apply left serverCtx.Viper nil: there is no resolved config to compare")
-	require.Equal(t, configtest.Settings(legacy.Viper), configtest.Settings(v2.Viper), msgAndArgs...)
+
+	legacySettings, v2Settings := configtest.Settings(legacy.Viper), configtest.Settings(v2.Viper)
+	if assert.ObjectsAreEqual(legacySettings, v2Settings) {
+		return
+	}
+	// The dump diff is the readable report. It is a rendering, so two different key sets
+	// can render alike (a key holding a newline), and the typed assertion below is the
+	// authority either way: whichever of the two fires, the difference is reported.
+	require.Equal(t, configtest.DumpViper(legacy.Viper), configtest.DumpViper(v2.Viper), msgAndArgs...)
+	require.Equal(t, legacySettings, v2Settings, msgAndArgs...)
 }
 
 // seedDefaultConfig returns a home carrying a complete, realistic config (all Sei
diff --git a/cmd/seid/cmd/envshadow_config_test.go b/cmd/seid/cmd/envshadow_config_test.go
index 8cd6c1d3d1..c7457c9edb 100644
--- a/cmd/seid/cmd/envshadow_config_test.go
+++ b/cmd/seid/cmd/envshadow_config_test.go
@@ -5,6 +5,7 @@ import (
 	"strings"
 	"testing"
 
+	gigaconfig "github.com/sei-protocol/sei-chain/giga/executor/config"
 	"github.com/sei-protocol/sei-chain/testutil/configtest"
 	"github.com/stretchr/testify/require"
 )
@@ -40,14 +41,23 @@ import (
 // writes `enabled = false` and has any non-empty SEID_GIGA_EXECUTOR in the process
 // environment therefore gets giga enabled and that consensus comparison relaxed, with
 // nothing in any log saying so.
+//
+// Two of the three links in that chain are asserted, and the third is described. This
+// file asserts the shadow itself and, through gigaconfig.ReadConfig on the resolved
+// viper, that the nil read leaves enabled at true. app's
+// TestGigaExecutorEnabledDrivesLastResultsHashValidation asserts that enabled reaches the
+// atomic, through a real app construction. Only the join between them is prose here,
+// because observing it needs a running node, which this suite deliberately does not do
+// (testutil/configtest/AGENTS.md, "Out of Scope").
 
 // sectionEnvVar returns the environment variable that shadows the given dotted path,
 // derived the way the server viper derives it rather than built by hand.
 //
 // Built by hand it is easy to get wrong in a way that reads as "the defect does not
-// exist": the replacer runs over the whole prefixed name, so folding only the key and
-// then joining with an underscore yields SEID_GIGA.EXECUTOR, which matches nothing and
-// shadows nothing.
+// exist". The replacer runs over the whole prefixed name, prefix included, and the prefix
+// is the running binary's basename — "cmd.test" under go test. Folding only the key and
+// joining it to the prefix therefore leaves that dot in place, yielding
+// CMD.TEST_GIGA_EXECUTOR, which no environment lookup matches and which shadows nothing.
 func sectionEnvVar(t *testing.T, path string) string {
 	t.Helper()
 	prefix, err := configtest.ServerEnvPrefix()
@@ -57,9 +67,9 @@ func sectionEnvVar(t *testing.T, path string) string {
 	return configtest.ServerEnvKey(prefix, path)
 }
 
-// bootWithAppTOML boots one fixture home carrying the given app.toml body through the
-// legacy manager and returns the resolved viper every appOpts.Get() call site reads.
-func bootWithAppTOML(t *testing.T, body string) *configtest.Home {
+// homeWithAppTOML returns a fresh fixture home whose app.toml holds the given body.
+// Booting it is applyLegacy's job.
+func homeWithAppTOML(t *testing.T, body string) *configtest.Home {
 	t.Helper()
 	home := configtest.NewHome(t)
 	home.WriteAppTOML(t, []byte(body))
@@ -89,7 +99,7 @@ max_log_bytes = 100
 	shadowVar := sectionEnvVar(t, "giga_executor")
 
 	// Baseline: no shadowing variable, so the operator's written values win.
-	base := applyLegacy(t, bootWithAppTOML(t, body), nil)
+	base := applyLegacy(t, homeWithAppTOML(t, body), nil)
 	require.NoError(t, base.err)
 	require.Equal(t, false, base.ctx.Viper.Get("giga_executor.enabled"),
 		"without a shadowing variable the operator's written value must win")
@@ -97,7 +107,7 @@ max_log_bytes = 100
 	for _, value := range []string{"true", "false", "1", "0", "anything-at-all"} {
 		t.Run("value="+value, func(t *testing.T) {
 			t.Setenv(shadowVar, value)
-			got := applyLegacy(t, bootWithAppTOML(t, body), nil)
+			got := applyLegacy(t, homeWithAppTOML(t, body), nil)
 			require.NoError(t, got.err, "shadowing must not refuse the boot")
 			v := got.ctx.Viper
 
@@ -108,6 +118,20 @@ max_log_bytes = 100
 			require.Nil(t, v.Get("giga_executor.occ_enabled"))
 			require.False(t, v.IsSet("giga_executor.enabled"))
 
+			// What the nil read costs, asserted through the section's real reader
+			// rather than inferred from it. ReadConfig is presence-guarded, so a key
+			// that resolves to nothing leaves the in-code default standing, and
+			// enabled comes back true from a file that says false. This is the step
+			// the consensus consequence in the header rests on.
+			cfg, err := gigaconfig.ReadConfig(v)
+			require.NoError(t, err, "a shadowed section must not make the reader fail")
+			require.True(t, cfg.Enabled,
+				"%s=%q must leave giga_executor.enabled at its in-code default of true, "+
+					"against an app.toml that sets it to false", shadowVar, value)
+			require.Equal(t, gigaconfig.DefaultConfig, cfg,
+				"a shadowed section must resolve to exactly the in-code defaults, so no "+
+					"value the operator wrote survives anywhere in it")
+
 			// A sibling section is untouched: the shadow is scoped to the prefix the
 			// variable names, not to the whole file.
 			require.Equal(t, int64(100), v.Get("evm.max_log_bytes"),
@@ -129,7 +153,7 @@ max_log_bytes = 100
 func TestEmptySectionEnvVarDoesNotShadow(t *testing.T) {
 	configtest.Isolate(t)
 
-	home := bootWithAppTOML(t, "[giga_executor]\nenabled = false\n")
+	home := homeWithAppTOML(t, "[giga_executor]\nenabled = false\n")
 	t.Setenv(sectionEnvVar(t, "giga_executor"), "")
 
 	got := applyLegacy(t, home, nil)
@@ -147,7 +171,7 @@ func TestEmptySectionEnvVarDoesNotShadow(t *testing.T) {
 func TestFullKeyEnvVarDeliversRatherThanShadows(t *testing.T) {
 	configtest.Isolate(t)
 
-	home := bootWithAppTOML(t, "[giga_executor]\nocc_enabled = false\n")
+	home := homeWithAppTOML(t, "[giga_executor]\nocc_enabled = false\n")
 
 	t.Setenv(sectionEnvVar(t, "giga_executor.occ_enabled"), "true")
 	delivered := applyLegacy(t, home, nil)
@@ -159,14 +183,14 @@ func TestFullKeyEnvVarDeliversRatherThanShadows(t *testing.T) {
 }
 
 // TestShadowFoldsSectionPunctuation records that the shadow does not care how the
-// section is punctuated, because the replacer folds ".", "-" and "_" to "_" before the
+// section is punctuated, because the replacer folds "." and "-" to "_" before the
 // lookup. A dashed section is shadowed by the same variable-name shape as an
 // underscored one — which is also why two differently-punctuated names cannot be
 // distinguished by environment delivery.
 func TestShadowFoldsSectionPunctuation(t *testing.T) {
 	configtest.Isolate(t)
 
-	home := bootWithAppTOML(t, "[state-commit]\nsc-enable = true\n")
+	home := homeWithAppTOML(t, "[state-commit]\nsc-enable = true\n")
 	shadowVar := sectionEnvVar(t, "state-commit")
 	require.True(t, strings.Contains(shadowVar, "STATE_COMMIT"),
 		"the replacer must fold the dash to an underscore: got %q", shadowVar)
diff --git a/testutil/configtest/viper.go b/testutil/configtest/viper.go
index acefc1e801..28e80d8282 100644
--- a/testutil/configtest/viper.go
+++ b/testutil/configtest/viper.go
@@ -38,14 +38,26 @@ func DumpViper(v *viper.Viper) string {
 // Settings renders a viper instance as a flat map from dotted key to resolved
 // value: one entry per AllKeys entry, each value as Get returns it.
 //
-// Use this, not AllSettings, to compare two vipers. AllSettings re-nests the flat
-// key space by splitting each key on "." and merging the pieces into a tree, and
-// when one key is a dotted prefix of another — "giga" alongside "giga.x" — whether
-// the scalar or the sub-tree survives depends on map iteration order. Two
-// AllSettings calls over one file then disagree, so an equality assertion between
-// two of them can fail on identical input. Get is unaffected: it tries longest
-// prefixes first, so both keys resolve. Settings therefore keys on AllKeys and
-// reads each key through Get, which is stable by construction.
+// Use this, not AllSettings, to compare two vipers, for two independent reasons.
+//
+// The first is determinism. AllSettings re-nests the flat key space by splitting each
+// key on "." and merging the pieces into a tree, and when one key is a dotted prefix
+// of another — "giga" alongside "giga.x" — whether the scalar or the sub-tree survives
+// depends on map iteration order. Two AllSettings calls over one file then disagree, so
+// an equality assertion between two of them can fail on identical input. Get is
+// unaffected: it tries longest prefixes first, so both keys resolve. Settings therefore
+// keys on AllKeys and reads each key through Get, which is stable by construction.
+//
+// The second is coverage, and it is the one that makes this comparison stronger rather
+// than merely repeatable. AllSettings omits every key whose Get returns nil, on the
+// reasoning that AllKeys lists only keys that hold a value. The section-level
+// environment shadow this suite pins is precisely where that reasoning fails: AllKeys
+// lists giga_executor.enabled while Get returns nil, so AllSettings drops the key
+// entirely and Settings records it as an explicit nil entry. Two vipers must therefore
+// agree on which keys resolve to nothing, not only on the values of the keys that
+// resolve. That is a strictly larger surface: under AllSettings a key one side enumerates
+// and resolves to nil is indistinguishable from a key the other side never enumerated at
+// all, which is the pair a manager reading a different file set would produce.
 //
 // A flat map rather than DumpViper's rendered string, for two reasons. Values keep
 // their concrete Go type, so int64(8) and "8" do not compare equal. And a key is a
diff --git a/testutil/configtest/viper_settings_test.go b/testutil/configtest/viper_settings_test.go
index e2290857f4..15fd7425b4 100644
--- a/testutil/configtest/viper_settings_test.go
+++ b/testutil/configtest/viper_settings_test.go
@@ -15,11 +15,26 @@ import (
 // no AutomaticEnv, no replacer. Settings's interaction with the environment is pinned
 // against the real server viper in cmd/seid/cmd, so nothing here needs an env layer.
 func newViperOver(t *testing.T, body string) *viper.Viper {
+	t.Helper()
+	return newViperOverFile(t, writeAppTOML(t, body))
+}
+
+// writeAppTOML writes body to an app.toml in a fresh temp directory and returns its path.
+func writeAppTOML(t *testing.T, body string) string {
 	t.Helper()
 	path := filepath.Join(t.TempDir(), "app.toml")
 	if err := os.WriteFile(path, []byte(body), 0o600); err != nil {
 		t.Fatalf("write app.toml: %v", err)
 	}
+	return path
+}
+
+// newViperOverFile returns a viper that has read the app.toml at path, the same single
+// file layer newViperOver builds. It is separate so a test can read one file many times:
+// the nondeterminism such a test is after lives in viper's map iteration, not in the
+// parse, so re-reading one path varies only the thing under study.
+func newViperOverFile(t *testing.T, path string) *viper.Viper {
+	t.Helper()
 	v := viper.New()
 	v.SetConfigFile(path)
 	if err := v.ReadInConfig(); err != nil {
@@ -36,10 +51,16 @@ func newViperOver(t *testing.T, body string) *viper.Viper {
 // the ambiguity by map iteration order, which Go randomizes per run. Settings never
 // re-nests, so there is no ambiguity to resolve.
 //
-// The assertion is deliberately asymmetric: it requires Settings to be stable, and it
-// only *reports* whether AllSettings varied on this run rather than requiring that it
-// does. Requiring instability would make this test depend on hitting both orderings
-// within the sample, which is the same coin-flip it is documenting.
+// Both halves are assertions, and the AllSettings half holds on every single read rather
+// than over the sample. Whichever order the re-nesting visits the two keys in, one
+// value is destroyed: reaching "giga.x" through a scalar "giga" replaces that scalar with
+// a fresh map, and writing "giga" over an existing sub-tree discards the sub-tree. So the
+// re-nested tree always holds one leaf fewer than AllKeys has keys. Only *which* value is
+// lost varies with the ordering, which is the instability the shape count reports.
+//
+// Asserting the loss rather than the variation is what makes this a regression guard: it
+// cannot depend on hitting both orderings within the sample, and a viper that fixed the
+// collision fails here instead of quietly making the test meaningless.
 func TestSettingsIsStableWhereAllSettingsIsNot(t *testing.T) {
 	const body = `
 [section]
@@ -48,26 +69,47 @@ func TestSettingsIsStableWhereAllSettingsIsNot(t *testing.T) {
 `
 	const runs = 200
 
+	// One file, read many times. Writing it once keeps the parse out of the experiment,
+	// leaving map iteration order as the only thing that differs between reads.
+	path := writeAppTOML(t, body)
+
 	settingsShapes := map[string]int{}
 	allSettingsShapes := map[string]int{}
 	for i := 0; i < runs; i++ {
-		v := newViperOver(t, body)
+		v := newViperOverFile(t, path)
 		settingsShapes[fmt.Sprint(configtest.Settings(v))]++
-		allSettingsShapes[fmt.Sprint(v.AllSettings())]++
+
+		all := v.AllSettings()
+		allSettingsShapes[fmt.Sprint(all)]++
+
+		keys := v.AllKeys()
+		require.Len(t, keys, 2,
+			"the fixture must present the prefix collision this test is about; got keys %v", keys)
+		require.Equal(t, len(keys)-1, countLeaves(all),
+			"AllSettings must lose exactly one of the two colliding values on every read, "+
+				"whichever way it re-nested; got %v for keys %v", all, keys)
 	}
 
 	require.Len(t, settingsShapes, 1,
 		"Settings must return one shape across %d reads of one file; got %d: %v",
 		runs, len(settingsShapes), settingsShapes)
 
-	if len(allSettingsShapes) > 1 {
-		t.Logf("AllSettings returned %d distinct shapes across %d reads, as expected: %v",
-			len(allSettingsShapes), runs, allSettingsShapes)
-	} else {
-		t.Logf("AllSettings happened to return one shape across %d reads this time; the "+
-			"instability is order-dependent, so a single stable sample does not clear it",
-			runs)
+	t.Logf("AllSettings returned %d distinct shapes across %d reads, each of them lossy: %v",
+		len(allSettingsShapes), runs, allSettingsShapes)
+}
+
+// countLeaves counts the non-map values in a nested settings tree, which is how many of
+// AllKeys's keys survived being re-nested into it.
+func countLeaves(m map[string]any) int {
+	n := 0
+	for _, v := range m {
+		if sub, ok := v.(map[string]any); ok {
+			n += countLeaves(sub)
+			continue
+		}
+		n++
 	}
+	return n
 }
 
 // TestSettingsPreservesConcreteTypes pins the property that makes Settings usable for a

From 5afa3189c5e07bf40112ab4c032cfff8f9062a6e Mon Sep 17 00:00:00 2001
From: bdchatham <bdchatham@gmail.com>
Date: Sat, 1 Aug 2026 09:54:12 -0700
Subject: [PATCH 4/8] test(config): anchor the dash-folding row on a baseline,
 and pin its reader
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The dash-folding row asserted only that the key resolves to nothing under the
shadow. A nil is evidence of shadowing only if the key is known to resolve
without the variable set: a fixture typo or a section rename would have satisfied
that assertion while the property the row exists to pin went untested. It now
anchors on its own baseline home first, the same shape the section-shadow row
already used.

It also pins what the nil read costs, because this key's reader is shaped
differently from giga_executor's. GetConfig reads it unguarded —
v.GetBool("state-commit.sc-enable") at sei-cosmos/server/config/config.go:621,
no presence check — so a nil read is not a fallback to an in-code default, it is
GetBool(nil), which is false. The two sections therefore fail in opposite
directions from one mechanism: a shadowed giga_executor.enabled keeps its true
default and silently enables what the operator disabled, while a shadowed
state-commit.sc-enable resolves false and silently disables what the operator
enabled. Which direction a section takes depends only on whether its reader
guards the read, and that is worth recording where the shadow is pinned.

Both new assertions were shown to fail when they should: omitting the key from
the fixture trips the baseline, and removing the shadow trips the reader
assertion once the earlier nil assertion is taken out of the way so it cannot
pre-empt it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 cmd/seid/cmd/envshadow_config_test.go | 31 +++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/cmd/seid/cmd/envshadow_config_test.go b/cmd/seid/cmd/envshadow_config_test.go
index c7457c9edb..5b336397d2 100644
--- a/cmd/seid/cmd/envshadow_config_test.go
+++ b/cmd/seid/cmd/envshadow_config_test.go
@@ -187,17 +187,44 @@ func TestFullKeyEnvVarDeliversRatherThanShadows(t *testing.T) {
 // lookup. A dashed section is shadowed by the same variable-name shape as an
 // underscored one — which is also why two differently-punctuated names cannot be
 // distinguished by environment delivery.
+//
+// It also records the sharper half of the consequence, because this key's reader is
+// shaped differently from giga_executor's. GetConfig reads it unguarded —
+// v.GetBool("state-commit.sc-enable") at sei-cosmos/server/config/config.go:621, with no
+// presence check — so a nil read is not a fallback to an in-code default, it is
+// GetBool(nil), which is false. The two sections therefore fail in opposite directions
+// from one mechanism: a shadowed giga_executor.enabled keeps its true default and
+// silently enables what the operator disabled, while a shadowed state-commit.sc-enable
+// resolves false and silently disables what the operator enabled. Which direction a
+// section takes depends only on whether its reader guards the read.
+//
+// The baseline is what makes the nil assertion mean anything. A nil is evidence of
+// shadowing only if the key is known to resolve without the variable set — otherwise a
+// fixture typo or a section rename would satisfy the assertion while the dash-folding
+// property it exists to pin went untested.
 func TestShadowFoldsSectionPunctuation(t *testing.T) {
 	configtest.Isolate(t)
 
-	home := homeWithAppTOML(t, "[state-commit]\nsc-enable = true\n")
+	const body = "[state-commit]\nsc-enable = true\n"
 	shadowVar := sectionEnvVar(t, "state-commit")
 	require.True(t, strings.Contains(shadowVar, "STATE_COMMIT"),
 		"the replacer must fold the dash to an underscore: got %q", shadowVar)
 
+	// Baseline, on its own fixture home: the dashed key resolves, and the reader agrees.
+	base := applyLegacy(t, homeWithAppTOML(t, body), nil)
+	require.NoError(t, base.err)
+	require.Equal(t, true, base.ctx.Viper.Get("state-commit.sc-enable"),
+		"without a shadowing variable the dashed key must resolve to the written value")
+	require.True(t, base.ctx.Viper.GetBool("state-commit.sc-enable"),
+		"the reader must see the operator's value when nothing shadows it")
+
 	t.Setenv(shadowVar, "x")
-	got := applyLegacy(t, home, nil)
+	got := applyLegacy(t, homeWithAppTOML(t, body), nil)
 	require.NoError(t, got.err)
 	require.Nil(t, got.ctx.Viper.Get("state-commit.sc-enable"),
 		"%s must shadow a dashed section the same way it shadows an underscored one", shadowVar)
+	require.False(t, got.ctx.Viper.GetBool("state-commit.sc-enable"),
+		"%s must make the unguarded reader resolve false, against an app.toml that sets it "+
+			"true — the opposite direction from giga_executor, whose guarded reader keeps its "+
+			"true default instead", shadowVar)
 }

From 60d1a52f9234494e56b5d2dad2a4a1bb662ad912 Mon Sep 17 00:00:00 2001
From: bdchatham <bdchatham@gmail.com>
Date: Sat, 1 Aug 2026 10:03:16 -0700
Subject: [PATCH 5/8] test(config): assert Settings' coverage claim, and name
 viper in the lossiness failure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Settings' doc argues it is stronger than AllSettings for two reasons, and only the
first was pinned. The second — AllSettings omits a key whose Get returns nil while
Settings records it as an explicit nil entry — had no assertion anywhere, because
the only thing that produces a key which enumerates but resolves to nothing is the
environment shadow, and configtest's own tests build vipers with no env layer on
purpose. The shadow rows are therefore the one place it can be asserted, so they
now assert it: Contains before the nil check, since indexing an absent key also
yields nil and only the pair distinguishes "recorded as nil" from "not recorded".
Verified by making Settings skip nil-resolving keys, which fails the new row.

The lossiness assertion characterizes upstream viper rather than this repo, so a
viper bump that fixes the prefix collision fails a test while nothing in sei
changed. The comment said so; the failure message did not. It now tells the reader
to delete the assertion rather than work around it, since Settings is correct
either way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 cmd/seid/cmd/envshadow_config_test.go      | 14 ++++++++++++++
 testutil/configtest/viper_settings_test.go |  5 ++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/cmd/seid/cmd/envshadow_config_test.go b/cmd/seid/cmd/envshadow_config_test.go
index 5b336397d2..5d8c3a29a3 100644
--- a/cmd/seid/cmd/envshadow_config_test.go
+++ b/cmd/seid/cmd/envshadow_config_test.go
@@ -141,6 +141,20 @@ max_log_bytes = 100
 			// reader cannot infer "in effect" from "enumerated".
 			require.True(t, slices.Contains(v.AllKeys(), "giga_executor.enabled"),
 				"AllKeys must still list a shadowed key")
+
+			// The same pair, through configtest.Settings, because this is the only place
+			// it can be asserted. Settings' coverage argument is that AllSettings omits a
+			// key whose Get returns nil while Settings records it as an explicit nil
+			// entry — and a key that enumerates but resolves to nothing is what the
+			// environment shadow produces and nothing else does, so the vipers in
+			// configtest's own tests cannot construct one. Contains before the nil check:
+			// indexing an absent key also yields nil, so the two together are what
+			// distinguish "recorded as nil" from "not recorded".
+			settings := configtest.Settings(v)
+			require.Contains(t, settings, "giga_executor.enabled",
+				"Settings must record a shadowed key, which is the entry AllSettings drops")
+			require.Nil(t, settings["giga_executor.enabled"],
+				"Settings must record a shadowed key as an explicit nil entry")
 		})
 	}
 }
diff --git a/testutil/configtest/viper_settings_test.go b/testutil/configtest/viper_settings_test.go
index 15fd7425b4..e97f302099 100644
--- a/testutil/configtest/viper_settings_test.go
+++ b/testutil/configtest/viper_settings_test.go
@@ -87,7 +87,10 @@ func TestSettingsIsStableWhereAllSettingsIsNot(t *testing.T) {
 			"the fixture must present the prefix collision this test is about; got keys %v", keys)
 		require.Equal(t, len(keys)-1, countLeaves(all),
 			"AllSettings must lose exactly one of the two colliding values on every read, "+
-				"whichever way it re-nested; got %v for keys %v", all, keys)
+				"whichever way it re-nested; got %v for keys %v. This characterizes viper, "+
+				"not this repo: if it fails after a viper upgrade, AllSettings may have "+
+				"fixed the prefix collision, in which case delete this assertion rather "+
+				"than working around it — Settings stays correct either way", all, keys)
 	}
 
 	require.Len(t, settingsShapes, 1,

From abcc32f22ca28fb18094affca9a19d3abfb01b5f Mon Sep 17 00:00:00 2001
From: bdchatham <bdchatham@gmail.com>
Date: Sat, 1 Aug 2026 10:14:24 -0700
Subject: [PATCH 6/8] test(config): richer failure output, and document the
 comparison rule in the guide
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

require.Contains for the two membership checks. require.True on a boolean reports
only "false"; Contains prints the collection, so an AllKeys failure now names the
keys that were present and a prefix failure names the derived variable. That also
drops the slices and strings imports, leaving the file with three. Note the key
list prints in AllKeys order, which this branch's own finding says is unstable, so
it varies between runs — still strictly more than a bare false.

The comparison rule now lives in testutil/configtest/AGENTS.md, which is the entry
point the root guide points contributors at before they change a configuration
read. It said nothing about how to compare two resolved vipers, so the
Settings-for-assertions and DumpViper-for-messages distinction existed only in a
doc comment a contributor reaches after already choosing wrong. The new section
states both, why AllSettings is neither, and why Settings panics on a nil viper.

Named the shape fingerprint's dependency on fmt sorting map keys. The test's
premise is that map ordering is untrustworthy, and its own mechanism relies on the
ordering fmt hides — worth saying out loud in that specific test, since without the
sort every read would look like a distinct shape and the counts would be
meaningless rather than merely wrong.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 cmd/seid/cmd/envshadow_config_test.go      |  8 +++-----
 testutil/configtest/AGENTS.md              | 20 ++++++++++++++++++++
 testutil/configtest/viper_settings_test.go |  7 +++++++
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/cmd/seid/cmd/envshadow_config_test.go b/cmd/seid/cmd/envshadow_config_test.go
index 5d8c3a29a3..beabeb64dc 100644
--- a/cmd/seid/cmd/envshadow_config_test.go
+++ b/cmd/seid/cmd/envshadow_config_test.go
@@ -1,8 +1,6 @@
 package cmd
 
 import (
-	"slices"
-	"strings"
 	"testing"
 
 	gigaconfig "github.com/sei-protocol/sei-chain/giga/executor/config"
@@ -139,7 +137,7 @@ max_log_bytes = 100
 
 			// Enumeration still reports the shadowed keys as present, which is why a
 			// reader cannot infer "in effect" from "enumerated".
-			require.True(t, slices.Contains(v.AllKeys(), "giga_executor.enabled"),
+			require.Contains(t, v.AllKeys(), "giga_executor.enabled",
 				"AllKeys must still list a shadowed key")
 
 			// The same pair, through configtest.Settings, because this is the only place
@@ -221,8 +219,8 @@ func TestShadowFoldsSectionPunctuation(t *testing.T) {
 
 	const body = "[state-commit]\nsc-enable = true\n"
 	shadowVar := sectionEnvVar(t, "state-commit")
-	require.True(t, strings.Contains(shadowVar, "STATE_COMMIT"),
-		"the replacer must fold the dash to an underscore: got %q", shadowVar)
+	require.Contains(t, shadowVar, "STATE_COMMIT",
+		"the replacer must fold the dash to an underscore")
 
 	// Baseline, on its own fixture home: the dashed key resolves, and the reader agrees.
 	base := applyLegacy(t, homeWithAppTOML(t, body), nil)
diff --git a/testutil/configtest/AGENTS.md b/testutil/configtest/AGENTS.md
index eae9208a03..e5a5a6dc2b 100644
--- a/testutil/configtest/AGENTS.md
+++ b/testutil/configtest/AGENTS.md
@@ -134,6 +134,26 @@ go test ./<pkg>/ -run FuzzXxx -fuzz FuzzXxx -fuzztime 60s   # extend a target by
 Seeds cover every row on an ordinary run. Running the fuzzer by hand explores past
 the seeds and is worth doing when changing a reader's cast or its guard.
 
+## Comparing two resolved vipers
+
+Two managers, two boots, or a before/after both reduce to "did these resolve the same
+values". Compare `Settings`, and report with `DumpViper`.
+
+- `configtest.Settings` is the value you assert on: a flat map from dotted key to what
+  `Get` returns, one entry per `AllKeys` entry. Use it, not `Viper.AllSettings`.
+  `AllSettings` re-nests the flat key space by splitting on `.`, so when one key is a
+  dotted prefix of another it drops one of them depending on map iteration order, and
+  an equality assertion between two of them can fail on identical input. It also omits
+  every key whose `Get` returns nil, so a key one side enumerates and resolves to
+  nothing looks the same as a key the other side never enumerated at all.
+- `configtest.DumpViper` is for the failure message, not the assertion. It renders one
+  sorted, type-qualified line per key, which is what a human reads — but it joins with
+  newlines and does not quote keys, and a TOML key may legally contain a newline, so two
+  different key sets can render identically. Assert on the map; print the dump.
+- `Settings` panics on a nil viper, deliberately. A `server.Context` carries no viper
+  until `Apply` populates one, and a nil-tolerant comparison would let two contexts
+  nobody populated compare equal and report a parity that was never established.
+
 ## Hermeticity
 
 These tests control more than the arguments to the function under test, because the
diff --git a/testutil/configtest/viper_settings_test.go b/testutil/configtest/viper_settings_test.go
index e97f302099..90b8fe49a2 100644
--- a/testutil/configtest/viper_settings_test.go
+++ b/testutil/configtest/viper_settings_test.go
@@ -73,6 +73,13 @@ func TestSettingsIsStableWhereAllSettingsIsNot(t *testing.T) {
 	// leaving map iteration order as the only thing that differs between reads.
 	path := writeAppTOML(t, body)
 
+	// The shape fingerprint is fmt.Sprint of a map, which is stable only because fmt
+	// sorts map keys before printing (Go 1.12+). Worth stating in a test whose whole
+	// premise is that map ordering is untrustworthy: the ordering fmt hides is the one
+	// this test relies on, and the ordering AllSettings exposes is the one it measures.
+	// Without the sort, every read would look like a distinct shape and the counts below
+	// would be meaningless rather than wrong.
+
 	settingsShapes := map[string]int{}
 	allSettingsShapes := map[string]int{}
 	for i := 0; i < runs; i++ {

From 11e25ad967ff08fffaf1488e90d4952977df03af Mon Sep 17 00:00:00 2001
From: bdchatham <bdchatham@gmail.com>
Date: Sat, 1 Aug 2026 10:26:01 -0700
Subject: [PATCH 7/8] test(config): close the Config half of the vacuous-parity
 hole, and make the collision fixture reach its collision
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The nil-viper guard closed one half of a two-channel premise. The Config
comparison beside it kept the other: Apply populates both fields, so a context it
never reached carries nil in both, and require.Equal on two nil *tmcfg.Config
pointers passes. It went unnoticed because AllSettings used to panic on the nil
viper two lines later, so the boot failure surfaced anyway — as a dereference from
inside viper rather than as a named premise. Guarding settings alone left this
masked only by the accident that the two are always compared together. Both
channels now go through one helper, since a test with grounds to compare either
has grounds to compare both, and splitting them is what let the hole sit next to
its own fix. I reported this twice as out of scope; with a reviewer asking and the
fix three lines, leaving it was the wrong call.

The guards also no longer discard the caller's message. A nil-viper failure from a
corpus row or a fuzz execution named the broken premise but not the input that
produced it, which is the half needed to reproduce it. Passing msgAndArgs straight
through would have made the caller's format string consume the helper's text as an
argument, so callerContext renders it as a parenthetical instead.

TestSettingsKeysCannotCollideThroughRendering never reached the collision its
comment described. The fixtures differed in size, so require.NotEqual held for a
reason unrelated to rendering: the two documents were never equal, because
DumpViper wraps the value as int64(1) while the old key text carried a bare 1. The
new fixture is a single key spelled `s.a = int64(1)\ns.b` holding int64(2), which
renders as exactly the two lines the other fixture's two keys produce — verified
identical. The test now asserts that premise first, so the property is proven
against a real collision rather than against two maps of different sizes.

Sample size down from 200 to 20. Both assertions in that test are deterministic per
read, so the sample fed only the informational shape count; at the measured ~4:1
split, 20 reads still show both orderings well over 99% of the time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../cmd/configmanager_differential_test.go    | 70 +++++++++++++++----
 testutil/configtest/viper_settings_test.go    | 47 ++++++++++---
 2 files changed, 93 insertions(+), 24 deletions(-)

diff --git a/cmd/seid/cmd/configmanager_differential_test.go b/cmd/seid/cmd/configmanager_differential_test.go
index 324290d293..38b8885ebd 100644
--- a/cmd/seid/cmd/configmanager_differential_test.go
+++ b/cmd/seid/cmd/configmanager_differential_test.go
@@ -156,8 +156,9 @@ func runManager(t *testing.T, mgr configmanager.ConfigManager, cmd *cobra.Comman
 // runs per fuzz execution.
 func requireSameSettings(t *testing.T, legacy, v2 *server.Context, msgAndArgs ...any) {
 	t.Helper()
-	require.NotNil(t, legacy.Viper, "legacy Apply left serverCtx.Viper nil: there is no resolved config to compare")
-	require.NotNil(t, v2.Viper, "v2 Apply left serverCtx.Viper nil: there is no resolved config to compare")
+	where := callerContext(msgAndArgs)
+	require.NotNil(t, legacy.Viper, "legacy Apply left serverCtx.Viper nil: there is no resolved config to compare"+where)
+	require.NotNil(t, v2.Viper, "v2 Apply left serverCtx.Viper nil: there is no resolved config to compare"+where)
 
 	legacySettings, v2Settings := configtest.Settings(legacy.Viper), configtest.Settings(v2.Viper)
 	if assert.ObjectsAreEqual(legacySettings, v2Settings) {
@@ -170,6 +171,53 @@ func requireSameSettings(t *testing.T, legacy, v2 *server.Context, msgAndArgs ..
 	require.Equal(t, legacySettings, v2Settings, msgAndArgs...)
 }
 
+// requireSameChannels asserts legacy and v2 produced the same two boot channels — the
+// Tendermint config struct and the resolved settings — having first asserted that both
+// exist.
+//
+// The Config half needs the same guard the settings half does, and for longer than it
+// had one. Apply populates both fields, so a context it never reached carries nil in
+// both, and require.Equal on two nil *tmcfg.Config pointers passes: the same vacuous
+// pass the settings comparison used to have, on the channel next to it. It went unnoticed
+// because AllSettings used to panic on the nil viper two lines later, so the boot failure
+// surfaced anyway — as a nil dereference from inside viper rather than as a named
+// premise. Guarding settings alone would have left this one masked only by the accident
+// that the two are always compared together.
+//
+// Both channels in one helper because they come from one Apply: a test that has grounds
+// to compare either has grounds to compare both, and splitting them is what let the hole
+// sit next to its own fix.
+func requireSameChannels(t *testing.T, legacy, v2 *server.Context, msgAndArgs ...any) {
+	t.Helper()
+	where := callerContext(msgAndArgs)
+	require.NotNil(t, legacy.Config, "legacy Apply left serverCtx.Config nil: there is no config to compare"+where)
+	require.NotNil(t, v2.Config, "v2 Apply left serverCtx.Config nil: there is no config to compare"+where)
+	require.Equal(t, legacy.Config, v2.Config,
+		"serverCtx.Config differs between legacy and v2"+where)
+	requireSameSettings(t, legacy, v2, msgAndArgs...)
+}
+
+// callerContext renders a testify msgAndArgs tail as a parenthetical, so a helper can
+// prepend its own explanation without discarding the caller's.
+//
+// The guards inside these helpers name which premise broke; the caller's message names
+// which input broke it. A corpus row or a fuzz execution needs both to be reproducible,
+// and passing msgAndArgs straight through would make the caller's format string consume
+// the helper's text as an argument.
+func callerContext(msgAndArgs []any) string {
+	switch len(msgAndArgs) {
+	case 0:
+		return ""
+	case 1:
+		return fmt.Sprintf(" (%v)", msgAndArgs[0])
+	default:
+		if format, ok := msgAndArgs[0].(string); ok {
+			return " (" + fmt.Sprintf(format, msgAndArgs[1:]...) + ")"
+		}
+		return fmt.Sprintf(" (%v)", msgAndArgs)
+	}
+}
+
 // seedDefaultConfig returns a home carrying a complete, realistic config (all Sei
 // sections), generated by letting the legacy creator write into a fresh home.
 func seedDefaultConfig(t *testing.T) *configtest.Home {
@@ -267,9 +315,7 @@ func TestConfigManagerLegacyVsV2Differential(t *testing.T) {
 	legacyCtx := runConfigManager(t, configmanager.LegacyConfigManager{}, home)
 	v2Ctx := runConfigManager(t, configmanager.SeiConfigManager{}, home)
 
-	require.Equal(t, legacyCtx.Config, v2Ctx.Config,
-		"serverCtx.Config differs between legacy and v2")
-	requireSameSettings(t, legacyCtx, v2Ctx,
+	requireSameChannels(t, legacyCtx, v2Ctx,
 		"serverCtx.Viper settings differ between legacy and v2")
 
 	// The start.go chain-id mutation is identical on both vipers; assert parity
@@ -312,9 +358,7 @@ func TestConfigManagerLegacyVsV2Differential_EnvHome(t *testing.T) {
 	require.Equal(t, home.Root, legacyCtx.Viper.GetString(flags.FlagHome),
 		"env-provided home did not drive legacy resolution")
 
-	require.Equal(t, legacyCtx.Config, v2Ctx.Config,
-		"serverCtx.Config differs between legacy and v2 on the env-home path")
-	requireSameSettings(t, legacyCtx, v2Ctx,
+	requireSameChannels(t, legacyCtx, v2Ctx,
 		"serverCtx.Viper settings differ between legacy and v2 on the env-home path")
 }
 
@@ -334,9 +378,7 @@ func TestConfigManagerLegacyVsV2Differential_Corpus(t *testing.T) {
 			legacyCtx := runConfigManager(t, configmanager.LegacyConfigManager{}, home)
 			v2Ctx := runConfigManager(t, configmanager.SeiConfigManager{}, home)
 
-			require.Equal(t, legacyCtx.Config, v2Ctx.Config,
-				"serverCtx.Config differs between legacy and v2 (%s)", tc.name)
-			requireSameSettings(t, legacyCtx, v2Ctx,
+			requireSameChannels(t, legacyCtx, v2Ctx,
 				"serverCtx.Viper settings differ between legacy and v2 (%s)", tc.name)
 		})
 	}
@@ -354,8 +396,7 @@ func TestConfigManagerV2AdvisoryNeverRefusesBoot(t *testing.T) {
 	require.NoError(t, v2Err, "advisory validation must never refuse boot on a valid config")
 
 	legacyCtx := runConfigManager(t, configmanager.LegacyConfigManager{}, home)
-	require.Equal(t, legacyCtx.Config, v2Ctx.Config)
-	requireSameSettings(t, legacyCtx, v2Ctx)
+	requireSameChannels(t, legacyCtx, v2Ctx)
 }
 
 // TestConfigManagerV2FreshHomeBoots exercises the fresh-home first-boot path: v2's
@@ -594,7 +635,6 @@ func FuzzConfigManagerLegacyVsV2Parity(f *testing.F) {
 			require.Equal(t, legacyErr.Error(), v2Err.Error(), "divergent error (case %q, suffix %q)", tc.name, appTOMLSuffix)
 			return
 		}
-		require.Equal(t, legacyCtx.Config, v2Ctx.Config, "Config diverges (case %q, suffix %q)", tc.name, appTOMLSuffix)
-		requireSameSettings(t, legacyCtx, v2Ctx, "settings diverge (case %q, suffix %q)", tc.name, appTOMLSuffix)
+		requireSameChannels(t, legacyCtx, v2Ctx, "case %q, app.toml suffix %q", tc.name, appTOMLSuffix)
 	})
 }
diff --git a/testutil/configtest/viper_settings_test.go b/testutil/configtest/viper_settings_test.go
index 90b8fe49a2..31da47c919 100644
--- a/testutil/configtest/viper_settings_test.go
+++ b/testutil/configtest/viper_settings_test.go
@@ -67,7 +67,12 @@ func TestSettingsIsStableWhereAllSettingsIsNot(t *testing.T) {
 "giga" = 1
 "giga.x" = 2
 `
-	const runs = 200
+	// Small on purpose. Both assertions below are deterministic per read — Settings cannot
+	// vary, and the leaf loss holds whichever way deepSearch re-nested — so the sample size
+	// feeds only the informational shape count. At the measured ~4:1 split, 20 reads show
+	// both orderings better than 99% of the time, and 180 fewer parses keeps this out of
+	// the package's runtime.
+	const runs = 20
 
 	// One file, read many times. Writing it once keeps the parse out of the experiment,
 	// leaving map iteration order as the only thing that differs between reads.
@@ -136,17 +141,41 @@ func TestSettingsPreservesConcreteTypes(t *testing.T) {
 }
 
 // TestSettingsKeysCannotCollideThroughRendering pins the second reason Settings is a map
-// rather than a rendered document. A key may legally contain a newline, and the parity
-// fuzz target appends arbitrary bytes to app.toml, so a newline-joined rendering can make
-// two different key sets produce one identical string. Map keys cannot collide that way.
+// rather than a rendered document: a rendering can collide where the underlying key sets
+// differ, and a map cannot.
+//
+// The fixtures are built to reach that collision rather than merely to differ, which is
+// the whole difficulty. DumpViper renders each key as `key = type(value)` and joins with
+// newlines, and a TOML key may legally contain a newline — so a single key whose text
+// reproduces one map's entire rendering makes the two documents byte-identical. The
+// second fixture is exactly that: one root key spelled `s.a = int64(1)\ns.b` holding
+// int64(2), which renders as `s.a = int64(1)\ns.b = int64(2)` — the same two lines the
+// first fixture's two keys produce.
+//
+// An earlier version of this test used `"a = 1\nb" = 2` and asserted only that the two
+// maps differ. That passed for a reason unrelated to the property: the renderings were
+// never equal, because DumpViper wraps the value as int64(1) while the key text carried a
+// bare 1. Two maps of different sizes are unequal, so the assertion held while the
+// collision it exists to demonstrate was never constructed. That is worth recording here,
+// because it is the failure this file is otherwise about: an assertion that passes for a
+// reason its own comment does not claim.
 func TestSettingsKeysCannotCollideThroughRendering(t *testing.T) {
-	two := configtest.Settings(newViperOver(t, "[s]\n\"a\" = 1\n\"b\" = 2\n"))
-	one := configtest.Settings(newViperOver(t, "[s]\n\"a = 1\\nb\" = 2\n"))
-
+	twoKeys := newViperOver(t, "[s]\n\"a\" = 1\n\"b\" = 2\n")
+	oneKey := newViperOver(t, "\"s.a = int64(1)\\ns.b\" = 2\n")
+
+	// The premise: the two renderings really are identical. Without this the assertion
+	// below is the vacuous one described above.
+	require.Equal(t, configtest.DumpViper(twoKeys), configtest.DumpViper(oneKey),
+		"the fixtures must reach the rendering collision this test is about, or the "+
+			"comparison below proves nothing about it")
+
+	// The property: Settings keeps them apart anyway, because it compares map keys rather
+	// than a joined document.
+	two, one := configtest.Settings(twoKeys), configtest.Settings(oneKey)
 	require.NotEqual(t, two, one,
-		"two keys and one newline-containing key must not compare equal")
+		"Settings must distinguish key sets whose rendering collides")
 	require.Len(t, two, 2)
-	require.Len(t, one, 1)
+	require.Len(t, one, 1, "a newline-containing key must stay a single entry")
 }
 
 // TestSettingsOnNilViperPanics pins the nil case as a failure rather than a tolerance,

From a14538c0980db3ba433dc4ec0bb39e028fa57aab Mon Sep 17 00:00:00 2001
From: bdchatham <bdchatham@gmail.com>
Date: Sat, 1 Aug 2026 10:52:12 -0700
Subject: [PATCH 8/8] test(config): let each channel name itself in a failure,
 and anchor the v2 fix
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Folding the Config comparison into the shared helper left the callers passing
settings-flavored messages, which callerContext then appended to the Config
failure — so a genuine Config mismatch reported "serverCtx.Config differs between
legacy and v2 (serverCtx.Viper settings differ between legacy and v2
(unknown-section))", naming the wrong channel as the one that differed. The
callers now name only the input and each helper supplies its own channel noun, so
the same failure reads "serverCtx.Config differs between legacy and v2 (case
\"unknown-section\")". The fuzz call site already did it this way; the other four
had not been updated when the helper absorbed the second channel.

callerContext's default branch printed msgAndArgs as a whole slice including index
0, so a non-string leading element would render as "([foo bar baz])" rather than a
message with arguments. Unreachable from the current call sites, but it degrades
gracefully now rather than oddly.

The header names PLT-775 as where the v2 correction lands, so the fix has an anchor
that outlives this comment rather than living only in prose.

The section-shadow fixture drops minimum-gas-prices, which the other three rows
never carried. It was realism the test does not use, and the difference between
rows was a distraction.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 cmd/seid/cmd/configmanager_differential_test.go | 11 ++++-------
 cmd/seid/cmd/envshadow_config_test.go           |  7 +++----
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/cmd/seid/cmd/configmanager_differential_test.go b/cmd/seid/cmd/configmanager_differential_test.go
index 38b8885ebd..159b1ed104 100644
--- a/cmd/seid/cmd/configmanager_differential_test.go
+++ b/cmd/seid/cmd/configmanager_differential_test.go
@@ -214,7 +214,7 @@ func callerContext(msgAndArgs []any) string {
 		if format, ok := msgAndArgs[0].(string); ok {
 			return " (" + fmt.Sprintf(format, msgAndArgs[1:]...) + ")"
 		}
-		return fmt.Sprintf(" (%v)", msgAndArgs)
+		return " (" + fmt.Sprint(msgAndArgs...) + ")"
 	}
 }
 
@@ -315,8 +315,7 @@ func TestConfigManagerLegacyVsV2Differential(t *testing.T) {
 	legacyCtx := runConfigManager(t, configmanager.LegacyConfigManager{}, home)
 	v2Ctx := runConfigManager(t, configmanager.SeiConfigManager{}, home)
 
-	requireSameChannels(t, legacyCtx, v2Ctx,
-		"serverCtx.Viper settings differ between legacy and v2")
+	requireSameChannels(t, legacyCtx, v2Ctx)
 
 	// The start.go chain-id mutation is identical on both vipers; assert parity
 	// holds after it too (covers the post-mutation snapshot).
@@ -358,8 +357,7 @@ func TestConfigManagerLegacyVsV2Differential_EnvHome(t *testing.T) {
 	require.Equal(t, home.Root, legacyCtx.Viper.GetString(flags.FlagHome),
 		"env-provided home did not drive legacy resolution")
 
-	requireSameChannels(t, legacyCtx, v2Ctx,
-		"serverCtx.Viper settings differ between legacy and v2 on the env-home path")
+	requireSameChannels(t, legacyCtx, v2Ctx, "env-home path")
 }
 
 // TestConfigManagerLegacyVsV2Differential_Corpus widens the parity proof from the
@@ -378,8 +376,7 @@ func TestConfigManagerLegacyVsV2Differential_Corpus(t *testing.T) {
 			legacyCtx := runConfigManager(t, configmanager.LegacyConfigManager{}, home)
 			v2Ctx := runConfigManager(t, configmanager.SeiConfigManager{}, home)
 
-			requireSameChannels(t, legacyCtx, v2Ctx,
-				"serverCtx.Viper settings differ between legacy and v2 (%s)", tc.name)
+			requireSameChannels(t, legacyCtx, v2Ctx, "case %q", tc.name)
 		})
 	}
 }
diff --git a/cmd/seid/cmd/envshadow_config_test.go b/cmd/seid/cmd/envshadow_config_test.go
index beabeb64dc..03d28ca639 100644
--- a/cmd/seid/cmd/envshadow_config_test.go
+++ b/cmd/seid/cmd/envshadow_config_test.go
@@ -28,7 +28,8 @@ import (
 // changing how configuration resolves could silently break an operator who has come to
 // depend on the current answer, so the behavior is recorded here and left alone.
 // SeiConfigManager corrects it when it owns resolution, and the divergence is ratified
-// against these rows rather than discovered against a production node.
+// against these rows rather than discovered against a production node. That correction is
+// tracked on PLT-775, so the fix has an anchor that outlives this comment.
 //
 // The consequence is worth stating where the pin lives, because it is what makes the
 // pin worth having rather than trivia. giga_executor.enabled defaults to true
@@ -85,9 +86,7 @@ func homeWithAppTOML(t *testing.T, body string) *configtest.Home {
 func TestSectionEnvVarShadowsItsWholeSection(t *testing.T) {
 	configtest.Isolate(t)
 
-	const body = `minimum-gas-prices = "0.1usei"
-
-[giga_executor]
+	const body = `[giga_executor]
 enabled = false
 occ_enabled = false