Skip to content

docs: add Ledger v3 migration plan - #129

Open
gfyrag wants to merge 2 commits into
mainfrom
codex/docs/ledger-v3-migration-plan
Open

gfyrag wants to merge 2 commits into
mainfrom
codex/docs/ledger-v3-migration-plan

Conversation

@gfyrag

@gfyrag gfyrag commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

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 --check
  • nix develop --command just lint — 0 issues

Open decisions

  • Required idempotency retention and whether the Ledger cluster is dedicated
  • Acceptable scan amplification for arbitrary custom metadata filters
  • Ownership of ledger and index provisioning
  • Handling of incompatible colored funds or typed Wallets metadata

@gfyrag
gfyrag requested a review from a team as a code owner September 16, 2026 12:27
@NumaryBot

Copy link
Copy Markdown
Contributor

🛑 Changes requested — automated review

The 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 NumaryBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [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 NumaryBot added risk: low NumaryBot classified this pull request as low risk. bot-reviewed NumaryBot completed its review workflow for the current head. changes-requested The current head has blocking findings or a human change request. labels Sep 16, 2026

@NumaryBot NumaryBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot-reviewed NumaryBot completed its review workflow for the current head. changes-requested The current head has blocking findings or a human change request. risk: low NumaryBot classified this pull request as low risk.

Development

Successfully merging this pull request may close these issues.

2 participants