fix: fall back to defaults for missing config keys#3893
Conversation
Allow omitting non-required keys such as log_to_file without parse failure. LoggingConfig, ServerConfig, StratumServerConfig, and ConfigMembers now use serde defaults aligned with Default impls. Addresses mimblewimble#3002.
| pub server: ServerConfig, | ||
| /// Logging config | ||
| /// Logging config (optional section; missing keys use LoggingConfig defaults) | ||
| #[serde(default)] |
There was a problem hiding this comment.
A missing logging section now becomes None, but node startup still calls .logging.clone().unwrap(). The minimal config therefore parses and then panics. Should this default to Some(LoggingConfig::default()) instead?
| /// | ||
| /// Missing optional keys fall back to [`Default`] (see #3002). | ||
| #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] | ||
| #[serde(default)] |
There was a problem hiding this comment.
These are always the mainnet defaults. A trimmed testnet or usernet config therefore misses the network-specific API, P2P, and stratum values from GlobalConfig::for_chain(). Could we apply defaults for the selected chain instead?
There was a problem hiding this comment.
With these defaults, a misspelled key can be silently ignored and replaced by its default, which is risky for settings such as archive_mode. Could we reject or at least report unknown fields?
2 participants
Summary
Fixes #3002.
Operators who trim
grin-server.tomlto only non-default keys hit parse failures for missing fields such aslog_to_file, even though the code has sensible defaults.Change
LoggingConfig:#[serde(default)]+ field defaults matchingDefault(so omittinglog_to_file→true, etc.)ConfigMembers: optionallogging/config_file_versionwith serde defaultServerConfig/StratumServerConfig: missing keys use the same values asDefaultMinimal configs can now omit keys safely; present keys still override.
Test plan
cargo test -p grin_config -- --test-threads=1(9 passed), including:test_logging_config_missing_keys_use_defaultstest_logging_section_optionaltest_stratum_partial_config_defaults