Skip to content

feat(mobile): swap execute screen with passkey gate - #512

Closed
collinsezedike wants to merge 1 commit into
Miracle656:mainfrom
collinsezedike:feat/mobile-swap-execute
Closed

feat(mobile): swap execute screen with passkey gate#512
collinsezedike wants to merge 1 commit into
Miracle656:mainfrom
collinsezedike:feat/mobile-swap-execute

Conversation

@collinsezedike

Copy link
Copy Markdown
Contributor

Summary

  • Adds \ rontend/mobile/app/swap.tsx: React Native swap screen ported from the web execute path in \swap/page.tsx\ and \lib/sorobanTx.ts\
  • Introduces distinct \signing\ state (biometric approval via
    eact-native-passkey) separate from \submitting\ (network broadcast), unlike the web's single \swapping\ state
  • Quotes via Soroswap SDK with automatic SDEX fallback using \pathPaymentStrictSend; slippage selector at 0.1 / 0.5 / 1.0%
  • Adds @stellar/stellar-sdk,
    eact-native-passkey, @react-native-async-storage/async-storage, and @soroswap/sdk\ to \ rontend/mobile/package.json\

Test plan

  • Open /swap\ route on a device or emulator with a registered passkey
  • Enter an amount and confirm the quote populates (Soroswap or SDEX label shown)
  • Tap Review swap - confirm summary card shows correct values and min. received
  • Tap Confirm swap - biometric sheet appears with signing state visible
  • Approve biometric - submitting spinner appears, then success card with tx hash
  • Reject biometric - error state shows cancellation message and Try again resets to form
  • Test with expired quote (wait 30s): verify quote refreshes before submit

Resolves #474

@vercel

vercel Bot commented Jul 27, 2026

Copy link
Copy Markdown

@collinsezedike is attempting to deploy a commit to the miracle656's projects Team on Vercel.

A member of the Team first needs to authorize it.

@drips-wave

drips-wave Bot commented Jul 27, 2026

Copy link
Copy Markdown

@collinsezedike Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Miracle656 Miracle656 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There's real craft in this one — separating signing from submitting so the UI can say what the user is actually waiting on is a genuine improvement over the web's single swapping state, the Soroswap→SDEX pathPaymentStrictSend fallback is the right resilience move, and refreshing an expired quote before submit rather than failing is a nice touch.

I have to block it on a security issue though, and I want to explain it properly because the root cause is a platform difference that's easy to miss when porting.

🚨 Blocker: the signing key is stored unencrypted

const [secret, credId] = await Promise.all([
  AsyncStorage.getItem('veil_signer_secret'),
  AsyncStorage.getItem('invisible_wallet_key_id'),
])

AsyncStorage is not encrypted on either platform. On Android it's a plaintext file in the app's data directory; on iOS it's an unencrypted file in the app container. It ends up in unencrypted device backups, and it's readable on a rooted or jailbroken device, by any process running as the app's user, and by forensic extraction tools.

And this key isn't decorative — it signs and broadcasts the swap:

const signer = Keypair.fromSecret(signerSecret)
const assembled = SorobanRpc.assembleTransaction(built, sim).build()
assembled.sign(signer)

Anyone who reads that string can execute swaps against the account without ever touching the device again.

It needs to be expo-secure-store, which is backed by the iOS Keychain and the Android Keystore:

import * as SecureStore from 'expo-secure-store'
const secret = await SecureStore.getItemAsync('veil_signer_secret', {
  requireAuthentication: true,
})

Note that requireAuthentication: true makes the OS itself gate retrieval behind biometrics — which is strictly stronger than the current arrangement, and I'll come back to why in a second.

Whatever writes this key elsewhere in the app needs the same treatment, otherwise the read just fails. Worth grepping for veil_signer_secret across the mobile tree before you push.

🚨 Blocker: the passkey gate isn't load-bearing

async function authorizeWithPasskey(credentialId: string): Promise<void> {
  ...
  const result = await Passkey.authenticate({ ... })
  if (!result) throw new Error('Passkey authentication was cancelled.')
}

