Skip to content

chore(pos-app): remove unused device ID - #644

Merged
ignaciosantise merged 1 commit into
mainfrom
austin-v2/dapps/pos-app-privacy-review
Oct 5, 2026
Merged

ignaciosantise merged 1 commit into
mainfrom
austin-v2/dapps/pos-app-privacy-review

Conversation

@ignaciosantise

Copy link
Copy Markdown
Collaborator

Summary

  • Removes the device ID that pos-app read with react-native-device-info's getUniqueId() on launch. It was left over from the old POS client setup (chore: use device id to init pos client #267), which needed a device ID to start.
  • After the move to @walletconnect/pay-core, its only use was the check if (!deviceId || !amount) return; before creating a payment in app/scan.tsx. The value was never sent to any API, logged, or passed to Sentry.
  • Removes deviceId and setDeviceId from useSettingsStore, bumps the persist version to 21, and adds a migration that deletes the device ID already saved on existing devices.
  • Removes getDeviceIdentifier() from utils/misc.ts, the effect in app/_layout.tsx, and the related tests and mocks. Adds a migration test.
  • react-native-device-info stays, because isTablet and isCameraPresent still use it.

Removing the check doesn't change behaviour: the root layout doesn't render until the settings store has loaded (_layout.tsx), so the scan screen never ran before the device ID existed.

Note: the email-receipt work also bumps the settings store to version 21. Whichever PR merges second needs to move to 22 and keep both migrations.

Test plan

  • npx jest: all 35 suites, 304 tests pass
  • npx expo lint is clean, and Prettier passes on the changed files
  • Start a payment on a device that already has saved settings (the migration runs) and on a fresh install

🤖 Generated with Claude Code

The device ID read via react-native-device-info's getUniqueId() was a
leftover from the old POS client init (#267). It was only used as a
gate before creating a payment and was never sent anywhere. Drop it
from the settings store (with a v21 migration that deletes the
persisted value), the root layout, the scan screen, and the tests.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
pos-demo Ready Ready Preview Oct 5, 2026 3:00pm UTC
2 Skipped Deployments
Project Deployment Actions Updated
react-native-appkit Ignored Ignored Oct 5, 2026 3:00pm UTC
react-native-walletkit Ignored Ignored Oct 5, 2026 3:00pm UTC

Request Review

@claude

claude Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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


Code Review

  • Read changed files
  • Run breaking changes analysis (in progress)
  • Post review findings

Found 1 issue(s)

Issue 1: Persist version 21 collision with in-flight email-receipt PR

ID: settingsstore-persist-version-collision-a3f1
File: dapps/pos-app/store/useSettingsStore.ts:260
Severity: MEDIUM
Category: correctness

Context:

  • Pattern: Both this PR and the email-receipt work bump version to 21 and add a version < 21 migration
  • Risk: Whichever merges second will have its version < 21 migration silently skipped on devices already at version 21; for those devices, deviceId will remain in persisted storage (or the email-receipt state won't be migrated, depending on ordering)
  • Impact: Stale deviceId persists in MMKV on affected devices, or email-receipt migration is skipped
  • Trigger: Any device that goes through the first PR's migration before the second PR ships

Recommendation: The PR description already flags this — whichever branch merges second must increment version to 22 and change its migration guard to version < 22. It's worth adding a CI check or reviewer checklist item to catch this before merge.

// Second PR to merge must become:
version: 22,
// and change the guard:
if (version < 22) {
  delete persistedState.deviceId; // or email-receipt migration
}

The rest of the change is clean:

  • Removed if (!deviceId || !amount) return guard in scan.tsx is safe — _layout.tsx renders nothing until _hasHydrated is true, which only flips after onRehydrateStorage completes, so deviceId was always already resolved by the time the scan screen mounted.
  • Migration delete persistedState.deviceId at version < 21 is correct and the test at useSettingsStore.test.ts:583 validates it properly.
  • react-native-device-info is correctly kept for isTablet / isCameraPresent usages.

Comment thread dapps/pos-app/store/useSettingsStore.ts Resolved
@ignaciosantise
ignaciosantise merged commit 589e262 into main Oct 5, 2026
9 of 11 checks passed
@ignaciosantise
ignaciosantise deleted the austin-v2/dapps/pos-app-privacy-review branch October 5, 2026 15:26
ignaciosantise added a commit that referenced this pull request Oct 5, 2026
Brings in the EVM transaction details view (#643) and the pos-app device
ID cleanup (#644). No conflicts.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
Preview – pos-demo — c07c8d7d Deployed Oct 5, 2026 by vercel[bot]
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.

1 participant