Skip to content

feat(core,react): add the unauthenticated invoice payment call - #206

Open
joshuakrueger-dfx wants to merge 5 commits into
DFXswiss:developfrom
joshuakrueger-dfx:feat/payment-link-invoice-payment
Open

feat(core,react): add the unauthenticated invoice payment call#206
joshuakrueger-dfx wants to merge 5 commits into
DFXswiss:developfrom
joshuakrueger-dfx:feat/payment-link-invoice-payment

Conversation

@joshuakrueger-dfx

@joshuakrueger-dfx joshuakrueger-dfx commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

⛔ Do not merge this before #201

#201 has to be merged first, then this one.
This branch returns PaymentLinkPayResponse, which #201 introduces, so merging here first would
land a method whose return type does not exist on develop. Until #201 is in, this diff carries
its two commits as well — that is why it reads +988 lines instead of the ~250 that belong to the
invoice-payment slice. After #201 lands, this branch gets rebased and the diff shrinks to its own
change; the final pass below expires with that rebase and will be redone.

Base is now develop, so pr.yaml can run. Rebased onto current develop again on 19 Aug (it was four commits behind, which is why a local run showed 9 suites where CI — testing the merge — showed 10).

Open items — what this pull request is waiting on

What has to happen Why not from here Who can do it
Merge #201 — before this pull request, not after. It introduces PaymentLinkPayResponse, the return type of the new method; merging this first would put a method on develop whose type is not there. It is also why this review currently is the whole stack. a reviewer of #201
Retarget this at develop. Done so CI can run. Still draft — not marked ready.
CI on Node 16. Ran green on ebf649b (13 Aug) and again on the current head after the rebase — core 10 suites / 120 tests, react 2 / 41, bip322 4 / 46.
Publish @dfx.swiss/react. Release process, not a branch. packages maintainers
Consume it in DFXswiss/app#1274 — swap the hand-built fetch in invoice.screen.tsx for this method. Needs the published version to exist. me, once published

Not symptom-driven: No incident and no error report. This exists because the review on DFXswiss/services#1274 (finding 8) named a rule the consuming repository cannot satisfy at the call site: API access goes through the SDK, and this endpoint has no SDK method.
Scale: GET paymentLink/payment is the call every OpenCryptoPay payer triggers after scanning an invoice QR code — the merchant that prompted #1274 bills more than 100 invoices a month, each one sending a payer through it. Today its verb, query shape and response type live in invoice.screen.tsx, where they can drift from the API without anything failing at build time.
Smaller fix considered: Leaving the hand-built fetch in place and only typing its response inside DFXswiss/services — no change in this repository at all. Insufficient because it puts endpoint knowledge in the consumer, which is the specific thing the guideline forbids, and it would have to be repeated in every further consumer; the whole point of #198 is that the contract lives once, here.

Why

GET paymentLink/payment is the call a payer makes after scanning an invoice QR code. It is the one payment-link endpoint with no SDK method, so its consumer builds the URL by hand and fires it with a raw fetchDFXswiss/services, src/screens/invoice.screen.tsx:36,127. Verb, query shape and response type therefore live in the consuming repository, where they go stale without anything failing at build time.

This came out of the review on DFXswiss/app#1274 (finding 8). That pull request cannot fix it at the call site without moving endpoint knowledge into the wrong repository, so the fix is here.

What is already there

Little of this is new:

  • PaymentLinksUrl.payment already exists and is already used — for POST (createPayment) and DELETE (cancelPayment). Only the GET on that same URL was missing, so no new constant.
  • The response needs no new modelling. The endpoint returns the quoted pay request or the terminal error payload, which is exactly the PaymentLinkPayResponse union feat(core,react): add the payment link pay flow contract #201 introduces. Field-by-field against the API (payment-link.controller.ts:215, dto/payment-link.dto.ts:55,69, shared/dto/error.dto.ts): PaymentLinkRequestBase matches PaymentLinkRequestDto, PaymentLinkPayRequest matches PaymentLinkPayRequestDto, and PaymentLinkPayTerminal matches PaymentLinkPaymentErrorResponseDto.
  • The endpoint carries no @UseGuards, so it is called with token: false — the pattern getStandards in feat(core,react): add the payment link pay flow contract #201 already uses.

