[ConfigManager] Register Node Sections 2/4

config/tendermintbase/tendermintbase.go

@@ -8,10 +8,40 @@ import (
 
 // The names these sections have in the configuration key space.
 const (
-	P2PSectionName = "p2p"
-	RPCSectionName = "rpc"
+	P2PSectionName       = "p2p"
+	RPCSectionName       = "rpc"
+	ConsensusSectionName = "consensus"
+	MempoolSectionName   = "mempool"
 )
 
+// removedSettings are the consensus paths this section does not declare.
+//
+// Every one is a setting the node removed, and the struct marks each field deprecated. The fields are kept
+// so a decode can tell that an operator set one, and declaring any of them would offer a key that changes
+// nothing about how the node runs.
+//
+// The reader has a check that names the removed settings an operator wrote, and it reaches eight of these
+// fifteen. Six are durations or booleans, where a written zero and an unwritten field are the same value,
+// so no check can tell them apart. One more the check simply omits. Nothing calls the check in any case, so
+// leaving these out of the file is what an operator actually gets.
+var removedSettings = []string{
+	"unsafe-overrides-enabled",
+	"unsafe-propose-timeout-override",
+	"unsafe-propose-timeout-delta-override",
+	"unsafe-vote-timeout-override",
+	"unsafe-vote-timeout-delta-override",
+	"unsafe-commit-timeout-override",
+	"unsafe-bypass-commit-timeout-override",
+	"timeout-propose",
+	"timeout-propose-delta",
+	"timeout-prevote",
+	"timeout-prevote-delta",
+	"timeout-precommit",
+	"timeout-precommit-delta",
+	"timeout-commit",
+	"skip-timeout-commit",
+}
+
 // Registration puts these sections in the configuration registry.
 //
 // Neither the package that defines these settings nor the package that decides them can register them. The
@@ -26,6 +56,10 @@ func init() {
 		filledFromTheCommandLine, "max-outbound-connections")
 	registry.RegisterSectionExcluding(RPCSectionName, &tmcfg.RPCConfig{}, rpcDefaults,
 		filledFromTheCommandLine)
+	registry.RegisterSectionExcluding(ConsensusSectionName, &tmcfg.ConsensusConfig{}, consensusDefaults,
+		append([]string{filledFromTheCommandLine}, removedSettings...)...)
+	registry.RegisterSectionExcluding(MempoolSectionName, &tmcfg.MempoolConfig{}, mempoolDefaults,
+		filledFromTheCommandLine)
 }
 
 // forMode is the configuration the seid init command writes for a kind of node.
@@ -70,3 +104,16 @@ func p2pDefaults(mode registry.Mode) any { return *forMode(mode).P2P }
 // Answered per mode, for the listen address alone: a node that serves queries binds one and a validator
 // does not.
 func rpcDefaults(mode registry.Mode) any { return *forMode(mode).RPC }
+
+// consensusDefaults is what a generated file carries for the consensus section.
+//
+// The same values for every mode. How long a node waits at each step of a round has to agree across the
+// validator set for the set to reach a decision, so a value that followed from the kind of node asking
+// would be this package proposing that they disagree.
+func consensusDefaults(mode registry.Mode) any { return *forMode(mode).Consensus }
+
+// mempoolDefaults is what a generated file carries for the mempool section.
+//
+// The same values for every mode. What a node holds before a transaction is decided is a limit on its own
+// memory and bandwidth, and nothing in the binary makes one follow from what kind of node is asking.
+func mempoolDefaults(mode registry.Mode) any { return *forMode(mode).Mempool }

config/tendermintbase/tendermintbase_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/sei-protocol/sei-chain/config/registry"
 	tmcfg "github.com/sei-protocol/sei-chain/sei-tendermint/config"
+	"github.com/spf13/viper"
 )
 
 // whatVariesByNodeKind is every key these sections answer differently depending on the kind of node.
@@ -108,6 +109,8 @@ func TestTheDeclaredKeysAreTheOnesTheReaderDecodes(t *testing.T) {
 	}{
 		{P2PSectionName, &tmcfg.P2PConfig{}, 2},
 		{RPCSectionName, &tmcfg.RPCConfig{}, 1},
+		{ConsensusSectionName, &tmcfg.ConsensusConfig{}, len(removedSettings) + 1},
+		{MempoolSectionName, &tmcfg.MempoolConfig{}, 1},
 	} {
 		registered, ok := registry.Lookup(tc.section)
 		if !ok {
@@ -186,6 +189,138 @@ func hasOpt(opts []string, want string) bool {
 	return false
 }
 
+// warningCannotName are the removed settings the reader's own deprecation check does not report.
+//
+// Six are durations or booleans, where a written zero and an unwritten field hold the same value, so the
+// check has nothing to test. The seventh is a pointer the check could name and does not. Recorded so that
+// making the check complete fails here rather than leaving a sentence quietly stale.
+var warningCannotName = map[string]bool{
+	"unsafe-overrides-enabled":              true,
+	"unsafe-propose-timeout-override":       true,
+	"unsafe-propose-timeout-delta-override": true,
+	"unsafe-vote-timeout-override":          true,
+	"unsafe-vote-timeout-delta-override":    true,
+	"unsafe-commit-timeout-override":        true,
+	"unsafe-bypass-commit-timeout-override": true,
+}
+
+// TestTheExcludedConsensusPathsAreTheRemovedOnes ties the exclusion list to the struct's own marking.
+//
+// Each excluded path has to name a field the struct itself marks as deprecated, and no declared path may.
+// The struct is the authority rather than the deprecation warning, because that warning is incomplete: it
+// names eight of these fifteen.
+//
+// Nothing in the binary calls the warning either, so an operator who still has one of these in their file
+// gets no error and no warning and the value is quietly ignored. That is what the exclusion is carrying. It
+// keeps the key out of the new format rather than relying on a diagnostic that never runs.
+func TestTheExcludedConsensusPathsAreTheRemovedOnes(t *testing.T) {
+	registered, ok := registry.Lookup(ConsensusSectionName)
+	if !ok {
+		t.Fatalf("%s is not registered; Defects: %v", ConsensusSectionName, registry.Defects())
+	}
+	marked := deprecatedPaths(reflect.TypeOf(tmcfg.ConsensusConfig{}))
+
+	excluded := map[string]bool{}
+	for _, key := range registered.Excluded {
+		excluded[key] = true
+	}
+	if want := len(removedSettings) + 1; len(excluded) != want {
+		t.Errorf("the section excludes %d paths and %d were expected, being the removed settings and the "+
+			"root directory", len(excluded), want)
+	}
+	for _, rel := range removedSettings {
+		key := ConsensusSectionName + "." + rel
+		if !excluded[key] {
+			t.Errorf("%s is listed as removed and the section does not exclude it", key)
+		}
+		if !marked[rel] {
+			t.Errorf("%s is excluded as a removed setting and the struct does not mark its field "+
+				"deprecated, so it is a setting an operator can use and belongs declared", key)
+		}
+		delete(marked, rel)
+	}
+	for rel := range marked {
+		t.Errorf("%s.%s names a field the struct marks deprecated and the section declares it",
+			ConsensusSectionName, rel)
+	}
+
+	for _, key := range registered.Keys {
+		rel := strings.TrimPrefix(key, ConsensusSectionName+".")
+		if err := writtenThenChecked(t, rel); err != nil {
+			t.Errorf("%s is declared and the deprecation warning names it: %v", key, err)
+		}
+	}
+}
+
+// TestTheDeprecationWarningReachesTheRecordedSubset measures the gap in the reader's own check.
+//
+// Eight of the fifteen removed settings make the warning name them and seven cannot, so an operator who
+// wrote one of those seven would get nothing back even from a caller that ran the check. Held so that
+// making the check complete shows up as a failure rather than as a sentence going quietly stale.
+func TestTheDeprecationWarningReachesTheRecordedSubset(t *testing.T) {
+	for _, rel := range removedSettings {
+		err := writtenThenChecked(t, rel)
+		switch {
+		case warningCannotName[rel] && err != nil:
+			t.Errorf("the warning now names %s, so it reaches one more removed setting and the row "+
+				"should go", rel)
+		case !warningCannotName[rel] && err == nil:
+			t.Errorf("the warning no longer names %s, so one more removed setting is now silent", rel)
+		}
+	}
+}
+
+// deprecatedPaths returns the mapstructure names of the fields a struct marks deprecated.
+func deprecatedPaths(t reflect.Type) map[string]bool {
+	out := map[string]bool{}
+	for i := 0; i < t.NumField(); i++ {
+		f := t.Field(i)
+		if !strings.HasPrefix(f.Name, "Deprecated") {
+			continue
+		}
+		if tag, ok := f.Tag.Lookup("mapstructure"); ok {
+			out[strings.Split(tag, ",")[0]] = true
+		}
+	}
+	return out
+}
+
+// writtenThenChecked writes one consensus path into a configuration and returns what the reader's
+// deprecation warning says about it.
+//
+// Written and decoded rather than assigned, because a removed setting is detected by its field being
+// non-nil after a decode, and assigning it directly would skip the step under test.
+func writtenThenChecked(t *testing.T, rel string) error {
+	t.Helper()
+	conf := tmcfg.DefaultConfig()
+	v := viper.New()
+	v.SetConfigType("toml")
+	body := "[consensus]\n" + rel + " = " + probeValueFor(rel) + "\n"
+	if err := v.ReadConfig(strings.NewReader(body)); err != nil {
+		t.Fatalf("compose a file setting %s: %v", rel, err)
+	}
+	if err := v.Unmarshal(conf); err != nil {
+		t.Skipf("%s does not decode from the probe value: %v", rel, err)
+	}
+	return conf.DeprecatedFieldWarning()
+}
+
+// probeValueFor returns a written value of the right shape for a consensus path.
+//
+// Three shapes appear: a duration written as a string, a boolean, and a whole number.
+func probeValueFor(rel string) string {
+	switch {
+	case strings.HasPrefix(rel, "skip-") || strings.HasPrefix(rel, "unsafe-") ||
+		strings.HasPrefix(rel, "double-sign-") || strings.HasSuffix(rel, "-enabled"):
+		return "true"
+	case strings.Contains(rel, "timeout") || strings.Contains(rel, "-delta") ||
+		strings.Contains(rel, "interval") || strings.Contains(rel, "period"):
+		return "\"1s\""
+	default:
+		return "1"
+	}
+}
+
 // TestNoSectionDeclaresTheRootDirectory covers a field five of these sections carry.
 //
 // Each holds a root directory tagged the same as the key at the top of the file, and the node fills every