feat: add Mint Swap operation ownership primitives - #414
Conversation
🦋 Changeset detectedLatest commit: 471631e 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 @@
## master #414 +/- ##
==========================================
- Coverage 93.42% 93.41% -0.01%
==========================================
Files 113 113
Lines 17981 18009 +28
==========================================
+ Hits 16798 16823 +25
- Misses 1183 1186 +3
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: 6452187749
ℹ️ 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".
| operation.terminalFailure ? JSON.stringify(operation.terminalFailure) : null, | ||
| JSON.stringify(operation.outputData), | ||
| operation.id, | ||
| operation.parent?.kind ?? null, |
There was a problem hiding this comment.
In multi-process/tab runs where a standalone executor has read an unparented mint operation before another transaction claims it with updateIfStateAndParentMatch, this regular update() path writes NULL for parentKind/parentId from the stale in-memory object and erases the durable parent claim. Existing mint flows still use mintOperationRepository.update() for pending/executing/finalized transitions, and the IndexedDB put path has the same replacement behavior when operation.parent is absent, so stale standalone writers can still overwrite ownership despite the new guarded method; preserve the stored parent or require the same parent precondition before clearing it.
AGENTS.md reference: AGENTS.md:L161-L164
Useful? React with 👍 / 👎.
| */ | ||
| updateIfStateAndParentMatch( | ||
| operation: MintOperation, | ||
| expected: { state: MintOperationState; parent?: OperationParent }, |
There was a problem hiding this comment.
When a batch worker read an unparented operation before another transaction marks it batchingDisabled, this guard still matches because it only checks state and parent, so the later batch claim can overwrite the opt-out marker while assigning a mint-batch parent. Since the new field is the durable batch-eligibility invariant, include the expected batchingDisabled value in the conditional update contract or provide a claim-specific method that requires it to be unset before mutating.
AGENTS.md reference: AGENTS.md:L161-L164
Useful? React with 👍 / 👎.
|
Hello @Egge21M good day, looking at the changes here and how the established primitive is going to be OperationParent, should I wait for this to get merged and remodel my work on the slice 2 implementation of mintswaps to fit into this shape ? |
3de5cc5 to
4454802
Compare
Egge21M
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 445480202a
ℹ️ 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".
| @@ -0,0 +1,12 @@ | |||
| --- | |||
| '@cashu/coco-core': minor | |||
There was a problem hiding this comment.
For consumers that implement the public MintOperationRepository contract, upgrading to this minor release makes their adapters fail to typecheck because assignMintSwapParentIfUnparented is a new required method. This repository treats required adapter-contract changes as major releases (for example, the existing atomic key-allocation changeset), so mark @cashu/coco-core as major to avoid shipping a breaking API change under a compatible version range.
Useful? React with 👍 / 👎.
| return false; | ||
| } | ||
|
|
||
| this.operations.set(operationId, { ...current, parent, updatedAt: Date.now() }); |
There was a problem hiding this comment.
When a caller mutates the parent object after a successful in-memory claim, this stores the same reference, so the operation's owner changes without another guarded call; getById and the query methods also return that same nested reference, allowing a read result to mutate repository state directly. Clone parent when storing and when returning operations so ownership can only change through repository mutation paths.
AGENTS.md reference: AGENTS.md:L167-L168
Useful? React with 👍 / 👎.
| `UPDATE coco_cashu_mint_operations | ||
| SET parentKind = ?, parentId = ?, updatedAt = ? | ||
| WHERE id = ? AND state = ? AND parentKind IS NULL AND parentId IS NULL`, | ||
| [parent.kind, parent.id, getUnixTimeSeconds(), operationId, expectedState], |
There was a problem hiding this comment.
When a caller supplies { kind: 'mint-swap', id: '' }, which satisfies the exported TypeScript interface, this SQL update succeeds and returns true because neither the method nor the schema validates parentId; the next repository read then throws from parseParent because an empty ID is treated as invalid. Validate the parent before mutating so a successful claim cannot make the operation unreadable; the IndexedDB implementation has the same write/read mismatch.
AGENTS.md reference: AGENTS.md:L153-L153
Useful? React with 👍 / 👎.
| /** Durable Mint Swap Operation ownership assigned at creation or by a conditional claim. */ | ||
| parent?: MintSwapOperationParent; |
There was a problem hiding this comment.
When a Mint Swap-owned child remains in init across a restart, initializeCoco() invokes MintOperationService.recoverPendingOperations(), which enumerates every init operation and deletes it in recoverInitOperation() after checking only its state; a concurrent init-state claim after that read can be erased the same way. The analogous melt recovery also deletes parented init melts, even though this change explicitly persists parents across init states. Fresh evidence is the unconditional cleanup in MintOperationService.ts:524-527,970-979 and MeltOperationService.ts:624-628,770-783; skip owned rows or make deletion conditional on the operation remaining unparented.
AGENTS.md reference: AGENTS.md:L167-L168
Useful? React with 👍 / 👎.
|
@igbopharaoh I think this is now ready to be merged. However Batch Minting no longer requires it. Do you want to keep it for the Mint Swaps or would you rather go with your approach? |
|
Hello @Egge21M kindly give me sometime to take a look at your approach here again and get back to you on this |
Hello Egge, I've been able to look at this pull request and compare it with mine particularly in the definition of the mint-swap primitives. I think this PR does a better job at defining the primitives and provides a clear way of defining the MintSwapOperationParent. I think I can adopt the modelling here into #402 particularly the MintSwapOperationParent with the kind and id fields. Mine was using only the id but your interface definition is better. What are your thoughts ? |
Yes great, whatever you think is best. I have closed this, but kept the branch alive on the remote. Feel free to pick whatever you need. |
2 participants
Related to #411. The scope is intentionally
narrower than that issue: Batch Mint Operations remain standalone aggregates under ADR 0009.
Problem
Future Mint Swap orchestration needs to durably associate its Mint and Melt Operations without
allowing stale ordinary updates to erase ownership. Although the persistence shape can support a
discriminator, the domain behavior should not imply that unknown future parent operations share
Mint Swap claim semantics.
Summary
MintSwapOperationParentreference to Mint and Melt Operations.claimForMintSwap(operationId, expectedState, mintSwapOperationId)repository operation; callers cannot supply arbitrary parent kinds.parentKindandparentIdacross SQL and IndexedDB adapters, withlegacy-row compatibility and validation of the currently supported
mint-swapkind.rollback.
primitives.
Verification
bun run --filter='@cashu/coco-core' test:unitbun run --filter='@cashu/coco-sql-storage' testChangeset
.changeset/operation-ownership-primitives.mdfor the public Mint Swap ownership type,repository interface, and adapter persistence changes.