Shape of the query

Long-form parameters only. The API also accepts short forms (r, e, m, l, n, a, c, d, s, w), but they are a url-length device for printed codes and the handler maps them onto the long forms before anything else runs. Putting both in a typed contract would offer two names for one field and no caller would gain from it.

Which fields are required was read off the validators rather than guessed: a route identity (routeId or route) and a payment identity (externalId or message) are each required through mutually alternative @ValidateIf chains, amount is required outright, and the rest carry @IsOptional().

Of those three rules, the type carries only one. amount is required in PaymentLinkInvoicePaymentQuery; routeId, route, externalId and message are all optional there, so getInvoicePayment({ amount: '5' }) compiles and is rejected by the API at runtime. The two either-or rules are expressible — a union of the four admissible shapes would do it — so this is a choice rather than a limit of the language: four overlapping shapes read badly at the call site and in the editor's hover, and the rules are documented on the interface instead. Worth knowing when reading the section above, which describes what the endpoint enforces, not what the compiler does.

Placement

Both layers, because that is what this repository already does: getRecipient lives on the core client and getPaymentRecipient in the react hook, independently — the hook does not use the core client anywhere. So the query type, the client method and the tests go into @dfx.swiss/core, and the thin hook method sits next to its neighbour in @dfx.swiss/react. Purely additive; no existing signature or behaviour changes.

Verification

All four commands from CONTRIBUTING.md, run with the repository's own binaries on Node 20 (Studio). CI pins Node 16.

lerna run lint exit 0 (core / react clean; nine pre-existing warnings in bip322-multisig only)
lerna run format:check exit 0, all four packages clean
lerna run build exit 0, all four packages
lerna run test exit 0 — core 10 suites / 120 tests, react 2 / 41, bip322 4 / 46

Coverage on the touched runtime files, collected on that same run: PaymentLinksApi.ts, definitions/route.ts and payment-routes.hook.ts are each 100 % statements / branches / functions / lines.

Mutations, each asserted to hit exactly one occurrence, each with the diff checked and the working tree proven clean afterwards:

Mutation Result
Core / hook token: false removed 3 of 5 getInvoicePayment tests fail
Core / hook GET becomes POST 3 of 5 fail
getRecipient URL becomes history 1 of 1 fails
amount loosened from required to optional 1 suite red (see below)

That last row used to read stays green, and it was the honest admission that the only rule the type carries was held by review rather than by a test. It is closed now, without a new dependency and without touching the configuration: packages/core runs Jest through ts-jest, which type-checks, so src/__tests__/invoice-payment-query-type.test.ts puts three calls that must not compile under @ts-expect-error. Relax the rule and the expected error disappears, the directive becomes unused, and TypeScript reports TS2578 — the suite fails.

Measured on head e40964af, Node 20, each mutation asserted to hit exactly one occurrence and the file verified restored afterwards:

Mutation of the type Result
amount: stringamount?: string 1 suite red, TS2578 — 10 passed / 120 tests becomes 1 failed + 9 passed, 119 tests
amount: stringamount: string | number 1 suite red, TS2578
index signature [key: string]: unknown added 2 suites red, TS2578 (the second is collateral — the signature also breaks an unrelated suite, so this is the weakest of the three)

Note the shape of that failure: a type error kills the suite, so its tests are not counted at all. Tests: 101 passed next to Test Suites: 1 failed is the red state, not a partial one — reading only the test line would show 101 green and hide it.

The two either-or rules (routeId | route, externalId | message) are still carried by the doc comment alone; expressing them means a union of four admissible shapes, which is a readability trade-off rather than an oversight.

Not verified

