fix(core): record melt quotes before settlement#275
Conversation
🦋 Changeset detectedLatest commit: 7445e8a The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #275 +/- ##
==========================================
+ Coverage 86.90% 86.99% +0.08%
==========================================
Files 130 130
Lines 17314 17427 +113
==========================================
+ Hits 15047 15160 +113
Misses 2267 2267
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdde50a8d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const res = | ||
| this.getPersistedSettlementResponse(ctx.canonicalQuote) ?? (await this.checkMeltQuote(ctx)); |
There was a problem hiding this comment.
When the cached quote is already PAID but lacks change, this fallback fetches the full settlement response and uses it only for local finalization. In the new checkPendingOperation flow, the preceding quote refresh has already stored a terminal PAID quote; QuoteLifecycle then ignores later observations for existing PAID quotes, so mints that report PAID before returning change/outpoint/preimage leave the canonical quote permanently missing those settlement fields even though this remote response had them. Record this fallback response (or avoid terminal persistence until settlement fields are present) before completing finalization.
Useful? React with 👍 / 👎.
Egge21M
force-pushed
the
issue-269-melt-observation-before-settlement
branch
from
bdde50a to
34c16d3
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26a9ddf0ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return 'finalize'; | ||
| } else if (decision === 'rollback') { | ||
| await this.rollback(op.id, 'Rollback requested by handler'); | ||
| await this.rollback(op.id, 'Rollback requested by handler', { canonicalQuote: quote }); |
There was a problem hiding this comment.
When checkPendingOperation() decides to roll back, this passes the pre-lock quote snapshot into rollback(), so rollback's own safety check reuses that stale canonicalQuote instead of querying the mint after acquiring the operation lock. If a pending melt is observed as UNPAID here but the remote quote advances to PAID before rollback wins the local lock, this path can still release inflight proofs and mark the operation rolled back; previously the rollback safety check re-queried the quote at that point. Let rollback() refresh/verify the quote under its lock rather than trusting the earlier decision snapshot.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ec4aa294c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return null; | ||
| } | ||
|
|
||
| const change = quote.change ?? []; |
There was a problem hiding this comment.
When a cached PAID quote does not have its settlement change serialized (for example, legacy rows or an earlier paid observation with change omitted), this new cached-settlement path converts the missing field to []. Finalization then marks the melt inputs spent, skips unblindAndSaveChangeProofs, and computes the whole excess as fees instead of fetching the full quote as the old path did, so any actual mint change for that pending melt is lost locally.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6f1216dc6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const persisted = await this.persistCanonicalMeltQuote({ | ||
| ...existing, | ||
| change: canonicalQuote.change, |
There was a problem hiding this comment.
When an existing terminal PAID quote was stored before settlement metadata was available and a later refresh returns PAID with change, this enrichment persists only the new change signatures while keeping the old payment_preimage/outpoint. Because BaseQuoteMeltHandler now treats any PAID quote with an array change as complete and skips the remote settlement fetch, BOLT/onchain pending operations can finalize without the preimage or outpoint even though the refreshed quote contained it; copy the method-specific settlement field when it is missing locally.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7445e8afea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (canonicalQuote.state === 'PAID' && !Array.isArray(canonicalQuote.change)) { | ||
| return this.quoteLifecycle.refreshMeltQuote(op.mintUrl, op.method, op.quoteId); | ||
| } | ||
| return canonicalQuote; |
There was a problem hiding this comment.
When the cached PAID quote already has a serialized change array but still lacks the method-specific settlement field (payment_preimage for BOLT or outpoint for onchain), this branch treats it as complete and returns it without a remote refresh. The new cached-PAID fast path means mergePaidMeltQuoteSettlement never gets a chance to copy that metadata later, so the pending operation can be finalized permanently without the preimage/outpoint even though a full quote check would now provide it.
Useful? React with 👍 / 👎.
1 participant
Parent PRD
Slice
Summary
Validation
bun run --filter='@cashu/coco-core' test -- test/unit/MeltOperationService.test.ts test/unit/MeltBolt11Handler.test.ts test/unit/MeltBolt12Handler.test.tsbun run --filter='@cashu/coco-core' typecheckbun run --filter='@cashu/coco-core' buildbun run --filter='@cashu/coco-core' test:unitNotes
Full
@cashu/coco-coretest was attempted by the implementation worker; unit tests passed, but live integration checks need the repo's mint/auth environment. This PR intentionally does not add the melt quote watcher, settlement processor, or manager lifecycle wiring.