Skip to content
Closed
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 @@ -8,6 +8,7 @@ import MarketsWonCard from './PredictPositionsHeader';

// Mock account utilities
jest.mock('../../utils/accounts', () => ({
...jest.requireActual('../../utils/accounts'),
getEvmAccountFromSelectedAccountGroup: jest.fn(() => ({
id: 'test-account-id',
address: '0x1234567890123456789012345678901234567890',
Expand Down
67 changes: 67 additions & 0 deletions app/components/UI/Predict/controllers/PredictController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2434,6 +2434,41 @@ describe('PredictController', () => {
},
);
});

it('succeeds when state key is checksummed but signer address is lowercase', async () => {
// Arrange - simulates the real-world case mismatch: selector returns
// checksummed EIP-55 address, but state was keyed by a differently-cased address
const checksummedAddress =
'0x1234567890123456789012345678901234567890'.toUpperCase() as `0x${string}`;
const mockBatchId = 'claim-batch-case';
await withController(async ({ controller }) => {
mockPolymarketProvider.getPositions = jest
.fn()
.mockResolvedValue([
createMockPosition({ id: 'pos-1', claimable: true }),
]);
mockPolymarketProvider.prepareClaim = jest
.fn()
.mockResolvedValue(mockClaim);
(addTransactionBatch as jest.Mock).mockResolvedValue({
batchId: mockBatchId,
});

// Seed state with the checksummed key
controller.updateStateForTesting((state) => {
state.claimablePositions[checksummedAddress] = [
createMockPosition({ id: 'pos-1', claimable: true }),
];
});

// Act - signer address comes back as lowercase (default mock)
const result = await controller.claimWithConfirmation({});

// Assert - should find the positions despite the case difference
expect(result.batchId).toBe(mockBatchId);
expect(result.status).toBe(PredictClaimStatus.PENDING);
});
});
});

describe('getUnrealizedPnL', () => {
Expand Down Expand Up @@ -4969,6 +5004,38 @@ describe('PredictController', () => {
expect(controller.state.claimablePositions[testAddress]).toEqual([]);
});
});

it('matches state key case-insensitively when address casing differs', async () => {
// Arrange - state keyed by checksummed address, but confirmClaim called with lowercase
await withController(async ({ controller }) => {
const checksummedKey = '0xABCDEF1234567890ABCDEF1234567890ABCDEF12';
const lowercaseArg = checksummedKey.toLowerCase() as `0x${string}`;
const mockPositions = [
createMockPosition({
id: 'position-1',
status: PredictPositionStatus.WON,
}),
];

controller.updateStateForTesting((state) => {
state.claimablePositions[checksummedKey] = mockPositions;
});

mockPolymarketProvider.confirmClaim = jest.fn();

// Act
controller.confirmClaim({ address: lowercaseArg });

// Assert - found the positions despite case mismatch; state cleared.
// getSigner is called with matchedKey (the checksummed state key),
// so the signer address reflects that casing.
expect(mockPolymarketProvider.confirmClaim).toHaveBeenCalledWith({
positions: mockPositions,
signer: expect.objectContaining({ address: checksummedKey }),
});
expect(controller.state.claimablePositions[checksummedKey]).toEqual([]);
});
});
});

describe('getPositions', () => {
Expand Down
34 changes: 19 additions & 15 deletions app/components/UI/Predict/controllers/PredictController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import {
PredictTradeStatus,
PredictTradeStatusValue,
} from '../constants/eventNames';
import { findAddressKey } from '../utils/accounts';
import { validateDepositTransactions } from '../utils/validateTransactions';
import { PolymarketProvider } from '../providers/polymarket/PolymarketProvider';
import { Signer } from '../providers/types';
Expand Down Expand Up @@ -1566,6 +1567,14 @@ export class PredictController extends BaseController<
}
}

private getClaimablePositionsByAddress(
address: string,
): { positions: PredictPosition[]; matchedKey: string } | undefined {
const matchedKey = findAddressKey(this.state.claimablePositions, address);
if (!matchedKey) return undefined;
return { positions: this.state.claimablePositions[matchedKey], matchedKey };
}

Comment on lines +1570 to +1577
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should not need this change. Also, we now use React Query for our positions queries and you should be reusing our hooks (or at least our queries) rather than creating custom fetch logic.

Let's have a sync on this if needed. We're happy to help fix the issues across the Predict section in the new homepage.

