feat: DFX App 2.0 preview at /app2 - #1162
Conversation
Ships the new standalone single-file DFX app under public/app2/, served from the same $web static site at /app2/index.html on dev and prod. - Coexists with the current React app; no changes to src/, build config, or the SPA-fallback routing. Entry is the explicit /app2/index.html. - All asset paths are document-relative and the service worker is scoped to root only, so the app runs unchanged under the subpath. - Adds an app2 upload step to dev.yml and prd.yml (mirrors the widget upload); the entry HTML is uploaded no-cache so redeploys are fresh.
- Fix DOM XSS (js/xss-through-dom): the image-fallback handler no longer assigns a DOM attribute value to img.src; the mono placeholder is rebuilt via mono(), whose encodeURIComponent neutralises any injected markup. - Fix prototype pollution (js/prototype-polluting-assignment x2): the wallet switcher lookup uses a null-prototype object, so URL-seeded addresses such as __proto__ can no longer reach Object.prototype. Verified with CodeQL 2.25.6 (javascript-code-scanning): 0 alerts. - Add SIL OFL 1.1 license for the self-hosted Inter fonts and a THIRD-PARTY-NOTICES.md for the vendored libraries. - Deploy the preview on dev only (remove the app2 upload from prd.yml).
Model which network the CONNECTED wallet can actually settle on (Lightning- aware) instead of treating every session chain as an interchangeable receive target: - Lightning is offered only for a Lightning identity (LNURL/LNNID) or a pure- Lightning session — never for an on-chain SegWit (bc1) address, which is paired with Lightning in the JWT but cannot receive on it. Fixes the BTC buy defaulting to / offering the Lightning network. - Swap now validates the receive chain (chain2) too, prompting a wallet connect instead of letting the output land on an unreachable chain. - Mode switches and defaults pick a wallet-reachable chain, not the first one. - Asset-picker filter chips are derived from the reachable pool (no dead-end 'no assets found' on single-chain wallets) with an 'All' chip and a helpful 'not available with your wallet' hint; sell fee never mislabels a crypto amount with a fiat symbol; corrected the 'we flag the cheapest' copy and the scary pre-filled sell/swap amount. CodeQL javascript-code-scanning: 0 alerts. Offline suite: 144 passed.
New DFX users auto-register on connect, but the backend blocks trading until a
referral is submitted (RecommendationRequired). Instead of linking out to
app.dfx.swiss, the confirm-sheet gate now:
- shows an 'Invitation needed' state explaining a referral is required,
- takes the invitation/referral code OR email inline and submits it to the KYC
Recommendation step (PUT the step session URL with { key }),
- falls back to the full KYC flow when prerequisite steps come first,
- maps 404/400 to clear 'no matching customer' / 'invalid code' messages,
- tells the user the referral must still be confirmed by whoever invited them.
The KYC Recommendation step form gains the same lead + confirmation note.
CodeQL javascript-code-scanning: 0 alerts. Offline suite: 146 passed.
… account - The drawer's OpenCryptoPay entry used a generic cube glyph; it now renders the actual OCP brand mark (buildMenu gains full-SVG icon support). - Account 'Display currency' was hard-wired to EUR; it now reflects the selected fiat (updated in renderAccount and on currency change). CodeQL: 0 alerts. Offline suite: 145 passed.
Replace the muddy squiggle glyph on the account Display-currency row with the universal currency sign — a clean circle with four rays — that reads clearly at icon size and matches the line-icon set.
…more blurry flag) Replace the low-res flag png in the buy/sell/swap fiat pill and the currency picker with a sharp SVG coin showing the currency symbol (EUR €, CHF Fr, USD $, GBP £), matching the round crypto coins. Language picker keeps its flags.
Bring back the flags (EUR/CHF/USD/GBP) but render them as inline SVG instead of <img src>: an img rasterises the SVG at CSS-px size and looks blurry on retina, while inline SVG renders at device resolution and stays crisp. Each flag uses a unique mask id so multiple inline flags don't collide on a shared '#a'. Currency symbol coin remains the fallback for currencies without a bundled flag.
…n fallback Harden shownChainsFor/supportedChainOf so a Taproot/on-chain BTC address never sees Lightning as a network even when the session's blockchain list is empty or unexpected (the previous fallback returned all of the asset's chains).
…nal-IBAN overclaim The pay box hid the reference and claimed 'This is your personal IBAN' whenever the API flagged isPersonalIban — misleading (most users have no personal IBAN) and money-critical: if a shared/collection IBAN ever carried isPersonalIban the required reference would be hidden and the transfer stranded. Now the reference is shown whenever the API returns a remittanceInfo (the authoritative signal), the label stays a neutral 'IBAN', and a no-reference IBAN just says 'no payment reference needed' without claiming it's personal. Added a money-safety test.
…es=) When the embedding wallet reports balances via the ?balances=amount@asset contract (DFX's own primary balance source), the sell / swap-source picker now lists ONLY the assets actually held, sorted by balance, with the amount shown per row. Without balance data (e.g. a bare hardware-wallet connect, no contract addresses available client-side) it falls back to all sellable assets.
The API's Swiss QR-bill SVG was inlined via innerHTML into a fixed 158x158 box that forced a non-square aspect (distorted modules) and was too small for a dense Swiss QR to scan. Now it renders as a data-URI <img> at 220px with object-fit:contain (never distorted), matching the production app; GiroCode text still gets encoded locally. The QR content itself is API-provided.
…i18n keys - The account Display-currency icon was a static € even when the selected fiat was CHF/USD; it now follows the currency (€/Fr/$/£), matching the value. - Remove the now-unused yourIban / personalIbanNote strings left over from the remittanceInfo-driven IBAN rework.
…ache CSP breakage On localhost the app no longer registers the SW and actively unregisters any leftover worker + clears caches. A stale cached index.html served by an old SW had a CSP hash that no longer matched the inline script, so the browser blocked ALL app JS — connect-wallet and every button went dead. Root cause removed for local dev; prod/root still gets the SW.
- The asset-picker 'not available with your wallet' message wrongly appeared when a category chip simply filtered out a reachable asset during search (e.g. Bitcoin chip + 'ETH'); it now shows plain 'no results' and only claims wallet-incompatibility when the asset is genuinely unreachable. - Escape the currency symbol in fiatGlyph, matching curSymSvg.
…verflow the rounded corners .panels had border-radius but no overflow:hidden, so the darker You-receive panel's square background poked past the rounded bottom corners. Added overflow:hidden (fab is centered, box-shadow is unaffected).
Review notesScope check: the diff outside Findings
What looks solid: isolation from the existing app is real (verified no One architectural question worth settling before this grows further: the app doesn't reuse any of the existing codebase's wallet-connect, chain-config, or KYC-flow logic — it's a from-scratch reimplementation, which is how it ends up re-solving (and re-introducing bugs in) problems the existing app already fixed. That's a reasonable tradeoff for a short-lived stakeholder-preview spike, but the PR description's own "ahead of any root cutover" framing suggests this might be heading toward production. Worth confirming the intended lifecycle now — disposable preview vs. seed of the real rewrite — since that changes how much investment (tests, shared logic, maintainability) makes sense here. |
Architecture suggestion — keep the isolation, gain reusabilityFollowing up on my review notes: the standalone approach makes sense for a fast stakeholder preview, but if there's any chance this graduates ("ahead of any root cutover"), the current shape — the full app hand-written in one inline Two observations from the repo:
Proposal: build the new app as a fourth build target
What this buys:
An optional step further would be giving the new entry its own Vite config (two devDependencies) instead of a third instance of the index-swap build hack — also a low-stakes pilot for eventually leaving CRA/react-app-rewired, which is deprecated upstream. But the zero-new-tooling variant above already delivers the main value. Naming: reconsider
|
…, own PWA identity, branded 404, hero spacing
|
@Danswar Thanks for the review — both the findings and the architecture proposal were on point. Rather than argue, I built it: your fourth-build-target proposal is now implemented on What it is — exactly the shape you sketched:
Your findings, addressed by construction or directly:
Login slice so far: MetaMask + WalletConnect v2 + email magic-link ( Open question where I'd like your take: keep shipping under |
|
Keep |
Danswar
left a comment
Danswar
left a comment
There was a problem hiding this comment.
Review round 2 — React build target (3e398fc4, 18b2c291)
The architecture is what was proposed, and a lot of it verifies clean (credit list below). But the PR currently contradicts its own description on the most important point, and the money path has two flow-bricking bugs plus a cluster of inverted API semantics — several of them regressions of bugs the static preview's own commit history already fixed once. Requesting changes.
The structural problem: both apps ship, and only the old one deploys
build-app2.shoutputs toapp2-dist/(gitignored) — no workflow builds or uploads it.dev.ymluploads./buildwith--pattern "app2/*", which is CRA's verbatim copy ofpublic/app2/— the static preview. Through CI,/app2/still serves the vanilla app; none of the React work reaches users.public/app2/is fully intact at head (94 files, 4.9 MB, including all ofassets/vendor/— 3.3 MB). "assets/vendor/* is gone" doesn't hold: the PR contains zero deletions.- Consequently round-1 findings 2 and 3 are not moot — the hand-maintained CSP hash and the wasm cache-order clobber (
dev.yml:117uploads*.wasmimmutable,:135re-uploadsapp2/assets/vendor/*.wasmat max-age=3600) are both still live in what deploys. - The React artifact itself (built locally — CI never has): 8.2 MB, with the full 5.2 MB static tree embedded at
app2-dist/app2/(CRA copiespublic/wholesale intoBUILD_PATH), plus the main app'sfavicon.ico/logo.png/manifest.json. And it ships no CSP at all — the strict policy exists only in the staticindex.html. On the CSP axis the React artifact is currently a regression, not a fix. - Round-1 finding 1 (tests) is still fully open: zero test files,
npm run testis--passWithNoTests, andpr.ymldoesn't runapp2:dev— green CI proves typecheck and nothing more for ~10.7k new lines of money-path UI. - The PR description still describes the static single-file drop-in — worth rewriting so approvals are based on what the PR now is.
Code findings
Details are inline. The two critical ones:
- New users are bricked in all three trade modes: every gate that explains/unblocks (email verification, KYC/limits, min/max) renders only inside the PaymentSheet, which only opens on a valid quote — and a new user's every quote fails (
EmailRequired/RecommendationRequired). Disabled CTA, "quote error", no way forward. (home.tsx:312) - Persisted WalletConnect sessions survive logout and pre-empt new pairings: silent auto-login as the previous wallet on shared machines, and a self-bricking hang after a phone-side disconnect, with no reachable code path that clears the stale session. (providers.ts:127)
High-severity, inline: fiat buyable/sellable filters inverted against the API convention (home.tsx:615); card/instant offered although the API rejects card unconditionally (PaymentMethodPicker.tsx:45); chain reachability wrong in both directions — blockchains[0] hides valid EVM chains (session.tsx:190) while the asset-pool fallback re-offers Lightning to non-LN wallets (asset-pool.ts:66); SEPA details missing the beneficiary (PaymentSheet.tsx:413); sell quotes fired without the mandatory IBAN and the 400 latched into the payment snapshot (useTradeQuote.ts:60); the cancel fix only covers the QR phase (session.tsx:238); injected addresses sent to /auth lowercase (providers.ts:102); Sumsub ident sessions dead-end silently (kyc.tsx:58); KYC data-entry steps loop with no portal link (kyc.tsx:366); failed support messages silently lost (support.tsx:255); PDF attachments can't be opened (support.tsx:507).
Not inline but worth fixing in the same pass: no accountsChanged/chainChanged handling anywhere in src/app2 (switching MetaMask accounts leaves the session — and sell instructions — on the old address; production re-verifies, src/hooks/wallets/metamask.hook.ts:111); no error boundary above the router (App.tsx — a provider throw is an unbranded error page in a money app); the Sheet background is neither inert nor focus-trapped, which is also what lets the "frozen" snapshot re-latch under an open sheet (home.tsx:287); ticket/thread load failures render as empty states ("No tickets yet") with unhandled rejections (support.tsx:167); transactions fetch and render the full history unbounded (transactions.tsx:132); the referral row copies the bare code while toasting "Referral link copied" (account.tsx:263); Drawer's Sell/Swap items route to / without setting the mode (Drawer.tsx:53) — decorative today.
Verified as claimed — credit where due
- Zero hand-rolled fetches; zero new dependencies — both checked mechanically. All hook usage exists in the pinned lib version. (One version edge:
err.code === 'TFA_REQUIRED'in kyc.tsx:145 needs ≥1.4.1 — fine for the locked build, silently dead when developing against a stale local install; worth a note.) - i18n claim verified: 659 keys × 4 languages, zero missing/mismatched, placeholder tokens consistent, all dynamic enum keys (
kn_*,ks_*,is_*) match the lib exactly. - Round-1 finding 4 properly fixed: real
URLparsing, rejectsjavascript:/data:/protocol-relative, applied at every API-derived href/window.opensink I could find — stricter than production, which iframes the ident URL unchecked. (Two identical copies of the helper exist —format.ts:89andamount.ts:85— merge them before they drift.) - No HTML-injection surface: no
dangerouslySetInnerHTML/innerHTMLin src/app2; the API-supplied QR-bill SVG is deliberately rendered viaimgdata-URI; everytarget="_blank"carriesnoopener. - The quote engine's ordering discipline (monotonic seq + key-tied freshness) is sound; there is zero client-side money arithmetic on the payment path; copy buttons copy raw machine values; the magic-link flow returns to
/app2/(not the root app), scrubs credentials from the URL, andauthMsgOkrefusing to blind-sign an unrelated challenge is a protection most dapps skip.
What I'd gate merge on
- Resolve the two-apps state: wire
app2:devintopr.yml, deployapp2-distindev.yml, decouplebuild-app2.shfrompublic/app2, then delete the static tree. - The two criticals + the fiat-flag inversion.
- A CSP for the React artifact (meta tag in the template or headers at the edge).
- A committed smoke-level test suite — the money-path fixes in this PR's own history are exactly the regressions tests should be pinning.
- Updated PR description.
|
@Danswar I addressed the complete round-2 review in commit Highlights:
Local verification:
I left the review threads open for your verification. |
There was a problem hiding this comment.
Review round 3 (22dd0cc)
Round-2 verification first: all 20 threads check out against the code — resolved. The structural fix is genuinely complete (CI builds and deploys app2-dist in all three workflows, static tree gone, CSP injected with fail-closed assertions), and the regression suite is real: every test imports the production modules, and this PR's own CI log shows PASS src/app2/__tests__/review-regressions.test.ts. Credit also for: i18n mechanically clean (660 keys x 4 languages, zero drift, all literal t() usages resolve), the clean-room import boundary holds (nothing escapes src/app2/ except the SDK and existing lockfile deps), the CSP checks out against the app's actual runtime traffic (API, WC relay/verify, blob attachments, self-hosted fonts), and build-app2.sh's EXIT trap correctly restores the entry swap on failure so a broken app2 build can't poison the widget build that runs after it.
This round: one finding I'd gate merge on, a band of mediums inline, and grouped smalls below.
Gate: the WalletConnect storage cleanup is a no-op after any page reload (providers.ts:228, details inline) — the round-2 fix only holds within a single page lifetime, and any reload re-opens the shared-machine path round 2 called critical.
Inline (mediums): URL-credential bootstrap dead-ends silently on RecommendationRequired; prod-hardcoded KYC/setup handoffs break the dev artifact; 2FA setup failure is an unrecoverable spinner; account-merge/switch responses loop; the sell sheet can auto-open unprompted; the buy rate renders a dangling slash; TranslationKey collapses to string; Drawer claims aria-modal without modal behavior; the postprocess identity strips are unasserted; support retry/timeout/empty-thread delivery gaps.
Smaller, fine as a fast-follow:
- The swap rate is computed client-side (
estimatedAmount / quote.amount, fee-inclusive) although the Swap response carriesexchangeRate— the one breach of the zero-client-money-math claim (FeesPanel.tsx:86); thefees.bank ??component-sum fallback on :83 is dead per the SDK type. "1,000"parses as 1.0 (amount.ts:14-22) — the mismatch is visible in the receive field, but it yields a wrong-amount quote rather than a rejection.AMOUNT_TOO_LOW/HIGHopens a gate titled "Setup required" with an external app.dfx.swiss CTA when the fix is editing the amount locally (PaymentSheet.tsx:315-338); theisCardbranch at :364 is dead code now that Bank is the only method.- Transaction states render as raw enum strings in all four languages (transactions.tsx:273); the language switch never propagates to the API user (LanguageSheet.tsx:22-26), so server-driven mails/ident sessions stay in the account's old language.
- CSP:
https://pulse.walletconnect.orgis missing from connect-src (telemetry fetch blocked, console noise on every WC connect); the single static CSP shipslocalhost:3000/dev-API entries in the prod artifact; and a meta-delivered CSP cannot carryframe-ancestors, so /app2 clickjacking protection needs a response header at the edge (infra, not this repo). - app2's content-hashed bundles upload at
max-age=3600instead of immutable, and app2 PNGs upload twice (86400 then 3600) — matches the widget precedent, perf nit only. WC_OPTIONAL_CHAINSclaims to mirror wagmi.config.ts but omits Citrea — harmless while app2 only doespersonal_sign; fix the list or the comment before the first on-chain feature.- No global 401-to-logout: an idle tab whose JWT expires stays visually logged in until the next data call. Shared gap with the production app, noting for parity, not as a regression.
One deploy-scope flag: this commit wires app2 into prd.yml for the first time — merging now publishes to app.dfx.swiss/app2/, where the previous state was dev-only. That matches the updated PR description; just making sure it's a conscious call.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… hardware BTC/EVM) Wire up every wallet DFX supports so the connect sheet no longer shows grey coming-soon entries: EVM browser wallets (EIP-6963), Solana/Tron via dynamic-imported adapters, hardware BitBox/Ledger/Trezor (Bitcoin + EVM), Alby (WebLN/LNURL), Cardano (CIP-30), and CLI manual paste-signing. Only DFX Taro and Internet Computer remain coming-soon, matching the original. Adds the scoped connect.trezor.io / sumsub CSP entries in postprocess.
Port the full OCP merchant experience from the original: home hub, apply (PartnershipRequest), payment routes (activate/create), invoice generator with real scannable LNURL QR (react-qr-code) + PNG download/print/sticker, payment links, POS terminal with live payment polling, history, config, and a demo mode. LNURL bech32 encoding is pure client-side; no new deps.
Bring the React rewrite to parity with the original static preview (the pixel-perfect + functional source of truth): buy/sell/swap, asset/chain picker, account, transactions, KYC steps, support/chat. Adds the limit increase request screen and the external-return routes (/account-merge, /buy/success cko-poll, /buy/failure), and wires the previously dead OpenCryptoPay / payment-routes / limit menu items. Also makes the pay and receive pills equal width and aligned within each half.
The buy panel drove its rate off the authenticated paymentInfos call, so any account gate (400 EmailRequired, KYC, limit) blanked both the rate and the calculation with no way to learn why. Buy and swap now run the panel on the public quote endpoint — as sell already did and as the static preview did — and only request payment details when the user moves to pay, where the gate UI lives. Display and payment are separate engines, so a gate can no longer take the rate down with it, and paymentInfos (which creates a transaction request server-side) no longer fires on every keystroke. Accepted trade-off, documented in useTradeQuote.ts: the public quote prices against the default fee set and the generic limit, so partner-wallet and individual fee agreements only appear in the payment response — which is what the sheet renders before anyone pays. Sell keeps its previous behaviour. Also in this change: - the wallet bar showed the Cardano mark for every CLI wallet, because the Cardano catalog entry declares walletType CLI and precedes the CLI entry; icon lookup now resolves own identity before a shared walletType, and DFX Wallet gets the DFX mark - fiat pills were shorter than asset pills (30px vs 38px glyph) in the trade panels and the picker rows - an invalid quote no longer renders as "0" with a refresh countdown, and the payment sheet fails closed on an invalid quote instead of printing an IBAN - the payment sheet's gate no longer shows a success tick and a "transfer the amount below" line with no amount below - real Open CryptoPay mark in the drawer and the OCP hero badge - display-currency row shows the currency's own symbol - .btn-mini had no horizontal padding, so "Verbindung entfernen" touched both edges
Sell was left on the old wiring: because the default payout account is
preselected, its panel ran on the authenticated paymentInfos endpoint from the
first render — so an account gate could still blank the rate, and every
debounced keystroke created a sell route plus a transaction request
server-side (api › sell.service.createSellPaymentInfo). It now quotes through
the public endpoint like the other two modes and asks for payment details only
when the user moves to pay.
That also collapses the two armed intents into one: picking the payout account
the sell CTA asked for arms the same state the CTA arms, so the sheet opens on
that exact keyed response. The 20s watchdog, the retry re-arm and the
cancel-on-input-change now cover all three modes identically, and the sell CTA
no longer needs a payout account before it can show a rate.
Verified against dev.api.dfx.swiss with a temporary payout account on both
paths (account on file, and adding one from the CTA's picker): the panel keeps
its rate and the sheet shows the real deposit details.
Removes hasSellQuoteInputs: it was already unused, and its test asserted the
opposite of the shipped behaviour ("no sell quote until an IBAN is present").
Merge gate: WalletConnect sessions restored from IndexedDB after logout on a shared browser (localStorage-only cleanup missed the layer the SDK actually persists to once @walletconnect/keyvaluestorage migrates off it). Clears the IndexedDB database too, with a bounded timeout so a blocked delete can never hang the logout path. Also addresses: - accountsChanged/chainChanged listened on window.ethereum instead of the provider instance a session actually authenticated with, mixing up account changes between installed wallets. - Instant SEPA was offered/rejected on the wrong currency flag and ignored the asset entirely; Card removed (the API hard-disables it unconditionally). - Invite codes sent as the wrong API field (usedRef vs recommendationCode), no uppercase normalization, truncated input, and no prefill on the recommendation-gate retry. - it/fr were missing two translation keys, and only the en dictionary was typed to catch that at compile time. - transaction status rendered the raw state enum instead of a translated label. - the swap exchange-rate line showed the reciprocal of the correct rate. - --passWithNoTests removed now that a real suite exists. - app2's content-hashed bundles (including the root-level wasm module) were losing their immutable cache header to a later, broader upload pass. - KYC document upload sent a bare base64 payload instead of the full data URL the API expects, causing a 500. - a failed quote refresh left its countdown timer running forever with no automatic or manual way to recover; added bounded backoff retries and a retry affordance in the UI. - country/nationality trading restrictions were mapped to the "try a different asset" message instead of an honest account-level one. - THIRD-PARTY-NOTICES.md misattributed the flag icons and didn't mention web3icons at all; both are now credited with their MIT license text shipped in the artifact.
…ssification Merge gate: the IndexedDB cleanup from the previous round was ineffective in the same tab that had just used WalletConnect. idb-keyval (the storage layer @walletconnect/keyvaluestorage builds on) opens its database once and never closes the connection or releases it on disconnect, so deleteDatabase() sat on onblocked until the timeout gave up — leaving the old session data intact rather than just being slow about removing it. Switched to clearing the object store's contents via a plain, non-version-bumping open() + readwrite transaction, which never needs exclusive access and so isn't blocked by that still-open connection; the connect path no longer pays the timeout at all in the common case. Also addresses: - the Alby hosted-account OAuth redirect sent the invite code unclassified, so a full recommendation code sent as usedRef 400'd server-side and (being a full-page redirect) dropped the user on a raw API error page. - the quote panel's retry button could appear where refresh() was a no-op (inputs not yet resolved, or mid TTL-rollover) — gated on the engine's own errorIsCurrent instead. - the recommendation-gate form could prefill a code shaped for the other field, guaranteeing its own resubmit would fail — only prefills a value that actually classifies as a recommendation code. - strengthened the countdown-interval test to cover a success followed by a later failure, the scenario the fix is actually about (the previous test only exercised repeated failures, which never created an interval to begin with). - Landing's invite placeholder and comment described a code shape neither API regex accepts; replaced with a real example and added an inline hint for a typed code that doesn't classify, instead of silently dropping it. - a stray Italian typographic apostrophe and an overly technical German status label brought back in line with their sibling translations; the dead Card payment-method strings (removed with Card itself) deleted from all four dictionaries.
… break WalletConnect storage The round-2 fallback path (databases() unavailable/throwing, database not yet created) opened the WalletConnect IndexedDB without a version argument and, on onupgradeneeded, only recorded that the database was new without creating the keyvaluestorage object store. That left the database at version 1 with no store at all. idb-keyval's own createStore() (what @walletconnect/keyvaluestorage is built on) also always opens without a version, and real IndexedDB never fires onupgradeneeded a second time for a database once it exists at a given version — so the SDK's own store-creation code would never run again, and every future WalletConnect connect attempt in that browser profile would throw NotFoundError. Since disconnectWalletConnect() runs before every connect attempt, this could trigger on the very first WalletConnect click in a browser without a working databases() API. Fixed by creating the keyvaluestorage store ourselves in onupgradeneeded, matching idb-keyval's own schema exactly, so a database our cleanup has to create is left in the same shape the SDK would have created it in.
…'s social identity The App2 build artifact replaced <title> but never touched the inherited twitter:*/alby:* meta tags from the shared public/index.html. Link-preview unfurls (Slack/X/Telegram/Signal) read those tags, not <title>, so every shared /app2 link rendered the old app's title, description, and screenshot. Fixed by stripping the inherited social-preview tags and injecting App2's own (no twitter:image, since there's no App2-specific screenshot — an absent card beats a wrong one). twitter:site/creator keep the real @dfx_swiss account, which is accurate for this product too. Both the title and description replacements now throw instead of silently no-op'ing if the CRA template no longer matches, and a final check fails the build if any of the main app's exact title/description/image/alby values are still present anywhere in the artifact. Also closed the shared public/ strip's blind spot: it only ever removed a hardcoded list of stale files, so a file added to public/ later would ride along into the artifact unnoticed. The artifact root is now checked against an explicit allow-list after every build step, failing the build on anything unaccounted for instead of shipping it quietly.
…export, fee-math labeling An abandoned WalletConnect teardown (cancelConnectAttempt's fire-and-forget disconnectWalletConnect()) nulled the cached provider promise immediately but kept awaiting the old provider's still-in-flight init(). If a fresh connect attempt started in that window, it would establish a new session before the stale teardown resumed and wiped it out again once the old init() finally settled. Fixed with a generation counter: the final clear only runs if nothing newer has taken over since. Also addresses: - a valid short partner ref code silently disappeared on the email login path (AuthMailDto has no field for it) with no feedback; now shown with its own hint distinct from "unrecognized code". - the recommendation-gate field's placeholder and lead text still promised "or email" support that the sign-up endpoint this form actually submits to never had, on top of now enforcing a code-only length and uppercase. - a leftover unused export in the postprocess helpers wired into an additional structural assertion (no twitter:/og:/alby: tag survives the removal step, independent of the value-based main-app-identity check). - the fee panel's collapsed summary and expanded detail rows showed two different numbers for the same pair with neither labeled: the summary still hand-divided the settled amounts (fee-inclusive) for sell/swap while the detail row showed the pre-fee market rate. Both now derive from the API's own fields (rate vs exchangeRate) and are labeled accordingly in all four languages.
…f a fixed list The artifact-root allow-list added in the previous commit did its job on the first build against a moved base: develop now ships public/_headers and public/_redirects for the Cloudflare Pages deploy, CRA copies public/ wholesale, and both landed in the App2 artifact root. A hand-maintained denylist means the next file the main app adds ships silently again, so the list is now read from public/ at build time — index.html excepted, since CRA rewrote that one into App2's own document. Verified by staging both files locally and building: the artifact root comes out with App2 entries only.
|
@Danswar — ready for another look. All 32 inline threads from rounds 2 and 3 are addressed and answered individually; the PR description is rewritten to match what this actually is now. Your round-3 merge gate needed three attempts, and the first two were wrong. The cleanup cleared Also fixed beyond the threads:
Test suite went from 29 suites / 318 tests to 40 / 382. Each new test was verified red against the state it fixes, and the two you called out as weak — the WalletConnect mock and the vacuous Known gaps, deliberately left open and listed in the description: OpenCryptoPay and the hardware/chain connectors move to a follow-up PR on top of this one. Buy is never confirmed via One note on your round-3 deploy flag: |
3 participants
DFX App 2.0 — React target at
/app2/This PR ships App 2.0 as a fourth React build target alongside the production app, the widget and the library. It is built from
src/app2/, talks to the API exclusively through@dfx.swiss/react, and deploys as its own isolated artifact under/app2/.What ships
scripts/build-app2.shproducesapp2-dist/;scripts/postprocess-app2.jsstrips the shared main-app payload, stages only App 2.0's own PWA assets, injects an environment-specific CSP and App 2.0's own social preview metadata, and fails the build if any of that does not hold.app2-dist/, stage it atbuild/app2/, upload it, and re-uploadapp2/index.htmlwithno-cache. Content-hashed assets are uploaded last withimmutable.public/app2/tree, its vendored browser bundles and its vendored WebAssembly are deleted.Changes since the last review round
The trade panel and the payment sheet were separated onto the two API calls they belong to:
PUT {buy,sell,swap}/quoteendpoints. The authenticatedpaymentInfoscall runs only when the user moves to pay. Previously any account-state gate (no e-mail on file, KYC, limit) blanked the rate and the calculation instead of surfacing in the sheet, andpaymentInfos— which creates a route and a transaction request server-side — fired on every debounced keystroke.0with a live countdown, and the payment sheet fails closed onisValid: falserather than printing an IBAN.rate(final, including fees) instead of client-side division; the breakdown line showsexchangeRate(market rate). Both are labelled so they cannot be read as the same number.Wallet session and artifact hardening:
localStorage, but@walletconnect/keyvaluestoragepersists to IndexedDB once it has migrated, so a shared browser could resume the previous owner's session after logout and reload. The store is now emptied in place (a delete would block on the SDK's own open connection), the upgrade path creates the store idb-keyval expects, and an aborted teardown can no longer wipe a newer session's state.accountsChanged/chainChangednow listen on the provider instance the session actually authenticated with (EIP-6963) rather thanwindow.ethereum, and only invalidate when the active address really changes.usedReforrecommendationCode, on the wallet, mail and Alby paths alike./app2link no longer unfurls as the production app. The artifact root is validated against an allow-list.currency.instantSellableandasset.instantBuyable); card is not offered at all, since the API rejects it unconditionally.de,itandfrare compile-checked against the English key set.Verification
npm run lint— cleannpx prettier --check "src/app2/**/*.{ts,tsx,css}"— cleannpm run test— 40 suites, 382 testsnpm run build:dev,npm run widget:dev,npm run app2:dev,npm run app2dev.api.dfx.swissin the browser, including the payment sheet with real deposit details and the e-mail gateKnown gaps
Deliberately not addressed here, to keep this reviewable:
PUT /buy/paymentInfos/:id/confirmis not called), so an open purchase does not appear under transactions.frame-ancestorscannot be delivered through a<meta>CSP and needs a response header at the edge.Deploy behaviour
On merge to
develop, DEV publishesdev.app.dfx.swiss/app2/. The production workflow publishes the same target toapp.dfx.swiss/app2/, which is a conscious change from the previously DEV-only state. The root app is unaffected.