Conversation
igbopharaoh
force-pushed
the
mint-operation-saga
branch
from
e2e9be4 to
26373c2
Compare
🦋 Changeset detectedLatest commit: a40834f The changes in this PR will be included in the next version bump. 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 |
![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: e2e9be4f89
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const initOp = await this.init(mintUrl, quoteId, 'bolt11', {}); | ||
| const preparedOp = await this.prepare(initOp.id); | ||
| return this.execute(preparedOp.id); |
There was a problem hiding this comment.
redeem() relies on getOperationByQuote() for deduplication, but the subsequent operation creation is not in the same critical section, so concurrent calls for the same mintUrl/quoteId can both pass the existence check and create separate operations. In that race, one path can end up rolled back with ALREADY_ISSUED even though redemption is expected to be idempotent. Add a per-quote lock (or transactional uniqueness on mint+quote active op) around lookup + create.
Useful? React with 👍 / 👎.
| const preparedOps = await this.mintOperationRepository.getByState('prepared'); | ||
| for (const op of preparedOps) { | ||
| try { | ||
| await this.execute(op.id); |
There was a problem hiding this comment.
recoverPendingOperations() replays stale prepared operations without re-checking trust, so operations created while a mint was trusted will still execute after that mint is later marked untrusted. Since startup now invokes this recovery flow, this can trigger wallet/mint calls against a mint the current trust policy rejects. Recovery should gate execute/recover paths with isTrustedMint before contacting the mint.
Useful? React with 👍 / 👎.
igbopharaoh
force-pushed
the
mint-operation-saga
branch
from
26373c2 to
5c044b0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c044b095a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (existing.state === 'init') { | ||
| const prepared = await this.prepare(existing.id); | ||
| return this.execute(prepared.id); | ||
| } |
There was a problem hiding this comment.
redeem() resumes persisted operations (init/prepared/executing) without re-validating isTrustedMint, so a quote created while a mint was trusted can still trigger prepare/execute/recoverExecuting calls after that mint is later marked untrusted. This bypasses the trust gate that existed before saga delegation and can still contact blocked mints on user-initiated redemption (not just startup recovery), so trust should be enforced before running any non-terminal operation transitions.
Useful? React with 👍 / 👎.
|
I see you were able to resolve merge hell, awesome! Thank you for taking this on, this already looks amazing! I noticed that you did not follow the same API structure that we have for other saga based operations (e.g. I wonder whether we should merge the operations API refactor first, and than have one for this one as well. What do you think? |
Oops, I think that is an oversight on my end, I'll push a commit to address it.
I think we can merge that first since reviewing this can take a bit of time and I've not added the public apis |
|
Awesome, I have added a placeholder API for the minting ops in #107 |
Great, I'll check it out |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 590ab6846b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| try { | ||
| releaseMintLock = await this.mintScopedLock.acquire(mintUrl); | ||
|
|
||
| let existing = await this.getOperationByQuote(mintUrl, quoteId); |
There was a problem hiding this comment.
redeem() deduplicates by reading getOperationByQuote() and then creating a new operation, but that protection is only guarded by an in-memory mintScopedLock for this process. In multi-instance setups (for example multiple tabs/processes sharing IndexedDB/SQLite), concurrent redeems for the same mintUrl/quoteId can both observe no active op and both insert, producing competing operations with different outputs where one later rolls back as ALREADY_ISSUED. Add a repository-level uniqueness guarantee (or transactional lookup+insert) for active operations per quote.
Useful? React with 👍 / 👎.
| const handler = this.handlerProvider.get(executing.method); | ||
| const { wallet } = await this.walletService.getWalletWithActiveKeysetId(executing.mintUrl); | ||
| const result = await handler.execute({ |
There was a problem hiding this comment.
execute() transitions a prepared operation to executing and calls wallet/mint APIs without checking isTrustedMint. If a quote was prepared while trusted and that mint is later marked untrusted, QuotesApi.executeMint() (or any direct execute call) will still contact the mint, bypassing trust policy on this path. Add a trust gate in execute() before network-facing handler calls.
Useful? React with 👍 / 👎.
|
GM! New API structure has been merged. Please wire you Service in the new |
igbopharaoh
force-pushed
the
mint-operation-saga
branch
from
590ab68 to
51d11b6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51d11b6c42
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const { wallet } = await this.walletService.getWalletWithActiveKeysetId(executing.mintUrl); | ||
| const result = await handler.recoverExecuting({ |
There was a problem hiding this comment.
recoverPendingOperations() now replays executing mint ops at startup, but recoverExecutingOperation() proceeds straight to wallet/handler calls without checking isTrustedMint. If a mint was trusted when the op started and later marked untrusted, startup recovery will still contact that mint on this path, bypassing the trust policy enforced for new operations. Add the same trust gate used for prepared-op recovery before invoking network-facing executing recovery.
Useful? React with 👍 / 👎.
igbopharaoh
force-pushed
the
mint-operation-saga
branch
from
51d11b6 to
6798c5f
Compare
igbopharaoh
force-pushed
the
mint-operation-saga
branch
from
6798c5f to
6797fb6
Compare
|
Hello @Egge21M wired the new service into the MintOpsApi, kindly review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6797fb6075
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| this.logger?.info('Mint quote already in ISSUED state, skipping redeem', { | ||
| mintUrl, | ||
| quoteId, | ||
| }); | ||
| return null; |
There was a problem hiding this comment.
The early return in redeem() when quote.state === 'ISSUED' can strand funds after partial saga failure: if a prior operation for this quote rolled back because proof persistence/recovery failed, and the quote state was later updated to ISSUED (for example by remote state sync), retries hit this branch and never attempt recovery from the operation’s stored outputData. In that state there is no remaining public path to re-run proof recovery for the quote, even though operation metadata still exists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8ad95f208
ℹ️ 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".
| async prepare(input: PrepareMintInput<TSupported>): Promise<PendingMintOperation> { | ||
| const initOperation = await this.mintOperationService.init( | ||
| input.mintUrl, | ||
| input.quoteId, | ||
| input.method, |
There was a problem hiding this comment.
manager.ops.mint.prepare() always goes straight into init() for the supplied mintUrl/quoteId, unlike redeem() which first looks up an existing operation for that quote. That means a retry, double-click, or a race with the default background auto-redeem path can persist a second mint operation for the same quote; if the first one already issued, the new op generates fresh outputData that cannot recover the original proofs and will later end in ALREADY_ISSUED/error instead of resuming the existing saga.
Useful? React with 👍 / 👎.
| const pendingOps = await this.mintOperationRepository.getByState('pending'); | ||
| for (const op of pendingOps) { | ||
| try { | ||
| if (await this.mintService.isTrustedMint(op.mintUrl)) { | ||
| await this.checkPendingOperation(op.id); |
There was a problem hiding this comment.
recoverPendingOperations() currently reconciles pending ops before executing ones, but recoverExecutingOperation() can downgrade an executing op back to pending via transitionToPending() when it cannot finish yet. In startup/manual recovery runs where no processor is draining paid quotes, those downgraded ops miss the pending pass entirely and remain stuck until a second recovery run or explicit user action.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10e22d7a70
ℹ️ 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 handler = this.handlerProvider.get(op.method); | ||
| const { wallet } = await this.walletService.getWalletWithActiveKeysetId(op.mintUrl); | ||
|
|
||
| const result = await handler.checkPending({ |
There was a problem hiding this comment.
recoverPendingOperations() already skips untrusted mints, but this method does not. manager.ops.mint.checkPayment() and manager.ops.mint.refresh() both route through here, so a quote that was prepared while a mint was trusted will still call checkMintQuoteState (and can even fall through to execute()) after the user marks that mint untrusted. Add the same isTrustedMint guard here so these inspection APIs do not bypass the trust policy.
Useful? React with 👍 / 👎.
| } else if (remoteQuote.state === 'UNPAID') { | ||
| return { | ||
| status: 'PENDING', | ||
| error: `Recovered: quote ${quoteId} is still UNPAID`, | ||
| }; |
There was a problem hiding this comment.
If mintProofsBolt11() fails with the mint’s “quote expired” error, execute() recovers through this branch. For Cashu quotes the expiry is carried separately from state, so an expired quote can still come back as UNPAID; returning PENDING here sends the operation back to the pending queue forever, and every retry/startup recovery will loop the same way instead of surfacing a terminal failure. Check remoteQuote.expiry before downgrading an UNPAID quote to pending.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b37541392d
ℹ️ 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".
| async redeemMintQuote(mintUrl: string, quoteId: string): Promise<void> { | ||
| this.logger?.info('Redeeming mint quote', { mintUrl, quoteId }); | ||
|
|
||
| const trusted = await this.mintService.isTrustedMint(mintUrl); | ||
| if (!trusted) { | ||
| throw new UnknownMintError(`Mint ${mintUrl} is not trusted`); | ||
| } | ||
|
|
||
| try { | ||
| const quote = await this.mintQuoteRepo.getMintQuote(mintUrl, quoteId); | ||
| if (!quote) { | ||
| this.logger?.warn('Mint quote not found', { mintUrl, quoteId }); | ||
| throw new Error('Quote not found'); | ||
| } | ||
| if (!quote.amount) { | ||
| this.logger?.warn('Mint quote had undefined amount', { mintUrl, quoteId }); | ||
| throw new Error('Quote amount undefined'); | ||
| } | ||
| const { wallet } = await this.walletService.getWalletWithActiveKeysetId(mintUrl); | ||
| const { keep } = await this.proofService.createOutputsAndIncrementCounters(mintUrl, { | ||
| keep: quote.amount, | ||
| send: 0, | ||
| }); | ||
| const proofs = await wallet.mintProofsBolt11(quote.amount, quote.quote, undefined, { type: 'custom', data: keep }); | ||
| await this.eventBus.emit('mint-quote:redeemed', { mintUrl, quoteId, quote }); | ||
| this.logger?.info('Mint quote redeemed, proofs minted', { | ||
| mintUrl, | ||
| quoteId, | ||
| amount: quote.amount, | ||
| proofs: proofs.length, | ||
| }); | ||
| await this.setMintQuoteState(mintUrl, quoteId, 'ISSUED'); | ||
| await this.proofService.saveProofs(mintUrl, mapProofToCoreProof(mintUrl, 'ready', proofs)); | ||
| this.logger?.debug('Proofs saved to repository', { mintUrl, count: proofs.length }); | ||
| } catch (err) { | ||
| this.logger?.error('Failed to redeem mint quote', { mintUrl, quoteId, err }); | ||
| throw err; | ||
| } | ||
| await this.mintOperationService.redeem(mintUrl, quoteId); |
There was a problem hiding this comment.
When a quote comes from createMintQuote(), addExistingMintQuotes(), or requeuePaidMintQuotes(), there is no mint operation yet—those paths only populate MintQuoteRepository. mintOperationService.redeem() returns null in that case, so this method now resolves successfully without minting proofs or surfacing an error, and MintQuoteProcessor will drop the PAID quote as if redemption succeeded. That breaks all legacy/service-driven mint redemption unless the caller manually created a saga first.
Useful? React with 👍 / 👎.
| if (failure) { | ||
| if (initOp) { | ||
| await this.tryRecoverInitOperation(initOp); |
There was a problem hiding this comment.
MintBolt11Handler.prepare() creates the remote quote before generating outputs, so failures from output generation or the first update() can leave a live quote behind even though prepareNewQuote() throws. This cleanup deletes the only persisted record in that case, which means the caller never learns the quote ID and startup recovery has nothing left to reconcile. That reintroduces a quote-loss window in the crash-safe flow this saga is supposed to provide.
Useful? React with 👍 / 👎.
| const observedPending: PendingMintOperation = { | ||
| ...op, | ||
| lastObservedRemoteState: result.observedRemoteState, | ||
| lastObservedRemoteStateAt: result.observedRemoteStateAt, | ||
| updatedAt: Date.now(), | ||
| }; | ||
| await this.mintOperationRepository.update(observedPending); |
There was a problem hiding this comment.
This method reads a pending snapshot, performs network I/O, and then blindly writes observedPending back to the repository without taking the per-operation lock. If execute()/finalize() runs on the same operation while handler.checkPending() is in flight, this stale write can revert an executing or finalized row back to pending, leading to duplicate issuance attempts or a stuck saga. manager.ops.mint.checkPayment(), refresh(), and startup recovery all reach this path, so it needs the same lock or a compare-and-swap style update.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c800ae99ba
ℹ️ 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".
| 'coco-cashu-sqlite-bun': patch | ||
| 'coco-cashu-indexeddb': patch | ||
| 'coco-cashu-sqlite3': patch | ||
| 'coco-cashu-core': patch |
There was a problem hiding this comment.
This changeset still publishes coco-cashu-core as a patch even though the diff removes public manager.quotes.* mint methods, renames the watcher/processor config keys and manager helpers, and drops the mint-quote:* event surface. Consumers on a normal ^ range will pick up this patch automatically and hit compile/runtime breaks without any semver warning, so this needs a breaking-version release or compatibility shims.
Useful? React with 👍 / 👎.
| async prepare( | ||
| ctx: PrepareContext<'bolt11'>, | ||
| ): Promise<PendingMintOperation<'bolt11'> & MintMethodMeta<'bolt11'>> { | ||
| const quote = ctx.importedQuote ?? (await ctx.wallet.createMintQuoteBolt11(ctx.operation.amount)); |
There was a problem hiding this comment.
manager.ops.mint.prepare() now accepts unit, but the bolt11 handler always creates the remote quote with createMintQuoteBolt11(ctx.operation.amount) and only afterwards rejects any unit mismatch. In practice, any caller that sets unit !== 'sat' gets a live quote plus an immediate error, because this path never asks the mint for a quote in the requested unit.
Useful? React with 👍 / 👎.
| case 'UNPAID': | ||
| return { | ||
| observedRemoteState: quote.state, | ||
| observedRemoteStateAt, | ||
| category: 'waiting', |
There was a problem hiding this comment.
Cashu quote expiry is carried separately from state, but this branch treats every UNPAID quote as still waiting. That leaves recoverPendingOperations() and manager.ops.mint.checkPayment()/refresh() stuck retrying expired quotes forever instead of moving the operation to a terminal failure once quote.expiry has passed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2a13657b3
ℹ️ 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 await this.finalizeIssuedOperation(executing, error); |
There was a problem hiding this comment.
If handler.execute() reports ALREADY_ISSUED after a local persistence/recovery problem, ensureOutputsSaved() can still return false here. This path still calls finalizeIssuedOperation(), which stores the saga as finalized and emits mint-op:finalized even though no proofs were recovered. In that scenario the wallet treats the mint as complete and there is no public retry path left to reconstruct the missing outputs.
Useful? React with 👍 / 👎.
| } catch (err) { | ||
| this.logger?.error('Failed to update quote state', { mintUrl, quoteId, state, err }); | ||
| } | ||
| this.logger?.error('Failed to process mint operation', { mintUrl, operationId, err }); |
There was a problem hiding this comment.
In the new saga flow, mintOperations.finalize() can legitimately leave an operation in pending again via transitionToPending() (for example when recoverExecutingOperation() cannot restore issued proofs yet). Those cases throw a plain Error, not NetworkError; this catch-all branch only logs and drops the queue item, so a PAID mint operation stops being retried by the processor until some manual refresh or startup recovery happens.
Useful? React with 👍 / 👎.
| if (!trusted) { | ||
| this.logger.debug('Skipping legacy mint quote reconciliation for untrusted mint', { | ||
| mintUrl: quote.mintUrl, | ||
| quoteId: quote.quote, | ||
| }); |
There was a problem hiding this comment.
I checked rg "reconcileLegacyMintQuotes\(" and this helper is only invoked from initializeCoco(). Once this branch skips a legacy quote because the mint is untrusted, nothing re-imports that row when the user trusts the mint later; the rest of the runtime only processes MintOperationRepository entries. That strands pre-migration pending/paid quotes unless the application explicitly calls the new helper.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 259d7b0142
ℹ️ 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 (!quote.amount || quote.amount <= 0) { | ||
| throw new ProofValidationError(`Mint quote ${quote.quote} has invalid amount`); | ||
| } |
There was a problem hiding this comment.
If a caller passes an ISSUED quote snapshot to manager.ops.mint.importQuote(), this path still creates a fresh pending operation and later generates new outputData for it. That output data can never match proofs that were already issued elsewhere, so the eventual ALREADY_ISSUED path cannot recover funds and the import ends in an unrecoverable error instead of failing fast. This is a new public API surface, so it should reject terminal quote states before persisting the init op.
Useful? React with 👍 / 👎.
| this.eventBus.on('mint-op:pending', ({ mintUrl, operation }) => { | ||
| if (operation.state !== 'pending') return; | ||
| this.handleMintOperationPending(mintUrl, operation as PendingMintOperation); | ||
| }); | ||
| this.eventBus.on('mint-quote:created', ({ mintUrl, quoteId, quote }) => { | ||
| this.handleMintQuoteCreated(mintUrl, quoteId, quote); | ||
| }); | ||
| this.eventBus.on('mint-quote:added', ({ mintUrl, quoteId, quote }) => { | ||
| this.handleMintQuoteAdded(mintUrl, quoteId, quote); | ||
| this.eventBus.on('mint-op:quote-state-changed', ({ mintUrl, operationId, quoteId, state }) => { |
There was a problem hiding this comment.
When a mint operation expires or otherwise transitions to failed, MintOperationService.failOperation()/failPendingOperation() only emit mint-op:finalized. HistoryService subscribes to mint-op:pending and mint-op:quote-state-changed only, so the existing mint history entry remains stuck at its last quote state (UNPAID/PAID) and no history:updated event is sent for the failure. Any UI or client code that relies on history to show terminal mint outcomes will miss these errors entirely.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b31621a5d9
ℹ️ 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 this.eventBus.emit('mint-op:quote-state-changed', { | ||
| mintUrl: current.mintUrl, | ||
| operationId: current.id, | ||
| operation: current, | ||
| quoteId: current.quoteId, |
There was a problem hiding this comment.
If mintOperationRepository.update(finalized) fails here (for example, a transient IndexedDB/SQLite write error), the code has already broadcast mint-op:quote-state-changed with ISSUED. MintOperationWatcherService stops watching on that event and HistoryService records the mint as issued, but the operation row is still executing, so the processor falls back to generic error handling and drops the op until a manual refresh or next startup recovery. Emitting the ISSUED notification only after the finalized row is stored avoids leaving the saga untracked in that failure mode.
Useful? React with 👍 / 👎.
Egge21M
force-pushed
the
mint-operation-saga
branch
from
b31621a to
151b547
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 151b5476ed
ℹ️ 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".
| 'mint-op:quote-state-changed', | ||
| async ({ mintUrl, operationId, operation, state }) => { | ||
| if (state === 'PAID') { | ||
| this.enqueue(mintUrl, quoteId, 'bolt11'); // Default to bolt11 for now | ||
| this.enqueue(mintUrl, operationId, operation.method); | ||
| } |
There was a problem hiding this comment.
When the first remote update a pending mint op sees is ISSUED—for example after reconnecting, after starting the watcher late, or when the subscription backend sends the current state on subscribe—MintOperationWatcherService still emits mint-op:quote-state-changed and then stops watching that quote, but this processor only enqueues PAID. In that case the operation stays persisted as pending and proof recovery/finalization never runs until the user manually refreshes or a later startup recovery sweep happens.
Useful? React with 👍 / 👎.
Egge21M
force-pushed
the
mint-operation-saga
branch
from
151b547 to
a40834f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a40834fe91
ℹ️ 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 quotes = await this.mintQuoteRepository.getPendingMintQuotes(); | ||
|
|
||
| for (const quote of quotes) { | ||
| if (mintUrl && quote.mintUrl !== mintUrl) continue; | ||
| if (quote.state === 'ISSUED') { | ||
| skipped.push(quote.quote); |
There was a problem hiding this comment.
reconcileLegacyMintQuotes() now only iterates mintQuoteRepository.getPendingMintQuotes() and then skips ISSUED rows, so upgraded wallets never import legacy quotes that were already marked issued. In the pre-saga flow, MintQuoteService.redeemMintQuote() set the quote state to ISSUED before saveProofs(), so a crash in that window leaves an ISSUED row with missing proofs. After this change, Manager.reconcileLegacyMintQuotes() is the only remaining path that reads MintQuoteRepository, which means those pre-existing crash cases become permanently unrecoverable instead of being migrated into the new recovery flow.
Useful? React with 👍 / 👎.
3 participants
Description
This PR replaces one-shot mint quote redemption with a persisted mint saga (
MintOperationService) to make minting crash-safe and recoverable.Why
Mint redemption previously depended on a single execution path, which left a crash window between successful remote issuance and local proof persistence. This change introduces a stateful operation flow with explicit recovery so retries and restarts are safe.
What changed
packages/core/operations/mint:MintOperationstate model (init -> prepared -> executing -> finalized | rolled_back)MintMethodHandlerabstraction andMintHandlerProviderMintOperationServicewithinit/prepare/execute/redeem/recoverPendingOperationsbolt11mint handler:prepareALREADY_ISSUEDreconciliation inexecuterecoverExecutinglogic based on remote quote state and restore flowoutputDataQuotesApi.redeemMintQuotenow routes tomintOperationService.redeemMintQuoteService.redeemMintQuotenow delegates to mint sagaManagernow wires mint handler/provider and runsrecoverPendingMintOperations()on startupmint-op:preparedmint-op:executingmint-op:finalizedmint-op:rolled-backmint-quote:*events remain intactCloses #96