The assertion is checked for truthiness and then discarded. It isn't verified, and it isn't bound to the transaction being signed. The signing path doesn't consume it at all — it goes straight to Keypair.fromSecret(...). So the prompt proves a human was present on that screen, and nothing more. Combined with the storage issue above, an attacker holding the secret skips the prompt entirely.

Your comment is honest that this is an intent confirmation rather than authentication, and I appreciate that. The problem is that a reader will reasonably assume a biometric prompt in front of a signing operation is protecting the signing operation.

Two ways to make it real, in ascending order of correctness:

  1. Short term — use SecureStore with requireAuthentication: true as above. The biometric check then actually gates key retrieval at the OS level instead of running beside it.
  2. Properly — this is an invisible wallet: the passkey is supposed to be the signer, via the contract's __check_auth, rather than sitting next to a locally-stored Stellar key. The SDK already exposes signAuthEntry for exactly this, and it's the path frontend/wallet uses. A locally-held secret with a passkey prompt beside it inverts the architecture the project is built on.

I'd accept (1) to unblock, but I'd like to understand why (2) wasn't the route — if there's a reason the SDK path doesn't work on RN yet, that's very much worth knowing, and it may be its own issue.

Also

Math.random() for the WebAuthn challenge:

// expo-crypto is not required; Math.random is fine for a presence check
for (let i = 0; i < challengeBytes.length; i++) {
  challengeBytes[i] = Math.floor(Math.random() * 256)
}

Math.random() is not cryptographically secure. The reasoning holds only as long as the challenge is never verified — so this is fine today precisely because of the previous point, which isn't a comfortable place to be. If you adopt option (2), this must become expo-crypto's getRandomBytes(32). I'd just change it now.

rpId: 'veil.app' — this is the third different app identity in the open queue: #508 uses app.veil.xyz / xyz.veil.wallet, your #514 uses veil.app / app.veil.mobile. I'm settling on one canonical pair and will confirm on #514. Don't change it here until I do.

Conflicts with mainapp/swap.tsx is an add/add conflict (main already has a Soroswap-quoting swap screen) and package.json conflicts too. Your version supersedes it, so take yours for swap.tsx, but please rebase so the diff is honest about what it replaces.

Depends on #514react-native-passkey can't run in Expo Go, so this needs the dev-client work in your #514 to land first. You add the dependency in both PRs; drop it here once #514 is in.

919 lines in one file — not blocking, but getStoredKeys / authorizeWithPasskey / signAndSubmitSoroban / fetchSoroswapQuote are all reusable and every other screen that signs will want them. lib/ (top level, not app/lib/) is the right home. Extracting them would also make them unit-testable, which matters a lot for the signing path.


Happy to re-review quickly once the storage change is in — the rest of this is good work and I don't want it stuck.

@collinsezedike
collinsezedike force-pushed the feat/mobile-swap-execute branch from 2843cd5 to 08fcf72 Compare July 28, 2026 18:01
@collinsezedike

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review.

Storage - moved veil_signer_secret entirely out of AsyncStorage and into expo-secure-store (iOS Keychain / Android Keystore). The getStoredSecret call now passes requireAuthentication: true, so the OS biometric gate is what actually controls access to the key. The separate authorizeWithPasskey / Passkey.authenticate() wrapper is gone - it was only running beside the signing path, not in front of it.

Challenge entropy - Math.random() was only in authorizeWithPasskey, which has been removed, so there is nothing left to replace.

rpId - left unchanged pending your confirmation on #514.

Rebase - done. Our app/swap.tsx supersedes the quoting-only screen that landed on main; package.json uses upstream versions for @soroswap/sdk and @stellar/stellar-sdk. react-native-passkey is dropped from this PR as requested.

expo-secure-store is the only new dep added here; @react-native-async-storage/async-storage is no longer imported in swap.tsx and has been removed from this PR's changes.

