Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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: 8 additions & 23 deletions app/src/screens/prove/ConfirmBelongingScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ 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';
import {
PassportEvents,
ProofEvents,
Expand All @@ -34,35 +37,17 @@ type ConfirmBelongingScreenProps = StaticScreenProps<Record<string, never>>;

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
2 changes: 1 addition & 1 deletion packages/mobile-sdk-alpha/src/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export { SCANNER_ERROR_CODES, notImplemented, sdkError } from './errors';

export { SdkEvents } from './types/events';

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

export {
clearPassportData,
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,9 +2,10 @@
// 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';

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

Expand Down Expand Up @@ -56,6 +57,35 @@ export function SelfClientProvider({
return <SelfClientContext.Provider value={client}>{children}</SelfClientContext.Provider>;
}

export function usePrepareDocumentProof() {
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.

}

/**
* Retrieves the current {@link SelfClient} from context.
*
Expand Down
2 changes: 1 addition & 1 deletion packages/mobile-sdk-alpha/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export { QRCodeScreen } from './components/screens/QRCodeScreen';
export { SdkEvents } from './types/events';

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

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