refactor: replace raw bigint with Amount VO on Proof and BlindedMessage - #609
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #609 +/- ##
==========================================
+ Coverage 91.49% 91.55% +0.05%
==========================================
Files 49 49
Lines 4493 4513 +20
Branches 1119 1120 +1
==========================================
+ Hits 4111 4132 +21
Misses 166 166
+ Partials 216 215 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50809ea30f
ℹ️ 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".
…igAll package uses regular JSON
…gonomics. Revise migration docs
8f9391c to
eb84d52
Compare
## Description Nothing has ever type checked `test/`. Vitest strips types via esbuild without checking them, and `test/tsconfig.json` existed only to give eslint's typed rules a project covering the test tree, which catches rule violations rather than assignability. `src/` was covered all along, indirectly: `api:check` runs `vite-plugin-dts`, which type checks it. The result was 89 errors across 12 files, accumulated silently as the library types moved. The great majority is one field: `method` became required on `MintQuoteBaseResponse` and `MeltQuoteBaseResponse`, and mint quotes also gained `amount_paid` / `amount_issued` / `updated_at`. ## Changes - `npm run check-types` runs `tsc -p test/tsconfig.json`. The tests import the library source directly rather than the built package, so tsc follows those imports and pulls all 87 files in `src/` into the same program. One command therefore covers both trees, and there is no need for a second project for `src/`. `test/tsconfig.json` gained `rootDir` and `noEmit` so a bare `tsc -p` behaves the same on the command line as it already did in an editor. - `test/consumer/` is excluded. Those harnesses deliberately import the package by its published name to simulate an external consumer, so they only resolve after a build. They are covered by `consumers.yml`, which builds first, and eslint skips them for the same reason. - Wired into `prtasks`, a `types.yml` workflow following the existing `lint.yml` shape, and the pre-push hook. A full pass takes about 3.5 seconds. - Fixtures realigned with the current types. Unpaid quotes get `0/0` accounting, which `validateMintQuoteAvailableAmount` short-circuits on, so those tests take the same runtime path as before. Deliberately malformed fixtures kept their hostile shape behind an explicit cast rather than being "fixed". - Fixed the pre-push condition. It read `if ! npm run check-lint && npm run check-format`, where `!` binds only to the first command. It aborted only when lint failed and format passed: when lint passed it short-circuited and format never ran, and when both failed it did nothing. - `AGENTS-CONTRIBUTING.md` said `Proof.amount` is `bigint` and to construct with `Amount.from(x).toBigInt()`. That was correct when written but has been wrong since #609 moved it to `Amount`, and it is where the payment request fixtures came from. ## Reviewer Notes - No files under `src/` are touched and `etc/cashu-ts.api.md` is unchanged, so there is no public API movement here. - The obvious alternative was shared fixture factories, so that a future field addition would need fixing in one place instead of 47. I left them out on purpose: with the gate in place, the next type change breaks the build immediately, at however many sites exist that day, rather than piling up unnoticed until someone opens the file. Fixing one or two sites as they break is cheaper than maintaining factories, and the factories would not have prevented today's cleanup anyway. - The gate was checked for teeth rather than assumed: planting a `Proof` with a raw `bigint` amount makes `check-types` exit 2 and the hook exit 1, and both return to 0 once removed. - The sweep also surfaced several `src/` defects where a declared type asserts more than the code delivers. Those are deliberately not in this PR; they follow in a separate one stacked on this branch.
Replaces raw bigint with the
Amountvalue object on the two most app-facing types (Proof.amount and SerializedBlindedMessage.amount) so downstream consumers no longer need to manually wrap every amount inAmount.from(). Also introducesProofLikeas inputs in the core wallet flows, so consumers get more flexibility using proofs from storage.Changes
AmountJsontype for round-trip convenience.ProofLikein the send/receive/melt wallet flows for flexible input