Skip to content

feat(rn_cli_wallet): add NFC tap-to-pay support for iOS and Android#421

Open
ignaciosantise wants to merge 1 commit intomainfrom
feat/nfc-wallet
Open

feat(rn_cli_wallet): add NFC tap-to-pay support for iOS and Android#421
ignaciosantise wants to merge 1 commit intomainfrom
feat/nfc-wallet

Conversation

@ignaciosantise
Copy link
Collaborator

Summary

Add NFC reader integration with automatic tag detection on Android via foreground dispatch, and iOS native CoreNFC support with universal links. Enables seamless tap-to-pay UX for both platforms.

Changes

  • Android: Foreground dispatch automatically intercepts NFC tags while app is visible, no user action needed
  • iOS: Native CoreNFC reader triggered by "Tap to Pay" button in header; uses app links for background tag handling
  • Dependencies: Added react-native-nfc-manager
  • Configuration: iOS entitlements, Android App Links, versionCode bump

Test Plan

  • Android: Tap NFC tag from home screen → wallet opens directly
  • iOS: Press "Tap to Pay" button → native reader opens
  • iOS: Scan NFC tag from home screen → notification → wallet opens
  • Both: Verify payment links are parsed and modal opens
  • Check logs for NFC scan debug events

🤖 Generated with Claude Code

Add NFC reader integration with foreground dispatch on Android for seamless
tag detection while app is visible. iOS uses native CoreNFC with universal
links. Add NFC button to header on iOS, Android App Links configuration, and
iOS App Site Association for deep linking from NFC tags.

- Add react-native-nfc-manager dependency
- Implement useNfc hook with foreground dispatch for Android
- Add NFC icon (NfcTap.tsx) with customizable fill color
- Add NFC button to Header on iOS (hidden on Android due to foreground dispatch)
- Update Android manifest with NFC permissions, app links, and autoVerify
- Update iOS entitlements with NDEF reading and universal links
- Bump Android versionCode to 60
- Add logging for debugging NFC scan events

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 19, 2026 17:46
@vercel
Copy link

vercel bot commented Mar 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pos-demo Ready Ready Preview, Comment Mar 19, 2026 5:48pm

Request Review

@claude
Copy link

claude bot commented Mar 19, 2026

Claude finished @ignaciosantise's task in 1m 4s —— View job


Code Review

  • Gather context and read changed files
  • Review NFC hook implementation (useNfc.ts)
  • Review Header component changes
  • Review Android/iOS configuration changes
  • License compliance, breaking changes, and data classification subagents
  • Compile and post final review

Found 4 issue(s)

Issue 1: bytesToString will stack-overflow on large byte arrays

ID: usenfc-bytestring-stackoverflow-a3f1
File: wallets/rn_cli_wallet/src/hooks/useNfc.ts:244
Severity: HIGH
Category: code_quality

Context:

  • Pattern: String.fromCharCode(...bytes) spreads the array as variadic arguments
  • Risk: JavaScript call stacks have a fixed size; spreading arrays larger than ~65K elements throws RangeError: Maximum call stack size exceeded
  • Impact: NFC scan silently fails for any tag with a large payload (e.g. concatenated Smart Poster records); the error propagates up through scanNfcTag and shows a generic "NFC scan failed" toast with no actionable feedback
  • Trigger: Any NFC tag whose NDEF payload, type field, or nested Smart Poster payload exceeds the engine's argument limit

Recommendation:

function bytesToString(bytes: number[]): string {
  let result = '';
  for (const byte of bytes) {
    result += String.fromCharCode(byte);
  }
  return result;
}

Fix this →


Issue 2: NfcManager.start() called on every foreground resume without deduplication

ID: usenfc-start-repeated-calls-b2c8
File: wallets/rn_cli_wallet/src/hooks/useNfc.ts:104
Severity: HIGH
Category: performance

Context:

  • Pattern: register() calls NfcManager.start() every time AppState transitions to 'active' (line 186-191). Additionally useNfc calls start() independently at line 28 with no coordination
  • Risk: react-native-nfc-manager warns that calling start() multiple times is a no-op on some platforms but logs errors or leaks resources on others; on Android the NFC adapter initialization is heavier than a no-op
  • Impact: Unnecessary repeated initialization on every foreground transition; no NfcManager.stop() is ever called in cleanup, leaving the adapter running after the component unmounts
  • Trigger: Every time the user backgrounds and re-foregrounds the app on Android

Recommendation: Track whether start() has already been called with a module-level flag, and call NfcManager.stop() in cleanup:

let nfcStarted = false;

const register = async () => {
  if (registered || foregroundDispatchPaused) return;
  try {
    if (!nfcStarted) {
      await NfcManager.start();
      nfcStarted = true;
    }
    // ... rest of register
  }
};

// In cleanup:
return () => {
  subscription.remove();
  resumeForegroundDispatch = null;
  unregister();
  NfcManager.stop(); // release NFC adapter
};

Fix this →


Issue 3: Module-level mutable globals for cross-hook coordination

ID: usenfc-global-state-anti-pattern-d4e2
File: wallets/rn_cli_wallet/src/hooks/useNfc.ts:12-13
Severity: MEDIUM
Category: code_quality

