ENG-29

[Connorbelez]

Summary by CodeRabbit

• New Features
  • New mint and issue operation supporting single-step share allocation to multiple lenders
  • Built-in validation ensures allocations meet minimum thresholds and sum correctly
  • Idempotent operation design—duplicate requests with the same key return consistent results
  • Double-mint protection prevents accidental duplicate minting for the same mortgage

[linear]

ENG-29 Implement mintAndIssue convenience mutation — atomic mint + initial allocation

[sourcery-ai]

Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[Connorbelez]

• ENG-29 #91 👈 (View in Graphite)
• ENG-27 #90 : 5 other dependent PRs (#92 , #93 , #94 and 2 others)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Warning

Rate limit exceeded

@Connorbelez has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 25 minutes and 3 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4babdaed-d274-4bfa-b9b8-b74797423d68

📥 Commits

Reviewing files that changed from the base of the PR and between 679eb84 and ffc35bf.

📒 Files selected for processing (7)

• convex/ledger/__tests__/mintAndIssue.test.ts
• convex/ledger/mutations.ts
• convex/ledger/validators.ts
• specs/ENG-29/chunks/chunk-01-implementation/context.md
• specs/ENG-29/chunks/chunk-01-implementation/tasks.md
• specs/ENG-29/chunks/manifest.md
• specs/ENG-29/tasks.md

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ENG-29

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Adds the new ENG-29 “mintAndIssue” convenience mutation to the mortgage ownership ledger, providing an atomic single-call flow to mint a mortgage’s total supply and immediately issue shares to initial lender position accounts.

Changes:

• Added allocationValidator and mintAndIssueArgsValidator for the new mutation’s arguments.
• Implemented mintAndIssue in convex/ledger/mutations.ts (mint + issue + invariant check in one ledger mutation).
• Added a dedicated Vitest suite covering happy paths, validation failures, double-mint protection, and idempotent replay.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
specs/ENG-29/tasks.md	Adds the ENG-29 task checklist for implementing the feature.
specs/ENG-29/chunks/manifest.md	Declares a single implementation chunk covering code + tests.
specs/ENG-29/chunks/chunk-01-implementation/tasks.md	Chunk-level task breakdown mirroring the main task list.
specs/ENG-29/chunks/chunk-01-implementation/context.md	Implementation context/acceptance criteria and expected patterns.
convex/ledger/validators.ts	Introduces validators for allocations and the new mutation’s args.
convex/ledger/mutations.ts	Implements the mintAndIssue mutation and wires it into ledger mutations.
convex/ledger/tests/mintAndIssue.test.ts	Adds tests for mint+issue behavior, validation, and idempotency.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

This PR implements mintAndIssue, an atomic convenience mutation that combines mintMortgage + issueShares into a single Convex transaction. It creates a TREASURY account, posts a MORTGAGE_MINTED entry (WORLD → TREASURY), then issues shares to each allocation (TREASURY → POSITION), validated by a post-hoc TREASURY balance invariant check. It follows the established ledgerMutation pattern and is accompanied by a well-structured 8-case test suite.

Issues found:

• P1 — Float amounts crash with unhandled TypeError: allocationValidator uses v.number(), which accepts non-integer floats. When BigInt(amount) is called on a float like 1500.5, the runtime throws TypeError: Cannot convert a non-integer to a BigInt — an unhandled error rather than a proper ConvexError. Should use v.int64() or guard with Number.isInteger() before conversion.
• P1 — Duplicate lenderId values in allocations are not validated: Two allocations with the same lenderId produce the same derived idempotency key ({key}:issue:{lenderId}). This either causes the postEntry idempotency mechanism to silently double-count (leaving TREASURY balance non-zero and triggering INVARIANT_VIOLATION) or propagates a low-level internal error. An explicit DUPLICATE_LENDER_IN_ALLOCATIONS check should be added before any DB writes.
• P2 — Idempotency replay uses unbounded .collect() and may return incorrect entries: The replay path fetches all journal entries for the mortgage with no limit, then filters in-memory. This is costly on active mortgages and — more importantly — returns all SHARES_ISSUED entries ever posted for the mortgage, not just those from the original call. Querying by derived idempotency keys would be both more efficient and semantically correct.
• P2 — Idempotency replay test doesn't verify entry identity: The test asserts issueEntries.length matches but not that the _id values are the same entries, weakening the idempotency guarantee being tested.

Confidence Score: 2/5

• Not safe to merge — two P1 logic bugs (float amounts crashing with unhandled TypeError, duplicate lenderIds producing confusing invariant violations) need to be addressed first.
• The core mutation structure and idempotency pattern are solid and consistent with the rest of the codebase, but two input-validation gaps can cause unhandled runtime errors that would be surfaced to callers without a meaningful error code. The idempotency replay correctness issue is a latent bug that could manifest later in the mortgage lifecycle.
• convex/ledger/validators.ts (float amount type) and convex/ledger/mutations.ts (duplicate lenderId check, idempotency replay entry collection).

Important Files Changed

Filename	Overview
convex/ledger/mutations.ts	Adds mintAndIssue atomic mutation; has two P1 bugs: (1) duplicate lenderId values in allocations are not rejected and lead to an INVARIANT_VIOLATION rather than a clear error, (2) the idempotency replay path collects all mortgage entries unboundedly and may return stale/wrong issueEntries after future ledger operations.
convex/ledger/validators.ts	Adds allocationValidator and mintAndIssueArgsValidator; amount: v.number() allows non-integer floats, causing an unhandled TypeError when BigInt(amount) is called — should be v.int64() or guarded with Number.isInteger().
convex/ledger/tests/mintAndIssue.test.ts	Good coverage of 8 test cases including happy paths, validation rejections, double-mint, and idempotency replay; idempotency test only checks issueEntries.length and not entry identity, and no test covers duplicate lenderId in allocations.
specs/ENG-29/chunks/chunk-01-implementation/context.md	Spec document with one ordering inaccuracy: states "double-mint check → idempotency" but the implementation (correctly) does the opposite — idempotency must come first or idempotent replays would always throw ALREADY_MINTED.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([mintAndIssue called]) --> B{Sum of allocations\n== 10,000?}
    B -- No --> ERR1([ALLOCATIONS_SUM_MISMATCH])
    B -- Yes --> C{Each allocation\n>= 1,000?}
    C -- No --> ERR2([ALLOCATION_BELOW_MINIMUM])
    C -- Yes --> D{Existing entry for\nidempotencyKey?}
    D -- Yes --> E{TREASURY exists\nfor mortgageId?}
    E -- No --> ERR3([IDEMPOTENT_REPLAY_FAILED])
    E -- Yes --> F[Collect ALL entries for mortgage\nfilter SHARES_ISSUED]
    F --> G([Return cached result])
    D -- No --> H{TREASURY exists\nfor mortgageId?}
    H -- Yes --> ERR4([ALREADY_MINTED])
    H -- No --> I[initializeWorldAccount]
    I --> J[Insert TREASURY account]
    J --> K[postEntry MORTGAGE_MINTED\nWORLD → TREASURY 10,000]
    K --> L[For each allocation:\ngetOrCreatePositionAccount\npostEntry SHARES_ISSUED\nTREASURY → POSITION]
    L --> M{TREASURY\nbalance == 0?}
    M -- No --> ERR5([INVARIANT_VIOLATION])
    M -- Yes --> N([Return treasuryAccountId\nmintEntry, issueEntries])

    style ERR1 fill:#f66,color:#fff
    style ERR2 fill:#f66,color:#fff
    style ERR3 fill:#f66,color:#fff
    style ERR4 fill:#f66,color:#fff
    style ERR5 fill:#f66,color:#fff
    style G fill:#6a6,color:#fff
    style N fill:#6a6,color:#fff

Loading

Comments Outside Diff (1)

1. convex/ledger/__tests__/mintAndIssue.test.ts, line 269-275 (link)

   Idempotency replay test verifies length but not entry identity

   The test checks that the second call returns the same number of issueEntries, but doesn't assert that they are the same entries (by _id). If the idempotency replay path were accidentally returning newly-fetched or re-created entries, this test would still pass.

   Consider adding identity checks alongside mintEntry:

_{Last reviewed commit: 679eb84}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@convex/ledger/mutations.ts`:
- Around line 246-259: The replay code in mintAndIssue returns all SHARES_ISSUED
entries for the mortgage, which can include unrelated entries; update the
post-query filter to only include entries whose causedBy equals the current mint
entry id (use existingEntry._id) so issueEntries returned are tied to this
specific mint operation (filter the issueEntries array by e.entryType ===
"SHARES_ISSUED" && e.causedBy === existingEntry._id).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b1a41178-5222-43e5-b43b-70de151b8779

📥 Commits

Reviewing files that changed from the base of the PR and between 2bc4fbd and 679eb84.

📒 Files selected for processing (7)

• convex/ledger/__tests__/mintAndIssue.test.ts
• convex/ledger/mutations.ts
• convex/ledger/validators.ts
• specs/ENG-29/chunks/chunk-01-implementation/context.md
• specs/ENG-29/chunks/chunk-01-implementation/tasks.md
• specs/ENG-29/chunks/manifest.md
• specs/ENG-29/tasks.md

[Connorbelez]

Merge activity

• Mar 17, 2:31 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Mar 17, 2:32 AM UTC: Graphite rebased this pull request as part of a merge.
• Mar 17, 2:33 AM UTC: @Connorbelez merged this pull request with Graphite.