fix: respect assetOut URL parameter when selecting target blockchain #875
Conversation
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
The previous fix (#873) added targetBlockchains?.length as a dependency, but this computed value depends on userAddresses being loaded from the V2 API. Adding userAddresses.length as an explicit dependency ensures the useEffect re-runs when linked addresses (including Lightning) are loaded, allowing asset-out=Lightning/BTC URL parameter to work correctly.
The swap screen was ignoring the asset-out URL parameter (e.g. Lightning/BTC) because the automatic address selection was setting the blockchain to Bitcoin before userAddresses were loaded. Changes: - Add useEffect to correct blockchain when assetOut is set and userAddresses are loaded, ensuring the target blockchain matches the assetOut parameter - Update setAddress() to prefer the blockchain from assetOut when available - Prevent selectedAddress useEffect from overriding blockchain when assetOut points to a different blockchain This fixes the issue where navigating to /swap?asset-out=Lightning/BTC would incorrectly show BTC/BTC instead of BTC/Lightning.
🤖 PR Review Bot❌ ESLint: 1 errors, 4 warnings❌ TypeScript: 1110 errors❌ Security: 2 critical, 33 high vulnerabilitiesThis is an automated review. Please address the issues above. |
TaprootFreak
added a commit
that referenced
this pull request
Jan 8, 2026
Add E2E test and baseline screenshots verifying that asset-out=Lightning/BTC URL parameter correctly selects Lightning as target blockchain. Screenshots confirm: - Target asset shows "BTC Lightning" (not Bitcoin) - Lightning LNURL address is displayed - Info text shows "Swaps von Bitcoin zu BTC auf Lightning" Validates fix from PR #875.
TaprootFreak
added a commit
that referenced
this pull request
Jan 8, 2026
Add E2E test and baseline screenshots verifying that asset-out=Lightning/BTC URL parameter correctly selects Lightning as target blockchain. Screenshots confirm: - Target asset shows "BTC Lightning" (not Bitcoin) - Lightning LNURL address is displayed - Info text shows "Swaps von Bitcoin zu BTC auf Lightning" Validates fix from PR #875.
TaprootFreak
added a commit
that referenced
this pull request
Jan 12, 2026
* feat(e2e): add Bitcoin and Lightning buy/sell tests (#866) * feat(e2e): add Bitcoin and Lightning buy/sell tests Add comprehensive E2E tests for Bitcoin and Lightning trading flows: Buy Bitcoin (6 tests): - Page load with session token - Amount input and currency selector - BTC as target asset - Trading restriction handling - Pre-filled amount flow - Exchange rate display Buy Lightning (6 tests): - Page load with Lightning blockchain - Amount input display - Lightning/BTC as target asset - Trading restriction or KYC handling - Pre-filled amount flow - Exchange rate or KYC requirement Sell Bitcoin (8 tests): - Page load with session token - BTC as sell asset - Amount input display - Bank account/IBAN selector - Pre-filled amount flow - Exchange rate display - Deposit address generation - Full UI flow completion Also includes: - Lightning authentication via lightning.space API - LNURL + ownershipProof for DFX API auth - Screenshot baselines for visual regression * fix(e2e): use correct URL parameter name 'amount-in' instead of 'amountIn' - Fix all E2E tests to use 'amount-in' (kebab-case) as expected by the API - Simplify Lightning exchange rate test to accept KYC state - Add comment explaining that placeholder is shown when KYC is required * fix(e2e): ensure Lightning amount input displays correctly in screenshot - Use direct DOM manipulation to set input value (Playwright's fill/type don't work reliably with the StyledInput React component) - Add wait time for React form initialization before setting value - Regenerate baseline screenshot showing "₣ 100" instead of placeholder * fix(e2e): use DOM manipulation to display input amounts in all screenshots - Fix buy-lightning-with-amount: now shows "₣ 100" - Fix sell-bitcoin-with-amount: now shows "BTC 0.001" - Fix sell-bitcoin-exchange-rate: now shows "BTC 0.01" Playwright's fill/type methods don't work reliably with the StyledInput React component, so we use direct DOM manipulation with event dispatch. * feat(e2e): add linked Ethereum address tests (buy/sell) (#869) Add E2E tests for Ethereum addresses linked to Bitcoin accounts: - Test linked address authentication flow (no second KYC needed) - 7 buy tests + 7 sell tests = 14 total - Add wallet3 with derivation path m/44'/60'/0'/0/1 for fresh addresses - Add authenticateLinkedAddress() helper for linked auth flow - Add getLinkedEvmAuth() helper for Bitcoin+Ethereum linked flow The linked address flow: 1. Authenticate with Bitcoin (primary wallet) 2. Link Ethereum address using POST /auth with existing token 3. New address shares same account/KYC status * feat(e2e): add Lightning sell flow tests (#868) Add 9 E2E tests for selling BTC via Lightning: - Load sell page with Lightning blockchain - Show BTC/Lightning as asset to sell - Display amount input - Show IBAN selector - Handle trading restrictions - Pre-filled amount (0.001 BTC) - Exchange rate/IBAN display (0.01 BTC) - Lightning invoice info - Full UI flow with screenshots Uses DOM manipulation for StyledInput compatibility. * feat(e2e): add linked Lightning address tests (buy/sell) (#870) Add E2E tests for Lightning Wallet 2 linked to Bitcoin account: - 7 buy tests + 7 sell tests = 14 total - Add Bitcoin/Lightning Wallet 2 with derivation path m/84'/0'/0'/0/1 - Add createBitcoinCredentialsWallet2() and createLightningCredentialsWallet2() - Add getLinkedLightningAuth() helper for the linked flow Linked Lightning Flow: 1. Authenticate Bitcoin Wallet 1 (bc1qq70e...) at DFX → primaryToken 2. Authenticate Bitcoin Wallet 2 (bc1qyxjt...) at lightning.space → LNURL 3. Link LNURL at DFX using primaryToken (no second KYC needed) * fix: add targetBlockchains dependency to swap screen useEffect (#873) * fix: add targetBlockchains dependency to swap screen useEffect The useEffect that sets source and target assets was missing targetBlockchains in its dependency array. This caused the effect to not re-run when user addresses (and thus targetBlockchains) were loaded asynchronously. Symptoms: - URL parameter asset-out=Lightning/BTC was ignored - Swap screen showed Bitcoin→Bitcoin instead of Bitcoin→Lightning - Lightning target only worked after manual page interaction Root cause: - targetBlockchains is computed from userAddressItems - userAddressItems depends on userAddresses from API (async) - Without the dependency, the useEffect ran once with empty targetBlockchains and never updated when data arrived Fix: Add targetBlockchains?.length to useEffect dependencies * fix: also add sourceBlockchains dependency to swap useEffect Complete the dependency fix by also adding sourceBlockchains?.length. Both computed arrays are used in the useEffect but were missing from the dependency array, which could cause stale data issues. * fix: add userAddresses.length dependency to swap screen useEffect (#874) The previous fix (#873) added targetBlockchains?.length as a dependency, but this computed value depends on userAddresses being loaded from the V2 API. Adding userAddresses.length as an explicit dependency ensures the useEffect re-runs when linked addresses (including Lightning) are loaded, allowing asset-out=Lightning/BTC URL parameter to work correctly. * fix: respect assetOut URL parameter when selecting target blockchain (#875) * fix: add userAddresses.length dependency to swap screen useEffect The previous fix (#873) added targetBlockchains?.length as a dependency, but this computed value depends on userAddresses being loaded from the V2 API. Adding userAddresses.length as an explicit dependency ensures the useEffect re-runs when linked addresses (including Lightning) are loaded, allowing asset-out=Lightning/BTC URL parameter to work correctly. * fix: respect assetOut URL parameter when selecting target blockchain The swap screen was ignoring the asset-out URL parameter (e.g. Lightning/BTC) because the automatic address selection was setting the blockchain to Bitcoin before userAddresses were loaded. Changes: - Add useEffect to correct blockchain when assetOut is set and userAddresses are loaded, ensuring the target blockchain matches the assetOut parameter - Update setAddress() to prefer the blockchain from assetOut when available - Prevent selectedAddress useEffect from overriding blockchain when assetOut points to a different blockchain This fixes the issue where navigating to /swap?asset-out=Lightning/BTC would incorrectly show BTC/BTC instead of BTC/Lightning. * test(e2e): add swap Lightning URL parameter baseline screenshots (#876) Add E2E test and baseline screenshots verifying that asset-out=Lightning/BTC URL parameter correctly selects Lightning as target blockchain. Screenshots confirm: - Target asset shows "BTC Lightning" (not Bitcoin) - Lightning LNURL address is displayed - Info text shows "Swaps von Bitcoin zu BTC auf Lightning" Validates fix from PR #875. * feat(compliance): add country dropdown to bank tx return screen (#872) - Replace text input with StyledSearchDropdown for country selection - Add country pre-fill from bank details - Add filter and match functions for country search by name/symbol Co-authored-by: Bernd <bernd@MacBookernd2026.galaxus.box> * feat: added new quote error * feat: enable Lightning as source asset in swap screen (#877) * feat: enable Lightning as source asset in swap screen Previously, only the primary wallet's blockchain was available as swap source. Now all user blockchains (including linked addresses like Lightning) are available. Change: sourceBlockchains now derives from userAddressItems (same as targetBlockchains) instead of only availableBlockchains from the wallet. This enables swaps FROM Lightning to other blockchains (e.g., Bitcoin Onchain). Also adds E2E test for Lightning -> Bitcoin Onchain swap flow. * fix: remove Monero exclusion from swap source blockchains Monero is already supported for Buy and Sell, so there's no reason to exclude it from Swap. All blockchains from user addresses are now available as swap source. * fix: remove unused availableBlockchains, improve E2E test timing - Remove unused availableBlockchains from useAppParams destructuring - E2E test: wait for exchange rate BEFORE capturing page content - E2E test: reduce screenshots from 3 to 2 (loaded + complete) * refactor(e2e): improve dialog handling and simplify sell flow (#879) * refactor(e2e): improve dialog handling and simplify sell flow * refactor(e2e): simplify sell flow test with improved MetaMask handling - Remove manual Sepolia network switching (DFX handles via popup) - Fix network switch approval to accept Sepolia requests - Fix popup close handling after approve click - Rename screenshots to simpler 5-step flow: 01-sell-page, 02-amount-entered, 03-before-transaction, 04-transaction-success, 05-etherscan-verification - Add TX hash capture via ethereum.request interception - Wallet2 test ready but needs Sepolia ETH for gas * test: add Bitcoin↔Lightning swap E2E tests with baseline screenshots (#880) - Add swap-bitcoin-to-lightning.spec.ts (symmetric to existing ln-to-btc test) - Add baseline screenshots for both swap directions: - Lightning → Bitcoin (source: Lightning, target: bc1 address) - Bitcoin → Lightning (source: Bitcoin, target: LNURL) Both tests verified against dev.app.dfx.swiss * fix: removed service worker * fix: allow address selection when session has no address (#882) Email login users were unable to select their existing address because: 1. Dropdown was disabled when only one address existed (forceEnable was false) 2. changeAddress was not called when selected address matched activeAddress Now checks if session has no address and enables selection accordingly. Co-authored-by: Bernd <bernd@MacBook-DFX-Bernd-2026.local> * chore(bank refund): refactoring * test(e2e): add mail login and subpages E2E tests (#886) - Add mail-login.spec.ts for testing mail login flow - Add subpages-test.spec.ts for testing authenticated page access - Add .env.test.manual to .gitignore * fix: address connect state fix * refactor: consolidate environment files into single .env (#888) * refactor: consolidate environment files into single .env - Replace 8 separate .env.* files with single .env + .env.sample - Add shell scripts that temporarily modify .env for different environments: - scripts/e2e-test.sh: E2E tests against dev API - scripts/start-dev.sh: Start app with dev API - scripts/build.sh: Build for prod/dev - scripts/build-widget.sh: Widget builds - Update package.json scripts to use new shell scripts - Update all E2E test files to reference .env instead of .env.test - Update Playwright configs to use npm start (reads .env directly) - Update .gitignore to ignore .env and .env.backup This simplifies configuration by having a single source of truth. Scripts explicitly modify .env when needed, making changes transparent. * fix: make shell scripts portable for Linux CI - Add OSTYPE check for sed -i (macOS uses '', Linux doesn't) - Auto-create .env from .env.sample if missing - Add loc option to build scripts for local builds - Restore widget:loc script in package.json * fix: update CI workflows and README for new env structure - Remove .env.* file writes from workflows (use env vars directly) - Update README with new configuration and scripts documentation * chore: remove unused env-cmd dependency * fix: move trap cleanup before modifications for robustness Set trap EXIT immediately after creating backup files, before any sed modifications. This ensures .env is restored even if sed fails. * fix: set API_URL for E2E auth-cache.ts direct API calls The auth-cache.ts uses API_URL (not REACT_APP_API_URL) for direct API authentication calls during E2E tests. - e2e-test.sh now sets both REACT_APP_API_URL and API_URL - Add API_URL to .env.sample documentation * refactor: E2E tests use REACT_APP_API_URL like the app Unify environment variables - E2E auth-cache.ts now reads REACT_APP_API_URL and appends /v1 for API calls. No separate API_URL variable needed anymore. * fix: api-timing-test.ts uses REACT_APP_API_URL for consistency * fix(e2e): improve wallet selection detection in subpages tests (#889) * fix(e2e): improve wallet selection detection in subpages tests - Add proper detection for disabled wallet buttons (accounts without linked blockchains) - For Buy/Sell/Swap pages: Accept wallet selection with disabled button as valid behavior - Change validators to return boolean for flexible assertion handling - Add WalletSelectionResult type for better state tracking The session token used in tests has blockchains:[] (no linked wallets), so the wallet button is disabled. Tests now correctly handle this case instead of timing out trying to click the disabled button. * test(e2e): add baseline screenshots for subpages tests * fix(e2e): update sell screenshot with correct page content Previous screenshot showed wallet selection screen instead of the actual Verkaufen form. Now correctly shows the sell form with payment fields and exchange rate. * fix: remove unused imports and variables (#890) - Remove unused useSell/useSwap imports from payment-info.tsx - Prefix unused error state with underscore in receive-interface.tsx - Remove unused getAuthToken destructuring from home.screen.tsx Fixes CI build failure due to eslint no-unused-vars errors. * fix(ci): correct widget chunk file paths in copy step (#892) The widget build outputs chunk files to ./widget/static/js/ and ./widget/static/css/, not to ./widget/$version-chunks/. Changes: - Fix chunk.js path: ./widget/static/js/*.chunk.js - Fix chunk.css path: ./widget/static/css/*.chunk.css - Fix wasm path: ./widget/static/js/*.wasm - Add || true to all copy commands to handle missing files gracefully * fix: remove unused variables in test files (#891) - Remove unused 'act' import from change.hook.test.ts - Remove unused 'originalWindow' variable from iframe.hook.test.ts - Remove unused 'result' destructuring from resize-observer.hook.test.ts * fix: resolve critical form-data vulnerability (#883) * fix: resolve critical form-data vulnerability Add npm override to upgrade form-data to ^2.5.5 within request dependency. This fixes GHSA-fjxv-7rqg-78g4 (unsafe random function for boundary). * fix: upgrade to Node 20 and fix TypeScript timeout types - Upgrade Node.js version from 16.x to 20.x in all CI workflows - Fix NodeJS.Timeout type incompatibility with browser setInterval/setTimeout - Use ReturnType<typeof setInterval/setTimeout> for cross-environment compatibility - Fix useEffect cleanup function to return void instead of falsy value * chore: add CODEOWNERS file (#897) Require approval from @davidleomay and @TaprootFreak for all PRs. * fix(ci): correct WASM file path in widget copy step (#896) * fix(ci): correct WASM file path in widget copy step The widget build outputs WASM files to the root of ./widget/, not to ./widget/static/js/. This was causing the error: "cp: cannot stat './widget/static/js/*.wasm': No such file or directory" Changes: - Fix WASM copy path in dev.yml: ./widget/*.wasm (was ./widget/static/js/*.wasm) - Fix WASM copy path in prd.yml: ./widget/*.wasm (was ./widget/static/js/*.wasm) - Fix build-widget-loc.sh: update all paths to match CI/CD workflows - chunk.js: ./widget/static/js/*.chunk.js (was ./widget/$version-chunks/) - chunk.css: ./widget/static/css/*.chunk.css (added, was missing) - wasm: ./widget/*.wasm (was ./widget/$version-chunks/*.module.wasm) - Add || true to all copy commands in build-widget-loc.sh for consistency Note: The WASM is placed in the widget root because CUSTOM_CHUNK_PATH is not set during widget builds, so webpack uses default output paths. * fix: enable CUSTOM_CHUNK_PATH for correct widget chunk loading Set CUSTOM_CHUNK_PATH=/widget/ in build-widget.sh to activate the webpack configuration in config-overrides.js. This ensures: - publicPath is set to absolute URL (e.g., https://app.dfx.swiss/widget/) - chunkFilename outputs to v1.0-chunks/ instead of static/js/ - Chunks load correctly when widget runs from different origins Also update CI/CD workflows to copy chunks from the new location (./widget/$version-chunks/) instead of ./widget/static/js/. Fixes chunk loading error: "file:///static/js/*.chunk.js ERR_FILE_NOT_FOUND" * fix: update build-widget-loc.sh chunk paths for CUSTOM_CHUNK_PATH With CUSTOM_CHUNK_PATH set, webpack outputs chunks to v1.0-chunks/ instead of static/js/. Update local build script to copy from the correct location. * fix: remove || true fallback from widget copy commands The copy commands should fail if chunk files are not found, rather than silently continuing. This helps catch configuration issues early. --------- Co-authored-by: bernd2022 <104787072+bernd2022@users.noreply.github.com> Co-authored-by: Bernd <bernd@MacBookernd2026.galaxus.box> Co-authored-by: David May <david.leo.may@gmail.com> Co-authored-by: Bernd <bernd@MacBook-DFX-Bernd-2026.local>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes the issue where
asset-out=Lightning/BTCURL parameter was being ignored because automatic address selection was overriding the target blockchain before userAddresses were loaded.Problem
PR #873 and #874 added dependencies but didn't fix the root cause:
asset-out=Lightning/BTCsetAddress()selects first address (Bitcoin) before Lightning address is availableblockchain: Bitcoinbased on selected addressuserAddressesload with Lightning,blockchainis already locked to BitcoinactiveTargetBlockchains = blockchain ? [Bitcoin] : targetBlockchains→ only Bitcoin assets shownSolution
Three changes to properly respect the
assetOutURL parameter:blockchainwhenassetOutis set anduserAddressesare loadedassetOutwhen availableassetOutpoints to different blockchainTest plan
/swap?session=...&asset-out=Lightning/BTCScreenshots
Before: BTC/BTC swap displayed
After: BTC Lightning swap displayed correctly