feat: support Expo Go with software-backed device signer fallback [WAL-9469]#1714
feat: support Expo Go with software-backed device signer fallback [WAL-9469]#1714albertoelias-crossmint wants to merge 3 commits intowallets-v1from
Conversation
…L-9469] Co-Authored-By: Alberto Elias <alberto.elias@paella.dev>
🦋 Changeset detectedLatest commit: 35685c5 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Original prompt from Alberto Elias
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Prompt To Fix All With AIThis is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 82
Comment:
**Base64 characters invalid in SecureStore keys**
`expo-secure-store` keys may only contain alphanumeric characters, `.`, `-`, and `_` ([docs](https://docs.expo.dev/versions/latest/sdk/securestore/)). Base64-encoded strings can contain `+`, `/`, and `=`, which are all invalid key characters. This will cause runtime errors when storing or retrieving pending keys.
For example, an uncompressed P-256 public key is 65 bytes → 88 base64 characters, and will very likely include `+`, `/`, or `=`.
The same issue applies to `mapAddressToKey` (line 90) and `deletePendingKey` (line 136) which also use `publicKeyBase64` in the key.
Consider using base64url encoding (replacing `+` with `-`, `/` with `_`, stripping `=`) or hex encoding for the SecureStore key portion:
```suggestion
await SecureStore.setItemAsync(`${PENDING_KEY_PREFIX}${publicKeyBase64.replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '')}`, privateKeyHex);
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 117-118
Comment:
**Verify signing prehash behavior matches native side**
With `@noble/curves` v1.x (resolved to 1.9.7), the default is `prehash: false`, meaning `p256.sign()` treats the input as an **already-hashed digest** and signs it directly without hashing.
The native iOS Secure Enclave typically hashes the message internally (via `SecKeyCreateSignature` with `ecdsaSignatureMessageX962SHA256`). If the native side hashes the incoming bytes and then signs, but this software fallback does **not** hash (it passes the bytes directly to the ECDSA math), then signatures will be incompatible — a key generated on one side cannot verify against the other.
Looking at `wallet.ts`, the API may already provide a pre-hashed digest (keccak256), which means the native side might be using a raw-signing mode too. Please verify that:
1. The native `SecureEnclaveKeyStorage.signMessage()` and `KeystoreKeyStorage.signMessage()` treat the decoded base64 message bytes the same way (raw ECDSA without re-hashing), **or**
2. If the native side does hash, add `{ prehash: true }` here to match.
This is the signing compatibility item flagged in the PR's review checklist and is critical to verify before merging.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 90-91
Comment:
**Inconsistent key encoding will break key lookups**
When `generateKey` stores a pending key at `PENDING_KEY_PREFIX + publicKeyBase64`, the `publicKeyBase64` contains standard base64 characters (`+`, `/`, `=`). If the base64 key issue from `generateKey` is fixed by using base64url, this lookup must use the same encoding. The same applies to `deletePendingKey` at line 136.
Recommend extracting a helper like `safeStoreKey(base64: string)` that consistently converts base64 to a SecureStore-safe format, and using it in all places that incorporate `publicKeyBase64` into a store key.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "feat: support Expo G..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| await SecureStore.setItemAsync(`${ADDRESS_KEY_PREFIX}${params.address}`, privateKeyHex); | ||
| await SecureStore.setItemAsync(`${ADDRESS_KEY_PREFIX}${params.address}_pub`, publicKeyBase64); | ||
| } else { | ||
| await SecureStore.setItemAsync(`${PENDING_KEY_PREFIX}${publicKeyBase64}`, privateKeyHex); |
There was a problem hiding this comment.
expo-secure-store keys may only contain alphanumeric characters, ., -, and _ (docs). Base64-encoded strings can contain +, /, and =, which are all invalid key characters. This will cause runtime errors when storing or retrieving pending keys.
For example, an uncompressed P-256 public key is 65 bytes → 88 base64 characters, and will very likely include +, /, or =.
The same issue applies to mapAddressToKey (line 90) and deletePendingKey (line 136) which also use publicKeyBase64 in the key.
Consider using base64url encoding (replacing + with -, / with _, stripping =) or hex encoding for the SecureStore key portion:
| await SecureStore.setItemAsync(`${PENDING_KEY_PREFIX}${publicKeyBase64}`, privateKeyHex); | |
| await SecureStore.setItemAsync(`${PENDING_KEY_PREFIX}${publicKeyBase64.replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '')}`, privateKeyHex); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 82
Comment:
**Base64 characters invalid in SecureStore keys**
`expo-secure-store` keys may only contain alphanumeric characters, `.`, `-`, and `_` ([docs](https://docs.expo.dev/versions/latest/sdk/securestore/)). Base64-encoded strings can contain `+`, `/`, and `=`, which are all invalid key characters. This will cause runtime errors when storing or retrieving pending keys.
For example, an uncompressed P-256 public key is 65 bytes → 88 base64 characters, and will very likely include `+`, `/`, or `=`.
The same issue applies to `mapAddressToKey` (line 90) and `deletePendingKey` (line 136) which also use `publicKeyBase64` in the key.
Consider using base64url encoding (replacing `+` with `-`, `/` with `_`, stripping `=`) or hex encoding for the SecureStore key portion:
```suggestion
await SecureStore.setItemAsync(`${PENDING_KEY_PREFIX}${publicKeyBase64.replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '')}`, privateKeyHex);
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Addressed in 35685c5. Added safeStoreKey() helper that converts base64 to base64url (+→-, /→_, strip =) before using public keys in SecureStore key names. Applied consistently in generateKey, mapAddressToKey, and deletePendingKey.
| const messageBytes = base64ToBytes(message); | ||
| const signature = p256.sign(messageBytes, privateKey, { lowS: true }); |
There was a problem hiding this comment.
With @noble/curves v1.x (resolved to 1.9.7), the default is prehash: false, meaning p256.sign() treats the input as an already-hashed digest and signs it directly without hashing.
The native iOS Secure Enclave typically hashes the message internally (via SecKeyCreateSignature with ecdsaSignatureMessageX962SHA256). If the native side hashes the incoming bytes and then signs, but this software fallback does not hash (it passes the bytes directly to the ECDSA math), then signatures will be incompatible — a key generated on one side cannot verify against the other.
Looking at wallet.ts, the API may already provide a pre-hashed digest (keccak256), which means the native side might be using a raw-signing mode too. Please verify that:
- The native
SecureEnclaveKeyStorage.signMessage()andKeystoreKeyStorage.signMessage()treat the decoded base64 message bytes the same way (raw ECDSA without re-hashing), or - If the native side does hash, add
{ prehash: true }here to match.
This is the signing compatibility item flagged in the PR's review checklist and is critical to verify before merging.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Line: 117-118
Comment:
**Verify signing prehash behavior matches native side**
With `@noble/curves` v1.x (resolved to 1.9.7), the default is `prehash: false`, meaning `p256.sign()` treats the input as an **already-hashed digest** and signs it directly without hashing.
The native iOS Secure Enclave typically hashes the message internally (via `SecKeyCreateSignature` with `ecdsaSignatureMessageX962SHA256`). If the native side hashes the incoming bytes and then signs, but this software fallback does **not** hash (it passes the bytes directly to the ECDSA math), then signatures will be incompatible — a key generated on one side cannot verify against the other.
Looking at `wallet.ts`, the API may already provide a pre-hashed digest (keccak256), which means the native side might be using a raw-signing mode too. Please verify that:
1. The native `SecureEnclaveKeyStorage.signMessage()` and `KeystoreKeyStorage.signMessage()` treat the decoded base64 message bytes the same way (raw ECDSA without re-hashing), **or**
2. If the native side does hash, add `{ prehash: true }` here to match.
This is the signing compatibility item flagged in the PR's review checklist and is critical to verify before merging.
How can I resolve this? If you propose a fix, please make it concise.
packages/client/device-signer-expo/src/SoftwareDeviceSignerKeyStorage.ts
Outdated
Show resolved
Hide resolved
Co-Authored-By: Alberto Elias <alberto.elias@paella.dev>
…clarify prehash behavior Co-Authored-By: Alberto Elias <alberto.elias@paella.dev>
🔥 Smoke Test Results✅ Status: Passed Statistics
✅ All smoke tests passed!All critical flows are working correctly. This is a non-blocking smoke test. Full regression tests run separately. |
|
Last reviewed commit: "fix: address PR revi..." |
|
Superseded by #1728 which targets |
2 participants
Description
The
@crossmint/expo-device-signerpackage uses a custom Expo native module (CrossmintDeviceSigner) that is unavailable in Expo Go. This causedCrossmintWalletProviderto crash on mount when running in Expo Go because it eagerly instantiatedNativeDeviceSignerKeyStorage.This PR adds a software-backed fallback (
SoftwareDeviceSignerKeyStorage) that uses@noble/curvesfor P-256 key operations andexpo-secure-storefor encrypted key persistence, along with acreateDeviceSignerKeyStorage()factory that auto-detects the environment and returns the appropriate implementation.Changes:
SoftwareDeviceSignerKeyStorage— pure JS implementation ofDeviceSignerKeyStorageusing@noble/curves/p256+expo-secure-store. Not hardware-backed, but functionally equivalent for development/prototyping.createDeviceSignerKeyStorage()— factory that tries to load the native module; falls back to the software implementation if unavailable.NativeDeviceSignerKeyStorage— improved error message when the native module is missing (was an opaque crash).CrossmintWalletProvider— swapped directNativeDeviceSignerKeyStorageinstantiation for the auto-detecting factory.Updates since last revision
Addressed automated review feedback:
safeStoreKey()helper that converts base64 to base64url (+→-,/→_, strip=) before using public keys in SecureStore key names. Applied consistently ingenerateKey,mapAddressToKey, anddeletePendingKey.wallet.tsconfirms the message passed tosignMessageis already a hash digest (e.g. keccak256). Added inline comment documenting whyprehash: false(the default) is correct.wallets-v1to incorporate upstream changes.Human review checklist
prehash: false, assuming the message is already a hash digest. This matches whatwallet.tssends (keccak256 hash), but should be verified against the nativeCrossmintDeviceSignerSwift package's actual signing behavior. If the native side applies SHA-256 internally viaSecKeyCreateSignature(ecdsaSignatureMessageX962SHA256), the software side would needprehash: trueto match.btoa/atobusage: The software impl usesbtoa/atobfor base64 encoding. These are available in Hermes (RN 0.74+) — confirm this covers the supported RN version range (>=0.74.0per peer deps).generateKeytype signature: The abstractDeviceSignerKeyStorageacceptsbiometricPolicy/biometricExpirationTime. The software impl ignores these params — confirm this is acceptable for the Expo Go use case.expo-modules-coremarked optional but statically imported:NativeDeviceSignerKeyStorage.tshas a top-levelimport { requireNativeModule } from "expo-modules-core". If the package were truly absent, this would fail before the try/catch runs. In practiceexpo-modules-coreis always present in Expo projects — confirm this assumption is safe.hasKey()lookup stores all public keys as a JSON array in a single SecureStore entry. For apps with many keys, this could approach SecureStore's 2KB value limit.Test plan
pnpm build:libspasses)pnpm lintpasses)pnpm test:vitest)Package updates
@crossmint/expo-device-signer: minor (newSoftwareDeviceSignerKeyStorage,createDeviceSignerKeyStorage,isNativeModuleAvailableexports; added@noble/curvesdependency;expo-secure-store+expo-modules-coreas optional peer deps)@crossmint/client-sdk-react-native-ui: patch (uses factory instead of directNativeDeviceSignerKeyStorage).changeset/expo-go-device-signer-fallback.mdLink to Devin session: https://crossmint.devinenterprise.com/sessions/92aaa470742247e88827749387401d9e
Requested by: @albertoelias-crossmint