diff --git a/go.mod b/go.mod index ed35f19..d3d188d 100644 --- a/go.mod +++ b/go.mod @@ -3,3 +3,5 @@ module github.com/sei-protocol/sei-config go 1.25.6 require github.com/BurntSushi/toml v1.5.0 + +require github.com/go-viper/mapstructure/v2 v2.5.0 diff --git a/go.sum b/go.sum index ff7fd09..3a30160 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,4 @@ github.com/BurntSushi/toml v1.5.0 h1:W5quZX/G/csjUnuI8SUYlsHs9M38FC7znL0lIO+DvMg= github.com/BurntSushi/toml v1.5.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= +github.com/go-viper/mapstructure/v2 v2.5.0 h1:vM5IJoUAy3d7zRSVtIwQgBj7BiWtMPfmPEgAXnvj1Ro= +github.com/go-viper/mapstructure/v2 v2.5.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= diff --git a/io.go b/io.go index eb18256..352474f 100644 --- a/io.go +++ b/io.go @@ -5,8 +5,11 @@ import ( "fmt" "os" "path/filepath" + "reflect" + "strings" "github.com/BurntSushi/toml" + "github.com/go-viper/mapstructure/v2" ) const ( @@ -23,12 +26,12 @@ func ReadConfigFromDir(homeDir string) (*SeiConfig, error) { appPath := filepath.Join(cfgDir, appTomlFile) var tm legacyTendermintConfig - if _, err := toml.DecodeFile(configPath, &tm); err != nil { + if err := decodeTOMLFile(configPath, &tm); err != nil { return nil, fmt.Errorf("reading %s: %w", configPath, err) } var app legacyAppConfig - if _, err := toml.DecodeFile(appPath, &app); err != nil { + if err := decodeTOMLFile(appPath, &app); err != nil { return nil, fmt.Errorf("reading %s: %w", appPath, err) } @@ -36,6 +39,70 @@ func ReadConfigFromDir(homeDir string) (*SeiConfig, error) { return cfg, nil } +// decodeTOMLFile decodes a TOML file into out, coercing quoted scalars the way +// the legacy reader does. The seid/tendermint config templates emit some +// numeric and bool fields quoted (e.g. `duplicate-txs-cache-size = "100000"`, +// `gossip-tx-key-only = "true"`); BurntSushi/toml alone is strict and rejects a +// quoted string into an int/bool field, so we decode to a generic map and then +// weakly-typed-decode into the struct. The TextUnmarshaller hook keeps +// string-encoded types (Duration) parsing. +// +// This approximates how cosmos/Viper tolerates quoted scalars (not full parity +// — Viper's hooks and key handling differ). WeaklyTypedInput widens tolerance +// two ways worth knowing: (a) a non-string scalar bound to a string field is +// stringified (e.g. a stray `true` becomes "1") — accepted as benign since seid +// templates never emit that form; (b) an empty string bound to a numeric/bool +// field would coerce to the zero value — this we reject (see +// rejectEmptyScalarStringHook) so blanking a limit errors rather than silently +// pinning it to zero. Genuinely malformed values (non-numeric strings, bad +// durations, overflow) still error (locked by io_quoted_scalars_test.go). +func decodeTOMLFile(path string, out any) error { + var raw map[string]any + if _, err := toml.DecodeFile(path, &raw); err != nil { + return err + } + dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: mapstructure.ComposeDecodeHookFunc( + rejectEmptyScalarStringHook, + mapstructure.TextUnmarshallerHookFunc(), + ), + WeaklyTypedInput: true, + TagName: "toml", + Result: out, + }) + if err != nil { + return err + } + return dec.Decode(raw) +} + +// rejectEmptyScalarStringHook fails an empty-string value bound to a numeric or +// bool field instead of letting WeaklyTypedInput silently coerce it to the zero +// value. Blanking a numeric (a connection limit, a cache size) should error, +// not silently pin it to 0/false. Non-empty strings pass through unchanged to +// the quoted-scalar coercion the template requires. +func rejectEmptyScalarStringHook(from, to reflect.Type, data any) (any, error) { + if from.Kind() != reflect.String { + return data, nil + } + if s, _ := data.(string); strings.TrimSpace(s) != "" { + return data, nil + } + t := to + for t.Kind() == reflect.Ptr { + t = t.Elem() + } + switch t.Kind() { + case reflect.Bool, + reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, + reflect.Float32, reflect.Float64: + return nil, fmt.Errorf("empty string for %s field", to) + default: + return data, nil + } +} + // WriteConfigToDir writes the SeiConfig as config.toml and app.toml into // homeDir/config/. Writes are atomic (temp file + rename) to prevent // corruption on crash. diff --git a/io_quoted_scalars_test.go b/io_quoted_scalars_test.go new file mode 100644 index 0000000..7b4ef0c --- /dev/null +++ b/io_quoted_scalars_test.go @@ -0,0 +1,81 @@ +package seiconfig + +import ( + "os" + "path/filepath" + "testing" +) + +// TestReadConfigFromDir_CoercesQuotedScalars reproduces the seid/tendermint +// config template's quoted primitives — e.g. `duplicate-txs-cache-size = "100000"` +// (a Go int) and `broadcast = "true"` (a Go bool) — and asserts ReadConfigFromDir +// coerces them instead of failing a strict decode. +// +// Regression for the v2 ConfigManager differential (PLT-775): a real seid +// config.toml is written with these primitives quoted, and the legacy reader +// (cosmos/Viper) tolerates it via weakly-typed coercion. ReadConfigFromDir must +// do the same, or v2 cannot read a real node's config and fails at boot. +func TestReadConfigFromDir_CoercesQuotedScalars(t *testing.T) { + home := t.TempDir() + cfgDir := filepath.Join(home, configDir) + if err := os.MkdirAll(cfgDir, 0o755); err != nil { + t.Fatal(err) + } + + // Quoted int, quoted bool, and a string-encoded Duration — the three + // coercion paths the lenient decoder must handle. + configToml := ` +[mempool] +duplicate-txs-cache-size = "100000" +broadcast = "true" +ttl-duration = "1s" +` + writeFile(t, filepath.Join(cfgDir, configTomlFile), configToml) + writeFile(t, filepath.Join(cfgDir, appTomlFile), "") // empty app.toml: fields default + + cfg, err := ReadConfigFromDir(home) + if err != nil { + t.Fatalf("ReadConfigFromDir failed on quoted scalars: %v", err) + } + if got := cfg.Mempool.DuplicateTxsCacheSize; got != 100000 { + t.Errorf("Mempool.DuplicateTxsCacheSize = %d, want 100000 (quoted int not coerced)", got) + } + if !cfg.Mempool.Broadcast { + t.Errorf("Mempool.Broadcast = false, want true (quoted bool not coerced)") + } +} + +func writeFile(t *testing.T, path, contents string) { + t.Helper() + if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { + t.Fatal(err) + } +} + +// TestReadConfigFromDir_LocksLeniencyBoundary pins what must STILL error after +// the lenient decode. The whole risk of weakly-typed coercion is silently +// widening; these cases (a genuinely non-numeric string, an empty string into a +// numeric field, a malformed duration) must keep failing, so a future decoder +// change cannot loosen the boundary with a green suite. +func TestReadConfigFromDir_LocksLeniencyBoundary(t *testing.T) { + cases := map[string]string{ + "non-numeric string into int": "[mempool]\nduplicate-txs-cache-size = \"banana\"\n", + "empty string into int": "[mempool]\nduplicate-txs-cache-size = \"\"\n", + "malformed duration": "[mempool]\nttl-duration = \"notaduration\"\n", + } + for name, configToml := range cases { + t.Run(name, func(t *testing.T) { + home := t.TempDir() + cfgDir := filepath.Join(home, configDir) + if err := os.MkdirAll(cfgDir, 0o755); err != nil { + t.Fatal(err) + } + writeFile(t, filepath.Join(cfgDir, configTomlFile), configToml) + writeFile(t, filepath.Join(cfgDir, appTomlFile), "") + + if _, err := ReadConfigFromDir(home); err == nil { + t.Fatalf("expected ReadConfigFromDir to error on %s, got nil", name) + } + }) + } +} diff --git a/version.json b/version.json index 3e76eee..6966a31 100644 --- a/version.json +++ b/version.json @@ -1,3 +1,3 @@ { - "version": "v0.0.20" + "version": "v0.0.21" }