Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 4 additions & 27 deletions app/src/screens/prove/ConfirmBelongingScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
// NOTE: Converts to Apache-2.0 on 2029-06-11 per LICENSE.

import LottieView from 'lottie-react-native';
import React, { useEffect, useState } from 'react';
import React, { useState } from 'react';
import { ActivityIndicator, View } from 'react-native';
import type { StaticScreenProps } from '@react-navigation/native';
import { usePreventRemove } from '@react-navigation/native';

import { loadSelectedDocument, useSelfClient } from '@selfxyz/mobile-sdk-alpha';
import { usePrepareDocumentProof, useSelfClient } from '@selfxyz/mobile-sdk-alpha';

Check warning on line 11 in app/src/screens/prove/ConfirmBelongingScreen.tsx

View workflow job for this annotation

GitHub Actions / workspace-lint

Replace `·usePrepareDocumentProof,·useSelfClient·` with `⏎··usePrepareDocumentProof,⏎··useSelfClient,⏎`

Check warning on line 11 in app/src/screens/prove/ConfirmBelongingScreen.tsx

View workflow job for this annotation

GitHub Actions / build-deps

Replace `·usePrepareDocumentProof,·useSelfClient·` with `⏎··usePrepareDocumentProof,⏎··useSelfClient,⏎`
import {
PassportEvents,
ProofEvents,
Expand All @@ -24,7 +24,6 @@
import { useSettingStore } from '@/stores/settingStore';
import { flushAllAnalytics, trackNfcEvent } from '@/utils/analytics';
import { black, white } from '@/utils/colors';
import { notificationSuccess } from '@/utils/haptic';
import {
getFCMToken,
requestNotificationPermission,
Expand All @@ -34,35 +33,13 @@

const ConfirmBelongingScreen: React.FC<ConfirmBelongingScreenProps> = () => {
const selfClient = useSelfClient();
const { useProvingStore, trackEvent } = selfClient;
const { trackEvent } = selfClient;
const navigate = useHapticNavigation('Loading', {
params: {},
});
const [_requestingPermission, setRequestingPermission] = useState(false);
const currentState = useProvingStore(state => state.currentState);
const init = useProvingStore(state => state.init);
const setUserConfirmed = useProvingStore(state => state.setUserConfirmed);
const { setUserConfirmed, isReadyToProve } = usePrepareDocumentProof();
const setFcmToken = useSettingStore(state => state.setFcmToken);
const isReadyToProve = currentState === 'ready_to_prove';
useEffect(() => {
notificationSuccess();

const initializeProving = async () => {
try {
const selectedDocument = await loadSelectedDocument(selfClient);
if (selectedDocument?.data?.documentCategory === 'aadhaar') {
init(selfClient, 'register');
} else {
init(selfClient, 'dsc');
}
} catch (error) {
console.error('Error loading selected document:', error);
init(selfClient, 'dsc');
}
};

initializeProving();
}, [init, selfClient]);

const onOkPress = async () => {
try {
Expand Down
32 changes: 31 additions & 1 deletion packages/mobile-sdk-alpha/src/context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
// SPDX-License-Identifier: BUSL-1.1
// NOTE: Converts to Apache-2.0 on 2029-06-11 per LICENSE.

import { createContext, type PropsWithChildren, useContext, useMemo } from 'react';
import { createContext, type PropsWithChildren, useContext, useEffect, useMemo } from 'react';

Check failure on line 5 in packages/mobile-sdk-alpha/src/context.tsx

View workflow job for this annotation

GitHub Actions / workspace-lint

Run autofix to sort these imports!

Check failure on line 5 in packages/mobile-sdk-alpha/src/context.tsx

View workflow job for this annotation

GitHub Actions / lint

Run autofix to sort these imports!

import { createSelfClient } from './client';
import { SdkEvents } from './types/events';
import type { Adapters, Config, SelfClient } from './types/public';
import { loadSelectedDocument } from './documents/utils';

/**
* React context holding a {@link SelfClient} instance.
Expand Down Expand Up @@ -66,3 +67,32 @@
if (!client) throw new Error('useSelfClient must be used within a SelfClientProvider');
return client;
}

export function usePrepareDocumentProof() {

Check failure on line 71 in packages/mobile-sdk-alpha/src/context.tsx

View workflow job for this annotation

GitHub Actions / workspace-lint

Expected usePrepareDocumentProof before useSelfClient

Check failure on line 71 in packages/mobile-sdk-alpha/src/context.tsx

View workflow job for this annotation

GitHub Actions / lint

Expected usePrepareDocumentProof before useSelfClient
const selfClient = useSelfClient();
const { useProvingStore } = selfClient;
const currentState = useProvingStore(state => state.currentState);
const init = useProvingStore(state => state.init);
const setUserConfirmed = useProvingStore(state => state.setUserConfirmed);
const isReadyToProve = currentState === 'ready_to_prove';

useEffect(() => {
const initializeProving = async () => {
try {
const selectedDocument = await loadSelectedDocument(selfClient);
if (selectedDocument?.data?.documentCategory === 'aadhaar') {
init(selfClient, 'register');
} else {
init(selfClient, 'dsc');
}
} catch (error) {
console.error('Error loading selected document:', error);
init(selfClient, 'dsc');
Comment on lines +77 to +79
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 1, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Suggested change
} 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this code has only been moved, can be tackled separately

Copy link
Contributor

Choose a reason for hiding this comment

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

@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?

}
Comment on lines +73 to +80
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 1, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Suggested change
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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this code has only been moved, can be tackled separately (if needed)

Copy link
Contributor

Choose a reason for hiding this comment

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

@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! 👍

};

initializeProving();
}, [init, selfClient]);

return { setUserConfirmed, isReadyToProve };
Copy link

Choose a reason for hiding this comment

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

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.

Fix in Cursor Fix in Web

Copy link
Contributor

Choose a reason for hiding this comment

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

you trippin.

}
7 changes: 6 additions & 1 deletion packages/mobile-sdk-alpha/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,12 @@
export { SdkEvents } from './types/events';

// Components
export { SelfClientContext, SelfClientProvider, useSelfClient } from './context';
export {

Check warning on line 71 in packages/mobile-sdk-alpha/src/index.ts

View workflow job for this annotation

GitHub Actions / workspace-lint

Replace `⏎··SelfClientContext,⏎··SelfClientProvider,⏎··useSelfClient,⏎··usePrepareDocumentProof⏎` with `·SelfClientContext,·SelfClientProvider,·useSelfClient,·usePrepareDocumentProof·`

Check failure on line 71 in packages/mobile-sdk-alpha/src/index.ts

View workflow job for this annotation

GitHub Actions / workspace-lint

Expected usePrepareDocumentProof before useSelfClient

Check warning on line 71 in packages/mobile-sdk-alpha/src/index.ts

View workflow job for this annotation

GitHub Actions / lint

Replace `⏎··SelfClientContext,⏎··SelfClientProvider,⏎··useSelfClient,⏎··usePrepareDocumentProof⏎` with `·SelfClientContext,·SelfClientProvider,·useSelfClient,·usePrepareDocumentProof·`

Check failure on line 71 in packages/mobile-sdk-alpha/src/index.ts

View workflow job for this annotation

GitHub Actions / lint

Expected usePrepareDocumentProof before useSelfClient
SelfClientContext,
SelfClientProvider,
useSelfClient,
usePrepareDocumentProof
} from './context';

// Documents utils
export { SelfMobileSdk } from './entry';
Expand Down
Loading