Skip to content

refactor: centralize NFC scanning into NFCUtils#3941

Open
myxmaster wants to merge 1 commit intoZeusLN:masterfrom
myxmaster:refactor/centralize-nfc-scan
Open

refactor: centralize NFC scanning into NFCUtils#3941
myxmaster wants to merge 1 commit intoZeusLN:masterfrom
myxmaster:refactor/centralize-nfc-scan

Conversation

@myxmaster
Copy link
Copy Markdown
Collaborator

Description

#3821 should be merged first.

  • Centralized enableNfc() from Send, Receive, ReceiveEcash, and OpenChannel into scanNfcTag() in NFCUtils
    • disableNfc() (which only cleared event listeners and was misnamed) is inlined into scanNfcTag()
  • Views keep a thin scanNfc() wrapper (renamed from enableNfc()) that calls scanNfcTag() and passes the result to their own validateAddress() / validateNodeUri()
  • Added NFCUtils.test.ts with unit tests for nfcUtf8ArrayToStr

Side effects:

  • OpenChannel now also has .catch() on NfcManager.start() (was missing)
  • Ndef.text.decodePayload is now applied universally (was previously skipped in OpenChannel; safe since node URIs never match the https?/lnurl guard and always fall back to nfcUtf8ArrayToStr)

Note:
I was unable to test NFC scanning end-to-end: the reading device consistently receives tag discovery events with an empty ndefMessage, making the scan silently fail. I suspect this requires a react-native-hce update to fix, but I don't have a second debug device available for broadcasting. The refactored logic is identical to what was previously spread across the four views though, so it should be fine.

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Configuration change
  • Locales update
  • Quality assurance
  • Other

Checklist

  • I’ve run yarn run tsc and made sure my code compiles correctly
  • I’ve run yarn run lint and made sure my code didn’t contain any problematic patterns
  • I’ve run yarn run prettier and made sure my code is formatted correctly
  • I’ve run yarn run test and made sure all of the tests pass

Testing

If you modified or added a utility file, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms (please specify OS version and phone model/VM):

  • Android
  • iOS

I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):

On-device

  • LDK Node
  • Embedded LND

Remote

  • LND (REST)
  • LND (Lightning Node Connect)
  • Core Lightning (CLNRest)
  • Nostr Wallet Connect
  • LndHub

Locales

  • I’ve added new locale text that requires translations
  • I’m aware that new translations should be made on the ZEUS Transfix page and not directly to this repo

Third Party Dependencies and Packages

  • Contributors will need to run yarn after this PR is merged in
  • 3rd party dependencies have been modified:
    • verify that package.json and yarn.lock have been properly updated
    • verify that dependencies are installed for both iOS and Android platforms

Other:

  • Changes were made that require an update to the README
  • Changes were made that require an update to onboarding

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors NFC functionality by centralizing logic into a shared utility, reducing duplication across multiple views including Send, Receive, and OpenChannel. It introduces a scanNfcTag helper and improves the user experience on Android by handling cases where NFC is disabled, providing a direct link to system settings. Review feedback suggests replacing console.warn with a dedicated logger and refining the handling of NFC scan results to ensure robustness and correct case-sensitivity for Lightning addresses.

@myxmaster
Copy link
Copy Markdown
Collaborator Author

After geminis suggestion (#3942 (comment)):

Updated nfcUtf8ArrayToStr to use TextDecoder instead of manual bit manipulation, and renamed it to decodeNdefTextPayload to better reflect what it actually does.

@kaloudis
Copy link
Copy Markdown
Contributor

kaloudis commented Apr 4, 2026

Please rebase

@kaloudis kaloudis added Refactor Code that needs to be refactored NFC labels Apr 4, 2026
@myxmaster myxmaster force-pushed the refactor/centralize-nfc-scan branch from aabe1d9 to 4e4847a Compare April 4, 2026 07:58
@myxmaster
Copy link
Copy Markdown
Collaborator Author

done

@myxmaster myxmaster force-pushed the refactor/centralize-nfc-scan branch from 4e4847a to bb513f4 Compare April 6, 2026 09:41
Copy link
Copy Markdown
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

tACK

@myxmaster myxmaster force-pushed the refactor/centralize-nfc-scan branch from bb513f4 to feff21f Compare April 9, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NFC Refactor Code that needs to be refactored

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants