Repository navigation
Conversation
🛑 Changes requested — automated reviewThe migration plan incorrectly describes a universal idempotency conflict guarantee as existing behavior. This makes its parity requirements inconsistent with the current balance-creation implementation. |
NumaryBot
left a comment
There was a problem hiding this comment.
NumaryBot posted 1 new inline finding.
Summary: #129 (comment)
| - deterministic wallet IDs for idempotent wallet creation; | ||
| - deterministic hold IDs for idempotent pending debits; | ||
| - stored immutable create-wallet responses and request fingerprints; | ||
| - stored per-key create-balance responses; |
There was a problem hiding this comment.
🟠 [major] Distinguish proposed idempotency guarantees from current behavior
For balance creation, reusing a key with the same wallet and balance name but a different priority or expiration currently returns the stored response: Manager.CreateBalance checks balanceIdempotencyMarker(ik) without comparing the request. Consequently, conflict-on-different-request is not an existing application-level protection for every operation, and the blanket Phase 5 exit criterion would change public behavior rather than preserve parity. Document the operation-specific baseline and explicitly identify any stronger conflict guarantee as a proposed behavior change.
NumaryBot
left a comment
There was a problem hiding this comment.
OpenCodeReview's independent Codex pass added 1 comment-only finding.
These comments do not affect the active verdict or commit status.
| } | ||
| ``` | ||
|
|
||
| Serialize canonical JSON with URL-safe Base64. Validate every field and reject: |
There was a problem hiding this comment.
OpenCodeReview · independent Codex pass · comment-only
🟠 [major] Preserve cursor-only continuation of filtered lists
At HEAD, mapAccountList and ListTransactions pass only the cursor on subsequent pages; clients need not resend filters. The proposed cursor stores only a filter hash, which cannot reconstruct those filters. Following a filtered request with ?cursor=... would therefore fail validation or lose its filtering. Store the original query state in the cursor (or durable server-side state), preserve cursor-only continuation, and add a filtered pagination compatibility test.
Summary
Wallets depends directly on Ledger v2 models, filters, cursors, and runtime behavior. This PR adds the engineering plan for moving Wallets to a Ledger-v3-only architecture while preserving the Wallets v1 API and current business behavior.
The plan covers the adapter boundary, Ledger v3 reads and writes, metadata filtering, cursor compatibility, idempotency, Numscript parity, required indexes, E2E coverage, provisioning on a new empty Ledger v3 ledger, smoke tests, observability, and follow-up Ledger v3 opportunities.
Migration of existing Ledger v2 data is explicitly out of scope. The plan contains no mirror, import, comparative data validator, cutover, or v2 rollback workflow.
It also bootstraps the repository-level
docs/index required by RFC 0004. The document is AI-assisted and requires maintainer review before merge.Validation
git diff --checknix develop --command just lint— 0 issuesOpen decisions