Skip to content

check and merge config from toml and env#1962

Draft
asmogo wants to merge 5 commits into
cashubtc:mainfrom
asmogo:feat/check-config
Draft

check and merge config from toml and env#1962
asmogo wants to merge 5 commits into
cashubtc:mainfrom
asmogo:feat/check-config

Conversation

@asmogo

@asmogo asmogo commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Description


Adds upfront, fail-fast validation of cdk-mintd settings after TOML + env-var merge, so misconfiguration produces a clear error pointing at both the offending TOML key and env var instead of a silent default or a confusing downstream crash.

Notes to the reviewers


  • New validate_settings() covers listen address, signing source (mnemonic / seed length / signatory URL), per-backend lightning config, database, auth, management-RPC, and Prometheus.
  • #[serde(default)] added across config structs so partial TOML files merge cleanly with defaults and env vars.
  • Settings::new split into try_new (Result) + new (panics on any error) — removes the old silent-fallback-to-defaults footgun.
  • Replaces an .expect() panic in Prometheus server startup with a propagated error.
  • Adds a shared test-utils module (env-mutex + unique temp dirs) and a full set of negative tests for every validator branch.

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED


Checklist

  • I followed the code style guidelines
  • I ran just quick-check before committing
  • If the Wallet API was modified (added/removed/changed), I have reflected those changes in the FFI bindings (crates/cdk-ffi)

@github-project-automation github-project-automation Bot moved this to Backlog in CDK May 13, 2026
@codecov

codecov Bot commented May 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.91599% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.83%. Comparing base (bfb5b8d) to head (5650198).

Files with missing lines Patch % Lines
crates/cdk-mintd/src/lib.rs 95.86% 35 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1962      +/-   ##
==========================================
+ Coverage   71.48%   71.83%   +0.34%     
==========================================
  Files         356      356              
  Lines       73857    74696     +839     
==========================================
+ Hits        52798    53656     +858     
+ Misses      21059    21040      -19     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@asmogo asmogo force-pushed the feat/check-config branch from d3a4185 to fdf24c0 Compare May 31, 2026 19:38
asmogo added 4 commits June 17, 2026 16:12
@asmogo asmogo force-pushed the feat/check-config branch from 1c0406f to 02961a9 Compare June 17, 2026 14:16
- Don't reject on-chain-only configs: validate_lightning_config no longer
  bails on an empty `[[ln]]`. from_env already guarantees at least one
  payment backend (Lightning or on-chain), so the check was both redundant
  and wrong for valid on-chain-only deployments.
- Remove duplicate TEST_MNEMONIC const that broke `cargo test` compilation.
- Share a single process-wide env lock between config.rs and lib.rs tests;
  two separate mutexes over global std::env raced and made
  test_env_var_only_config_all_backends flaky.
- Collapse the no-op LdkNode validation arm to `=> {}`.
- Drop misleading "of entropy" from the short-seed error (it measures
  string bytes, not entropy).
- Fix indentation to satisfy `cargo fmt --check`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

1 participant