Regarding option (2) - using the SDK's signAuthEntry path so the passkey is the signer rather than a locally-held secret: agreed that is the correct architecture. The constraint for this PR was getting the execute path working at all on mobile; the invisible-wallet signing path is likely worth its own issue once the dev-client work in #514 is confirmed (it needs the native module to be loadable before webAuthnProvider.authenticate can be exercised on-device).

@collinsezedike

Copy link
Copy Markdown
Contributor Author

One more thing I should have caught before pushing: grepping veil_signer_secret across frontend/mobile shows only reads (the two SecureStore.getItemAsync calls in swap.tsx) and no writes. That means getStoredSecret() will always return null on a fresh install - the key is never written to SecureStore in the first place.

The web wallet writes it in app/page.tsx and app/recover/page.tsx (to localStorage / sessionStorage). The mobile app needs an equivalent setup or onboarding screen that calls SecureStore.setItemAsync('veil_signer_secret', secret, { requireAuthentication: true }). That screen is out of scope for this issue, but worth tracking - I can open a follow-up if helpful.

@Miracle656

Copy link
Copy Markdown
Owner

Thanks for this — there's real substance in the execute path, and the biometric reasoning is documented carefully. Closing it though, because the file it rewrites has been replaced underneath it and the branch now duplicates several pieces that landed separately.

The base moved

app/swap.tsx was 150 lines when you branched. Since then #534 (feat(mobile): build the swap UI surface) rewrote it to 303 lines against the shared component system, and #513 landed the Soroswap quote library. Merging origin/main into this branch produces four conflict hunks that between them span essentially the whole file, so there's no meaningful automatic reconciliation — it would be a rewrite either way.

What's now duplicated

  • Soroswap client. lib/soroswap.ts exists on main (from Add mobile bulk payout screen and Soroswap quote lib #513) and already exposes getSoroswapQuote, buildSoroswapSwapXdr and resolveTokenAddress. This branch re-implements all of it inline against @soroswap/sdk, including its own token-list fetch from raw.githubusercontent.com.
  • UI primitives. main's swap.tsx builds on components/ui (Screen, Card, Button) and the theme/colors + theme/typography tokens. This branch carries its own StyleSheet throughout, so merging it would reintroduce a second visual language on the one screen that already uses the shared one.
  • Network config. HORIZON_URL, RPC_URL and NETWORK_PASSPHRASE are hardcoded to testnet at the top of the file. lib/network.ts resolves these from EXPO_PUBLIC_* so the testnet/mainnet switch (feat(mobile): switch between testnet and mainnet at runtime #526) keeps working.

expo-secure-store version

The package.json change pins ~14.1.4. main is on ~57.0.1 for the Expo SDK 57 toolchain, so this is a large downgrade — I think it was carried over from an older reference project.

On the passkey gate

The gate here is SecureStore.getItemAsync('veil_signer_secret', { requireAuthentication: true }) — the OS holds the fee-payer secret in the Keychain/Keystore and won't release it without biometrics. That's a legitimate mechanism and your comment about the prompt being load-bearing is accurate.

It isn't the same thing as the passkey path the rest of the wallet uses, though. lib/passkey.ts on main drives react-native-passkeys, passes the Soroban authorization-entry hash as the WebAuthn challenge, and produces an assertion the contract verifies in __check_auth. #474 asks to "authorize it with the passkey" in that sense — a signature the chain checks — rather than a device-local biometric unlock of a stored key. Worth deciding explicitly which one the swap flow should use before rebuilding this.

Suggested path forward

#474 is still open. On current main the remaining work is much smaller than 856 lines: swap.tsx already has the token selectors, amount input and flip; it needs the quote wired through lib/soroswap.ts, then build → authorize → submit → result states. Keep components/ui and theme/ for the presentation, take network values from lib/network.ts, and leave package.json alone. Happy to review that.

@Miracle656 Miracle656 closed this Jul 30, 2026
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.

46. Swap execute + sign

2 participants