fix(io): lenient decode so ReadConfigFromDir parses real seid config.toml#36
Merged
Merged
Conversation
…toml The seid/tendermint config templates emit some numeric and bool fields QUOTED (e.g. `duplicate-txs-cache-size = "100000"`, `gossip-tx-key-only = "true"`). The legacy reader (cosmos/Viper) tolerates this via weakly-typed mapstructure coercion, but ReadConfigFromDir decoded with strict BurntSushi/toml, which rejects a quoted string into an int/bool field: toml: line 308 "mempool.duplicate-txs-cache-size": incompatible types: TOML value has type string; destination has type integer So a node whose config.toml came from the standard template could not be read into the unified model at all — the v2 in-binary ConfigManager (sei-chain PLT-775) would fail at boot. Fix: decode TOML to a generic map, then mapstructure-decode into the legacy structs with WeaklyTypedInput (coerces quoted int/bool/uint) and the TextUnmarshaller hook (keeps Duration and other string-encoded types parsing). This mirrors how cosmos/Viper reads the same files, so a real seid config.toml round-trips through the model. Regression test reproduces the quoted int / bool / Duration case. version -> v0.0.21. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR SummaryMedium Risk Overview The write path is unchanged. New tests cover successful coercion and cases that must still error (invalid numbers, empty numeric strings, bad durations). Version bumped to v0.0.21. Reviewed by Cursor Bugbot for commit 3296aa4. Bugbot is set up for automated code reviews on this repo. Configure here. |
Addresses the /xreview slate on #36: - dep: github.com/mitchellh/mapstructure (archived) -> github.com/go-viper/ mapstructure/v2 (maintained fork viper migrated to); drop-in, and respects the repo's minimal/maintained-deps invariant (idiomatic-reviewer). - guard: rejectEmptyScalarStringHook errors on an empty string bound to a numeric/bool field instead of letting WeaklyTypedInput silently coerce it to zero/false (the dissenter's empty-string->zero footgun, e.g. a blanked connection limit pinning to 0). - tests: TestReadConfigFromDir_LocksLeniencyBoundary pins what must still error (non-numeric string, empty-string-into-int, malformed duration) so a future decoder change can't silently widen coercion. - comment: soften "mirrors cosmos/Viper" -> "approximates", and document the one remaining accepted widening (non-string scalar -> string field), which seid templates never emit (sei-network-specialist). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Suggested version: Changes in (empty)
Cutting a Release (and modifying non-markdown files)This PR is modifying both Automatically created GitHub ReleaseA draft GitHub Release has been created. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
ReadConfigFromDircould not parse aconfig.tomlrendered by the standard seid/tendermint template, because the template emits some numeric and bool fields quoted — e.g.duplicate-txs-cache-size = "100000",gossip-tx-key-only = "true"— while the reader used strictBurntSushi/toml, which rejects a quoted string into anint/boolfield:The legacy reader (cosmos/Viper) tolerates this via weakly-typed
mapstructurecoercion; this library did not.Why it matters
This was caught by the v2 in-binary ConfigManager differential in sei-chain (PLT-775). The v2 manager reads the existing two-file config into the unified model, re-writes it atomically, and re-enters the legacy reader. Because
ReadConfigFromDirerrored on the quoted primitives, v2 would fail at boot on any real node whoseconfig.tomlcame from the standard template. It's a class, not one field (every quoted numeric/bool field is affected —gossip-tx-key-only, etc.).Fix
Decode the TOML to a generic map, then
mapstructure-decode into the legacy structs with:WeaklyTypedInput: true— coerces quotedint/bool/uint(the whole class, generically), andTextUnmarshallerHookFunc— keepsDurationand other string-encoded types parsing as before.This mirrors how cosmos/Viper reads the same files, so a real seid
config.tomlround-trips through the unified model. Write path is unchanged.Test
TestReadConfigFromDir_CoercesQuotedScalarsreproduces the quoted int / bool / Duration case (previously a hard decode error). Full suite green.version→ v0.0.21 (needs a tag on merge so sei-chain can pin it).🤖 Generated with Claude Code