Skip to content

feat: add method to quote responses - #2178

Merged
thesimplekid merged 2 commits into
cashubtc:mainfrom
asmogo:feat/add-method-field-to-quote-responses
Jul 6, 2026
Merged

feat: add method to quote responses#2178
thesimplekid merged 2 commits into
cashubtc:mainfrom
asmogo:feat/add-method-field-to-quote-responses

Conversation

@asmogo

@asmogo asmogo commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Description

Implementing cashubtc/nuts#387


Notes to the reviewers


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 Jun 30, 2026

@cdk-bot cdk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified findings approved for disclosure:

  • Required method field breaks quote response deserialization from older mints (high) - Wallets built from this PR cannot create or check quotes against existing mints that still emit the previous quote response shape without method, causing mint/melt operations to fail during response deserialization.
    Additional locations included in summary:
    • crates/cashu/src/nuts/nut23.rs:268
    • crates/cashu/src/nuts/nut25.rs:50
    • crates/cashu/src/nuts/nut04.rs:403
    • crates/cashu/src/nuts/nut05.rs:501
    • crates/cashu/src/nuts/nut30.rs:46
    • crates/cashu/src/nuts/nut30.rs:175

Comment thread crates/cashu/src/nuts/nut23.rs

@cdk-bot cdk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified findings approved for disclosure:

  • Custom WebSocket quote notifications do not backfill the new required method field (medium) - Custom WebSocket quote notifications from mints that omit the newly-added method field are rejected as parsing errors, while the HTTP fallback path continues to work.
    Unanchored locations included in summary:
    • crates/cdk/src/wallet/subscription.rs:273
  • Bolt12 melt responses without method default to Bolt11 through shared type alias (low) - Bolt12 melt quote responses that omit the new method field are recorded as Bolt11, which can misroute or misclassify quote state downstream.
    Unanchored locations included in summary:
    • crates/cashu/src/nuts/nut25.rs:113

Comment thread crates/cashu/src/nuts/nut23.rs
crodas
crodas previously approved these changes Jul 1, 2026
@thesimplekid

Copy link
Copy Markdown
Collaborator

Thanks for this @asmogo I'll take it over and rebase and get it merged.

@thesimplekid
thesimplekid force-pushed the feat/add-method-field-to-quote-responses branch from f2bfbf8 to ea6ed42 Compare July 4, 2026 17:13
@thesimplekid

Copy link
Copy Markdown
Collaborator

@cdk-bot

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.85502% with 103 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.77%. Comparing base (7c1ed40) to head (52ef5d9).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
crates/cdk-common/src/mint.rs 25.58% 32 Missing ⚠️
crates/cashu/src/nuts/nut25.rs 70.00% 30 Missing ⚠️
crates/cdk-ffi/src/types/quote.rs 0.00% 7 Missing ⚠️
crates/cdk/src/wallet/subscription.rs 90.32% 6 Missing ⚠️
crates/cashu/src/nuts/nut08.rs 28.57% 5 Missing ⚠️
crates/cashu/src/nuts/nut17/mod.rs 92.85% 4 Missing ⚠️
crates/cdk-common/src/melt.rs 50.00% 4 Missing ⚠️
crates/cdk/src/event.rs 25.00% 3 Missing ⚠️
crates/cdk/src/mint/melt/mod.rs 0.00% 3 Missing ⚠️
crates/cashu/src/nuts/nut23.rs 97.05% 2 Missing ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2178      +/-   ##
==========================================
+ Coverage   72.72%   72.77%   +0.04%     
==========================================
  Files         356      356              
  Lines       77588    78079     +491     
==========================================
+ Hits        56427    56821     +394     
- Misses      21161    21258      +97     

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

@github-project-automation github-project-automation Bot moved this from Backlog to Todo in CDK Jul 6, 2026
asmogo and others added 2 commits July 6, 2026 13:32
Add a required `method` field to all mint and melt quote responses
(bolt11, bolt12, onchain, and custom) so wallets can tell which payment
method a quote belongs to without relying on the request route.

For wire compatibility with mints that don't send the field yet,
missing values deserialize to the method implied by the response type
(bolt11, bolt12, or onchain), and the wallet HTTP client fills in the
method for custom responses from the request route.

BREAKING CHANGE: quote response structs gain a required `method` field;
constructing them now requires providing it.
`MeltQuoteBolt12Response` was a type alias for the bolt11 response,
which made melt notifications ambiguous: every bolt12 melt event had to
be broadcast to both the bolt11 and bolt12 subscription topics as a
workaround. Make it a distinct struct with its own
`NotificationPayload` variant and a `method` default of bolt12, and
drop the dual-topic broadcast.

Also fill the `method` field from the notification kind when websocket
payloads omit it, so custom and bolt12 quote notifications from older
mints deserialize with the correct method.

BREAKING CHANGE: `MeltQuoteBolt12Response` is no longer
interchangeable with `MeltQuoteBolt11Response`.
@thesimplekid
thesimplekid force-pushed the feat/add-method-field-to-quote-responses branch from ea6ed42 to 52ef5d9 Compare July 6, 2026 12:32
@thesimplekid
thesimplekid merged commit 52ef5d9 into cashubtc:main Jul 6, 2026
28 of 29 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in CDK Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants