Skip to content

fix: improve mint/melt limit ux - #2161

Open
orangeshyguy21 wants to merge 2 commits into
cashubtc:mainfrom
orangeshyguy21:fix/optional-mint-melt-bounds
Open

fix: improve mint/melt limit ux#2161
orangeshyguy21 wants to merge 2 commits into
cashubtc:mainfrom
orangeshyguy21:fix/optional-mint-melt-bounds

Conversation

@orangeshyguy21

@orangeshyguy21 orangeshyguy21 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

[ln]/[onchain] mint/melt bounds stay required, but a missing config now reports explicitly instead of an opaque error. Example config also sets values to more appropriate default values to align with network policy.

Description

Since 0.17.0 a single [ln]/[onchain] block missing the mint/melt bounds fails with:

data did not match any variant of untagged enum LnOneOrMany for key `ln`

The #[serde(untagged)] deserializer (from the multi-backend Vec<Ln> change) masks the real cause. This swaps it for a visitor that deserializes the single table directly, yielding missing field "ln.min_mint".


Notes to the reviewers

  • Fixes the diagnostic; bounds remain mandatory by design.
  • Improves example values; onchain mirrors bdk configs: min_receive_amount_sat, min_send_amount_sat

Suggested CHANGELOG Updates

FIXED

  • cdk-mintd: [ln]/[onchain] missing min_mint/max_mint/min_melt/max_melt now fails with a clear "missing field" error instead of the untagged-enum message (regressed in 0.17.0).

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) — N/A

Default min/max mint/melt on `Ln` and `Onchain` so a single `[ln]`/`[onchain]`
table that omits them loads again — the multi-backend `Vec<Ln>` deserializer
had made these fields de facto required. Backends stay mandatory. Updates the
example config and adds parse tests for the bounds-omitted case.
@github-project-automation github-project-automation Bot moved this to Backlog in CDK Jun 27, 2026
@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.89%. Comparing base (08fde41) to head (8a3fdca).

Files with missing lines Patch % Lines
crates/cdk-mintd/src/config.rs 93.33% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2161      +/-   ##
==========================================
+ Coverage   71.88%   71.89%   +0.01%     
==========================================
  Files         356      356              
  Lines       74532    74574      +42     
==========================================
+ Hits        53574    53612      +38     
- Misses      20958    20962       +4     

☔ 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.

@prusnak

prusnak commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

I am OK with setting min to 1 by default.

Not sure how I feel about having some default max value. Maybe it is fine that the mint does not boot until an admin sets the proper maximum values?

@thesimplekid

Copy link
Copy Markdown
Collaborator

A min of one can't really be used for onchain as it would be dust the mint could never move. I would prefer we force this being set by the operator and fix the example to show this must be set and recommend something higher.

@orangeshyguy21

Copy link
Copy Markdown
Contributor Author

Tend to agree here that they should be required configs, with more sensible defaults per backend. These values have increased implications with onchain.

Will rework with new intent and sensible defaults.

@orangeshyguy21 orangeshyguy21 changed the title fix: make ln/onchain limits optional fix: improve mint/melt limit ux Jun 28, 2026
Replace the untagged `[ln]` deserializer with a visitor so a missing
min/max mint/melt reports the field (e.g. `missing field "ln.min_mint"`)
instead of the opaque "untagged enum LnOneOrMany" error. Bounds remain
required; the example config now sets them explicitly. Tests updated to
assert the clear error.
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.

3 participants