The four commands above and the type mutations were run on Node 20; CI pins Node 16. TypeScript comes from the repository either way, so the directives should behave the same — but the Node 16 run is the one that counts, and it is the check on this head.

An earlier version of this section said the first retarget started no workflow because pull_request does not run on edited. That reason was wrong: edited is in the workflow's trigger list. It did not run because the base at that moment was feat/payment-link-pay-contract, and pr.yaml filters on main and develop.

Nothing holds the route-identity and payment-identity rules but the doc comment on the interface — not the compiler and not a test (see Shape of the query). amount is now held by the compiler through the type test above.

No live call against the API; the tests use a mocked HTTP client. No consumption in DFXswiss/services yet — that follows the release, as the rule requires. The react hook is exercised through a Jest harness that stubs useCallback / useMemo as identity; it does not render with Testing Library.

The two approvals on this pull request were given on 6a5abfc6. Since then came the test commit, the type test and two rebases — at the current head this is formally unreviewed, and a re-review is due before it goes anywhere near a merge.


Final pass (e40964a):
Coherent: Every file serves the one title — the query type, the client method, the thin react method beside its neighbour, their tests, and the type test that holds the one rule the type carries. Nothing here belongs to another topic; what looks larger than that is #201, whose two commits this branch still carries because it is not merged.
Nothing extra: No new dependency, no configuration change, no new URL constant (PaymentLinksUrl.payment already existed for POST and DELETE), and the short-form query parameters (r, e, m, …) are deliberately left out — they are a url-length device for printed codes and the handler maps them onto the long forms before anything else runs. Deliberately not built: the two either-or rules as a union of four admissible shapes (readability trade-off, documented under Shape of the query), and any consumption in DFXswiss/services — that follows the release and gets its own pull request, as #1239 to #1243 already do.
Sources closed: DFXswiss/services#1274, review finding 8 — the SDK method exists here; the swap at the call site follows the publish and is the last row of the table above. #198 (umbrella), step 3 — this is the invoice-payment slice on top of #201. Commits — each says what its diff does. Reviews — marassteiner approved twice, both on 6a5abfc6, so those approvals do not cover this head; no other review exists. Open#201 must land first, then the release, then the consumer pull request; and this pass expires with the next commit.

@joshuakrueger-dfx

Copy link
Copy Markdown
Contributor Author

@marassteiner please review

This is a PR, not an issue: 8 files against its base branch, head 6a5abfc. Note: still marked Draft, no CI configured on it yet.

@marassteiner

Copy link
Copy Markdown
Collaborator

Sorry — I've reached my hourly job limit and can't take this on right now. I'll pick it up automatically once the limit resets.

@marassteiner marassteiner left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review — approve

Recommendation: approve. No merge-blockers for the change set itself. PR remains correctly draft and stacked on #201 until that lands and this is retargeted to develop.

Gate A — AI diff review (two independent lanes)

Lane A — Konformität/Qualität

  • Source-only diff (8 files, +146): core client method, query type, exports, tests, thin React hook re-export — matches CONTRIBUTING (no version/changelog/lockfile bumps; conventional feat(core,react) commit; tests for core).
  • Dual-layer placement matches existing getRecipient / neighbour patterns (hook does not go through the core client).
  • Hygiene clean (no console.log / TODOs / debug).
  • Stacking on #201 instead of develop is documented and intentional for a stacked PR; not a defect in the code.

Lane B — Logik/Korrektheit

  • GET + PaymentLinksUrl.payment + token: false is correct for the unauthenticated invoice pay endpoint.
  • Utils.buildQuery encoding (spaces, /, Date, enum) covered by tests; optional omission covered; both PaymentLinkPayResponse branches (quoted + terminal) covered.
  • Core and React request shapes match for the happy path.

Gate B — Local software run

On this review host (Node 24.19.0; CI pins Node 16):

