feat(core): adopt NUT-04 mint quote accounting#303
Conversation
🦋 Changeset detectedLatest commit: 12a606e 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❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## codex/nut04-quote-accounting-docs #303 +/- ##
====================================================================
Coverage ? 87.68%
====================================================================
Files ? 131
Lines ? 18496
Branches ? 0
====================================================================
Hits ? 16218
Misses ? 2278
Partials ? 0
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: c187989371
ℹ️ 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".
| await beforePersist?.(canonicalQuote); | ||
| const persisted = await this.persistCanonicalMintQuote(canonicalQuote); | ||
| return { quote: persisted, remoteStateChanged }; | ||
| await this.mintQuoteRepository.setMintQuoteState( |
There was a problem hiding this comment.
When a BOLT11 quote already has a remoteUpdatedAt and we later record a local state observation (for example finalization writes ISSUED), the resolution above intentionally sets remoteUpdatedAt: null, but this call persists through setMintQuoteState; the memory, SQL, and IndexedDB implementations only update state/accounting fields and leave the old remote timestamp in place. A later remote snapshot with a higher updated_at but stale amount_issued can then be accepted as newer and regress a completed quote back to ready/pending. Persist the resolved quote here or clear remoteUpdatedAt in setMintQuoteState.
Useful? React with 👍 / 👎.
| beforePersist?: (quote: MintQuote) => Promise<void>; | ||
| } = {}, | ||
| ): Promise<{ quote: MintQuote; remoteQuoteChanged: boolean }> { | ||
| await options.beforePersist?.(canonicalQuote); |
There was a problem hiding this comment.
When importMintQuote is called for a quote that already exists and the incoming snapshot is stale (for example a lower updated_at), this line still runs the import-only accounting/capability validation before resolveMintQuoteObservation can discard the stale observation. In that case a stale snapshot with an unsupported unit or impossible accounting throws instead of returning the canonical stored quote, even though the same observation would otherwise be ignored. Move this validation after freshness resolution, or skip it when the observation will not be persisted.
Useful? React with 👍 / 👎.
| 'amount' | 'state' | ||
| > & { | ||
| amount: Amount; | ||
| state?: MintQuoteBolt11Response['state']; |
There was a problem hiding this comment.
Once BOLT11 quote snapshots are allowed to omit state, the subscription path can silently drop valid v5 accounting updates: MintOperationWatcherService only records BOLT11 payloads when payload.state is PAID or ISSUED. If a mint sends an update with amount_paid/amount_issued but no compatibility state, the watcher ignores it, so the quote is never marked ready and pending mint operations are not processed until another polling/recovery path runs. The BOLT11 watch policy needs to treat accounting fields as recordable too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2f9654270
ℹ️ 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 keyset = wallet.getKeyset(); | ||
|
|
||
| const sendOT = OutputData.createP2PKData({ pubkey }, amount, keyset); | ||
| const sendOT = OutputData.createP2PKData({ kind: 'P2PK', data: pubkey }, amount, keyset); |
There was a problem hiding this comment.
When preparing any P2PK send, this now passes { kind, data } to OutputData.createP2PKData, but the upgraded cashu-ts helper still expects P2PKOptions with a pubkey field. With pubkey undefined, cashu-ts throws while building the P2PK secret, so P2PK send preparation fails before outputs are reserved; I verified this with bun run --filter='@cashu/coco-core' test -- test/unit/P2pkSendHandler.test.ts, where all prepare-path P2PK tests fail at this call.
Useful? React with 👍 / 👎.
Egge21M
force-pushed
the
codex/nut04-quote-accounting-docs
branch
from
71560d3 to
740e6ea
Compare
Egge21M
force-pushed
the
codex/nut04-mint-quote-accounting
branch
from
f2f9654 to
12a606e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12a606ed8f
ℹ️ 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 canonicalQuote = mintQuoteFromBolt11Response(mintUrl, quote); | ||
| const accountingStatus = getBolt11MintQuoteAccountingStatus(canonicalQuote); |
There was a problem hiding this comment.
When a BOLT11 quote response has invalid accounting, e.g. amount_issued > amount_paid, this status calculation can classify it as completed as soon as amount_issued >= amount. recordMintQuoteSnapshot later ignores the invalid snapshot, but checkPendingOperation still sees the completed category and calls finalize, so a pending operation can be moved into execution based only on an invalid remote observation. Validate the BOLT11 accounting before deriving the category, similar to the onchain/BOLT12 handlers.
Useful? React with 👍 / 👎.
1 participant
Description
This pull request implements the NUT-04 mint quote accounting PRD on top of the PRD docs branch
codex/nut04-quote-accounting-docs.Implements #294.
Closes #297.
Closes #298.
Closes #299.
Closes #300.
Closes #301.
Closes #302.
Problem
Coco still treated remote NUT-04 mint quote state as the primary readiness signal. With cashu-ts v5 and reusable or partially issued quotes, readiness needs to come from persisted accounting fields (
amount_paid,amount_issued, and observation time) while preserving atomic mint operation behavior.Summary
Verification
git diff --checkbun run format:checkbun run security:release-agebun run buildbun run typecheckbun run --filter='@cashu/coco-core' test:unitbun test packages/sql-storage/src/test/schema.test.tsbun --cwd packages/expo-sqlite test src/test/contract.test.tsChangeset
.changeset/cashu-ts-v5-boundary.md..changeset/mint-quote-accounting-columns.md.