async claimWithConfirmation(
_params: ClaimParams = {},
): Promise<PredictClaim> {
Expand Down Expand Up @@ -1595,8 +1604,10 @@ export class PredictController extends BaseController<

const signer = this.getSigner();

// Get claimable positions from state
const claimablePositions = this.state.claimablePositions[signer.address];
// Get claimable positions from state (case-insensitive address match)
const claimablePositions = this.getClaimablePositionsByAddress(
signer.address,
)?.positions;

if (!claimablePositions || claimablePositions.length === 0) {
throw new Error('No claimable positions found');
Expand Down Expand Up @@ -1723,30 +1734,23 @@ export class PredictController extends BaseController<
public confirmClaim({ address }: { address?: string }): void {
const provider = this.provider;

const normalizedAddress = (
address ?? this.getSigner().address
).toLowerCase();
const matchedAddress = Object.keys(this.state.claimablePositions).find(
(addressKey) => addressKey.toLowerCase() === normalizedAddress,
);
const resolvedAddress = address ?? this.getSigner().address;
const claimResult = this.getClaimablePositionsByAddress(resolvedAddress);

if (!matchedAddress) {
if (!claimResult || claimResult.positions.length === 0) {
return;
}

const signer = this.getSigner(matchedAddress);
const claimedPositions = this.state.claimablePositions[matchedAddress];
if (!claimedPositions || claimedPositions.length === 0) {
return;
}
const { positions: claimedPositions, matchedKey } = claimResult;
const signer = this.getSigner(matchedKey);

provider.confirmClaim?.({
positions: claimedPositions,
signer,
});

this.update((state) => {
state.claimablePositions[matchedAddress] = [];
state.claimablePositions[matchedKey] = [];
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ jest.mock('../../../../core/Engine', () => ({
}));

jest.mock('../utils/accounts', () => ({
...jest.requireActual('../utils/accounts'),
getEvmAccountFromSelectedAccountGroup: jest.fn(() => ({
address: MOCK_ADDRESS,
})),
Expand Down
13 changes: 11 additions & 2 deletions app/components/UI/Predict/hooks/usePredictClaim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { ensureError } from '../utils/predictErrorHandler';
import { usePredictTrading } from './usePredictTrading';
import { ConfirmationLoader } from '../../../Views/confirmations/components/confirm/confirm-component';
import Routes from '../../../../constants/navigation/Routes';
import { PredictClaimStatus } from '../types';

export const usePredictClaim = () => {
const { navigateToConfirmation } = useConfirmNavigation();
Expand All @@ -20,15 +21,22 @@ export const usePredictClaim = () => {
const { toastRef } = useContext(ToastContext);
const navigation = useNavigation();

const claim = useCallback(async () => {
const claim = useCallback(async (): Promise<{
wasCancelled: boolean;
succeeded: boolean;
}> => {
try {
navigateToConfirmation({
headerShown: false,
loader: ConfirmationLoader.PredictClaim,
// TODO: remove once navigation stack is fixed properly
stack: Routes.PREDICT.ROOT,
});
await claimWinnings({});
const result = await claimWinnings({});
return {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We weren’t using the result before. Since we’re using it now, instead of returning flags, let’s return the result object itself so whoever consumes it can use it however they need.

wasCancelled: result?.status === PredictClaimStatus.CANCELLED,
succeeded: result?.status === PredictClaimStatus.PENDING,
};
} catch (err) {
// Log error with claim context
Logger.error(ensureError(err), {
Expand Down Expand Up @@ -70,6 +78,7 @@ export const usePredictClaim = () => {
},
},
});
return { wasCancelled: false, succeeded: false };
}
}, [
claimWinnings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ jest.mock('../selectors/predictController', () => ({
}));

jest.mock('../utils/accounts', () => ({
...jest.requireActual('../utils/accounts'),
getEvmAccountFromSelectedAccountGroup: () => ({
address: '0x1234567890123456789012345678901234567890',
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const mockGetEvmAccountFromSelectedAccountGroup = jest.fn(() => ({
type: 'eip155:eoa',
}));
jest.mock('../utils/accounts', () => ({
...jest.requireActual('../utils/accounts'),
getEvmAccountFromSelectedAccountGroup: () =>
mockGetEvmAccountFromSelectedAccountGroup(),
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ jest.mock('./usePredictWithdraw', () => ({
}));

jest.mock('../utils/accounts', () => ({
...jest.requireActual('../utils/accounts'),
getEvmAccountFromSelectedAccountGroup: jest.fn(() => ({
address: selectedAddress,
})),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { selectSelectedAccountGroupId } from '../../../../selectors/multichainAc
import type { Hex } from '@metamask/utils';
import { usePredictClaim } from './usePredictClaim';
import { usePredictDeposit } from './usePredictDeposit';
import { _clearPositionsCache } from '../../../Views/Homepage/Sections/Predictions/hooks/usePredictPositionsForHomepage';
import { usePredictWithdraw } from './usePredictWithdraw';
import { store } from '../../../../store';
import { selectTransactionMetadataById } from '../../../../selectors/transactionController';
Expand Down Expand Up @@ -254,6 +255,9 @@ export const usePredictToastRegistrations = (): ToastRegistration[] => {
queryKey: predictQueries.positions.keys.all(),
});

// Clear homepage positions cache so next visit fetches fresh data
_clearPositionsCache();

showSuccessToast({
showToast,
title: strings('predict.deposit.account_ready'),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createSelector } from 'reselect';
import { RootState } from '../../../../../reducers';
import { PredictPositionStatus } from '../../types';
import { findAddressKey } from '../../utils/accounts';

const selectPredictControllerState = (state: RootState) =>
state.engine.backgroundState.PredictController;
Expand All @@ -25,10 +26,10 @@ const selectPredictClaimablePositionsByAddress = ({
}: {
address: string;
}) =>
createSelector(
selectPredictClaimablePositions,
(claimablePositions) => claimablePositions[address] || [],
);
createSelector(selectPredictClaimablePositions, (claimablePositions) => {
const matchedKey = findAddressKey(claimablePositions, address);
return matchedKey ? claimablePositions[matchedKey] : [];
});
Comment thread
vinnyhoward marked this conversation as resolved.

const selectPredictWonPositions = ({ address }: { address: string }) =>
createSelector(
Expand Down
34 changes: 33 additions & 1 deletion app/components/UI/Predict/utils/accounts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,41 @@ export const createMockEngineContext = () => ({
const mockEngineContext = createMockEngineContext();
jest.mock('../../../../core/Engine', () => ({ context: mockEngineContext }));

import { getEvmAccountFromSelectedAccountGroup } from './accounts';
import {
findAddressKey,
getEvmAccountFromSelectedAccountGroup,
} from './accounts';
import Engine from '../../../../core/Engine';

describe('findAddressKey', () => {
const CHECKSUMMED = '0xABCDEF1234567890ABCDeF1234567890ABCDEF12';
const LOWERCASE = CHECKSUMMED.toLowerCase();

it('finds a key when the map uses checksummed casing and query is lowercase', () => {
const map = { [CHECKSUMMED]: ['pos1'] };
expect(findAddressKey(map, LOWERCASE)).toBe(CHECKSUMMED);
});

it('finds a key when the map uses lowercase casing and query is checksummed', () => {
const map = { [LOWERCASE]: ['pos1'] };
expect(findAddressKey(map, CHECKSUMMED)).toBe(LOWERCASE);
});

it('finds a key when both casings match exactly', () => {
const map = { [CHECKSUMMED]: ['pos1'] };
expect(findAddressKey(map, CHECKSUMMED)).toBe(CHECKSUMMED);
});

it('returns undefined when address is not in the map', () => {
const map = { '0x1111111111111111111111111111111111111111': [] };
expect(findAddressKey(map, CHECKSUMMED)).toBeUndefined();
});

it('returns undefined for an empty map', () => {
expect(findAddressKey({}, CHECKSUMMED)).toBeUndefined();
});
});

describe('accountUtils', () => {
beforeEach(() => {
jest.clearAllMocks();
Expand Down
17 changes: 17 additions & 0 deletions app/components/UI/Predict/utils/accounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,23 @@
import { isEvmAccountType } from '@metamask/keyring-api';
import Engine from '../../../../core/Engine';

/**
* Finds the key in a map whose address matches the given address case-insensitively.
* Polymarket and EVM tooling can return addresses in checksummed or lowercase form,
* so direct key lookups may miss a valid entry.
*
* @param map - Object keyed by Ethereum addresses
* @param address - Address to match (any casing)
* @returns The matching key as stored in the map, or undefined if not found
*/
export const findAddressKey = (
map: Record<string, unknown>,
address: string,
): string | undefined => {
const normalized = address.toLowerCase();
return Object.keys(map).find((key) => key.toLowerCase() === normalized);
};

/**
* Gets the EVM account from the selected account group
* Extracts the duplicated pattern used throughout PerpsController
Expand Down
Loading
Loading