Context:

  • Pattern: foregroundDispatchPaused and resumeForegroundDispatch are module-level variables mutated by both useNfc (via pauseForegroundDispatch/unpauseForegroundDispatch) and useNfcForegroundDispatch
  • Risk: Module globals persist across component unmounts/remounts and are shared across all instances. If useNfcForegroundDispatch is ever mounted in more than one component tree simultaneously, the second mount overwrites resumeForegroundDispatch (line 183), silently breaking the first instance's resume logic
  • Impact: Hard-to-debug "NFC stopped working" issues after component re-renders or fast refresh in dev
  • Trigger: Any remount of App.tsx, React strict mode double-effects in development, or future refactor that uses either hook in a second component

Recommendation: Encapsulate in a React context or merge both hooks into one so the coordinator state is scoped to the hook's closure, not the module.


Issue 4: NFC-sourced URIs passed to payment handler without allowlist validation

ID: usenfc-uri-no-allowlist-f9a3
File: wallets/rn_cli_wallet/src/hooks/useNfc.ts:129 / wallets/rn_cli_wallet/src/components/Header.tsx:29
Severity: MEDIUM
Category: security

Context:

  • Pattern: Any URI extracted from a physical NFC tag is forwarded directly to handleUriOrPaymentLink without checking the scheme or host
  • Risk: An attacker in physical proximity can place or substitute an NFC tag containing an arbitrary URI (e.g. a crafted wc: pairing URI pointing to a malicious relay, or a payment URL with a tampered amount/recipient)
  • Impact: Wallet may pair with an attacker-controlled session or process a payment request from a spoofed origin; user sees the standard connection/payment modal with no indication the source was an untrusted NFC tag
  • Trigger: Any NFC tag brought close to the device while the app is in the foreground (Android) or after the user taps "Tap to Pay" (iOS)

Recommendation: Before calling handleUriOrPaymentLink, validate the URI against expected schemes/hosts:

const ALLOWED_NFC_HOSTS = ['pay.walletconnect.com', 'staging.pay.walletconnect.com'];

function isAllowedNfcUri(uri: string): boolean {
  try {
    const { protocol, hostname } = new URL(uri);
    return (
      (protocol === 'wc:') ||
      (protocol === 'https:' && ALLOWED_NFC_HOSTS.includes(hostname))
    );
  } catch {
    return false;
  }
}

Fix this →


Subagent checks:

  • ✅ License: react-native-nfc-manager is MIT licensed — no compliance issues
  • ✅ Breaking changes: Private example app, all changes additive — no breaking changes
  • ✅ Data classification: No secrets, PII, or sensitive data concerns

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds NFC tap-to-pay support to rn_cli_wallet, wiring NFC tag scans into the existing URI/payment-link handling flow and updating mobile platform configuration for NFC and app/universal links.

Changes:

  • Add react-native-nfc-manager and implement NFC scanning + Android foreground tag interception.
  • Route NFC/app-link payment URLs through the centralized handleUriOrPaymentLink handler.
  • Update iOS entitlements and Android manifest/versionCode to support NFC + pay.walletconnect.com link handling.

Reviewed changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
wallets/rn_cli_wallet/package.json Adds react-native-nfc-manager dependency.
wallets/rn_cli_wallet/yarn.lock Locks react-native-nfc-manager resolution.
wallets/rn_cli_wallet/ios/Podfile.lock Adds CocoaPods entry for react-native-nfc-manager.
wallets/rn_cli_wallet/src/hooks/useNfc.ts Introduces NFC scan hook + Android foreground dispatch listener.
wallets/rn_cli_wallet/src/screens/App.tsx Registers Android foreground dispatch and adds pay link handling in deep-link handler.
wallets/rn_cli_wallet/src/components/Header.tsx Adds iOS “Tap” NFC button to trigger manual scan.
wallets/rn_cli_wallet/src/assets/NfcTap.tsx Adds NFC icon asset component.
wallets/rn_cli_wallet/ios/RNWeb3Wallet/RNWeb3Wallet.entitlements Adds associated domains + NFC session formats.
wallets/rn_cli_wallet/ios/RNWeb3Wallet/RNWeb3WalletDebug.entitlements Same as above for Debug.
wallets/rn_cli_wallet/android/app/src/main/AndroidManifest.xml Adds NFC feature flag and NDEF intent-filter for pay wallet domains.
wallets/rn_cli_wallet/android/app/build.gradle Bumps Android versionCode.
wallets/rn_cli_wallet/ios/RNWeb3Wallet.xcodeproj/xcshareddata/xcschemes/RNWallet.xcscheme Scheme version change.
wallets/rn_cli_wallet/ios/RNWeb3Wallet.xcodeproj/project.pbxproj Xcode project metadata updates related to build phases/provisioning.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@ignaciosantise
Copy link
Collaborator Author

All 4 issues from the Claude review have been addressed:

  1. bytesToString stack overflow — Replaced String.fromCharCode(...bytes) with a for loop to avoid spreading large arrays.

  2. NfcManager.start() called repeatedly — Added a module-level nfcStarted flag with ensureNfcStarted() so start() is only called once.

  3. Module-level mutable globals — Replaced the local registered flag in useNfcForegroundDispatch with the shared foregroundRegistered module-level flag. unpauseForegroundDispatch() now resets it so both hooks stay in sync.

  4. NFC URIs not validated — Added isAllowedNfcUri() allowlist (checks scheme + hostname) applied in both the Header's iOS NFC button and the Android foreground dispatch callback in App.tsx.

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.

2 participants