Command Result
npx lerna run lint exit 0 (8 pre-existing warnings in bip322-multisig only; core/react clean)
npx lerna run format:check exit 0
npx lerna run build exit 0 (all 4 packages)
npx lerna run test exit 0 — core 7 suites / 82 tests, bip322 4 / 46

DONE review PR 206 LOCAL_RUN_OK

Note: no CI checks appear on this PR because pr.yaml only triggers against main/develop; that is expected for the stacked base, as the PR body states. Local gates green on Node 24; Node-16 parity remains for when this is retargeted.

Follow-up findings (not merge-blockers)

  1. Query type does not encode either-or identity rulesPaymentLinkInvoicePaymentQuery requires only amount; routeId|route and externalId|message stay optional at the type level though the API validators require them. Documented intentional trade-off in the PR body; runtime API still rejects incomplete queries.
    #221

  2. React useApi 401 retry can attach a session token after token: false — first request is correctly unauthenticated; on 401 with an existing session, api.hook.ts retries with getAuthToken(). Pre-existing useApi behaviour; this PR is the first usePaymentRoutes caller with token: false. Fix belongs in api.hook.ts, not in this method.
    #222

Open process items (from PR body — not code defects)

  • Merge #201 first, then retarget this PR to develop and mark ready so CI runs.
  • Publish @dfx.swiss/react / consume in DFXswiss/app#1274 after release.

No merge from this review.

@joshuakrueger-dfx

Copy link
Copy Markdown
Contributor Author

@marassteiner please review

