Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ describe('PermissionApproval', () => {
expect(navigationMock.navigate).toHaveBeenCalledTimes(0);
});

it('re-runs effect when pendingApprovals changes', async () => {
it('does not re-navigate for the same approval when pendingApprovals changes', async () => {
const navigationMock = {
navigate: jest.fn(),
};
Expand Down Expand Up @@ -329,12 +329,13 @@ describe('PermissionApproval', () => {
anotherRequestId: anotherApprovalRequest,
};

// The current approvalRequest is still the same (same metadata.id),
// so navigation should not fire again even though the queue changed.
mockApprovalRequest(approvalRequest, pendingApprovals2);

rerender(<PermissionApproval navigation={navigationMock} />);

// Effect should re-run when pendingApprovals content changes, causing navigation again
expect(navigationMock.navigate).toHaveBeenCalledTimes(2);
expect(navigationMock.navigate).toHaveBeenCalledTimes(1);
});

it('navigates when new approval added after queue cleared', async () => {
Expand Down Expand Up @@ -369,7 +370,10 @@ describe('PermissionApproval', () => {

const approvalRequest2 = {
type: ApprovalTypes.REQUEST_PERMISSIONS,
requestData: HOST_INFO_MOCK,
requestData: {
...HOST_INFO_MOCK,
metadata: { id: 'newRequestId' },
},
id: 'newRequestId',
// TODO: Replace "any" with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
16 changes: 11 additions & 5 deletions app/components/Approvals/PermissionApproval/PermissionApproval.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect } from 'react';
import { useEffect, useRef } from 'react';
import useApprovalRequest from '../../Views/confirmations/hooks/useApprovalRequest';
import { ApprovalTypes } from '../../../core/RPCMethods/RPCMethodMiddleware';
import { MetaMetricsEvents } from '../../../core/Analytics';
Expand Down Expand Up @@ -27,6 +27,9 @@ const PermissionApproval = (props: PermissionApprovalProps) => {
const { approvalRequest } = useApprovalRequest();
const totalAccounts = useSelector(selectAccountsLength);

// Prevents re-navigation for the same approval when pendingApprovals changes.
const lastNavigatedApprovalIdRef = useRef<string | null>(null);

const eventSource = useOriginSource({
origin: approvalRequest?.requestData?.metadata?.origin,
});
Expand All @@ -52,6 +55,11 @@ const PermissionApproval = (props: PermissionApprovalProps) => {
return;
}

if (lastNavigatedApprovalIdRef.current === id) {
return;
}
lastNavigatedApprovalIdRef.current = id;

const chainIds = getAllScopesFromPermission(
requestData.permissions[Caip25EndowmentPermissionName],
);
Expand Down Expand Up @@ -83,10 +91,8 @@ const PermissionApproval = (props: PermissionApprovalProps) => {
trackEvent,
createEventBuilder,
eventSource,
// Include pendingApprovals to ensure the effect re-runs when the approval queue changes.
// This prevents the queue from getting permanently stuck and handles cases where new approvals
// are added to the list, ensuring previous approvals that weren't rendered for other reasons
// can be processed. Ideally we can identify all cases where approval fail to render and then remove this dependency.
// Re-run when the queue changes so new approvals are picked up.
// The ref guard above prevents re-navigation for the same approval.
pendingApprovals,
]);

Expand Down
5 changes: 5 additions & 0 deletions app/components/UI/SDKLoading/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import { EdgeInsets, useSafeAreaInsets } from 'react-native-safe-area-context';
import Device from '../../../../app/util/device';
import { useTheme, useAssetFromTheme } from '../../../util/theme';

// Intrinsic dimensions of the Lottie assets (logo-light.json / logo-dark.json).
const LOTTIE_INTRINSIC_WIDTH = 1000;
const LOTTIE_INTRINSIC_HEIGHT = 1624;

const animationSize = Device.getDeviceWidth() / 2;

const loadingLight = require('./logo-light.json');
Expand Down Expand Up @@ -36,6 +40,7 @@ const createStyles = (colors: ThemeColors, _safeAreaInsets: EdgeInsets) =>
},
animation: {
width: animationSize,
aspectRatio: LOTTIE_INTRINSIC_WIDTH / LOTTIE_INTRINSIC_HEIGHT,
alignSelf: 'center',
alignItems: 'center',
justifyContent: 'center',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function connectWithWC({
}),
)
.catch((err) => {
console.warn(`DeepLinkManager failed to connect`, err);
console.warn(`connectWithWC failed`, err);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ export function handleMetaMaskDeeplink({
url.startsWith(`${PREFIXES.METAMASK}${ACTIONS.WC}`) ||
url.startsWith(`${PREFIXES.METAMASK}/${ACTIONS.WC}`)
) {
// console.debug(`test now deeplink hack ${url}`);
let fixedUrl = wcURL;
if (url.startsWith(`${PREFIXES.METAMASK}/${ACTIONS.WC}`)) {
fixedUrl = url.replace(
Expand Down
61 changes: 58 additions & 3 deletions app/core/WalletConnect/WalletConnectV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ export const ERROR_MESSAGES = {
INVALID_ID: 'Invalid Id',
};

// Safety timeout for the proposal serialization lock.
const PROPOSAL_LOCK_TIMEOUT_MS = 60_000;

// How long a pairing topic stays in seenTopics. Long enough to block the
// OS duplicate-delivery burst (~1 s), short enough to allow manual retries.
const SEEN_TOPIC_TTL_MS = 5_000;

export class WC2Manager {
private static instance: WC2Manager;
private static _initialized = false;
Expand All @@ -67,6 +74,15 @@ export class WC2Manager {
[pairingTopic: string]: { redirectUrl?: string; origin: string };
} = {};

// Serializes proposal handling so concurrent proposals don't fight
// over shared state (wc2Metadata, navigation, approval queue).
private proposalLock: Promise<void> = Promise.resolve();
// Deduplicates connect() calls for the same pairing topic.
// Entries auto-expire after SEEN_TOPIC_TTL_MS to allow manual retries.
private seenTopics = new Set<string>();
// Deduplicates session_proposal events (the relay can fire duplicates).
private handledProposalIds = new Set<number>();

private constructor(
web3Wallet: IWalletKit,
deeplinkSessions: {
Expand Down Expand Up @@ -417,6 +433,33 @@ export class WC2Manager {
}

async onSessionProposal(proposal: WalletKitTypes.SessionProposal) {
const handle = () =>
Promise.race([
this._handleSessionProposal(proposal),
new Promise<void>((resolve) =>
setTimeout(() => {
console.warn(
`WC2::session_proposal lock timeout for id=${proposal.id}`,
);
resolve();
}, PROPOSAL_LOCK_TIMEOUT_MS),
),
]);

// Chain onto the lock. The error handler ensures a failed proposal
// doesn't prevent subsequent proposals from being processed.
this.proposalLock = this.proposalLock.then(handle, handle);
await this.proposalLock;
}

private async _handleSessionProposal(
proposal: WalletKitTypes.SessionProposal,
) {
if (this.handledProposalIds.has(proposal.id)) {
return;
}
this.handledProposalIds.add(proposal.id);

// Open session proposal modal for confirmation / rejection
const { id, params } = proposal;

Expand Down Expand Up @@ -472,7 +515,7 @@ export class WC2Manager {
`WC2::session_proposal metadata url=${origin} hostname=${hostname}`,
);

// Save Connection info to redux store to be retrieved in ui.
// Save connection info to redux store for the approval UI.
store.dispatch(updateWC2Metadata({ url, name, icon, id: `${id}` }));

// Get the current chain ID to include in permissions
Expand Down Expand Up @@ -505,7 +548,6 @@ export class WC2Manager {
},
);

// Request permissions via the permissions controller
await permissionsController.requestPermissions(
{ origin: channelId },
{
Expand Down Expand Up @@ -550,11 +592,14 @@ export class WC2Manager {
id: proposal.id,
reason: getSdkError('USER_REJECTED_METHODS'),
});
// Clear stale metadata so it doesn't bleed into the next proposal.
store.dispatch(
updateWC2Metadata({ url: '', name: '', icon: '', id: '' }),
);
return;
}

try {
// Use the hostname for consistent permissions
const approvedAccounts = getPermittedAccounts(channelId);

DevLogger.log(`WC2::session_proposal getScopedPermissions`, {
Expand Down Expand Up @@ -735,6 +780,16 @@ export class WC2Manager {
return;
}

// Deduplicate: the OS can deliver the same deeplink multiple times.
if (this.seenTopics.has(params.topic)) {
return;
}
this.seenTopics.add(params.topic);
setTimeout(
() => this.seenTopics.delete(params.topic),
SEEN_TOPIC_TTL_MS,
);

// cleanup uri before pairing.
const cleanUri = wcUri.startsWith('wc://')
? wcUri.replace('wc://', 'wc:')
Expand Down
Loading