introduce usePrepareDocumentProof in mobile SDK #1176
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces direct ProvingStore usage in ConfirmBelongingScreen with a new SDK hook Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Screen as ConfirmBelongingScreen
participant Hook as usePrepareDocumentProof (SDK)
participant Client as SelfClient
participant Util as loadSelectedDocument
participant Nav as Navigation
rect rgb(240,248,255)
note right of Screen: Component mounts
Screen->>Hook: call usePrepareDocumentProof()
Hook->>Client: read proving refs/state
Hook->>Util: loadSelectedDocument(Client)
alt load succeeds
Util-->>Hook: selectedDocument
Hook->>Hook: init proving (register if Aadhaar else dsc)
else load fails
Util-->>Hook: error
Hook->>Hook: init proving (fallback dsc)
end
Hook-->>Screen: { isReadyToProve, setUserConfirmed }
end
rect rgb(245,255,240)
note right of User: User taps OK
User->>Screen: OK
Screen->>Screen: request notification perms & store FCM token
Screen->>Hook: setUserConfirmed()
Screen->>Nav: navigate to loading
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)packages/mobile-sdk-alpha/**/*.{ts,tsx}📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
Files:
**/*.{js,ts,tsx,jsx,sol,nr}📄 CodeRabbit inference engine (.cursorrules)
Files:
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (2)📚 Learning: 2025-08-24T18:54:04.809ZApplied to files:
📚 Learning: 2025-08-29T15:31:15.924ZApplied to files:
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/screens/prove/ConfirmBelongingScreen.tsx(2 hunks)packages/mobile-sdk-alpha/src/context.tsx(2 hunks)packages/mobile-sdk-alpha/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/src/index.ts
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/src/index.tsapp/src/screens/prove/ConfirmBelongingScreen.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/context.tsxpackages/mobile-sdk-alpha/src/index.ts
packages/mobile-sdk-alpha/src/index.ts
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Files:
packages/mobile-sdk-alpha/src/index.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/screens/prove/ConfirmBelongingScreen.tsx
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/screens/prove/ConfirmBelongingScreen.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
packages/mobile-sdk-alpha/src/context.tsx
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Applied to files:
packages/mobile-sdk-alpha/src/index.ts
🧬 Code graph analysis (2)
packages/mobile-sdk-alpha/src/context.tsx (2)
packages/mobile-sdk-alpha/src/index.ts (3)
usePrepareDocumentProof(75-75)useSelfClient(74-74)loadSelectedDocument(85-85)packages/mobile-sdk-alpha/src/proving/provingMachine.ts (1)
useProvingStore(367-1417)
app/src/screens/prove/ConfirmBelongingScreen.tsx (2)
packages/mobile-sdk-alpha/src/context.tsx (1)
usePrepareDocumentProof(71-98)packages/mobile-sdk-alpha/src/index.ts (1)
usePrepareDocumentProof(75-75)
🪛 GitHub Actions: Mobile SDK CI
packages/mobile-sdk-alpha/src/context.tsx
[error] 5-5: simple-import-sort/imports: Run autofix to sort these imports.
packages/mobile-sdk-alpha/src/index.ts
[warning] 1-1: Code style issues found in src/index.ts. Run Prettier with --write to fix.
🪛 GitHub Actions: Web CI
app/src/screens/prove/ConfirmBelongingScreen.tsx
[error] 11-11: Build failed: usePrepareDocumentProof is not exported by '@selfxyz/mobile-sdk-alpha' (imported in ConfirmBelongingScreen.tsx). Ensure the function is exported by the package version in use or adjust the import.
🪛 GitHub Actions: Workspace CI
packages/mobile-sdk-alpha/src/context.tsx
[error] 5-5: simple-import-sort/imports: Run autofix to sort these imports!
🪛 GitHub Check: lint
packages/mobile-sdk-alpha/src/context.tsx
[failure] 5-5:
Run autofix to sort these imports!
[failure] 71-71:
Expected usePrepareDocumentProof before useSelfClient
packages/mobile-sdk-alpha/src/index.ts
[warning] 71-71:
Replace ⏎··SelfClientContext,⏎··SelfClientProvider,⏎··useSelfClient,⏎··usePrepareDocumentProof⏎ with ·SelfClientContext,·SelfClientProvider,·useSelfClient,·usePrepareDocumentProof·
[failure] 71-71:
Expected usePrepareDocumentProof before useSelfClient
🪛 GitHub Check: workspace-lint
packages/mobile-sdk-alpha/src/context.tsx
[failure] 5-5:
Run autofix to sort these imports!
[failure] 71-71:
Expected usePrepareDocumentProof before useSelfClient
packages/mobile-sdk-alpha/src/index.ts
[warning] 71-71:
Replace ⏎··SelfClientContext,⏎··SelfClientProvider,⏎··useSelfClient,⏎··usePrepareDocumentProof⏎ with ·SelfClientContext,·SelfClientProvider,·useSelfClient,·usePrepareDocumentProof·
[failure] 71-71:
Expected usePrepareDocumentProof before useSelfClient
app/src/screens/prove/ConfirmBelongingScreen.tsx
[warning] 11-11:
Replace ·usePrepareDocumentProof,·useSelfClient· with ⏎··usePrepareDocumentProof,⏎··useSelfClient,⏎
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-deps
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
| init(selfClient, 'register'); | ||
| } else { | ||
| init(selfClient, 'dsc'); | ||
| } | ||
| } catch (error) { | ||
| console.error('Error loading selected document:', error); | ||
| init(selfClient, 'dsc'); | ||
| } |
There was a problem hiding this comment.
Await init call
Line 84 and Line 86 invoke init without awaiting. Because init returns a Promise, any rejection (e.g., missing private key or failing protocol fetch) bubbles up as an unhandled rejection, leaving the proving store half-initialized and making downstream UI appear stuck. Please await both branches (and the fallback in Line 90) so failures surface through the surrounding try/catch and the store can be safely reset.
- init(selfClient, 'register');
+ await init(selfClient, 'register');
} else {
- init(selfClient, 'dsc');
+ await init(selfClient, 'dsc');
}
} catch (error) {
- console.error('Error loading selected document:', error);
- init(selfClient, 'dsc');
+ console.error('Error loading selected document:', error instanceof Error ? error.message : 'unknown');
+ await init(selfClient, 'dsc');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| init(selfClient, 'register'); | |
| } else { | |
| init(selfClient, 'dsc'); | |
| } | |
| } catch (error) { | |
| console.error('Error loading selected document:', error); | |
| init(selfClient, 'dsc'); | |
| } | |
| await init(selfClient, 'register'); | |
| } else { | |
| await init(selfClient, 'dsc'); | |
| } | |
| } catch (error) { | |
| console.error( | |
| 'Error loading selected document:', | |
| error instanceof Error ? error.message : 'unknown' | |
| ); | |
| await init(selfClient, 'dsc'); | |
| } |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/context.tsx around lines 84 to 91, the calls to
init(selfClient, 'register') and init(selfClient, 'dsc') (including the fallback
in the catch) are not awaited even though init returns a Promise; change those
three calls to await init(...) so any rejection is caught by the surrounding
try/catch and the store can be reset safely; ensure the enclosing function is
async (or convert it to async) so awaiting is valid.
There was a problem hiding this comment.
this code has only been moved, can be tackled separately (if needed)
There was a problem hiding this comment.
@shazarre, understood! Since this is moved code, it makes sense to keep the refactoring focused and address the async handling in a follow-up if needed. Thanks for clarifying! 👍
| } catch (error) { | ||
| console.error('Error loading selected document:', error); | ||
| init(selfClient, 'dsc'); |
There was a problem hiding this comment.
Avoid logging PII
Line 89 logs the full error object from loadSelectedDocument. That error can include passport metadata or other sensitive identifiers, which would then land in device logs—clear violation of our no-PII logging rule for SDK code. Please log only a sanitized message (e.g., the generic string or a whitelisted code) instead of the entire object.
- console.error('Error loading selected document:', error);
+ console.error('Error loading selected document');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| console.error('Error loading selected document:', error); | |
| init(selfClient, 'dsc'); | |
| } catch (error) { | |
| console.error('Error loading selected document'); | |
| init(selfClient, 'dsc'); | |
| } |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/context.tsx around lines 88 to 90, the catch
block currently calls console.error with the full error object which may contain
PII; change this to log only a sanitized message or whitelisted error code (for
example: console.error('Error loading selected document: <LOAD_DOC_ERROR>') or
use a telemetry call that records a non-PII error code), and do not include the
error object or stack in logs; keep the existing init(selfClient, 'dsc') call
unchanged so recovery still runs.
There was a problem hiding this comment.
this code has only been moved, can be tackled separately
There was a problem hiding this comment.
@shazarre, understood—I'll defer this for now since it's pre-existing code. Would you like me to open a follow-up issue to track the PII logging cleanup separately?
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/src/screens/prove/ConfirmBelongingScreen.tsx (1)
48-50: Haptic feedback is presentThe
notificationSuccess()haptic feedback fires on component mount via theuseEffectwith empty dependencies. The past review concern appears to be resolved or was based on outdated code.
🧹 Nitpick comments (1)
app/src/screens/prove/ConfirmBelongingScreen.tsx (1)
117-130: Consider extracting inline style objectThe inline style object at line 125
{{ flexDirection: 'row', alignItems: 'center' }}creates a new reference on each render. While React Native optimizes this, extracting it to a constant would be slightly more performant.const loadingContainerStyle = { flexDirection: 'row' as const, alignItems: 'center' as const }; // Then in JSX: <View style={loadingContainerStyle}>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/screens/prove/ConfirmBelongingScreen.tsx(2 hunks)packages/mobile-sdk-alpha/src/context.tsx(2 hunks)packages/mobile-sdk-alpha/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/mobile-sdk-alpha/src/context.tsx
🧰 Additional context used
📓 Path-based instructions (6)
packages/mobile-sdk-alpha/src/index.ts
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts
Files:
packages/mobile-sdk-alpha/src/index.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/src/index.ts
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-alpha/src/index.tsapp/src/screens/prove/ConfirmBelongingScreen.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/index.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (app/AGENTS.md)
Type checking must pass before PRs (yarn types)
Files:
app/src/screens/prove/ConfirmBelongingScreen.tsx
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/screens/prove/ConfirmBelongingScreen.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.
Applied to files:
packages/mobile-sdk-alpha/src/index.ts
🧬 Code graph analysis (1)
app/src/screens/prove/ConfirmBelongingScreen.tsx (1)
packages/mobile-sdk-alpha/src/context.tsx (1)
usePrepareDocumentProof(60-87)
🪛 GitHub Actions: Web CI
app/src/screens/prove/ConfirmBelongingScreen.tsx
[error] 12-12: Build failed: 'usePrepareDocumentProof' is not exported by '@selfxyz/mobile-sdk-alpha'; import may be broken due to export mismatch. Check package exports or update the import.
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Cursor Bugbot
- GitHub Check: Cursor Bugbot
- GitHub Check: build-deps
- GitHub Check: e2e-ios
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
🔇 Additional comments (4)
app/src/screens/prove/ConfirmBelongingScreen.tsx (3)
11-14: LGTM! Clean import structureThe import correctly includes the new
usePrepareDocumentProofhook from the SDK, following the established pattern.
39-46: LGTM! Proper hook integrationThe refactor correctly replaces direct
useProvingStoreaccess with the high-levelusePrepareDocumentProofhook, simplifying the component and delegating initialization to the SDK.
52-87: LGTM! Robust error handling and security-consciousThe handler properly manages async operations, notification permissions, and error states. No sensitive data is logged (only generic error messages), adhering to security guidelines.
packages/mobile-sdk-alpha/src/index.ts (1)
71-71: Export correctly declared
usePrepareDocumentProofis exported inpackages/mobile-sdk-alpha/src/context.tsx(line 60), so the re-export insrc/index.tsis valid.Likely an incorrect or invalid review comment.
| initializeProving(); | ||
| }, [init, selfClient]); | ||
|
|
||
| return { setUserConfirmed, isReadyToProve }; |
There was a problem hiding this comment.
Bug: Stale Closures and Missing Dependencies
The useEffect in usePrepareDocumentProof re-initializes excessively due to init (from a Zustand selector) changing on every render, and omits loadSelectedDocument from its dependencies, potentially causing stale closures. Separately, the returned setUserConfirmed function expects a selfClient argument but is called without it, leading to incorrect behavior.
Based on @aaronmgdr work in #1154 this PR introduces first high level hook (
usePrepareDocumentProof) for interacting with SDK as a PoC inConfirmBelongingSreen.Note
Introduces
usePrepareDocumentProofin the SDK and refactorsConfirmBelongingScreento rely on it for proof readiness and confirmation, updating exports accordingly.usePrepareDocumentProofinpackages/mobile-sdk-alpha/src/context.tsxto initialize proving (based onloadSelectedDocument) and exposeisReadyToProveandsetUserConfirmed.usePrepareDocumentProoffrompackages/mobile-sdk-alpha/src/browser.tsandpackages/mobile-sdk-alpha/src/index.ts.app/src/screens/prove/ConfirmBelongingScreen.tsxto useusePrepareDocumentProofinstead of manual proving store/init logic; simplifies effect and readiness handling.Written by Cursor Bugbot for commit feb40c4. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Refactor