This is a PR, not an issue: 8 files against feat/payment-link-pay-contract (stacked on #201), head 6a5abfc6, 0/0 checks (workflow only runs on main/develop).

@marassteiner

Copy link
Copy Markdown
Collaborator

Sorry — I've reached my hourly job limit and can't take this on right now. I'll pick it up automatically once the limit resets.

@marassteiner

Copy link
Copy Markdown
Collaborator

EN: Working on this now — job dfxswiss__packages__206__pr-review.
DE: Ich arbeite jetzt daran — Auftrag dfxswiss__packages__206__pr-review.

@marassteiner marassteiner left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review — approve

Recommendation: approve. No merge-blockers in this change set. The PR correctly remains draft and stacked on #201 until that lands and this is retargeted to develop.

Gate A — AI diff review (two independent lanes)

Lane A — Konformität/Qualität

  • 0 findings. Full pass confirmed.
  • Source-only (+146, 8 files): query type, core client method, public exports, core tests, thin React hook — matches CONTRIBUTING (no version/CHANGELOG/lockfile bumps; conventional feat(core,react) commit; tests extended in @dfx.swiss/core).
  • End-to-end wiring: PaymentLinkInvoicePaymentQueryPaymentLinksApi.getInvoicePayment → core/react exports → usePaymentRoutes.getInvoicePayment (interface + useCallback + useMemo deps).
  • Reuses PaymentLinkPayResponse / hasPaymentQuote from #201; long-form query only; pattern matches existing public endpoints (token: false, Utils.buildQuery).

Lane B — Logik/Korrektheit

  • 1 non-blocking finding (already tracked).
  • Correct: GET + PaymentLinksUrl.payment + token: false on both core and react; Utils.buildQuery encoding (ISO dates, omitted optionals, enum string) covered by tests; both quoted and terminal response branches asserted; no semantic merge risk (pure additive).
Severity Finding Evidence Disposition
medium (follow-up) PaymentLinkInvoicePaymentQuery documents either-or route/payment identity, but only amount is type-required — { amount: '5' } typechecks and fails only at the API packages/core/src/definitions/route.ts:382-396 Intentional per PR body; same looseness pattern as PaymentLinkPaymentQuery. Tracked in #221

Pre-existing / out-of-diff note (not re-raised as a new defect): React useApi 401-retry can re-attach a session token when the original call set token: false (packages/react/src/hooks/api.hook.ts:89-107). #206 is the first token: false consumer on usePaymentRoutes. Primary fix is in api.hook.ts, not this method. Tracked in #222.

Gate B — Local software run

Repository is a Lerna monorepo (SDK packages), not a long-running app. Documented path from CONTRIBUTING.md was executed on this checkout (6a5abfc):

Command Result
npm ci exit 0
npx lerna run lint exit 0 (8 pre-existing warnings in bip322-multisig only; core/react clean)
npx lerna run format:check exit 0 (all four packages)
npx lerna run build exit 0 (all four packages)
npx lerna run test exit 0 — core 7 suites / 82 tests, bip322-multisig 4 / 46

Host Node: v24.19.0 (CI pins Node 16; PR body already notes that CI does not run on stacked base branches — pr.yaml only targets main/develop). This is environmental for the missing GH checks, not a defect in the diff.

Follow-up issues (linked)

  • #221 — tighten either-or identity rules on PaymentLinkInvoicePaymentQuery
  • #222useApi must not attach session token on 401 retry when token: false

Merge checklist (outside this review)

  • Merge #201 first (base + PaymentLinkPayResponse)
  • Retarget this PR to develop and mark ready so CI runs
  • Publish @dfx.swiss/react / @dfx.swiss/core, then consume in DFXswiss/app#1274

No merge from this review.

@joshuakrueger-dfx
joshuakrueger-dfx changed the base branch from feat/payment-link-pay-contract to develop August 13, 2026 08:46
@joshuakrueger-dfx
joshuakrueger-dfx force-pushed the feat/payment-link-invoice-payment branch from 34839fd to ebf649b Compare August 13, 2026 08:50
TaprootFreak and others added 5 commits August 19, 2026 14:33
`GET paymentLink/payment` is the call a payer makes after scanning an invoice
QR code. It was the one payment-link endpoint with no SDK method, so its
consumer built the URL by hand and fired it with a raw fetch — verb, query
shape and response type living in the consuming repository, where they go
stale without anything failing at build time.

The URL constant was already here and already used for POST and DELETE; only
the GET was missing. The response type needs no new modelling either: the
endpoint returns the quoted pay request or the terminal error payload, which
is exactly the PaymentLinkPayResponse union from the pay-flow contract this
branch builds on.

The query carries the long-form parameters only. The short forms the API also
accepts (r, e, m, l, n, a, c, d, s, w) are a url-length device for printed
codes and the handler maps them onto the long forms before anything else, so
putting them in a typed contract would offer two names for one field with no
gain.

Tests cover the built query, omitted optionals staying out of it entirely, the
verb and the suppressed session token, and both branches of the response.
The type says amount is required and everything else optional, and until
now nothing held that: the mutation from required to optional stayed
green because Jest does not see types. ts-jest does type-check, so three
calls now sit under @ts-expect-error. Relaxing amount to optional, widening
it to string | number, or adding an index signature each makes a directive
unused (TS2578) and fails the suite - measured 1, 1 and 2 suites red.
@joshuakrueger-dfx
joshuakrueger-dfx force-pushed the feat/payment-link-invoice-payment branch from f6be1b0 to e40964a Compare August 19, 2026 12:34
@joshuakrueger-dfx

Copy link
Copy Markdown
Contributor Author

EN:
The one rule this type carried is now held by the compiler instead of by review: three calls that must not compile sit under @ts-expect-error in packages/core/src/__tests__/invoice-payment-query-type.test.ts, and ts-jest type-checks, so relaxing amount makes the directive unused (TS2578) and fails the suite. Also rebased onto current develop — the branch was four commits behind, which is why a local run showed 9 suites where CI, testing the merge, showed 10.

DE:
Die einzige Regel, die dieser Typ trägt, hält jetzt der Compiler statt der Review: drei Aufrufe, die nicht kompilieren dürfen, stehen unter @ts-expect-error in packages/core/src/__tests__/invoice-payment-query-type.test.ts, und ts-jest prüft Typen — wird amount gelockert, ist die Direktive ungenutzt (TS2578) und die Suite fällt. Ausserdem auf aktuelles develop rebased; der Branch lag vier Commits zurück, daher zeigte ein lokaler Lauf 9 Suites, während die CI auf dem Merge 10 sah.

Details

Why this and not tsd / expect-type

No new dependency, no configuration change: packages/core/jest.config.js already runs ts-jest with tsconfig.test.json, and ts-jest reports type errors as a failing suite. Verified before writing the test — a throwaway spec calling getInvoicePayment({ routeId: '1' }) failed with TS2345 ... Property 'amount' is missing.

Counter-proof on head e40964af (Node 20)

Each mutation asserted to hit exactly one occurrence in route.ts, the file verified restored afterwards:

Mutation Result
amount: stringamount?: string Test Suites: 1 failed, 9 passed / Tests: 119 (from 10 passed / 120), TS2578
amount: stringamount: string | number 1 suite red, TS2578
[key: string]: unknown added to the interface 2 suites red, TS2578 — the second is collateral damage, so this is the weakest of the three

Worth noting how that failure reads: a type error kills the whole suite, so its tests are not counted. Tests: 119 passed beside Test Suites: 1 failed is the red state — looking only at the test line would show 119 green and hide it.

Gates on this head

lerna run lint, format:check, build and test all exit 0 locally on Node 20 — core 10 suites / 120 tests, react 2 / 41, bip322 4 / 46 (1 todo). CI on Node 16 is green on this head with the same numbers, and the new suite is in its PASS list.

What is still open

Unchanged: this waits on #201, then the publish, then the consumer pull request in DFXswiss/services. The two either-or rules (routeId | route, externalId | message) remain carried by the doc comment — expressing them means a union of four admissible shapes, which is a readability trade-off rather than an oversight. Say the word and it can be added the same way.

@joshuakrueger-dfx

Copy link
Copy Markdown
Contributor Author

EN:
Ready after 1 review pass on head e40964af — two independent lanes over the full diff, five findings, all rejected with evidence rather than implemented.
Merge order: #201 first, then this — this branch returns PaymentLinkPayResponse, which #201 introduces.

DE:
Bereit nach 1 Review-Durchlauf auf Head e40964af — zwei unabhängige Lanes über den vollständigen Diff, fünf Funde, alle mit Beleg abgelehnt statt umgesetzt.
Merge-Reihenfolge: erst #201, dann dieser — der Branch gibt PaymentLinkPayResponse zurück, das #201 einführt.

Details

The five findings and why none of them changed the code

1 — as any in the two new test files (low, conformity lane). Rejected on repository precedent, counted rather than assumed: any or as any appears in seven of the existing test files here — http-client.test.ts has 11 occurrences, transaction-api.test.ts (which landed on develop last week) 1, and auth-api, bank-api, buy-api, validations one each. There is no no-explicit-any rule configured, and lerna run lint is clean. The two files in question carry 6 and 5. That is the house style for fixtures in this repository, not a departure from it. It could still be tightened — worth doing repo-wide, not in a feature branch.

2 — PaymentLinkPaymentQuery accepts {} (medium, logic lane). Real, but it is #201's type, not this slice — waitForPayment and confirmPayment come from that pull request. Same class as finding 4 below; it belongs to whoever reviews #201.

3 — frontendUrl is a new required field on the public PaymentLink (medium, logic lane). Also #201, and it is a known, deliberate decision: the author wrote it out in the pull request himself — "Note for downstream consumers: PaymentLink gained a required frontendUrl. Reading code is unaffected; code that builds a PaymentLink literal (fixtures, mocks) needs the field." Flagging it here would re-open a call that has already been made and documented.

4 — PaymentLinkInvoicePaymentQuery allows { amount: '5' } (high, logic lane). The most substantial of the five, and the one that belongs to this slice. The API requires a route identity (routeId or route) and a payment identity (externalId or message) through mutually alternative @ValidateIf chains; the type carries neither, so a call missing both compiles and is rejected at runtime. It is expressible — two identity unions intersected with the base — and the type test added here would extend to it naturally. It is not an oversight: the trade-off is written out under Shape of the query in the description, because four admissible shapes read badly at the call site and in editor hovers, and it changes the public signature of a published package. Left as documented, pending an explicit decision. Say so and it goes in the same way amount did.

5 — see 1.

What did change since the approvals

Both approvals sit on 6a5abfc6. Since then: the test commit (13 Aug), the type test that puts three non-compiling calls under @ts-expect-error, and two rebases onto develop. The head is therefore formally unreviewed by a human, which is part of why this is going to review now rather than to merge.

Gates on e40964af

lerna run lint, format:check, build and test all exit 0 locally on Node 20 — core 10 suites / 120 tests, react 2 / 41, bip322 4 / 46. CI on Node 16 is green on this head with the same numbers. mergeable: MERGEABLE, all five commits signed and verified, no unresolved review threads.

@joshuakrueger-dfx
joshuakrueger-dfx marked this pull request as ready for review August 19, 2026 13:14
@joshuakrueger-dfx

Copy link
Copy Markdown
Contributor Author

@marassteiner please review

This is a PR, not an issue: 10 files against develop, head e40964af, 1/1 check green (Lint, build and test on Node 16 — core 10 suites / 120 tests, react 2 / 41, bip322 4 / 46).

Your two approvals sit on 6a5abfc6 and do not cover this head. Since then: the test commit of 13 Aug, a type test that puts three non-compiling calls under @ts-expect-error so the amount rule is held by the compiler rather than by review, and two rebases onto current develop.

Please note the merge order: #201 has to land before this one — this branch returns PaymentLinkPayResponse, which #201 introduces, and until then this diff carries its two commits as well.

One point is deliberately left open and flagged in the description rather than fixed: PaymentLinkInvoicePaymentQuery compiles a call that carries neither a route identity (routeId/route) nor a payment identity (externalId/message), which the API rejects at runtime. Expressing it means intersecting two identity unions and changing a published signature — your call whether that belongs in this PR.

@marassteiner

Copy link
Copy Markdown
Collaborator

EN: Working on this now — job dfxswiss__packages__206__pr-review__r2.
DE: Ich arbeite jetzt daran — Auftrag dfxswiss__packages__206__pr-review__r2.

@marassteiner

Copy link
Copy Markdown
Collaborator

EN: The required independent review of head e40964af could not be completed because the review toolchain hit a usage limit; it can be retried after 20 Aug 2026, 05:33 UTC, and this pass does not approve or request changes.
DE: Der vorgeschriebene unabhängige Review von Head e40964af konnte wegen eines Nutzungslimits der Review-Toolchain nicht abgeschlossen werden; Wiederholung möglich ab 20. Aug 2026, 05:33 UTC, und dieser Durchlauf enthält weder Approve noch Request-changes.

Details

Head under review: e40964af. The two existing approvals on this pull request cover 6a5abfc6 only and do not apply to this head.

The mandatory two-lane review did not run: both lanes aborted on a usage limit before reading the diff. No substitute review was used.

Local software run on this checkout (Node v24.19.0; CI pins Node 16) completed with exit 0:

Command Result
npm install exit 0
npx lerna run lint exit 0 (8 pre-existing warnings in bip322-multisig only; core/react clean)
npx lerna run format:check exit 0 (all four packages)
npx lerna run build exit 0 (all four packages)
npx lerna run test exit 0 — core 10 suites / 120 tests, react 2 / 41, bip322 4 / 46 (1 todo)

CI on this head is already green (Lint, build and test).

This is not a code verdict. Merge order remains as stated in the pull request body: #201 has to land first.

@joshuakrueger-dfx

Copy link
Copy Markdown
Contributor Author

@marassteiner please review

This is a PR, not an issue: 10 files against develop, head e40964a, 1/1 checks green.

Your last completed review was on 6a5abfc; the __r2 pass on e40964af stopped at a usage limit on 2026-08-19.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants