Release: develop -> main - #1189
Merged
Merged
Conversation
* List RealUnit on the payment-link page (ZCHF on Ethereum) RealUnit settles OpenCryptoPay payments on-chain in ZCHF on Ethereum (it consumes the LNURL only as the payment-request identifier), so it is not a Lightning wallet. Two changes let a correctly-classified non-Lightning wallet work on /pl: - getDeeplinkByWalletId: a RealUnit case that builds the payload-bearing deeplink (<deepLink>lightning:<LNURL>) from the backend deepLink prefix and the LNURL — the generic path only injects the payload for Lightning wallets. - hasActionDeepLink: treat RealUnit as having an action deeplink so the UI shows 'Pay in app' rather than 'Open app and scan QR code again'. Wallet.qualifiesForPayment already matches a ZCHF/Ethereum wallet via supportedAsset.name === 'ZCHF'; tests added for that plus the deeplink/label. * Make the RealUnit deeplink config-driven instead of a name check Drop the hardcoded case 'RealUnit' and the name === 'RealUnit' label check. Instead honor the backend wallet_app.hasActionDeepLink flag (additive to the existing Lightning check) as the generic signal that a wallet takes the OpenCryptoPay payment via its <deepLink>lightning:<LNURL> action deeplink. Behaviour is unchanged for every existing wallet (Lightning wallets still append via the method check; bare-open wallets like Cake Wallet/urble stay bare); any future non-Lightning OCP wallet just sets hasActionDeepLink=true in its wallet_app row, no frontend code. Companion: DFXswiss/api#4352 sets the flag for RealUnit. * Cover the Lightning hasActionDeepLink path with a regression test Adds a Lightning-only wallet fixture (supportedMethods ['Lightning'], no backend flag) asserting it gets hasActionDeepLink=true and the <deepLink>lightning:<LNURL> append via the Lightning half of the OR alone — so a future regression that drops that half would be caught.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 24, 2026 08:26
* test(safe): document the Safe screen with baseline screenshots The Safe screen had no spec and no baselines at all. The only coverage was a smoke test in payment-and-special.spec.ts asserting that the body is visible, so a UI change to this feature produced no visual diff for a reviewer to look at. Adds e2e/safe.spec.ts covering eight distinct states: the loaded portfolio, the two currency switches, deposit via crypto, withdraw via fiat and via crypto, the swap interface and the PDF statement modal. Following CONTRIBUTING.md these run locally only and are not wired into CI. Two states are deliberately not captured separately, because they are already contained in the shots: "Deposit / Fiat" is the initial state of the transaction interface, and the activity list sits below the interface and is visible in every shot. Capturing them again produced byte-identical images. The spec raises the viewport height. The screen scrolls inside an overflow-auto container, so fullPage still only yields a 1280x720 image and the deposit, withdraw and swap forms would be missing from their own screenshots. The screenshot tolerance is deliberately small. A large maxDiffPixels budget makes --update-snapshots silently keep an outdated baseline when the content changed, which turns the documentation stale without anyone noticing. * test(safe): pin the clock so the PDF baseline cannot rot The PDF modal prefills the current date via new Date() in safe.screen.tsx, so the committed screenshot carried a moving value. Later runs would either fail on the small pixel budget or, worse, stay silently stale when the changed glyphs happen to fit inside it. The clock is now fixed before navigation and the baselines are regenerated from that state.
* feat(safe): open the Safe without a preselected transaction area The Safe screen started with Deposit selected, so the deposit form with its Fiat/Crypto switch, amount field and NEXT button appeared before the user had asked for anything. Deposit, Withdraw and Swap now render as plain text with nothing selected, and nothing below them until one is chosen. While nothing is selected all three read equally - the existing dimmed style for unselected entries would suggest they were disabled. * test(safe): regenerate the affected baselines and pin the empty default state Baselines 01 to 03 documented the Safe with the deposit form open, which is no longer the default. They are regenerated from the same synthetic dataset, so the only difference is the now empty transaction area; 04 to 08 are untouched because they click into a mode first. Step 1 now also asserts that neither the Type row nor the Next button is present before a mode is chosen - otherwise a regression could put a form back in front of a user who asked for none, and only the image would notice. * test(safe): give Deposit/Fiat its own baseline Deposit/Fiat used to be the preselected state and was therefore documented by the portfolio shots. Those now show no form at all, so that interface would have been the only one without a baseline - a regression in it could pass review unseen. It gets its own shot right after the portfolio states, where a user meets it: the form that opens on the first click. The header comment claimed 01-03 cover Deposit/Fiat, which is no longer true; it now says what those shots actually document.
…1194) * feat(safe): let a user switch between the Safe accounts they may see The Safe screen only ever showed the signed-in user's own holdings. Someone granted access to another person's Safe could not reach it: the data is there and the API serves it, but no screen offered a way to ask for it. An account picker now appears whenever more than one account is reachable, and the whole screen — chart, holdings, activity, statement — follows the chosen one. With a single account nothing changes, which is what almost everyone has. On an account held by inspection only, the deposit, withdraw and swap section is not rendered at all. The API refuses those anyway; offering buttons that fail on click would be worse than not offering them. * fix(safe): repair the legacy Safe, and tie each load to the account it was for Three defects, the first of which broke the screen for nearly everyone. The legacy Safe appears in the account list with a null id — it has no account row and is read through the plain endpoints. Treating that entry like any other requested `custody/account/null/…`, which the API rejects, so the Safe showed nothing but "Something went wrong" for every user still on legacy. Today that is almost all of them. The DTO now says `id: number | null` and one helper decides the path, so the case cannot be overlooked again. Switching accounts while a request was in flight let the late answer win: one account's holdings could be painted under another account's name. Each load is now tied to the account it was started for and discards its result if the selection moved on. The selection is held as a stable key rather than the account object. An object in a dependency list is a new reference on every render, which turned the loads into an endless loop — visible as the same three requests repeating without end. Also: the statement download refuses while the account list is still loading rather than quietly falling back to the caller's own Safe, and the transaction area stays hidden until the access level is known instead of appearing for a moment and vanishing again. * test(safe): cover the account switcher with visual baselines Three states that only exist once a user can reach more than one Safe: the owned account with its transaction area, the picker listing both reachable accounts, and a shared account held by inspection only. The last one carries the assertion this whole feature rests on — deposit, withdraw and swap are absent, not merely disabled. A screenshot alone would not prove that, so the test insists on it before taking the picture. The single-account and legacy cases stay with safe.spec.ts and are not repeated: the switcher is not rendered at all there, and those baselines are unaffected by this change. The dataset behind these images is invented — this repository is public. * fix(safe): stop the empty picker wrapper from shifting every layout The wrapper around the account picker rendered as soon as an account was selected, which is every user — the picker itself then returned null when there was only one account, but the wrapper stayed. As a flex child of a stack with a gap it pushed everything below it down by 40px, for exactly those users the change was supposed to leave alone. The claim that existing baselines were unaffected was therefore wrong. The wrapper now applies the same condition the picker applies internally. Measured against the committed baselines: the differing pixels drop from 22750 to 3434, and what remains is numbers from a local dataset, with every element back at its original position. The manual order reload had no protection against a late answer either. It is the path taken after a completed order, so closing that screen and switching accounts before the answer arrives would have painted one account's activity under another's name — the same defect the effects were already guarded against. Both paths now share one loader that accepts a result only while the account it was fetched for is still selected. * fix(safe): accept only the newest answer, and refuse to act without an account Guarding a result by "is this account still selected" left a gap: switching away and back leaves the same selection in place, so a slow first answer was still accepted and overwrote the fresher one that had arrived meanwhile. Two clicks were enough to see stale activity under the right account name. Each kind of load now carries a counter and only the newest request may commit its result — the account list included, which had no guard at all. An empty account list resolved to no account, and no account was treated as permission to act. It now refuses: acting requires an account that actually grants it. The legacy Safe is recognised by its own flag rather than by a null id. The field was in the DTO but unused, and inferring the case from a missing id meant the distinction lived in two places instead of one. * refactor(safe): make the load kinds a type, and let effects invalidate their request The kind of load was an unconstrained string dispatched through a chain of ifs. A typo at a call site, or a fourth kind added later, would have compiled fine and then matched no branch — leaving that spinner running forever with nothing to flag it. The kinds are a union type now and each has its setter in one place, so adding one forces a decision instead of failing silently. Starting a request now returns a way to invalidate it, and the effects use it on cleanup. Leaving the screen mid-request no longer applies the answer to a component that is gone. The account list effect had this already; the other three did not. Also adds isAccountsLoaded to the dependency list it was read from but missing in — harmless today, since it is set in the same batch as the account list, but the coupling was implicit and nothing here enforces it. * style(safe): drop a rule-less eslint-disable by removing what it suppressed The comment named no rule, so it silenced everything on the next line, and it was the only one of its kind here — every other disable in this repository names its rule. Naming it was not an option either: the rule it meant is not registered in the config that npm run lint uses, so mentioning it turns the lint run red. Moving the setter table inside the callback removes the reason for the comment altogether. The table is rebuilt per call, which costs nothing and keeps the callback genuinely dependency-free. * fix(safe): catch leaving the screen on the manual reload path too The invalidation returned when a request starts only helps callers that can register it as cleanup — effects. The reload after a completed order is not an effect: it runs from a close handler, and its invalidation had nowhere to go. Leaving the Safe right after closing that screen therefore still applied the answer to a hook whose screen was gone. A mount flag closes it for every path at once, manual or not. * fix(safe): set the mount flag on the way in, not only clear it on the way out The flag only ever went to false. An effect that is torn down and run again on the same instance — which StrictMode does on every mount — would have left it false for good, and with it every load discarding its answer: three spinners turning forever on a screen that is very much still there. Not reachable today, since nothing here wraps the tree in StrictMode. It would have surfaced the moment someone turned it on, which is what React recommends. * fix(safe): only offer to act on one's own Safe, not on a mandate over someone else's A write grant on another person's account showed the full transaction area, but the order endpoints carry no account: the order would have been booked against the caller's own holdings while the screen named someone else's. That is a silent misdirection of money, and it is reachable — grants may hand out write, and acting on someone else's behalf does not exist in the backend (initiatedBy is never set anywhere). Acting therefore requires one's own Safe with full disposal. Verified against a running instance: with a write grant on a foreign account the transaction area is absent, while the caller's own account still offers it. The default selection had the same confusion: it picked the first account with write rather than the caller's own. An owner who had limited themselves and held a mandate elsewhere would have landed on the foreign account unprompted. Ownership is now what it says — the account list marks foreign accounts with their owner and leaves the field off one's own.
…g one (#1191) * feat(support): let customers type the receiver IBAN instead of picking one When a customer reports that a transfer never arrived, the form asks which account it went to. That field was a required dropdown filled from GET /bank, which lists only the collective accounts. A customer who deposits through a personal IBAN can never find their actual receiver in that list, and because the field is required they had to pick an account they never transferred to. The wrong IBAN then sits in the ticket and reads like a statement from the customer - in exactly the situation where someone needs help. There are more than three thousand personal IBANs in use, so this is the normal case rather than an edge case. The dropdown becomes a free-text field, and the API checks what was typed. The hint below the field says whether the IBAN was recognised, whether it does not look like an IBAN at all, whether it could not be attributed, or that a login is needed before personal IBANs can be checked. The check is advisory and never blocks submission. Any failure, rate limiting included, shows a neutral "could not check right now" and leaves the form submittable - a support ticket must never fail because an IBAN was not recognised. The field stays required in the sense that it may not be empty. Three things the client library does not handle, so the screen does: the input is debounced by 500 ms, the value used elsewhere in the codebase, rather than checked on every keystroke; a request counter discards answers that arrive after a newer one, so a slow answer for a prefix cannot overwrite the answer for the complete IBAN; and the counter is also bumped when the field is cleared, so an in-flight request cannot bring a hint back. The wording for a non-attributable IBAN deliberately avoids claiming that it is not a DFX IBAN. The status does not carry that meaning: another customer's personal IBAN is never checked, and after an account merge personal IBANs stay with the former account, so a customer's own older IBAN can land there too. getBanks is no longer used on this screen and is removed along with its state and effect. The sender IBAN field, which comes from the customer's own bank accounts, is untouched. * fix(support): stop judging a half-typed IBAN and drop the blame from the hint Review found three things the first version got wrong. The check fired on every typing pause, and the API answers incomplete input with InvalidIban by design. A customer copying an IBAN off a bank statement and glancing up mid-way was told their IBAN looks invalid, with the form jumping as the hint appeared and vanished. For the customers this change exists for that is the normal way of filling the field, not an edge case, and every pause also spent a request from a limit shared by everyone behind the same network. The call now waits until the normalized value reaches fifteen characters, the shortest IBAN in use. A real typo is still caught, because the customer stops at twenty-one or twenty-two characters and the checksum fails there. The hint for an unattributable IBAN asked the customer to check whether it is correct. The status does not support that: another customer's personal IBAN is never checked, and after an account merge personal IBANs stay with the former account. So the one customer this feature was built for - the one whose personal IBAN cannot be matched - was being told to correct an IBAN that was already right, and replacing it with a guess would restore exactly the wrong-IBAN-in-the-ticket problem this change removes. The hint now says only that we could not assign it, and that the request can be submitted anyway. A blank-only entry passed the required rule, sanitized to an empty string server-side and produced a ticket with no receiver IBAN at all - something the dropdown made impossible. An emptiness check now rejects it. It is a check on presence, never on the IBAN itself, so an unrecognized value is still accepted. * fix(support): hold back a verdict until the customer leaves the IBAN field Two independent reviews showed the length threshold does not do what it was added for. Fifteen characters is the shortest IBAN in existence and a Swiss one needs twenty-one, so a customer pausing at sixteen still triggered a request and still read "this does not look like a valid IBAN" while typing a correct one. The threshold moved the problem, it did not solve it, and a country-length table in the frontend would only add a second authority on IBAN validity next to the API. What actually matters is which hint can do harm, and that is only the negative one. A positive confirmation can occur only for a complete IBAN that matched, so it still appears immediately; every other verdict now waits until the field loses focus. That also keeps the form from jumping while someone is still typing. The threshold itself was measuring the wrong string: it stripped only ASCII spaces while the API strips every non-alphanumeric character, so a dash-grouped entry counted separators towards the length and was checked too early. Normalization now matches the API exactly and is the single source for the threshold, the request and the equality guard. The debounce sits on the normalized value, so retyping a space or a dash costs no request. The check could also start for a debounced value that was no longer in the field, if both updates landed in one commit - the response then wrote a hint under the new or empty value. Rather than reason about batching, the effect now refuses to run unless the debounced value is still the current one. And the unmatched wording is more careful again: the status does not establish that DFX does not own the IBAN, since another customer's personal IBAN is never checked and neither is one left behind by an account merge. The text now says we could not recognize it with the information available to us. * test(support): cover the receiver IBAN check and capture its states The advisory receiver IBAN check had no test at all. These tests pin the two properties the feature depends on: the check never blocks, delays or alters a support request, and a hint never belongs to a value that has left the field. The component tests render the screen and cover field visibility per reason, the length threshold at both its boundaries, the normalization the request shares with the API, debouncing, the fact that a formatting-only edit costs no request, the reset that clears hint, spinner and unavailable state on every edit, discarding a late answer for a superseded value whether it resolves or rejects, the focus gating that holds back every non-positive verdict until the field is left, the four hint texts and their colour, the unavailable path for a failed or rate-limited check, the spinner, the requirement that the field be non-empty, and submits that actually go through while a check is in flight and after one has failed, proving the raw text the customer typed is what gets sent. Each behaviour was checked by breaking the screen on purpose and confirming the named test turns red. One constant cannot be pinned this way: setting ReceiverIbanCheckDelay to 0 changes nothing, because useDebounce computes delay || 500 and a falsy 0 becomes 500 again. The language files get their own test for the five new keys, pinning the exact text per language so a swapped or hollowed-out translation fails. It is scoped to those keys: the three files already differ on unrelated entries, so full parity would fail on gaps this change did not create. The Playwright spec captures the six visible states of the field for review. Its baselines are not part of this commit: they are platform-bound, and CONTRIBUTING.md asks for them to be generated on the same platform as the existing ones. @dfx.swiss/react and @dfx.swiss/react-components are mocked rather than loaded, because their published ESM does not load under this repo's test command. The mocks reproduce the rule merging and validation shapes the screen relies on and wire the input through react-hook-form, so focus, validation and the watched value behave as they do in the app. Form deliberately renders no submit handler of its own, matching the real component, so the button's own handler stays the only path a submit test can take. * build(deps): take @dfx.swiss/react 1.7.0-beta.0 for the receive-IBAN check CI installs from the lockfile, so the range alone does not move the build; the pin has to ship with the change. This is the prerelease line, taken knowingly: the stable release of the same version is not out yet, and the screen cannot compile without checkReceiveIban. The range is ^1.7.0-beta.0, which the stable 1.7.0 also satisfies, so a later install moves off the prerelease without another range edit. @dfx.swiss/core follows transitively to 0.6.0-beta.0; it is not a direct dependency here. Verified against the published package rather than a local build: build:dev, the stricter widget:dev, lint, and the full test suite at 465 tests. * fix(support): bind the IBAN verdict to the value it was computed for The hint was cleared in an effect on the normalized value, and effects run after the render. On the first render after an edit the previous verdict was therefore still in state and was shown under the new value - most visibly the positive confirmation, which is deliberately shown while the field still has focus. The result now carries the IBAN it was computed for, and the hint is derived only when that value still matches the field. Clearing it in the effect becomes unnecessary and is gone, along with the same clearing in the check effect's guard branch; the reset effect keeps only the request counter and the spinner. Two tests did not hold what their names promised, both shown by mutating the screen: Validations.Iban was mocked as always passing, so adding real IBAN validation to the receiver field left the suite green - while in the app it would reject a free-text entry and block a support request, which is the one thing this screen must never do. The mock now fails, and a test keeps a complete form submittable with a non-IBAN value in the field. The translate mock returns its key, so removing the translation call around the hint also left the suite green, and every non-English customer would have seen English text. A test now pins that the hint goes through translate under the screens/support namespace. The visual spec gained the LoginRequired state, which the docblock promised but did not record, and now forces lang=en: it asserts English text while authenticating as a real account, and the language of that account would otherwise decide whether the selectors match. * test(support): make the submit-button mock disable like the real one The button mock dropped isLoading, while the real component disables itself on disabled || isLoading. Putting the submit button into its loading state while a check is in flight would therefore have blocked the very thing this screen promises never to block, and the suite would have stayed green. The mock now mirrors the real component, and the two in-flight tests fail against that change. Four comments claimed things a reader cannot check here: how the service answers incomplete input and how its rate limit is shared, how the input component handles focus handlers internally, that the tested translation keys contain colons - they contain periods - and that the visual baselines are deterministic and free of production data, when only the check endpoint is intercepted and everything else comes from whatever stack the run points at. Each now says what holds in this repository, and the reasoning they carried is kept where it was about this code. * test(support): prove submission survives every verdict, not just a negative one Submission was proven unaffected for an unrecognized IBAN, for a check still in flight and for one that failed - but not for the positive verdict. Blocking the button or normalizing the submitted value specifically when the IBAN was recognized therefore left the suite green, and the customer who gets the friendliest answer is the one whose request would have been swallowed. The submission test now runs over all four statuses, so any verdict-specific special case fails it. * test(support): actually submit in the non-IBAN case, and fix two wrong comments The non-IBAN test checked that the field carries no error and the button stays enabled, but it never pressed the button. An onSubmit that quietly returned for a receiver value below the check threshold therefore passed, while in the app the customer's request would simply never have been created. The test now clicks and asserts the request carries the raw value. Two comments were wrong rather than merely unverifiable. One said the hints appear only after the field is left - the positive confirmation deliberately appears immediately, which is the whole reason focus gating is selective. The other justified the missing autocomplete by saying the receiver IBAN is not the customer's own; with a personal deposit IBAN it is exactly theirs, which is the case this screen was rewritten for. The remaining reason, that the sender field above already claims the IBAN autofill, is true and stays. A third comment pointed into node_modules for its evidence and now describes what the mock does and why. * test(support): pin the no-form-error promise against more than rate limiting The guarantee that a failed check never becomes a form error was pinned against rate limiting only. Reporting every other failure through the screen's error banner therefore passed, and a customer whose check answered with a server error would have seen a form error under an advisory field. The failure case now runs over two kinds of rejection, and each of them also submits and asserts the raw value, so the two former single-status tests are covered by one parameterized case rather than duplicated. * test(support): let the receiver IBAN spec past the screen's KYC guard The spec drove /support/issue with a freshly registered account. The screen sits behind useKycLevelGuard, so the app redirected to contact data and the Receiver IBAN field never rendered: all seven tests timed out waiting for a field that was not there. The route setup now pins kyc.level in the GET /v2/user response, next to the receiveIban mock the tests already install. Nothing changes on the server, so the account other specs share keeps the state they rely on, and the screenshots no longer depend on whatever level the local account happens to have. * test(support): add the receiver IBAN check screenshot baselines The spec captures the field in seven states: empty, check in flight, recognized, not matched, invalid, check unavailable and login required. Its baselines were missing, so the spec had nothing to compare against. Taken on macOS against a local API stack, as CONTRIBUTING requires, and confirmed by a second run without --update-snapshots. No other baseline is touched.
The DNS cutover is done: dev.app.dfx.swiss and app.dfx.swiss are served by the Pages projects, and the Front Door profiles that sat in front of the storage accounts no longer exist. The upload step therefore fails, and because it fails every later step is skipped -- including the Cloudflare deploy right behind it. A push to develop currently ships nothing at all, and a release to main would behave the same way. Both pipelines now go straight from the build to the Pages deploy, which is what the comment above that step announced. The Azure templates under infrastructure/ stay for now; the storage accounts still exist and their removal belongs with the resource teardown, not with this pipeline fix.
TaprootFreak
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist