From da7e5f8763c3710d57387f499f4fc7b15b868e53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Tani=C3=A7a?= Date: Wed, 10 Dec 2025 06:20:12 -0700 Subject: [PATCH 01/12] feat(predict): configure fees via feature flag cp-7.61.0 (#23857) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Currently, fee collection settings are all hardcoded (amounts, collector address, waived categories). However, this creates a security concern in a scenario where the collector account gets compromised. This change enables the use of LD feature flag to configure the Predict fee collection. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://github.com/MetaMask/metamask-mobile/issues/23856 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Adds remote-configurable Predict fee collection (percentages, collector, waivers) and wires it through order preview and Polymarket flows, replacing hardcoded fees. > > - **Predict Fees (Feature Flag)** > - Add `PredictFeeCollection` type and `DEFAULT_FEE_COLLECTION_FLAG`. > - Load `remoteFeatureFlags.predictFeeCollection` in `PredictController.previewOrder` and pass `feeCollection` to providers. > - **Polymarket Provider & Utils** > - Update `previewOrder` and fee logic to accept `feeCollection`; compute fees from `metamaskFee`/`providerFee`, respect `waiveList`, and include `collector`. > - Use `fees.collector` for Safe fee authorization in `placeOrder`; remove hardcoded `FEE_PERCENTAGE` and `FEE_COLLECTOR_ADDRESS`. > - **Types** > - Extend `PredictFees` with `collector`; update provider interfaces to accept `feeCollection`. > - **Tests** > - Adjust/add tests to validate flag-driven fee calculation, collector propagation, waived-fee cases, and controller/provider integrations. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ec2cc32546326f16bc677a88a8783266c2766d3e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- app/components/UI/Predict/constants/flags.ts | 12 +++ .../controllers/PredictController.test.ts | 44 ++++++--- .../Predict/controllers/PredictController.ts | 11 ++- .../hooks/usePredictOrderPreview.test.ts | 2 + .../polymarket/PolymarketProvider.test.ts | 33 +++---- .../polymarket/PolymarketProvider.ts | 9 +- .../Predict/providers/polymarket/constants.ts | 6 -- .../providers/polymarket/utils.test.ts | 95 +++++++++++++++---- .../UI/Predict/providers/polymarket/utils.ts | 48 +++++++--- app/components/UI/Predict/providers/types.ts | 7 +- app/components/UI/Predict/types/flags.ts | 9 ++ 11 files changed, 203 insertions(+), 73 deletions(-) create mode 100644 app/components/UI/Predict/constants/flags.ts create mode 100644 app/components/UI/Predict/types/flags.ts diff --git a/app/components/UI/Predict/constants/flags.ts b/app/components/UI/Predict/constants/flags.ts new file mode 100644 index 00000000000..fac4ae44252 --- /dev/null +++ b/app/components/UI/Predict/constants/flags.ts @@ -0,0 +1,12 @@ +import { PredictFeeCollection } from '../types/flags'; + +export const DEFAULT_FEE_COLLECTION_FLAG = { + enabled: true, + collector: + process.env.METAMASK_ENVIRONMENT === 'dev' + ? '0xe6a2026d58eaff3c7ad7ba9386fb143388002382' + : '0x100c7b833bbd604a77890783439bbb9d65e31de7', + metamaskFee: 0.02, // 2% + providerFee: 0.02, // 2% + waiveList: [], +} satisfies PredictFeeCollection; diff --git a/app/components/UI/Predict/controllers/PredictController.test.ts b/app/components/UI/Predict/controllers/PredictController.test.ts index 3f30178f3f2..24e1a7320e0 100644 --- a/app/components/UI/Predict/controllers/PredictController.test.ts +++ b/app/components/UI/Predict/controllers/PredictController.test.ts @@ -79,6 +79,19 @@ jest.mock('../../../../core/Engine', () => ({ }, }), }, + RemoteFeatureFlagController: { + state: { + remoteFeatureFlags: { + predictFeeCollection: { + enabled: true, + collector: '0x100c7b833bbd604a77890783439bbb9d65e31de7', + metamaskFee: 0.02, + providerFee: 0.02, + waiveList: [], + }, + }, + }, + }, }, })); @@ -3269,19 +3282,26 @@ describe('PredictController', () => { }); expect(result).toEqual(mockOrderPreview); - expect(mockPolymarketProvider.previewOrder).toHaveBeenCalledWith({ - providerId: 'polymarket', - marketId: 'market-1', - outcomeId: 'outcome-1', - outcomeTokenId: 'token-1', - side: Side.BUY, - size: 100, - signer: expect.objectContaining({ - address: '0x1234567890123456789012345678901234567890', - signTypedMessage: expect.any(Function), - signPersonalMessage: expect.any(Function), + expect(mockPolymarketProvider.previewOrder).toHaveBeenCalledWith( + expect.objectContaining({ + marketId: 'market-1', + outcomeId: 'outcome-1', + outcomeTokenId: 'token-1', + side: Side.BUY, + size: 100, + signer: expect.objectContaining({ + address: '0x1234567890123456789012345678901234567890', + signTypedMessage: expect.any(Function), + signPersonalMessage: expect.any(Function), + }), + feeCollection: expect.objectContaining({ + enabled: true, + collector: expect.any(String), + metamaskFee: expect.any(Number), + providerFee: expect.any(Number), + }), }), - }); + ); }); }); diff --git a/app/components/UI/Predict/controllers/PredictController.ts b/app/components/UI/Predict/controllers/PredictController.ts index b9375ecd1bd..a40a9589be6 100644 --- a/app/components/UI/Predict/controllers/PredictController.ts +++ b/app/components/UI/Predict/controllers/PredictController.ts @@ -80,6 +80,8 @@ import { PREDICT_CONSTANTS, PREDICT_ERROR_CODES } from '../constants/errors'; import { getEvmAccountFromSelectedAccountGroup } from '../utils/accounts'; import { GEO_BLOCKED_COUNTRIES } from '../constants/geoblock'; import { MATIC_CONTRACTS } from '../providers/polymarket/constants'; +import { DEFAULT_FEE_COLLECTION_FLAG } from '../constants/flags'; +import { PredictFeeCollection } from '../types/flags'; /** * State shape for PredictController @@ -1265,9 +1267,16 @@ export class PredictController extends BaseController< throw new Error('Provider not available'); } + const { RemoteFeatureFlagController } = Engine.context; + const feeCollection = + (RemoteFeatureFlagController.state.remoteFeatureFlags + .predictFeeCollection as unknown as + | PredictFeeCollection + | undefined) ?? DEFAULT_FEE_COLLECTION_FLAG; + const signer = this.getSigner(); - return provider.previewOrder({ ...params, signer }); + return provider.previewOrder({ ...params, signer, feeCollection }); } catch (error) { // Log to Sentry with preview context (no sensitive amounts) Logger.error( diff --git a/app/components/UI/Predict/hooks/usePredictOrderPreview.test.ts b/app/components/UI/Predict/hooks/usePredictOrderPreview.test.ts index b4789052dcb..14af7b24670 100644 --- a/app/components/UI/Predict/hooks/usePredictOrderPreview.test.ts +++ b/app/components/UI/Predict/hooks/usePredictOrderPreview.test.ts @@ -3,6 +3,7 @@ import { usePredictOrderPreview } from './usePredictOrderPreview'; import { usePredictTrading } from './usePredictTrading'; import { OrderPreview, PreviewOrderParams } from '../providers/types'; import { Side } from '../types'; +import { DEFAULT_FEE_COLLECTION_FLAG } from '../constants/flags'; jest.mock('./usePredictTrading'); @@ -26,6 +27,7 @@ describe('usePredictOrderPreview', () => { providerFee: 1, totalFee: 2, totalFeePercentage: 4, + collector: DEFAULT_FEE_COLLECTION_FLAG.collector, }, }; diff --git a/app/components/UI/Predict/providers/polymarket/PolymarketProvider.test.ts b/app/components/UI/Predict/providers/polymarket/PolymarketProvider.test.ts index dc27f555712..726c0b53314 100644 --- a/app/components/UI/Predict/providers/polymarket/PolymarketProvider.test.ts +++ b/app/components/UI/Predict/providers/polymarket/PolymarketProvider.test.ts @@ -38,6 +38,7 @@ import { Side, } from '../../types'; import { PREDICT_ERROR_CODES } from '../../constants/errors'; +import { DEFAULT_FEE_COLLECTION_FLAG } from '../../constants/flags'; import { OrderPreview, PlaceOrderParams } from '../types'; import { PolymarketProvider } from './PolymarketProvider'; import { @@ -741,6 +742,7 @@ describe('PolymarketProvider', () => { providerFee: 0.02, totalFee: 0.04, totalFeePercentage: 0.04, + collector: DEFAULT_FEE_COLLECTION_FLAG.collector, }, ...overrides, }; @@ -1212,6 +1214,11 @@ describe('PolymarketProvider', () => { describe('previewOrder', () => { it('calls previewOrder utility function with correct parameters', async () => { const provider = createProvider(); + const mockSigner = { + address: '0x1234567890123456789012345678901234567890', + signTypedMessage: jest.fn(), + signPersonalMessage: jest.fn(), + }; const mockParams = { marketId: 'market-123', outcomeId: 'outcome-456', @@ -1219,6 +1226,7 @@ describe('PolymarketProvider', () => { side: Side.BUY, amount: 100, size: 100, + signer: mockSigner, }; await provider.previewOrder(mockParams); @@ -1364,6 +1372,7 @@ describe('PolymarketProvider', () => { providerFee: 0.02, totalFee: 0.04, totalFeePercentage: 0.04, + collector: DEFAULT_FEE_COLLECTION_FLAG.collector, }, }); const orderParams: PlaceOrderParams = { @@ -1385,6 +1394,7 @@ describe('PolymarketProvider', () => { providerFee: 0.02, totalFee: 0.04, totalFeePercentage: 0.04, + collector: DEFAULT_FEE_COLLECTION_FLAG.collector, }, }); const orderParams: PlaceOrderParams = { @@ -1411,6 +1421,7 @@ describe('PolymarketProvider', () => { providerFee: 0.02, totalFee: 0.04, totalFeePercentage: 0.04, + collector: DEFAULT_FEE_COLLECTION_FLAG.collector, }, }); const orderParams: PlaceOrderParams = { @@ -1437,6 +1448,7 @@ describe('PolymarketProvider', () => { providerFee: 0.02, totalFee: 0.04, totalFeePercentage: 0.04, + collector: DEFAULT_FEE_COLLECTION_FLAG.collector, }, }); const orderParams: PlaceOrderParams = { @@ -1464,7 +1476,7 @@ describe('PolymarketProvider', () => { ); }); - it('uses FEE_COLLECTOR_ADDRESS as recipient', async () => { + it('uses collector from fees as recipient', async () => { const { provider, mockSigner } = setupPlaceOrderTest(); const preview = createMockOrderPreview({ side: Side.BUY, @@ -1473,6 +1485,7 @@ describe('PolymarketProvider', () => { providerFee: 0.02, totalFee: 0.04, totalFeePercentage: 0.04, + collector: DEFAULT_FEE_COLLECTION_FLAG.collector, }, }); const orderParams: PlaceOrderParams = { @@ -1503,6 +1516,7 @@ describe('PolymarketProvider', () => { providerFee: 0, totalFee: 0, totalFeePercentage: 0, + collector: '0x0', }, }); @@ -3459,6 +3473,8 @@ describe('PolymarketProvider', () => { metamaskFee: 0.5, providerFee: 0.5, totalFee: 1, + totalFeePercentage: 1, + collector: DEFAULT_FEE_COLLECTION_FLAG.collector, }, }); }; @@ -3487,21 +3503,6 @@ describe('PolymarketProvider', () => { expect(sellPreview.rateLimited).toBe(true); }); - it('does not set rateLimited when signer is not provided', async () => { - setupPreviewOrderMock(); - const { provider } = setupPlaceOrderTest(); - - const preview = await provider.previewOrder({ - marketId: 'market-1', - outcomeId: 'outcome-1', - outcomeTokenId: '0', - side: Side.BUY, - size: 10, - }); - - expect(preview.rateLimited).toBeUndefined(); - }); - it('does not set rateLimited when address has never placed an order', async () => { setupPreviewOrderMock(); const { provider, mockSigner } = setupPlaceOrderTest(); diff --git a/app/components/UI/Predict/providers/polymarket/PolymarketProvider.ts b/app/components/UI/Predict/providers/polymarket/PolymarketProvider.ts index 1fd66ab8f7d..bbd57365954 100644 --- a/app/components/UI/Predict/providers/polymarket/PolymarketProvider.ts +++ b/app/components/UI/Predict/providers/polymarket/PolymarketProvider.ts @@ -50,7 +50,6 @@ import { SignWithdrawResponse, } from '../types'; import { - FEE_COLLECTOR_ADDRESS, MATIC_CONTRACTS, MIN_COLLATERAL_BALANCE_FOR_CLAIM, ORDER_RATE_LIMIT_MS, @@ -96,6 +95,7 @@ import { roundOrderAmount, submitClobOrder, } from './utils'; +import { PredictFeeCollection } from '../../types/flags'; export type SignTypedMessageFn = ( params: TypedMessageParams, @@ -868,7 +868,10 @@ export class PolymarketProvider implements PredictProvider { } public async previewOrder( - params: Omit & { signer?: Signer }, + params: Omit & { + signer: Signer; + feeCollection?: PredictFeeCollection; + }, ): Promise { const basePreview = await previewOrder(params); @@ -1062,7 +1065,7 @@ export class PolymarketProvider implements PredictProvider { safeAddress, signer, amount: feeAmountInUsdc, - to: FEE_COLLECTOR_ADDRESS, + to: fees.collector, }); } diff --git a/app/components/UI/Predict/providers/polymarket/constants.ts b/app/components/UI/Predict/providers/polymarket/constants.ts index d0114bfb23c..6e3f7243fcc 100644 --- a/app/components/UI/Predict/providers/polymarket/constants.ts +++ b/app/components/UI/Predict/providers/polymarket/constants.ts @@ -2,12 +2,6 @@ import { ContractConfig, RoundConfig, TickSize } from './types'; export const POLYMARKET_PROVIDER_ID = 'polymarket'; -export const FEE_PERCENTAGE = 4; // 4% -export const FEE_COLLECTOR_ADDRESS = - process.env.METAMASK_ENVIRONMENT === 'dev' - ? '0xe6a2026d58eaff3c7ad7ba9386fb143388002382' - : '0x100c7b833bbd604a77890783439bbb9d65e31de7'; - /** * Default slippage for market orders. */ diff --git a/app/components/UI/Predict/providers/polymarket/utils.test.ts b/app/components/UI/Predict/providers/polymarket/utils.test.ts index f31d5743555..4bd4609d915 100644 --- a/app/components/UI/Predict/providers/polymarket/utils.test.ts +++ b/app/components/UI/Predict/providers/polymarket/utils.test.ts @@ -14,12 +14,12 @@ import { PREDICT_ERROR_CODES } from '../../constants/errors'; import { ClobAuthDomain, EIP712Domain, - FEE_PERCENTAGE, HASH_ZERO_BYTES32, MATIC_CONTRACTS, MSG_TO_SIGN, POLYGON_MAINNET_CHAIN_ID, } from './constants'; +import { DEFAULT_FEE_COLLECTION_FLAG } from '../../constants/flags'; import { ApiKeyCreds, ClobHeaders, @@ -2556,24 +2556,45 @@ describe('polymarket utils', () => { }); describe('calculateFees', () => { - it('calculates fee using FEE_PERCENTAGE constant', async () => { + const feeCollection = DEFAULT_FEE_COLLECTION_FLAG; + const totalFeePercentage = + (feeCollection.metamaskFee + feeCollection.providerFee) * 100; + + beforeEach(() => { + // Mock the Gamma API response for market details + mockFetch.mockResolvedValue({ + ok: true, + json: jest.fn().mockResolvedValue({ + id: 'market-1', + tags: [], + }), + }); + }); + + it('calculates fee using feeCollection config', async () => { const params = { + feeCollection, marketId: 'market-1', userBetAmount: 1, }; const fees = await calculateFees(params); - const expectedTotal = (params.userBetAmount * FEE_PERCENTAGE) / 100; - const expectedEach = expectedTotal / 2; + const expectedMetamaskFee = + params.userBetAmount * feeCollection.metamaskFee; + const expectedProviderFee = + params.userBetAmount * feeCollection.providerFee; + const expectedTotal = expectedMetamaskFee + expectedProviderFee; expect(fees.totalFee).toBe(expectedTotal); - expect(fees.providerFee).toBe(expectedEach); - expect(fees.metamaskFee).toBe(expectedEach); - expect(fees.totalFeePercentage).toBe(FEE_PERCENTAGE); + expect(fees.providerFee).toBe(expectedProviderFee); + expect(fees.metamaskFee).toBe(expectedMetamaskFee); + expect(fees.totalFeePercentage).toBe(totalFeePercentage); + expect(fees.collector).toBe(feeCollection.collector); }); it('calculates fees correctly for various amounts', async () => { const params = { + feeCollection, marketId: 'market-1', userBetAmount: 1, }; @@ -2583,27 +2604,34 @@ describe('polymarket utils', () => { expect(fees.providerFee).toBeGreaterThanOrEqual(0); expect(fees.metamaskFee).toBeGreaterThanOrEqual(0); expect(fees.totalFee).toBeGreaterThanOrEqual(0); - expect(fees.totalFeePercentage).toBe(FEE_PERCENTAGE); + expect(fees.totalFeePercentage).toBe(totalFeePercentage); + expect(fees.collector).toBe(feeCollection.collector); }); it('handles large amounts correctly', async () => { const params = { + feeCollection, marketId: 'market-1', userBetAmount: 100, }; const fees = await calculateFees(params); - const expectedTotal = (params.userBetAmount * FEE_PERCENTAGE) / 100; - const expectedEach = expectedTotal / 2; + const expectedMetamaskFee = + params.userBetAmount * feeCollection.metamaskFee; + const expectedProviderFee = + params.userBetAmount * feeCollection.providerFee; + const expectedTotal = expectedMetamaskFee + expectedProviderFee; expect(fees.totalFee).toBe(expectedTotal); - expect(fees.providerFee).toBe(expectedEach); - expect(fees.metamaskFee).toBe(expectedEach); - expect(fees.totalFeePercentage).toBe(FEE_PERCENTAGE); + expect(fees.providerFee).toBe(expectedProviderFee); + expect(fees.metamaskFee).toBe(expectedMetamaskFee); + expect(fees.totalFeePercentage).toBe(totalFeePercentage); + expect(fees.collector).toBe(feeCollection.collector); }); it('handles small amounts correctly', async () => { const params = { + feeCollection, marketId: 'market-1', userBetAmount: 0.25, }; @@ -2613,24 +2641,50 @@ describe('polymarket utils', () => { expect(typeof fees.providerFee).toBe('number'); expect(typeof fees.metamaskFee).toBe('number'); expect(typeof fees.totalFee).toBe('number'); - const expectedTotal = (params.userBetAmount * FEE_PERCENTAGE) / 100; - const expectedEach = expectedTotal / 2; + const expectedMetamaskFee = + params.userBetAmount * feeCollection.metamaskFee; + const expectedProviderFee = + params.userBetAmount * feeCollection.providerFee; + const expectedTotal = expectedMetamaskFee + expectedProviderFee; expect(fees.totalFee).toBe(expectedTotal); - expect(fees.providerFee).toBe(expectedEach); - expect(fees.metamaskFee).toBe(expectedEach); - expect(fees.totalFeePercentage).toBe(FEE_PERCENTAGE); + expect(fees.providerFee).toBe(expectedProviderFee); + expect(fees.metamaskFee).toBe(expectedMetamaskFee); + expect(fees.totalFeePercentage).toBe(totalFeePercentage); + expect(fees.collector).toBe(feeCollection.collector); }); - it('waives fees for markets with middle-east tag', async () => { + it('returns zero fees when feeCollection is not provided', async () => { + const params = { + marketId: 'market-1', + userBetAmount: 100, + }; + + const fees = await calculateFees(params); + + expect(fees.providerFee).toBe(0); + expect(fees.metamaskFee).toBe(0); + expect(fees.totalFee).toBe(0); + expect(fees.totalFeePercentage).toBe(0); + expect(fees.collector).toBe('0x0'); + }); + + it('waives fees for markets in waiveList', async () => { + // Mock market with a tag that's in the waiveList mockFetch.mockResolvedValueOnce({ ok: true, - json: async () => ({ + json: jest.fn().mockResolvedValue({ id: 'market-with-waived-fees', tags: [{ slug: 'middle-east' }], }), }); + const feeCollectionWithWaiveList = { + ...feeCollection, + waiveList: ['middle-east'], + }; + const params = { + feeCollection: feeCollectionWithWaiveList, marketId: 'market-with-waived-fees', userBetAmount: 100, }; @@ -2641,6 +2695,7 @@ describe('polymarket utils', () => { expect(fees.metamaskFee).toBe(0); expect(fees.totalFee).toBe(0); expect(fees.totalFeePercentage).toBe(0); + expect(fees.collector).toBe('0x0'); }); }); diff --git a/app/components/UI/Predict/providers/polymarket/utils.ts b/app/components/UI/Predict/providers/polymarket/utils.ts index 2ffe8817ff8..daab1e5a1ca 100644 --- a/app/components/UI/Predict/providers/polymarket/utils.ts +++ b/app/components/UI/Predict/providers/polymarket/utils.ts @@ -29,7 +29,6 @@ import type { import { ClobAuthDomain, EIP712Domain, - FEE_PERCENTAGE, HASH_ZERO_BYTES32, MATIC_CONTRACTS, MSG_TO_SIGN, @@ -57,6 +56,7 @@ import { OrderBook, } from './types'; import { PREDICT_ERROR_CODES } from '../../constants/errors'; +import { PredictFeeCollection } from '../../types/flags'; export const getPolymarketEndpoints = () => ({ GAMMA_API_ENDPOINT: 'https://gamma-api.polymarket.com', @@ -915,44 +915,60 @@ export function encodeClaim( }); } -async function waiveFees({ marketId }: { marketId: string }) { +async function waiveFees({ + marketId, + waiveList, +}: { + marketId: string; + waiveList: string[]; +}) { const market = await getMarketDetailsFromGammaApi({ marketId }); const { tags } = market; - return tags?.map((t) => t.slug).includes('middle-east') ?? false; + const slugs = tags?.map((t) => t.slug); + return slugs?.some((slug) => waiveList.includes(slug)) ?? false; } export async function calculateFees({ + feeCollection, marketId, userBetAmount, }: { + feeCollection?: PredictFeeCollection; marketId: string; userBetAmount: number; }): Promise { - if (await waiveFees({ marketId })) { + if ( + !feeCollection?.enabled || + (await waiveFees({ marketId, waiveList: feeCollection.waiveList })) + ) { return { metamaskFee: 0, providerFee: 0, totalFee: 0, totalFeePercentage: 0, + collector: '0x0', }; } - let totalFee = 0; + const totalFeePercentage = + (feeCollection.metamaskFee + feeCollection.providerFee) * 100; - totalFee = (userBetAmount * FEE_PERCENTAGE) / 100; + let metamaskFee = userBetAmount * feeCollection.metamaskFee; + let providerFee = userBetAmount * feeCollection.providerFee; - // Round to 4 decimals - totalFee = Math.round(totalFee * 10000) / 10000; + // Round to 3 decimals + metamaskFee = Math.round(metamaskFee * 1000) / 1000; + providerFee = Math.round(providerFee * 1000) / 1000; - // split total 50/50 between metamask and provider - const metamaskFee = totalFee / 2; - const providerFee = totalFee - metamaskFee; + // Rounded to 4 decimals + const totalFee = metamaskFee + providerFee; return { metamaskFee, providerFee, totalFee, - totalFeePercentage: FEE_PERCENTAGE, + totalFeePercentage, + collector: feeCollection.collector, }; } @@ -1300,9 +1316,12 @@ export const roundOrderAmount = ({ }; export const previewOrder = async ( - params: Omit, + params: Omit & { + feeCollection?: PredictFeeCollection; + }, ): Promise => { - const { marketId, outcomeId, outcomeTokenId, side, size } = params; + const { marketId, outcomeId, outcomeTokenId, side, size, feeCollection } = + params; const book = await getOrderBook({ tokenId: outcomeTokenId }); if (!book) { throw new Error(PREDICT_ERROR_CODES.PREVIEW_NO_ORDER_BOOK); @@ -1337,6 +1356,7 @@ export const previewOrder = async ( minOrderSize: parseFloat(book.min_order_size), negRisk: book.neg_risk, fees: await calculateFees({ + feeCollection, marketId, userBetAmount: size, }), diff --git a/app/components/UI/Predict/providers/types.ts b/app/components/UI/Predict/providers/types.ts index 6b93e136723..8fc112c2f95 100644 --- a/app/components/UI/Predict/providers/types.ts +++ b/app/components/UI/Predict/providers/types.ts @@ -13,6 +13,7 @@ import { } from '../types'; import { Hex } from '@metamask/utils'; import { TransactionType } from '@metamask/transaction-controller'; +import { PredictFeeCollection } from '../types/flags'; export interface GetMarketsParams { providerId?: string; @@ -73,6 +74,7 @@ export interface PredictFees { providerFee: number; totalFee: number; totalFeePercentage: number; + collector: Hex; } export interface GeoBlockResponse { @@ -234,7 +236,10 @@ export interface PredictProvider { // Order management previewOrder( - params: Omit & { signer: Signer }, + params: Omit & { + signer: Signer; + feeCollection?: PredictFeeCollection; + }, ): Promise; placeOrder( params: Omit & { signer: Signer }, diff --git a/app/components/UI/Predict/types/flags.ts b/app/components/UI/Predict/types/flags.ts new file mode 100644 index 00000000000..49fc4cc2486 --- /dev/null +++ b/app/components/UI/Predict/types/flags.ts @@ -0,0 +1,9 @@ +import type { Hex } from '@metamask/utils'; + +export interface PredictFeeCollection { + enabled: boolean; + collector: Hex; + metamaskFee: number; + providerFee: number; + waiveList: string[]; +} From ea7986a8689c6cf7c2c96ab0bb04a457ffe11bc4 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 10 Dec 2025 09:57:42 -0330 Subject: [PATCH 02/12] chore: Improve readability of ESLint config (continued) (#23838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** The previous PR to improve ESLint config readability missed a few cases where a number was used instead of a string for the rule severity. The last ones have been updated now. ## **Changelog** CHANGELOG entry: null ## **Related issues** This is a continuation of #23821 ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Replaces remaining numeric ESLint severities with string "error" in .eslintrc.js. > > - **ESLint config (`.eslintrc.js`)**: > - Standardizes severity to strings (`'error'`) for: > - `no-constant-condition` > - `no-unneeded-ternary` > - `no-use-before-define` (now `['error', 'nofunc']`) > - `react/no-multi-comp` > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3c4c8daf437cd7358be054d82d993dcd2170d1df. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .eslintrc.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index efebbabc783..10416af6ea5 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -229,7 +229,7 @@ module.exports = { 'import/prefer-default-export': 'off', 'no-alert': 'error', 'no-constant-condition': [ - 2, + 'error', { checkLoops: false, }, @@ -253,14 +253,14 @@ module.exports = { 'no-throw-literal': 'error', 'no-unmodified-loop-condition': 'error', 'no-unneeded-ternary': [ - 2, + 'error', { defaultAssignment: false, }, ], 'no-unsafe-negation': 'error', 'no-unused-expressions': 'off', - 'no-use-before-define': [2, 'nofunc'], + 'no-use-before-define': ['error', 'nofunc'], 'no-useless-call': 'error', 'no-useless-computed-key': 'error', 'no-useless-concat': 'error', @@ -290,7 +290,7 @@ module.exports = { 'react/no-did-update-set-state': 'error', 'react/no-find-dom-node': 'error', 'react/no-multi-comp': [ - 2, + 'error', { ignoreStateless: true, }, From f408850bd8e66edf4561eec7d6921da9681de1b1 Mon Sep 17 00:00:00 2001 From: Salim TOUBAL Date: Wed, 10 Dec 2025 14:41:51 +0100 Subject: [PATCH 03/12] fix: fix token balances init (#23668) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** fix token balances init ## **Changelog** CHANGELOG entry: fix token balances init ## **Related issues** Fixes: https://github.com/MetaMask/metamask-mobile/issues/23637 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Initialize TokenBalancesController with an empty `tokenBalances` state when persisted state is missing or null, and add tests covering persisted/default cases. > > - **Controllers**: > - Use `persistedState?.TokenBalancesController ?? { tokenBalances: {} }` in `app/core/Engine/controllers/token-balances-controller-init.ts` to safely default state when absent. > - **Tests**: > - Add `beforeEach` to clear mocks. > - Add tests asserting: persisted state is passed through; default empty state is used when `TokenBalancesController` state is `undefined` or when `persistedState` is `null`. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 0770ddb81dc5429ccf499d408bbbc15a91be7dd3. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../token-balances-controller-init.test.ts | 60 ++++++++++++++++++- .../token-balances-controller-init.ts | 4 +- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/app/core/Engine/controllers/token-balances-controller-init.test.ts b/app/core/Engine/controllers/token-balances-controller-init.test.ts index 28046a94f53..b241f5b3ee8 100644 --- a/app/core/Engine/controllers/token-balances-controller-init.test.ts +++ b/app/core/Engine/controllers/token-balances-controller-init.test.ts @@ -40,18 +40,72 @@ function getInitRequestMock(): jest.Mocked< } describe('TokenBalancesControllerInit', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('initializes the controller', () => { const { controller } = tokenBalancesControllerInit(getInitRequestMock()); + expect(controller).toBeInstanceOf(TokenBalancesController); }); - it('passes the proper arguments to the controller', () => { - tokenBalancesControllerInit(getInitRequestMock()); + it('passes the persisted state to the controller when available', () => { + const mockPersistedState = { + tokenBalances: { + '0x123': { + '0x1': { + '0xtoken': '0x100' as const, + }, + }, + }, + } as const; + const requestMock = getInitRequestMock(); + requestMock.persistedState.TokenBalancesController = mockPersistedState; + + tokenBalancesControllerInit(requestMock); + + const controllerMock = jest.mocked(TokenBalancesController); + expect(controllerMock).toHaveBeenCalledWith({ + messenger: expect.any(Object), + state: mockPersistedState, + interval: 30_000, + allowExternalServices: expect.any(Function), + queryMultipleAccounts: expect.any(Boolean), + accountsApiChainIds: expect.any(Function), + platform: 'mobile', + }); + }); + + it('uses default state with empty tokenBalances when persisted state is undefined', () => { + const requestMock = getInitRequestMock(); + requestMock.persistedState.TokenBalancesController = undefined; + + tokenBalancesControllerInit(requestMock); + + const controllerMock = jest.mocked(TokenBalancesController); + expect(controllerMock).toHaveBeenCalledWith({ + messenger: expect.any(Object), + state: { tokenBalances: {} }, + interval: 30_000, + allowExternalServices: expect.any(Function), + queryMultipleAccounts: expect.any(Boolean), + accountsApiChainIds: expect.any(Function), + platform: 'mobile', + }); + }); + + it('uses default state with empty tokenBalances when persistedState is null', () => { + const requestMock = getInitRequestMock(); + // @ts-expect-error: Testing null case + requestMock.persistedState = null; + + tokenBalancesControllerInit(requestMock); const controllerMock = jest.mocked(TokenBalancesController); expect(controllerMock).toHaveBeenCalledWith({ messenger: expect.any(Object), - state: undefined, + state: { tokenBalances: {} }, interval: 30_000, allowExternalServices: expect.any(Function), queryMultipleAccounts: expect.any(Boolean), diff --git a/app/core/Engine/controllers/token-balances-controller-init.ts b/app/core/Engine/controllers/token-balances-controller-init.ts index d2e1b9438bd..6c6ed8969e4 100644 --- a/app/core/Engine/controllers/token-balances-controller-init.ts +++ b/app/core/Engine/controllers/token-balances-controller-init.ts @@ -23,7 +23,9 @@ export const tokenBalancesControllerInit: ControllerInitFunction< const controller = new TokenBalancesController({ messenger: controllerMessenger, - state: persistedState.TokenBalancesController, + state: persistedState?.TokenBalancesController ?? { + tokenBalances: {}, + }, interval: 30_000, allowExternalServices: () => selectBasicFunctionalityEnabled(getState()), queryMultipleAccounts: preferencesState.isMultiAccountBalancesEnabled, From 7924ed44bc6213940f0bdd61d60fbe1ae6b38f8f Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 10 Dec 2025 10:30:27 -0330 Subject: [PATCH 04/12] chore: Ignore `build` directory when linting (#23837) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This directory never exists on CI when the linter is run, but it often exists locally and creates a ton of spurious lint errors and warnings. ## **Changelog** CHANGELOG entry: null ## **Related issues** N/A ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Add `build` to `.eslintignore` to exclude build artifacts from linting. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 95f5c5cf8ef197f8f10f6827e97052f94bd02ca4. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .eslintignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.eslintignore b/.eslintignore index fefba93adbd..231f697a1ce 100644 --- a/.eslintignore +++ b/.eslintignore @@ -3,6 +3,7 @@ /app/util/blockies.js __snapshots__ android +build coverage ios jest.preprocessor.js From 9f03fc9551091a0481b0ed5705eece30bdc7d6f9 Mon Sep 17 00:00:00 2001 From: Bryan Fullam Date: Wed, 10 Dec 2025 06:25:47 -0800 Subject: [PATCH 05/12] fix: tron compute fee fallback cp-7.61.0 (#23849) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Bumps the Tron snap and patches bridge controller to allow for proper fee computation ## **Changelog** CHANGELOG entry: Fixed a bug where Tron TRC20 swap fees were underestimated ## **Related issues** Fixes: https://github.com/MetaMask/metamask-mobile/issues/23850 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Patch bridge-controller to include `feeLimit` in non-EVM fee computations and bump Tron snap to ^1.16.0. > > - **Bridge fee computation**: > - `dist/utils/quote-fees.{cjs,mjs}`: Include `feeLimit` (`trade.raw_data?.fee_limit`) when building options for `computeFeeRequest` in `appendNonEvmFees`. > - **Dependencies**: > - Use Yarn patch for `@metamask/bridge-controller@61.0.0`. > - Bump `@metamask/tron-wallet-snap` to `^1.16.0`. > - Update `package.json`/`yarn.lock` to reflect these changes. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5cc61fcc635a9958d1f8dd1dff0d1eadd28bcf21. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- ...dge-controller-npm-61.0.0-8c413c463f.patch | 24 ++++++ package.json | 7 +- yarn.lock | 78 ++++++------------- 3 files changed, 51 insertions(+), 58 deletions(-) create mode 100644 .yarn/patches/@metamask-bridge-controller-npm-61.0.0-8c413c463f.patch diff --git a/.yarn/patches/@metamask-bridge-controller-npm-61.0.0-8c413c463f.patch b/.yarn/patches/@metamask-bridge-controller-npm-61.0.0-8c413c463f.patch new file mode 100644 index 00000000000..012c7bd87b6 --- /dev/null +++ b/.yarn/patches/@metamask-bridge-controller-npm-61.0.0-8c413c463f.patch @@ -0,0 +1,24 @@ +diff --git a/dist/utils/quote-fees.cjs b/dist/utils/quote-fees.cjs +index accd7751a6e94db0c0a43b16787ad8252ee5a20a..4e093c7b93731c7dc389eb339db5f4b633a86c39 100644 +--- a/dist/utils/quote-fees.cjs ++++ b/dist/utils/quote-fees.cjs +@@ -91,6 +91,7 @@ const appendNonEvmFees = async (quotes, messenger, selectedAccount) => { + ? { + visible: trade.visible, + type: trade.raw_data?.contract?.[0]?.type, ++ feeLimit: trade.raw_data?.fee_limit, + } + : undefined; + const response = (await messenger.call('SnapController:handleRequest', (0, snaps_1.computeFeeRequest)(selectedAccount.metadata.snap?.id, transaction, selectedAccount.id, scope, options))); +diff --git a/dist/utils/quote-fees.mjs b/dist/utils/quote-fees.mjs +index 9dd2aa0b29ef32c710fd5aea067512a3104be764..23fa279da0f6e35740940d67bb707494bf59b30a 100644 +--- a/dist/utils/quote-fees.mjs ++++ b/dist/utils/quote-fees.mjs +@@ -88,6 +88,7 @@ const appendNonEvmFees = async (quotes, messenger, selectedAccount) => { + ? { + visible: trade.visible, + type: trade.raw_data?.contract?.[0]?.type, ++ feeLimit: trade.raw_data?.fee_limit, + } + : undefined; + const response = (await messenger.call('SnapController:handleRequest', computeFeeRequest(selectedAccount.metadata.snap?.id, transaction, selectedAccount.id, scope, options))); diff --git a/package.json b/package.json index a01993f0a3b..35537b25525 100644 --- a/package.json +++ b/package.json @@ -176,7 +176,8 @@ "@ethereumjs/util@npm:^9.0.2": "patch:@ethereumjs/util@npm%3A9.1.0#~/.yarn/patches/@ethereumjs-util-npm-9.1.0-7e85509408.patch", "@metamask/key-tree@npm:^10.1.1": "patch:@metamask/key-tree@npm%3A10.1.1#~/.yarn/patches/@metamask-key-tree-npm-10.1.1-0bfab435ac.patch", "@metamask/key-tree@npm:^10.0.2": "patch:@metamask/key-tree@npm%3A10.1.1#~/.yarn/patches/@metamask-key-tree-npm-10.1.1-0bfab435ac.patch", - "@metamask/transaction-controller@npm:^62.5.0": "patch:@metamask/transaction-controller@npm%3A62.5.0#~/.yarn/patches/@metamask-transaction-controller-npm-61.0.0-cccac388c7.patch" + "@metamask/transaction-controller@npm:^62.5.0": "patch:@metamask/transaction-controller@npm%3A62.5.0#~/.yarn/patches/@metamask-transaction-controller-npm-61.0.0-cccac388c7.patch", + "@metamask/bridge-controller@npm:^64.0.0": "patch:@metamask/bridge-controller@npm%3A61.0.0#~/.yarn/patches/@metamask-bridge-controller-npm-61.0.0-8c413c463f.patch" }, "dependencies": { "@config-plugins/detox": "^9.0.0", @@ -200,7 +201,7 @@ "@metamask/assets-controllers": "patch:@metamask/assets-controllers@npm%3A93.0.0#~/.yarn/patches/@metamask-assets-controllers-npm-93.0.0-ea998cb0bd.patch", "@metamask/base-controller": "^9.0.0", "@metamask/bitcoin-wallet-snap": "^1.8.0", - "@metamask/bridge-controller": "^61.0.0", + "@metamask/bridge-controller": "patch:@metamask/bridge-controller@npm%3A61.0.0#~/.yarn/patches/@metamask-bridge-controller-npm-61.0.0-8c413c463f.patch", "@metamask/bridge-status-controller": "^61.0.0", "@metamask/chain-agnostic-permission": "^1.2.2", "@metamask/composable-controller": "^12.0.0", @@ -289,7 +290,7 @@ "@metamask/token-search-discovery-controller": "^4.0.0", "@metamask/transaction-controller": "patch:@metamask/transaction-controller@npm%3A62.5.0#~/.yarn/patches/@metamask-transaction-controller-npm-61.0.0-cccac388c7.patch", "@metamask/transaction-pay-controller": "^10.4.0", - "@metamask/tron-wallet-snap": "^1.15.1", + "@metamask/tron-wallet-snap": "^1.16.0", "@metamask/utils": "^11.8.1", "@ngraveio/bc-ur": "^1.1.6", "@nktkas/hyperliquid": "^0.27.1", diff --git a/yarn.lock b/yarn.lock index 8a0ab36b903..95867297444 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7186,7 +7186,7 @@ __metadata: languageName: node linkType: hard -"@metamask/assets-controllers@npm:^93.0.0, @metamask/assets-controllers@npm:^93.1.0": +"@metamask/assets-controllers@npm:^93.1.0": version: 93.1.0 resolution: "@metamask/assets-controllers@npm:93.1.0" dependencies: @@ -7374,7 +7374,7 @@ __metadata: languageName: node linkType: hard -"@metamask/bridge-controller@npm:^61.0.0": +"@metamask/bridge-controller@npm:61.0.0": version: 61.0.0 resolution: "@metamask/bridge-controller@npm:61.0.0" dependencies: @@ -7406,34 +7406,35 @@ __metadata: languageName: node linkType: hard -"@metamask/bridge-controller@npm:^64.0.0": - version: 64.0.0 - resolution: "@metamask/bridge-controller@npm:64.0.0" +"@metamask/bridge-controller@patch:@metamask/bridge-controller@npm%3A61.0.0#~/.yarn/patches/@metamask-bridge-controller-npm-61.0.0-8c413c463f.patch": + version: 61.0.0 + resolution: "@metamask/bridge-controller@patch:@metamask/bridge-controller@npm%3A61.0.0#~/.yarn/patches/@metamask-bridge-controller-npm-61.0.0-8c413c463f.patch::version=61.0.0&hash=edc633" dependencies: "@ethersproject/address": "npm:^5.7.0" "@ethersproject/bignumber": "npm:^5.7.0" "@ethersproject/constants": "npm:^5.7.0" "@ethersproject/contracts": "npm:^5.7.0" "@ethersproject/providers": "npm:^5.7.0" - "@metamask/accounts-controller": "npm:^35.0.0" - "@metamask/assets-controllers": "npm:^93.0.0" "@metamask/base-controller": "npm:^9.0.0" - "@metamask/controller-utils": "npm:^11.16.0" - "@metamask/gas-fee-controller": "npm:^26.0.0" + "@metamask/controller-utils": "npm:^11.15.0" + "@metamask/gas-fee-controller": "npm:^25.0.0" "@metamask/keyring-api": "npm:^21.0.0" "@metamask/messenger": "npm:^0.3.0" "@metamask/metamask-eth-abis": "npm:^3.1.1" - "@metamask/multichain-network-controller": "npm:^3.0.0" - "@metamask/network-controller": "npm:^27.0.0" - "@metamask/polling-controller": "npm:^16.0.0" - "@metamask/remote-feature-flag-controller": "npm:^2.0.1" - "@metamask/snaps-controllers": "npm:^14.0.1" - "@metamask/transaction-controller": "npm:^62.4.0" + "@metamask/multichain-network-controller": "npm:^2.0.0" + "@metamask/polling-controller": "npm:^15.0.0" "@metamask/utils": "npm:^11.8.1" bignumber.js: "npm:^9.1.2" reselect: "npm:^5.1.1" uuid: "npm:^8.3.2" - checksum: 10/d9a73530421d74606ebcabccd6348a38a21ef786eb42d529bd73b05aee567e44e952482b26c2a7d5f93863afc955ced8dbd4979f76b3f0c7a0d9805e2abcceab + peerDependencies: + "@metamask/accounts-controller": ^34.0.0 + "@metamask/assets-controllers": ^89.0.0 + "@metamask/network-controller": ^25.0.0 + "@metamask/remote-feature-flag-controller": ^2.0.0 + "@metamask/snaps-controllers": ^14.0.0 + "@metamask/transaction-controller": ^61.0.0 + checksum: 10/1beb273ef7bb5e419a82fd4a974569327d7dafd72142eb11de18ee2d7461bc53e38eddb35b56e0e882f3957952bc90af67122168c832760020ac0d09ef0fc896 languageName: node linkType: hard @@ -8548,26 +8549,6 @@ __metadata: languageName: node linkType: hard -"@metamask/multichain-network-controller@npm:^3.0.0": - version: 3.0.0 - resolution: "@metamask/multichain-network-controller@npm:3.0.0" - dependencies: - "@metamask/base-controller": "npm:^9.0.0" - "@metamask/controller-utils": "npm:^11.16.0" - "@metamask/keyring-api": "npm:^21.0.0" - "@metamask/keyring-internal-api": "npm:^9.0.0" - "@metamask/messenger": "npm:^0.3.0" - "@metamask/superstruct": "npm:^3.1.0" - "@metamask/utils": "npm:^11.8.1" - "@solana/addresses": "npm:^2.0.0" - lodash: "npm:^4.17.21" - peerDependencies: - "@metamask/accounts-controller": ^35.0.0 - "@metamask/network-controller": ^26.0.0 - checksum: 10/b167cd4bed12285c1e37f74a681371c453936e1aaa7e1207fb98cd97cbfa8831ca9e96a569a8787caac3f5a831627435e001dc8febaad2e61a142e4298f57d2f - languageName: node - linkType: hard - "@metamask/multichain-transactions-controller@npm:^6.0.0": version: 6.0.0 resolution: "@metamask/multichain-transactions-controller@npm:6.0.0" @@ -9112,19 +9093,6 @@ __metadata: languageName: node linkType: hard -"@metamask/remote-feature-flag-controller@npm:^2.0.1": - version: 2.0.1 - resolution: "@metamask/remote-feature-flag-controller@npm:2.0.1" - dependencies: - "@metamask/base-controller": "npm:^9.0.0" - "@metamask/controller-utils": "npm:^11.16.0" - "@metamask/messenger": "npm:^0.3.0" - "@metamask/utils": "npm:^11.8.1" - uuid: "npm:^8.3.2" - checksum: 10/4815f68b368331a6876b339b6204d965725938e459036f486af5b4403604285a864617649f8877d85e9461ddac3cded921b0cc55004cd86ef03224b3fd26a74d - languageName: node - linkType: hard - "@metamask/remote-feature-flag-controller@npm:^3.0.0": version: 3.0.0 resolution: "@metamask/remote-feature-flag-controller@npm:3.0.0" @@ -9767,10 +9735,10 @@ __metadata: languageName: node linkType: hard -"@metamask/tron-wallet-snap@npm:^1.15.1": - version: 1.15.1 - resolution: "@metamask/tron-wallet-snap@npm:1.15.1" - checksum: 10/1249bd818ca7b15bd724e96dae34a1893d56651c456f2ddb064906d33f27a93773903154722cd52975a0e9dd5e5c63f1bf15fae5554a223881ae708db6e2779d +"@metamask/tron-wallet-snap@npm:^1.16.0": + version: 1.16.0 + resolution: "@metamask/tron-wallet-snap@npm:1.16.0" + checksum: 10/86be8ef0b7258b8375b9ab43eb4a8f55018f02a226a3784b727a25c03c249afa75e9adacecbf72f982dbf720db4107882a1b8304634bc5dad25ce2db760beff0 languageName: node linkType: hard @@ -34299,7 +34267,7 @@ __metadata: "@metamask/auto-changelog": "npm:^5.3.0" "@metamask/base-controller": "npm:^9.0.0" "@metamask/bitcoin-wallet-snap": "npm:^1.8.0" - "@metamask/bridge-controller": "npm:^61.0.0" + "@metamask/bridge-controller": "patch:@metamask/bridge-controller@npm%3A61.0.0#~/.yarn/patches/@metamask-bridge-controller-npm-61.0.0-8c413c463f.patch" "@metamask/bridge-status-controller": "npm:^61.0.0" "@metamask/browser-passworder": "npm:^5.0.0" "@metamask/build-utils": "npm:^3.0.0" @@ -34399,7 +34367,7 @@ __metadata: "@metamask/token-search-discovery-controller": "npm:^4.0.0" "@metamask/transaction-controller": "patch:@metamask/transaction-controller@npm%3A62.5.0#~/.yarn/patches/@metamask-transaction-controller-npm-61.0.0-cccac388c7.patch" "@metamask/transaction-pay-controller": "npm:^10.4.0" - "@metamask/tron-wallet-snap": "npm:^1.15.1" + "@metamask/tron-wallet-snap": "npm:^1.16.0" "@metamask/utils": "npm:^11.8.1" "@ngraveio/bc-ur": "npm:^1.1.6" "@nktkas/hyperliquid": "npm:^0.27.1" From 75ce034cc42704a3b2b4f416b130a82810b1a4b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20=C5=81ucka?= <5708018+PatrykLucka@users.noreply.github.com> Date: Wed, 10 Dec 2025 16:04:35 +0100 Subject: [PATCH 06/12] fix: fix Asset component to support selectedAddressForAsset for EVM and non-EVM assets cp-7.61.0 (#23597) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** **What is the reason for the change?** The Asset view was incorrectly determining the selectedAddress when viewing non-EVM assets (like Solana). The previous implementation used selectedInternalAccount.address which always returns the globally selected account's address. This meant when viewing a Solana asset, the view was incorrectly using the EVM address instead of the Solana address, causing issues with transaction filtering and display. **What is the improvement/solution?** Updated the Asset view to use selectSelectedInternalAccountByScope selector which returns the correct account based on the asset's chain scope: Converts the asset's chainId to CAIP format using formatChainIdToCaip Looks up the appropriate account for that chain scope (e.g., EVM account for eip155:1, Solana account for solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp) Falls back to the standard selected account address if no scope-specific account is found This follows the same pattern used in the bridge selectors (selectSourceWalletAddress), ensuring consistency across the codebase. **Key changes:** - Added selectedAddressForAsset prop computed in mapStateToProps using chain-scoped account selection - Updated component to use selectedAddressForAsset instead of deriving address from selectedInternalAccount - Updated componentDidUpdate to properly react to address changes - Added tests covering EVM assets, Solana assets, chainId format conversion, and fallback behavior ## **Changelog** CHANGELOG entry: Fixed a bug that was causing wrong url redirect on solana asset "View full history" button ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TMCU-244 ## **Manual testing steps** ```gherkin Feature: Asset view uses correct chain-specific address Scenario: user views a Solana asset and sees transactions for their Solana address Given user has an account group with both EVM and Solana accounts And user has SOL transactions on Solana mainnet When user navigates to the SOL asset detail view Then the transaction list displays transactions associated with the Solana address And the asset overview shows the correct SOL balance ``` ## **Screenshots/Recordings** ### **Before** https://github.com/user-attachments/assets/1228271c-5cdb-4938-b7a0-35927c7f9099 ### **After** https://github.com/user-attachments/assets/91bf8e05-8504-4751-b5db-34b7293e6bbe ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Asset view now resolves the correct chain-scoped address (EVM and non-EVM) and applies improved filtering/sorting for non-EVM transactions. > > - **Asset view (`app/components/Views/Asset/index.js`)** > - Use `selectedAddressForAsset` derived via CAIP `chainId` and `selectSelectedInternalAccountByScope`; fallback to `selectSelectedInternalAccountAddress`. > - Update `componentDidUpdate` to react to `selectedAddressForAsset` changes; initialize `selectedAddress` from this prop. > - Use route `chainId` throughout (e.g., ramp checks) instead of global selector. > - For non-EVM assets: render `MultichainTransactionsView` and apply filtering logic: > - Native assets: include only transactions where all participants carry the chain’s native asset. > - Tokens: filter by token address or unit; exclude empty-asset transactions. > - Sort by `time` desc; add simple cache (`cacheKey`, `cachedFilteredTransactions`). > - **Tests (`app/components/Views/Asset/index.test.js`)** > - Add tests for chain-scoped address resolution (EVM and Solana), CAIP conversion, and fallback behavior. > - Add/adjust tests for non-EVM filtering (native vs SPL, mixed tx exclusion, empty state) and sorting; update snapshots accordingly. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8566187f3e48efaad4224bfd1a94dc8975412a7a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../Asset/__snapshots__/index.test.js.snap | 1606 ++++------------- app/components/Views/Asset/index.js | 58 +- app/components/Views/Asset/index.test.js | 225 ++- 3 files changed, 615 insertions(+), 1274 deletions(-) diff --git a/app/components/Views/Asset/__snapshots__/index.test.js.snap b/app/components/Views/Asset/__snapshots__/index.test.js.snap index 78c2c04f701..8ab3736db74 100644 --- a/app/components/Views/Asset/__snapshots__/index.test.js.snap +++ b/app/components/Views/Asset/__snapshots__/index.test.js.snap @@ -252,7 +252,7 @@ exports[`Asset Fund Button Visibility should show fund button when only ramp is `; -exports[`Asset Multichain Functionality should exclude mixed token/SOL transactions from native SOL view 1`] = ` +exports[`Asset Multichain Functionality renders empty state when no multichain transactions exist 1`] = ` - - - - - - - Buy - - - - - `; -exports[`Asset Multichain Functionality should exclude transactions with empty asset data 1`] = ` +exports[`Asset Multichain Functionality returns empty list for unknown SPL tokens 1`] = ` - SOL + UNKNOWN - SOL + UNKNOWN - - - - - - - Buy - - - - - - - - - + testID="token-avatar-image" + /> - SOL + UNKNOWN - 0 SOL + 0 UNKNOWN + > + + + Contract address + + + + Unknown...dress + + + + + @@ -3749,7 +3564,7 @@ exports[`Asset Multichain Functionality should exclude transactions with empty a } } > - SOL activity + UNKNOWN activity @@ -3854,7 +3669,7 @@ exports[`Asset Multichain Functionality should exclude transactions with empty a `; -exports[`Asset Multichain Functionality should filter SPL token transactions correctly 1`] = ` +exports[`Asset Multichain Functionality should exclude mixed token/SOL transactions from native SOL view 1`] = ` - USDC + SOL - USDC + SOL - - - - - - - Buy - - - - - - + + > + + - USDC + SOL - 0 USDC + 0 SOL - - - Contract address - - - - EPjFWdd...TDt1v - - - - - + /> @@ -5591,7 +5252,7 @@ exports[`Asset Multichain Functionality should filter SPL token transactions cor } } > - USDC activity + SOL activity @@ -5696,7 +5357,7 @@ exports[`Asset Multichain Functionality should filter SPL token transactions cor `; -exports[`Asset Multichain Functionality should filter native SOL transactions correctly 1`] = ` +exports[`Asset Multichain Functionality should exclude transactions with empty asset data 1`] = ` - - - - - - - Buy - - - - - `; -exports[`Asset Multichain Functionality should handle state with no multichain transactions 1`] = ` +exports[`Asset Multichain Functionality should filter SPL token transactions correctly 1`] = ` - SOL + USDC - SOL + USDC - - - - - - - Buy - - - - - - - - - + testID="token-avatar-image" + /> - SOL + USDC - 0 SOL + 0 USDC + > + + + Contract address + + + + EPjFWdd...TDt1v + + + + + @@ -9193,7 +8669,7 @@ exports[`Asset Multichain Functionality should handle state with no multichain t } } > - SOL activity + USDC activity @@ -9298,7 +8774,7 @@ exports[`Asset Multichain Functionality should handle state with no multichain t `; -exports[`Asset Multichain Functionality should handle unknown SPL token filtering gracefully 1`] = ` +exports[`Asset Multichain Functionality should filter native SOL transactions correctly 1`] = ` - UNKNOWN + SOL - UNKNOWN + SOL - - - - - - - Buy - - - - - - + + > + + - UNKNOWN + SOL - 0 UNKNOWN + 0 SOL - - - - Token details - - - - - Contract address - - - - Unknown...dress - - - - - + } + > + + + + Token details + + @@ -11035,7 +10357,7 @@ exports[`Asset Multichain Functionality should handle unknown SPL token filterin } } > - UNKNOWN activity + SOL activity @@ -12601,119 +11923,6 @@ exports[`Asset Multichain Functionality should render non-EVM assets with Multic } } > - - - - - - - Buy - - - - - - - - - - - - Buy - - - - - { const { @@ -273,10 +283,18 @@ class Asset extends PureComponent { } componentDidUpdate(prevProps) { + // Update selectedAddress if the address for the asset's chain has changed + if ( + prevProps.selectedAddressForAsset !== this.props.selectedAddressForAsset + ) { + this.selectedAddress = isHexAddress(this.props.selectedAddressForAsset) + ? safeToChecksumAddress(this.props.selectedAddressForAsset) + : this.props.selectedAddressForAsset; + } + if ( prevProps.chainId !== this.props.chainId || - prevProps.selectedInternalAccount.address !== - this.props.selectedInternalAccount?.address + prevProps.selectedAddressForAsset !== this.props.selectedAddressForAsset ) { this.showLoaderAndNormalize(); } else { @@ -634,6 +652,23 @@ const mapStateToProps = (state, { route }) => { const evmTransactions = selectTransactions(state); const asset = route.params; + // Get the correct selected address for the asset's chain + // For non-EVM assets (like Solana), we need to get the address from the account + // that matches the asset's chain scope + let selectedAddressForAsset; + + if (asset?.chainId) { + const caipChainId = formatChainIdToCaip(asset.chainId); + const accountByScope = + selectSelectedInternalAccountByScope(state)(caipChainId); + selectedAddressForAsset = accountByScope?.address; + } + + // Fallback to the standard selected account address + if (!selectedAddressForAsset) { + selectedAddressForAsset = selectSelectedInternalAccountAddress(state); + } + let allTransactions = evmTransactions; ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) @@ -742,17 +777,18 @@ const mapStateToProps = (state, { route }) => { conversionRate: selectConversionRate(state), currentCurrency: selectCurrentCurrency(state), selectedInternalAccount, - chainId: selectChainId(state), + selectedAddressForAsset, + chainId: route.params.chainId, tokens: selectTokens(state), transactions: allTransactions, rpcUrl: selectRpcUrl(state), networkConfigurations: selectNetworkConfigurations(state), isNetworkRampSupported: isNetworkRampSupported( - selectChainId(state), + route.params.chainId, getRampNetworks(state), ), isNetworkBuyNativeTokenSupported: isNetworkRampNativeTokenSupported( - selectChainId(state), + route.params.chainId, getRampNetworks(state), ), isDepositEnabled: (() => { diff --git a/app/components/Views/Asset/index.test.js b/app/components/Views/Asset/index.test.js index fb4f628a377..971e43d286d 100644 --- a/app/components/Views/Asset/index.test.js +++ b/app/components/Views/Asset/index.test.js @@ -277,12 +277,12 @@ jest.mock('../../../selectors/earnController', () => ({ }, })); -jest.mock( - '../../../selectors/featureFlagController/multichainAccounts/enabledMultichainAccounts', - () => ({ - selectMultichainAccountsState2Enabled: () => false, - }), -); +const mockSelectSelectedInternalAccountByScope = jest.fn(() => () => undefined); +jest.mock('../../../selectors/multichainAccounts/accounts', () => ({ + ...jest.requireActual('../../../selectors/multichainAccounts/accounts'), + selectSelectedInternalAccountByScope: (...args) => + mockSelectSelectedInternalAccountByScope(...args), +})); describe('Asset', () => { it('should render correctly', () => { @@ -571,7 +571,7 @@ describe('Asset', () => { expect(toJSON()).toMatchSnapshot(); }); - it('should handle unknown SPL token filtering gracefully', () => { + it('returns empty list for unknown SPL tokens', () => { const testState = createMockStateWithAccount(SolAccountType.DataAccount); const { toJSON } = renderScreen( @@ -672,7 +672,7 @@ describe('Asset', () => { expect(toJSON()).toMatchSnapshot(); }); - it('should handle state with no multichain transactions', () => { + it('renders empty state when no multichain transactions exist', () => { const stateWithoutMultichain = { ...createMockStateWithAccount(SolAccountType.DataAccount), engine: { @@ -958,4 +958,213 @@ describe('Asset', () => { expect(toJSON()).toMatchSnapshot(); }); }); + + describe('selectedAddressForAsset', () => { + const MOCK_EVM_ADDRESS = '0xC4966c0D659D99699BFD7EB54D8fafEE40e4a756'; + const MOCK_SOLANA_ADDRESS = '8A4AptCThfbuknsbteHgGKXczfJpfjuVA9SLTSGaaLGC'; + + beforeEach(() => { + jest.clearAllMocks(); + mockSelectSelectedInternalAccountByScope.mockReturnValue(() => undefined); + }); + + it('calls selectSelectedInternalAccountByScope with EVM CAIP chainId', () => { + const mockScopedSelector = jest.fn().mockReturnValue({ + id: 'evm-account-id', + address: MOCK_EVM_ADDRESS, + type: EthAccountType.Eoa, + }); + mockSelectSelectedInternalAccountByScope.mockReturnValue( + mockScopedSelector, + ); + + renderWithProvider( + , + { state: mockInitialState }, + ); + + expect(mockSelectSelectedInternalAccountByScope).toHaveBeenCalledTimes(1); + expect(mockScopedSelector).toHaveBeenCalledWith('eip155:1'); + }); + + it('calls selectSelectedInternalAccountByScope with Solana CAIP chainId', () => { + const mockScopedSelector = jest.fn().mockReturnValue({ + id: 'solana-account-id', + address: MOCK_SOLANA_ADDRESS, + type: SolAccountType.DataAccount, + }); + mockSelectSelectedInternalAccountByScope.mockReturnValue( + mockScopedSelector, + ); + + const testState = createMockStateWithAccount(SolAccountType.DataAccount); + + renderScreen( + (props) => ( + + ), + { name: 'Asset' }, + { state: testState }, + { + symbol: 'SOL', + address: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + isNative: true, + chainId: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + }, + ); + + expect(mockSelectSelectedInternalAccountByScope).toHaveBeenCalled(); + expect(mockScopedSelector).toHaveBeenCalledWith( + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + ); + }); + + it('attempts scope lookup even when it returns undefined', () => { + const mockScopedSelector = jest.fn().mockReturnValue(undefined); + mockSelectSelectedInternalAccountByScope.mockReturnValue( + mockScopedSelector, + ); + + renderWithProvider( + , + { state: mockInitialState }, + ); + + // Selector was called, returned undefined, component still renders (fallback) + expect(mockSelectSelectedInternalAccountByScope).toHaveBeenCalledTimes(1); + expect(mockScopedSelector).toHaveBeenCalledWith('eip155:1'); + }); + + it('converts EVM hex chainId to CAIP format when looking up account by scope', () => { + const mockScopedSelector = jest.fn().mockReturnValue({ + id: 'evm-account-id', + address: MOCK_EVM_ADDRESS, + type: EthAccountType.Eoa, + }); + mockSelectSelectedInternalAccountByScope.mockReturnValue( + mockScopedSelector, + ); + + renderWithProvider( + , + { state: mockInitialState }, + ); + + // formatChainIdToCaip converts '0x1' to 'eip155:1' + expect(mockScopedSelector).toHaveBeenCalledWith('eip155:1'); + }); + + it('passes Solana CAIP chainId directly when looking up account by scope', () => { + const mockScopedSelector = jest.fn().mockReturnValue({ + id: 'solana-account-id', + address: MOCK_SOLANA_ADDRESS, + type: SolAccountType.DataAccount, + }); + mockSelectSelectedInternalAccountByScope.mockReturnValue( + mockScopedSelector, + ); + + const testState = createMockStateWithAccount(SolAccountType.DataAccount); + + renderScreen( + (props) => ( + + ), + { name: 'Asset' }, + { state: testState }, + { + symbol: 'SOL', + address: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + isNative: true, + chainId: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + }, + ); + + // Solana chainId is already in CAIP format, passed through as-is + expect(mockScopedSelector).toHaveBeenCalledWith( + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + ); + }); + + it('still calls scope selector when account has null address', () => { + // Test that selector is called even when account has null address (fallback scenario) + const mockScopedSelector = jest.fn().mockReturnValue({ + id: 'account-without-address', + address: null, + type: EthAccountType.Eoa, + }); + mockSelectSelectedInternalAccountByScope.mockReturnValue( + mockScopedSelector, + ); + + renderWithProvider( + , + { state: mockInitialState }, + ); + + // Selector was called with correct chainId, even though address was null + expect(mockSelectSelectedInternalAccountByScope).toHaveBeenCalledTimes(1); + expect(mockScopedSelector).toHaveBeenCalledWith('eip155:1'); + }); + }); }); From 1fc0cc9724b963ccf11915835791e415f73d0f45 Mon Sep 17 00:00:00 2001 From: Guillaume Roux Date: Wed, 10 Dec 2025 16:05:06 +0100 Subject: [PATCH 07/12] chore: Bump ENS Snap to `1.1.0` (#23871) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR bumps the ENS resolver snap to `v1.1.0`. ## **Changelog** CHANGELOG entry: Update ENS snap ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Update `@metamask/ens-resolver-snap` from `^1.0.0` to `^1.1.0` in `package.json` and `yarn.lock`. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 83e4f9919f810519aa9325b399ea5f5f4e568f72. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- package.json | 2 +- yarn.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 35537b25525..f85f1eca76b 100644 --- a/package.json +++ b/package.json @@ -215,7 +215,7 @@ "@metamask/earn-controller": "^10.0.0", "@metamask/eip-5792-middleware": "^2.0.0", "@metamask/eip1193-permission-middleware": "^1.0.2", - "@metamask/ens-resolver-snap": "^1.0.0", + "@metamask/ens-resolver-snap": "^1.1.0", "@metamask/error-reporting-service": "^3.0.0", "@metamask/eth-hd-keyring": "^13.0.0", "@metamask/eth-json-rpc-filters": "^9.0.0", diff --git a/yarn.lock b/yarn.lock index 95867297444..1dac32b3fc2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7738,10 +7738,10 @@ __metadata: languageName: node linkType: hard -"@metamask/ens-resolver-snap@npm:^1.0.0": - version: 1.0.0 - resolution: "@metamask/ens-resolver-snap@npm:1.0.0" - checksum: 10/c97e0d300a275fac053233391f38d7146a98bb0a49ec8e03a42a27bf4db9864a7f8b2d5b2a9701bcb1dd043d0edf9a9e9f7e52115893db39e51c140e7202e24d +"@metamask/ens-resolver-snap@npm:^1.1.0": + version: 1.1.0 + resolution: "@metamask/ens-resolver-snap@npm:1.1.0" + checksum: 10/e7d636ae8934aa31e0b0ee916557e1911b37147d7a6eca74fbe3581df3652162104f190eddba6c5ae100166e79168855ac364393b57abfbc6f8bc82047d71082 languageName: node linkType: hard @@ -34283,7 +34283,7 @@ __metadata: "@metamask/earn-controller": "npm:^10.0.0" "@metamask/eip-5792-middleware": "npm:^2.0.0" "@metamask/eip1193-permission-middleware": "npm:^1.0.2" - "@metamask/ens-resolver-snap": "npm:^1.0.0" + "@metamask/ens-resolver-snap": "npm:^1.1.0" "@metamask/error-reporting-service": "npm:^3.0.0" "@metamask/eslint-config-typescript": "npm:^9.0.0" "@metamask/eslint-plugin-design-tokens": "npm:^1.0.0" From 339418715ddc2402a84fd26b989932261fffb9ca Mon Sep 17 00:00:00 2001 From: Priya Date: Wed, 10 Dec 2025 22:32:23 +0700 Subject: [PATCH 08/12] test: fix e2e timeouts by disabling fox animation on login screen (#23800) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** ## **Changelog** CHANGELOG entry: ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Skips rendering Rive/Fox animations in E2E mode across onboarding, login, and password screens; updates animation logic/tests and disables Detox sync in a wallet details spec. > > - **UI/Animations (E2E gating)**: > - `UI/OnboardingAnimation/OnboardingAnimation.tsx`: Do not render `Rive` when `isE2E`; initialize `isPlaying` to `isE2E`; fast-forward animation state and call `setStartFoxAnimation(true)` in E2E; keep logo/buttons visible via animated values. > - `Views/Login`: Wrap `FoxAnimation` with `!isE2E` to prevent rendering during E2E. > - `Views/Onboarding`: Wrap `FoxAnimation` with `!isE2E`. > - `Views/ChoosePassword`: Wrap `FoxRiveLoaderAnimation` with `!isE2E`. > - `Views/OnboardingSuccess/OnboardingSuccessEndAnimation`: Wrap `Rive` with `!isE2E` and no-op side effects in E2E. > - **Tests**: > - `UI/OnboardingAnimation/OnboardingAnimation.test.tsx`: Update E2E-mode test to assert the `Rive` component is not rendered (mock methods undefined); align descriptions. > - **E2E spec**: > - `e2e/specs/accounts/wallet-details.spec.ts`: Add `device.disableSynchronization()` before login. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 2359cdfad09b28c2d04023fa504153db2e05ee11. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../OnboardingAnimation.test.tsx | 12 +++----- .../OnboardingAnimation.tsx | 28 ++++++++++--------- app/components/Views/ChoosePassword/index.js | 3 +- app/components/Views/Login/index.tsx | 19 +++++++------ app/components/Views/Onboarding/index.tsx | 4 ++- .../OnboardingSuccessEndAnimation/index.tsx | 18 ++++++------ e2e/specs/accounts/wallet-details.spec.ts | 1 + 7 files changed, 46 insertions(+), 39 deletions(-) diff --git a/app/components/UI/OnboardingAnimation/OnboardingAnimation.test.tsx b/app/components/UI/OnboardingAnimation/OnboardingAnimation.test.tsx index bb6d995ad9a..135b64dff63 100644 --- a/app/components/UI/OnboardingAnimation/OnboardingAnimation.test.tsx +++ b/app/components/UI/OnboardingAnimation/OnboardingAnimation.test.tsx @@ -215,7 +215,7 @@ describe('OnboardingAnimation', () => { expect(mockSetStartFoxAnimation).toHaveBeenCalledWith(true); }); - it('skips Rive animation in E2E mode', () => { + it('does not render Rive component in E2E mode', () => { // Start with a fresh render in E2E mode const { rerender } = render( , @@ -233,15 +233,11 @@ describe('OnboardingAnimation', () => { />, ); - // In E2E mode, Rive methods should not be called + // In E2E mode, Rive component is not rendered at all const mockedMethods = __getLastMockedMethods(); - // The Rive component should exist but methods should not be called - expect(mockedMethods).toBeDefined(); - if (mockedMethods) { - expect(mockedMethods.setInputState).not.toHaveBeenCalled(); - expect(mockedMethods.fireState).not.toHaveBeenCalled(); - } + // The Rive component should not be rendered in E2E mode + expect(mockedMethods).toBeUndefined(); }); }); diff --git a/app/components/UI/OnboardingAnimation/OnboardingAnimation.tsx b/app/components/UI/OnboardingAnimation/OnboardingAnimation.tsx index b2e5d8f88a0..d30e3e05769 100644 --- a/app/components/UI/OnboardingAnimation/OnboardingAnimation.tsx +++ b/app/components/UI/OnboardingAnimation/OnboardingAnimation.tsx @@ -73,7 +73,7 @@ const OnboardingAnimation = ({ const { themeAppearance } = useAppThemeFromContext(); const styles = createStyles(); - const [isPlaying, setIsPlaying] = useState(false); + const [isPlaying, setIsPlaying] = useState(isE2E); const moveLogoUp = useCallback(() => { Animated.parallel([ @@ -143,18 +143,20 @@ const OnboardingAnimation = ({ ]} pointerEvents="none" > - { - setIsPlaying(true); - }} - /> + {!isE2E && ( + { + setIsPlaying(true); + }} + /> + )} diff --git a/app/components/Views/ChoosePassword/index.js b/app/components/Views/ChoosePassword/index.js index 517ea9d36d9..ed8443add4f 100644 --- a/app/components/Views/ChoosePassword/index.js +++ b/app/components/Views/ChoosePassword/index.js @@ -78,6 +78,7 @@ import { import { uint8ArrayToMnemonic } from '../../../util/mnemonic'; import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english'; import { setDataCollectionForMarketing } from '../../../actions/security'; +import { isE2E } from '../../../util/test/utils'; const createStyles = (colors) => StyleSheet.create({ @@ -723,7 +724,7 @@ class ChoosePassword extends PureComponent { {loading ? ( - + {!isE2E && } ) : ( = ({ saveOnboardingEvent }) => { - - - + {!isE2E && ( + + + + )} ); diff --git a/app/components/Views/Onboarding/index.tsx b/app/components/Views/Onboarding/index.tsx index fcabc14b0d9..27eabef0b31 100644 --- a/app/components/Views/Onboarding/index.tsx +++ b/app/components/Views/Onboarding/index.tsx @@ -903,7 +903,9 @@ const Onboarding = () => { - + {!isE2E && ( + + )} {handleSimpleNotification()} diff --git a/app/components/Views/OnboardingSuccess/OnboardingSuccessEndAnimation/index.tsx b/app/components/Views/OnboardingSuccess/OnboardingSuccessEndAnimation/index.tsx index 39feb36f8a1..4591e198fb3 100644 --- a/app/components/Views/OnboardingSuccess/OnboardingSuccessEndAnimation/index.tsx +++ b/app/components/Views/OnboardingSuccess/OnboardingSuccessEndAnimation/index.tsx @@ -54,14 +54,16 @@ const OnboardingSuccessEndAnimation: React.FC< style={styles.animationContainer} > - + {!isE2E && ( + + )} ); diff --git a/e2e/specs/accounts/wallet-details.spec.ts b/e2e/specs/accounts/wallet-details.spec.ts index a2270746fbe..d87d2473a70 100644 --- a/e2e/specs/accounts/wallet-details.spec.ts +++ b/e2e/specs/accounts/wallet-details.spec.ts @@ -33,6 +33,7 @@ describe(SmokeAccounts('Wallet details'), () => { testSpecificMock, }, async () => { + await device.disableSynchronization(); await loginToApp(); await WalletView.tapIdenticon(); From 142e2d9176638312751bf5b04ececabf5fe40d2c Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 10 Dec 2025 12:02:54 -0330 Subject: [PATCH 09/12] chore: Update `@metamask/eslint-config-typescript` (#23835) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This package had a peer dependency on ESLint v7, but we're using v8. This update eliminates that peer dependency warning, and moves us closer to updating to ESLint v9. ## **Changelog** CHANGELOG entry: null ## **Related issues** N/A ## **Manual testing steps** N/A ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Upgrades the ESLint TypeScript config to v10 and adds a temporary `consistent-type-definitions` override enforcing `interface` in `.eslintrc.js`. > > - **Tooling / ESLint**: > - Upgrade `@metamask/eslint-config-typescript` to `^10.0.0` in `package.json` (lockfile updated accordingly). > - Update `.eslintrc.js` overrides for `*.{ts,tsx}`: > - Add temporary `@typescript-eslint/consistent-type-definitions: ['error', 'interface']` rule to defer a breaking change. > - Remove previous custom settings for `@typescript-eslint/no-unused-vars`. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 75899b36075fb1fa6a9d044875ae5610687f885f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .eslintrc.js | 12 ++++-------- package.json | 2 +- yarn.lock | 18 +++++++++--------- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 10416af6ea5..22628cc987f 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -34,15 +34,11 @@ module.exports = { 'react/no-unused-prop-types': 'off', 'react/prop-types': 'off', 'react/self-closing-comp': 'off', - // This change is included in `@metamask/eslint-config-typescript@10.0.0 - '@typescript-eslint/no-unused-vars': [ + // Temporarily overriding this rule to postpone this breaking change: https://github.com/MetaMask/eslint-config/pull/216 + // TODO: Remove this override and align on prefering type over interface. + '@typescript-eslint/consistent-type-definitions': [ 'error', - { - vars: 'all', - args: 'all', - argsIgnorePattern: '[_]+', - ignoreRestSiblings: true, // this line is what has changed - }, + 'interface', ], '@typescript-eslint/no-explicit-any': 'error', // Under discussion diff --git a/package.json b/package.json index f85f1eca76b..fa7c367a483 100644 --- a/package.json +++ b/package.json @@ -501,7 +501,7 @@ "@metamask/auto-changelog": "^5.3.0", "@metamask/browser-passworder": "^5.0.0", "@metamask/build-utils": "^3.0.0", - "@metamask/eslint-config-typescript": "^9.0.0", + "@metamask/eslint-config-typescript": "^10.0.0", "@metamask/eslint-plugin-design-tokens": "^1.0.0", "@metamask/foundryup": "1.0.0", "@metamask/mobile-provider": "^3.0.0", diff --git a/yarn.lock b/yarn.lock index 1dac32b3fc2..72a0de086db 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7755,16 +7755,16 @@ __metadata: languageName: node linkType: hard -"@metamask/eslint-config-typescript@npm:^9.0.0": - version: 9.0.1 - resolution: "@metamask/eslint-config-typescript@npm:9.0.1" +"@metamask/eslint-config-typescript@npm:^10.0.0": + version: 10.0.0 + resolution: "@metamask/eslint-config-typescript@npm:10.0.0" peerDependencies: - "@metamask/eslint-config": ^9.0.0 - "@typescript-eslint/eslint-plugin": ^4.20.0 - "@typescript-eslint/parser": ^4.20.0 - eslint: ^7.23.0 + "@metamask/eslint-config": ^10.0.0 + "@typescript-eslint/eslint-plugin": ^5.33.0 + "@typescript-eslint/parser": ^5.33.0 + eslint: ^8.21.0 typescript: ^4.0.7 - checksum: 10/df6c630e285b1a125caffce1988c23b3ba0f76507c337a849fb30fb5f9b9df4bb563419f9bb2ec7e39072601b7e95a4d5be52ddff1643bde65206f33d73440d3 + checksum: 10/08c052d3fe034c9712a69fda395975438ea18c10ee73bb573a6758e068d7020e62d9a7347eb2d67e5e7a38275fcd96b8f33e975d4f37363a8631c253d30f7dd9 languageName: node linkType: hard @@ -34285,7 +34285,7 @@ __metadata: "@metamask/eip1193-permission-middleware": "npm:^1.0.2" "@metamask/ens-resolver-snap": "npm:^1.1.0" "@metamask/error-reporting-service": "npm:^3.0.0" - "@metamask/eslint-config-typescript": "npm:^9.0.0" + "@metamask/eslint-config-typescript": "npm:^10.0.0" "@metamask/eslint-plugin-design-tokens": "npm:^1.0.0" "@metamask/eth-hd-keyring": "npm:^13.0.0" "@metamask/eth-json-rpc-filters": "npm:^9.0.0" From 745bb08170baa362e2406604a02627469ff80d09 Mon Sep 17 00:00:00 2001 From: Juanmi <95381763+juanmigdr@users.noreply.github.com> Date: Wed, 10 Dec 2025 17:39:57 +0100 Subject: [PATCH 10/12] fix: nft auto detection not triggered on network switch (#23858) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Changing network through the network selector does not trigger NFT detection, we have to refresh the tab by switching tabs which is not great user experience. Other bugs solved: - Now we show the NFT auto detection disabled when its needed, previously we were hiding it even when the setting was off - We dont make any API calls for NFT detection when auto-detection is off NOTE: as part of this PR I have also centralized some logic and moved other logic around ## **Changelog** CHANGELOG entry: fix nft auto detection not triggered on network switch ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-2072 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** ### **Before** https://github.com/user-attachments/assets/b639c099-9da8-4747-ae2a-749ed863eb33 ### **After** https://github.com/user-attachments/assets/d9f9ac5f-cc42-4abe-abb2-b8a103e43b8a ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Centralizes NFT detection in a reusable hook and triggers detection on network changes and collectibles views, updating components and tests accordingly. > > - **Core** > - Added `useNftDetection` hook encapsulating detection, loading indicators, tracing, and analytics; respects user preference to avoid API calls when disabled. > - **UI** > - `NftGrid`: triggers `detectNfts` on enabled network changes and when full view gains focus; renders `NftGridHeader` outside the list; integrates new hook. > - `CollectibleDetectionModal`: enables preferences and calls `detectNfts` via hook on CTA. > - `NftGridRefreshControl`: uses hook for detection and updates ownership across networks. > - `Wallet`: calls `detectNfts` when switching to the Collectibles tab; removes NFT loading indicator actions from props. > - **Tests** > - Added `useNftDetection.test.ts` and updated `NftGrid.test.tsx` and `CollectibleDetectionModal` test to mock/use the new hook. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5eceec280d174905a5d5a22883fdcf323680c979. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../CollectibleDetectionModal/index.test.tsx | 19 +- .../UI/CollectibleDetectionModal/index.tsx | 30 +- app/components/UI/NftGrid/NftGrid.test.tsx | 9 + app/components/UI/NftGrid/NftGrid.tsx | 31 +- .../UI/NftGrid/NftGridRefreshControl.tsx | 76 +-- app/components/Views/Wallet/index.tsx | 95 +--- app/components/hooks/useNftDetection.test.ts | 447 ++++++++++++++++++ app/components/hooks/useNftDetection.ts | 104 ++++ 8 files changed, 624 insertions(+), 187 deletions(-) create mode 100644 app/components/hooks/useNftDetection.test.ts create mode 100644 app/components/hooks/useNftDetection.ts diff --git a/app/components/UI/CollectibleDetectionModal/index.test.tsx b/app/components/UI/CollectibleDetectionModal/index.test.tsx index db34426e056..fde5e9f071a 100644 --- a/app/components/UI/CollectibleDetectionModal/index.test.tsx +++ b/app/components/UI/CollectibleDetectionModal/index.test.tsx @@ -3,7 +3,8 @@ import { backgroundState } from '../../../util/test/initial-root-state'; import React from 'react'; import renderWithProvider from '../../../util/test/renderWithProvider'; import { fireEvent } from '@testing-library/react-native'; -import Engine from '../../../core/Engine'; + +const mockDetectNfts = jest.fn(); jest.mock('../../../core/Engine', () => ({ context: { @@ -22,6 +23,13 @@ jest.mock('../../../core/Engine', () => ({ }, })); +jest.mock('../../hooks/useNftDetection', () => ({ + useNftDetection: () => ({ + detectNfts: mockDetectNfts, + chainIdsToDetectNftsFor: ['0x1'], + }), +})); + const initialState = { engine: { backgroundState, @@ -29,12 +37,17 @@ const initialState = { }; describe('CollectibleDetectionModal', () => { - it('calls NFT detection controller', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('calls detectNfts from useNftDetection hook when button is pressed', () => { const { getByTestId } = renderWithProvider(, { state: initialState, }); fireEvent.press(getByTestId(`collectible-detection-modal-button`)); - expect(Engine.context.NftDetectionController.detectNfts).toHaveBeenCalled(); + + expect(mockDetectNfts).toHaveBeenCalled(); }); }); diff --git a/app/components/UI/CollectibleDetectionModal/index.tsx b/app/components/UI/CollectibleDetectionModal/index.tsx index 2f32c7474fc..3413864f7d6 100644 --- a/app/components/UI/CollectibleDetectionModal/index.tsx +++ b/app/components/UI/CollectibleDetectionModal/index.tsx @@ -15,14 +15,9 @@ import { } from '../../../component-library/components/Icons/Icon'; import { useTheme } from '../../../util/theme'; import Engine from '../../../core/Engine'; -import { - hideNftFetchingLoadingIndicator, - showNftFetchingLoadingIndicator, -} from '../../../reducers/collectibles'; import { UserProfileProperty } from '../../../util/metrics/UserSettingsAnalyticsMetaData/UserProfileAnalyticsMetaData.types'; import { useMetrics } from '../../hooks/useMetrics'; -import { useNftDetectionChainIds } from '../../hooks/useNftDetectionChainIds'; -import { endTrace, trace, TraceName } from '../../../util/trace'; +import { useNftDetection } from '../../hooks/useNftDetection'; const styles = StyleSheet.create({ alertBar: { @@ -35,9 +30,9 @@ const CollectibleDetectionModal = () => { const { colors } = useTheme(); const { toastRef } = useContext(ToastContext); const { addTraitsToUser } = useMetrics(); - const chainIdsToDetectNftsFor = useNftDetectionChainIds(); + const { detectNfts } = useNftDetection(); - const showToastAndEnableNFtDetection = useCallback(async () => { + const showToastAndEnableNFtDetection = useCallback(() => { // show toast toastRef?.current?.showToast({ variant: ToastVariants.Icon, @@ -48,7 +43,7 @@ const CollectibleDetectionModal = () => { hasNoTimeout: false, }); // set nft autodetection - const { PreferencesController, NftDetectionController } = Engine.context; + const { PreferencesController } = Engine.context; PreferencesController.setDisplayNftMedia(true); PreferencesController.setUseNftDetection(true); const traits = { @@ -56,21 +51,8 @@ const CollectibleDetectionModal = () => { [UserProfileProperty.NFT_AUTODETECTION]: UserProfileProperty.ON, }; addTraitsToUser(traits); - // Call detect nfts - showNftFetchingLoadingIndicator(); - try { - trace({ name: TraceName.DetectNfts }); - await NftDetectionController.detectNfts(chainIdsToDetectNftsFor); - endTrace({ name: TraceName.DetectNfts }); - } finally { - hideNftFetchingLoadingIndicator(); - } - }, [ - colors.primary.inverse, - toastRef, - addTraitsToUser, - chainIdsToDetectNftsFor, - ]); + detectNfts(); + }, [colors.primary.inverse, toastRef, addTraitsToUser, detectNfts]); return ( diff --git a/app/components/UI/NftGrid/NftGrid.test.tsx b/app/components/UI/NftGrid/NftGrid.test.tsx index 1229b72aa57..45e0d548c2b 100644 --- a/app/components/UI/NftGrid/NftGrid.test.tsx +++ b/app/components/UI/NftGrid/NftGrid.test.tsx @@ -25,6 +25,7 @@ jest.mock('@react-navigation/native', () => ({ navigate: mockNavigate, push: mockPush, }), + useFocusEffect: jest.fn(), })); // Mock react-redux @@ -49,6 +50,14 @@ jest.mock('../../hooks/useMetrics'); getMetaMetricsId: jest.fn(), }); +// Mock useNftDetection +jest.mock('../../hooks/useNftDetection', () => ({ + useNftDetection: () => ({ + detectNfts: jest.fn(), + chainIdsToDetectNftsFor: ['0x1'], + }), +})); + // Mock FlashList jest.mock('@shopify/flash-list', () => ({ FlashList: ({ diff --git a/app/components/UI/NftGrid/NftGrid.tsx b/app/components/UI/NftGrid/NftGrid.tsx index 69244062985..c2cd2d4fcc5 100644 --- a/app/components/UI/NftGrid/NftGrid.tsx +++ b/app/components/UI/NftGrid/NftGrid.tsx @@ -20,7 +20,7 @@ import ActionSheet from '@metamask/react-native-actionsheet'; import NftGridItemActionSheet from './NftGridItemActionSheet'; import NftGridHeader from './NftGridHeader'; import NftGridSkeleton from './NftGridSkeleton'; -import { useNavigation } from '@react-navigation/native'; +import { useNavigation, useFocusEffect } from '@react-navigation/native'; import { StackNavigationProp } from '@react-navigation/stack'; import { MetaMetricsEvents, useMetrics } from '../../hooks/useMetrics'; import { CollectiblesEmptyState } from '../CollectiblesEmptyState'; @@ -39,6 +39,7 @@ import ButtonIcon, { import { IconName } from '../../../component-library/components/Icons/Icon'; import { useTailwind } from '@metamask/design-system-twrnc-preset'; import { selectHomepageRedesignV1Enabled } from '../../../selectors/featureFlagController/homepage'; +import { useNftDetection } from '../../hooks/useNftDetection'; interface NFTNavigationParamList { AddAsset: { assetType: string }; @@ -104,6 +105,10 @@ const NftGrid = ({ isFullView = false }: NftGridProps) => { multichainCollectiblesByEnabledNetworksSelector, ); + const { detectNfts, chainIdsToDetectNftsFor } = useNftDetection(); + + const isInitialMount = useRef(true); + const allFilteredCollectibles: Nft[] = useMemo(() => { trace({ name: TraceName.LoadCollectibles }); @@ -129,6 +134,25 @@ const NftGrid = ({ isFullView = false }: NftGridProps) => { return itemsToProcess; }, [allFilteredCollectibles, maxItems]); + // Trigger NFT detection when enabled networks change (after initial mount) + useEffect(() => { + if (isInitialMount.current) { + isInitialMount.current = false; + return; + } + + detectNfts(); + }, [chainIdsToDetectNftsFor, detectNfts]); + + // Trigger NFT detection when the full view is focused + useFocusEffect( + useCallback(() => { + if (isFullView) { + detectNfts(); + } + }, [isFullView, detectNfts]), + ); + useEffect(() => { if (longPressedCollectible) { actionSheetRef.current.show(); @@ -160,7 +184,6 @@ const NftGrid = ({ isFullView = false }: NftGridProps) => { const nftRowList = useMemo( () => ( } data={collectiblesToRender} renderItem={({ item, index }) => ( @@ -207,12 +230,16 @@ const NftGrid = ({ isFullView = false }: NftGridProps) => { hideSort style={isFullView ? tw`px-4 pb-4` : tw`pb-3`} /> + + + + {/* View all NFTs button - shown when there are more items than maxItems */} {maxItems && allFilteredCollectibles.length > maxItems && ( diff --git a/app/components/UI/NftGrid/NftGridRefreshControl.tsx b/app/components/UI/NftGrid/NftGridRefreshControl.tsx index 74d5bb565e7..b97fd701336 100644 --- a/app/components/UI/NftGrid/NftGridRefreshControl.tsx +++ b/app/components/UI/NftGrid/NftGridRefreshControl.tsx @@ -4,44 +4,19 @@ import { useSelector } from 'react-redux'; import { useTheme } from '../../../util/theme'; import Engine from '../../../core/Engine'; -import { cloneDeep } from 'lodash'; -import { selectSelectedInternalAccountFormattedAddress } from '../../../selectors/accountsController'; -import { endTrace, trace, TraceName } from '../../../util/trace'; -import { MetaMetricsEvents, useMetrics } from '../../hooks/useMetrics'; -import { useNftDetectionChainIds } from '../../hooks/useNftDetectionChainIds'; -import { prepareNftDetectionEvents } from '../../../util/assets'; -import { getDecimalChainId } from '../../../util/networks'; -import { Nft } from '@metamask/assets-controllers'; -import Logger from '../../../util/Logger'; import { selectTokenNetworkFilter } from '../../../selectors/preferencesController'; import { selectEvmNetworkConfigurationsByChainId } from '../../../selectors/networkController'; +import { useNftDetection } from '../../hooks/useNftDetection'; const NftGridRefreshControl = React.forwardRef((props, ref) => { const { colors } = useTheme(); - const selectedAddress = useSelector( - selectSelectedInternalAccountFormattedAddress, - ); const allEVMNetworks = useSelector(selectEvmNetworkConfigurationsByChainId); const tokenNetworkFilter = useSelector(selectTokenNetworkFilter); - const chainIdsToDetectNftsFor = useNftDetectionChainIds(); - - const { trackEvent, createEventBuilder } = useMetrics(); + const { detectNfts } = useNftDetection(); const [refreshing, setRefreshing] = useState(false); - const getNftDetectionAnalyticsParams = useCallback((nft: Nft) => { - try { - return { - chain_id: getDecimalChainId(nft.chainId), - source: 'detected' as const, - }; - } catch (error) { - Logger.error(error as Error, 'Wallet.getNftDetectionAnalyticsParams'); - return undefined; - } - }, []); - const allNetworkClientIds = useMemo( () => Object.keys(tokenNetworkFilter).flatMap((chainId) => { @@ -58,22 +33,12 @@ const NftGridRefreshControl = React.forwardRef((props, ref) => { const onRefresh = useCallback(async () => { requestAnimationFrame(async () => { - // Return early if no address selected - if (!selectedAddress) return; - - // Get initial state of NFTs before refresh - const { NftDetectionController, NftController } = Engine.context; - const previousNfts = cloneDeep( - NftController.state.allNfts[selectedAddress.toLowerCase()], - ); - trace({ name: TraceName.DetectNfts }); - setRefreshing(true); - const actions = [ - NftDetectionController.detectNfts(chainIdsToDetectNftsFor), - ]; + const { NftController } = Engine.context; + const actions = [detectNfts()]; + // Also check and update ownership status for all networks allNetworkClientIds.forEach((networkClientId) => { actions.push( NftController.checkAndUpdateAllNftsOwnershipStatus(networkClientId), @@ -82,37 +47,8 @@ const NftGridRefreshControl = React.forwardRef((props, ref) => { await Promise.allSettled(actions); setRefreshing(false); - endTrace({ name: TraceName.DetectNfts }); - - // Get updated state after refresh - const newNfts = cloneDeep( - NftController.state.allNfts[selectedAddress.toLowerCase()], - ); - - const eventParams = prepareNftDetectionEvents( - previousNfts, - newNfts, - getNftDetectionAnalyticsParams, - ); - eventParams.forEach((params) => { - trackEvent( - createEventBuilder(MetaMetricsEvents.COLLECTIBLE_ADDED) - .addProperties({ - chain_id: params.chain_id, - source: params.source, - }) - .build(), - ); - }); }); - }, [ - chainIdsToDetectNftsFor, - allNetworkClientIds, - createEventBuilder, - getNftDetectionAnalyticsParams, - selectedAddress, - trackEvent, - ]); + }, [detectNfts, allNetworkClientIds]); return ( void; currentRouteName: string; storePrivacyPolicyClickedOrClosed: () => void; - showNftFetchingLoadingIndicator: () => void; - hideNftFetchingLoadingIndicator: () => void; } interface WalletTokensTabViewProps { navigation: WalletProps['navigation']; @@ -517,8 +504,6 @@ const Wallet = ({ shouldShowPna25Toast, storePna25Acknowledged, storePrivacyPolicyClickedOrClosed, - showNftFetchingLoadingIndicator, - hideNftFetchingLoadingIndicator, }: WalletProps) => { const { navigate } = useNavigation(); const route = useRoute>(); @@ -727,10 +712,6 @@ const Wallet = ({ ///: END:ONLY_INCLUDE_IF ]); - const selectedAddress = useSelector( - selectSelectedInternalAccountFormattedAddress, - ); - const isDataCollectionForMarketingEnabled = useSelector( (state: RootState) => state.security.dataCollectionForMarketing, ); @@ -1035,7 +1016,7 @@ const Wallet = ({ isAllNetworks && isPopularNetworks ? allDetectedTokens : detectedTokens; const selectedNetworkClientId = useSelector(selectNetworkClientId); - const chainIdsToDetectNftsFor = useNftDetectionChainIds(); + const { detectNfts } = useNftDetection(); /** * Shows Nft auto detect modal if the user is on mainnet, never saw the modal and have nft detection off @@ -1294,20 +1275,8 @@ const Wallet = ({ selectedNetworkClientId, ]); - const getNftDetectionAnalyticsParams = useCallback((nft: Nft) => { - try { - return { - chain_id: getDecimalChainId(nft.chainId), - source: 'detected' as const, - }; - } catch (error) { - Logger.error(error as Error, 'Wallet.getNftDetectionAnalyticsParams'); - return undefined; - } - }, []); - const onChangeTab = useCallback( - async (obj: { i: number; ref: React.ReactNode }) => { + (obj: { i: number; ref: React.ReactNode }) => { const tabLabel = React.isValidElement(obj.ref) && obj.ref.props ? (obj.ref.props as { tabLabel?: string })?.tabLabel @@ -1319,59 +1288,13 @@ const Wallet = ({ createEventBuilder(MetaMetricsEvents.DEFI_TAB_SELECTED).build(), ); } else if (tabLabel === strings('wallet.collectibles')) { - // Return early if no address selected - if (!selectedAddress) return; - - const formattedSelectedAddress = toFormattedAddress(selectedAddress); - trackEvent( createEventBuilder(MetaMetricsEvents.WALLET_COLLECTIBLES).build(), ); - // Call detect nfts - const { NftDetectionController, NftController } = Engine.context; - const previousNfts = cloneDeep( - NftController.state.allNfts[formattedSelectedAddress], - ); - - try { - trace({ name: TraceName.DetectNfts }); - showNftFetchingLoadingIndicator(); - await NftDetectionController.detectNfts(chainIdsToDetectNftsFor); - endTrace({ name: TraceName.DetectNfts }); - } finally { - hideNftFetchingLoadingIndicator(); - } - - const newNfts = cloneDeep( - NftController.state.allNfts[formattedSelectedAddress], - ); - - const eventParams = prepareNftDetectionEvents( - previousNfts, - newNfts, - getNftDetectionAnalyticsParams, - ); - eventParams.forEach((params) => { - trackEvent( - createEventBuilder(MetaMetricsEvents.COLLECTIBLE_ADDED) - .addProperties({ - chain_id: params.chain_id, - source: params.source, - }) - .build(), - ); - }); + detectNfts(); } }, - [ - trackEvent, - createEventBuilder, - selectedAddress, - showNftFetchingLoadingIndicator, - chainIdsToDetectNftsFor, - hideNftFetchingLoadingIndicator, - getNftDetectionAnalyticsParams, - ], + [trackEvent, createEventBuilder, detectNfts], ); const turnOnBasicFunctionality = useCallback(() => { @@ -1494,10 +1417,6 @@ const mapDispatchToProps = (dispatch: any) => ({ dispatch(storePrivacyPolicyShownDateAction(Date.now())), storePrivacyPolicyClickedOrClosed: () => dispatch(storePrivacyPolicyClickedOrClosedAction()), - showNftFetchingLoadingIndicator: () => - dispatch(showNftFetchingLoadingIndicatorAction()), - hideNftFetchingLoadingIndicator: () => - dispatch(hideNftFetchingLoadingIndicatorAction()), storePna25Acknowledged: () => dispatch(storePna25AcknowledgedAction()), }); diff --git a/app/components/hooks/useNftDetection.test.ts b/app/components/hooks/useNftDetection.test.ts new file mode 100644 index 00000000000..982dbdfbfe4 --- /dev/null +++ b/app/components/hooks/useNftDetection.test.ts @@ -0,0 +1,447 @@ +import { renderHook, act } from '@testing-library/react-hooks'; +import { useSelector, useDispatch } from 'react-redux'; +import { useNftDetection } from './useNftDetection'; +import Engine from '../../core/Engine'; +import { endTrace, trace } from '../../util/trace'; +import { MetaMetricsEvents, useMetrics } from './useMetrics'; +import { useNftDetectionChainIds } from './useNftDetectionChainIds'; +import { prepareNftDetectionEvents } from '../../util/assets'; +import { getDecimalChainId } from '../../util/networks'; +import Logger from '../../util/Logger'; +import { + hideNftFetchingLoadingIndicator, + showNftFetchingLoadingIndicator, +} from '../../reducers/collectibles'; +import { Nft } from '@metamask/assets-controllers'; +import { Hex } from '@metamask/utils'; + +// Mock all dependencies +jest.mock('react-redux', () => ({ + useSelector: jest.fn(), + useDispatch: jest.fn(), +})); + +jest.mock('../../core/Engine'); + +jest.mock('../../util/trace', () => ({ + trace: jest.fn(), + endTrace: jest.fn(), + TraceName: { + DetectNfts: 'DetectNfts', + }, +})); + +jest.mock('./useMetrics', () => ({ + useMetrics: jest.fn(), + MetaMetricsEvents: { + COLLECTIBLE_ADDED: 'Collectible Added', + }, +})); + +jest.mock('./useNftDetectionChainIds', () => ({ + useNftDetectionChainIds: jest.fn(), +})); + +jest.mock('../../util/assets', () => ({ + prepareNftDetectionEvents: jest.fn(), +})); + +jest.mock('../../util/networks', () => ({ + getDecimalChainId: jest.fn(), +})); + +jest.mock('../../util/Logger', () => ({ + error: jest.fn(), +})); + +jest.mock('../../reducers/collectibles', () => ({ + showNftFetchingLoadingIndicator: jest.fn(), + hideNftFetchingLoadingIndicator: jest.fn(), +})); + +describe('useNftDetection', () => { + const mockDispatch = jest.fn(); + const mockTrackEvent = jest.fn(); + const mockCreateEventBuilder = jest.fn(); + const mockAddProperties = jest.fn(); + const mockBuild = jest.fn(); + const mockDetectNfts = jest.fn(); + + const mockUseSelector = useSelector as jest.MockedFunction< + typeof useSelector + >; + const mockUseDispatch = useDispatch as jest.MockedFunction< + typeof useDispatch + >; + const mockUseMetrics = useMetrics as jest.MockedFunction; + const mockUseNftDetectionChainIds = + useNftDetectionChainIds as jest.MockedFunction< + typeof useNftDetectionChainIds + >; + const mockPrepareNftDetectionEvents = + prepareNftDetectionEvents as jest.MockedFunction< + typeof prepareNftDetectionEvents + >; + const mockGetDecimalChainId = getDecimalChainId as jest.MockedFunction< + typeof getDecimalChainId + >; + const mockTrace = trace as jest.MockedFunction; + const mockEndTrace = endTrace as jest.MockedFunction; + const mockLoggerError = Logger.error as jest.MockedFunction< + typeof Logger.error + >; + + const mockSelectedAddress = '0x1234567890abcdef'; + const mockChainIds = ['0x1', '0x89'] as Hex[]; + + const mockNftControllerState = { + allNfts: { + [mockSelectedAddress.toLowerCase()]: { + '0x1': [ + { + address: '0xNFT1', + tokenId: '1', + name: 'NFT 1', + standard: 'ERC721', + } as Nft, + ], + }, + }, + }; + + const mockEngine = { + context: { + NftDetectionController: { + detectNfts: mockDetectNfts, + }, + NftController: { + state: mockNftControllerState, + }, + PreferencesController: { + state: { + useNftDetection: true, + }, + }, + }, + }; + + beforeEach(() => { + jest.clearAllMocks(); + + // Setup useDispatch mock + mockUseDispatch.mockReturnValue(mockDispatch); + + // Setup useSelector mock + mockUseSelector.mockReturnValue(mockSelectedAddress); + + // Setup useMetrics mock + mockAddProperties.mockReturnThis(); + mockBuild.mockReturnValue({ event: 'test-event', properties: {} }); + mockCreateEventBuilder.mockReturnValue({ + addProperties: mockAddProperties, + build: mockBuild, + }); + + mockUseMetrics.mockReturnValue({ + trackEvent: mockTrackEvent, + createEventBuilder: mockCreateEventBuilder, + isEnabled: jest.fn(), + enable: jest.fn(), + addTraitsToUser: jest.fn(), + createDataDeletionTask: jest.fn(), + checkDataDeleteStatus: jest.fn(), + getDeleteRegulationCreationDate: jest.fn(), + getDeleteRegulationId: jest.fn(), + isDataRecorded: jest.fn(), + getMetaMetricsId: jest.fn(), + }); + + // Setup useNftDetectionChainIds mock + mockUseNftDetectionChainIds.mockReturnValue(mockChainIds); + + // Setup Engine mock + (Engine as unknown as { context: typeof mockEngine.context }).context = + mockEngine.context; + + // Setup prepareNftDetectionEvents mock + mockPrepareNftDetectionEvents.mockReturnValue([]); + + // Setup getDecimalChainId mock + mockGetDecimalChainId.mockReturnValue(1); + + // Setup detectNfts to resolve + mockDetectNfts.mockResolvedValue(undefined); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('detectNfts', () => { + it('returns early when selectedAddress is undefined', async () => { + mockUseSelector.mockReturnValue(undefined); + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + await result.current.detectNfts(); + }); + + expect(mockDetectNfts).not.toHaveBeenCalled(); + expect(mockDispatch).not.toHaveBeenCalled(); + }); + + it('returns early when NFT detection is disabled', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = false; + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + await result.current.detectNfts(); + }); + + expect(mockDetectNfts).not.toHaveBeenCalled(); + expect(mockDispatch).not.toHaveBeenCalled(); + }); + + it('dispatches showNftFetchingLoadingIndicator before detection', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = true; + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + await result.current.detectNfts(); + }); + + expect(mockDispatch).toHaveBeenCalledWith( + showNftFetchingLoadingIndicator(), + ); + }); + + it('dispatches hideNftFetchingLoadingIndicator after detection', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = true; + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + await result.current.detectNfts(); + }); + + expect(mockDispatch).toHaveBeenCalledWith( + hideNftFetchingLoadingIndicator(), + ); + }); + + it('calls NftDetectionController.detectNfts with correct chain IDs', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = true; + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + await result.current.detectNfts(); + }); + + expect(mockDetectNfts).toHaveBeenCalledTimes(1); + expect(mockDetectNfts).toHaveBeenCalledWith(mockChainIds); + }); + + it('starts trace before detection', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = true; + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + await result.current.detectNfts(); + }); + + expect(mockTrace).toHaveBeenCalledWith({ name: 'DetectNfts' }); + }); + + it('ends trace after detection', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = true; + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + await result.current.detectNfts(); + }); + + expect(mockEndTrace).toHaveBeenCalledWith({ name: 'DetectNfts' }); + }); + + it('tracks analytics events for newly detected NFTs', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = true; + + const mockEventParams = [ + { chain_id: 1, source: 'detected' as const }, + { chain_id: 137, source: 'detected' as const }, + ]; + mockPrepareNftDetectionEvents.mockReturnValue(mockEventParams); + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + await result.current.detectNfts(); + }); + + expect(mockTrackEvent).toHaveBeenCalledTimes(2); + expect(mockCreateEventBuilder).toHaveBeenCalledWith( + MetaMetricsEvents.COLLECTIBLE_ADDED, + ); + expect(mockAddProperties).toHaveBeenCalledWith({ + chain_id: 1, + source: 'detected', + }); + expect(mockAddProperties).toHaveBeenCalledWith({ + chain_id: 137, + source: 'detected', + }); + }); + + it('does not track events when no new NFTs detected', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = true; + mockPrepareNftDetectionEvents.mockReturnValue([]); + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + await result.current.detectNfts(); + }); + + expect(mockTrackEvent).not.toHaveBeenCalled(); + }); + + it('hides loading indicator even when detection fails', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = true; + mockDetectNfts.mockRejectedValue(new Error('Detection failed')); + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + try { + await result.current.detectNfts(); + } catch (error) { + // Expected error + } + }); + + expect(mockDispatch).toHaveBeenCalledWith( + hideNftFetchingLoadingIndicator(), + ); + }); + + it('ends trace even when detection fails', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = true; + mockDetectNfts.mockRejectedValue(new Error('Detection failed')); + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + try { + await result.current.detectNfts(); + } catch (error) { + // Expected error + } + }); + + expect(mockEndTrace).toHaveBeenCalledWith({ name: 'DetectNfts' }); + }); + + it('calls prepareNftDetectionEvents with previous and new NFT states', async () => { + mockEngine.context.PreferencesController.state.useNftDetection = true; + + const { result } = renderHook(() => useNftDetection()); + + await act(async () => { + await result.current.detectNfts(); + }); + + expect(mockPrepareNftDetectionEvents).toHaveBeenCalledWith( + mockNftControllerState.allNfts[mockSelectedAddress.toLowerCase()], + mockNftControllerState.allNfts[mockSelectedAddress.toLowerCase()], + expect.any(Function), + ); + }); + }); + + describe('getNftDetectionAnalyticsParams', () => { + it('returns correct analytics params for valid NFT', async () => { + mockGetDecimalChainId.mockReturnValue(1); + + const { result } = renderHook(() => useNftDetection()); + + const mockNft = { + address: '0xNFT1', + tokenId: '1', + name: 'Test NFT', + description: 'Test NFT Description', + image: 'test-image.jpg', + chainId: '0x1' as Hex, + standard: 'ERC721', + } as unknown as Nft; + + await act(async () => { + await result.current.detectNfts(); + }); + + // Extract the param builder from the mock call + const prepareEventsCall = mockPrepareNftDetectionEvents.mock.calls[0]; + expect(prepareEventsCall).toBeDefined(); + + const paramBuilder = prepareEventsCall[2]; + const params = paramBuilder(mockNft); + + expect(params).toEqual({ + chain_id: 1, + source: 'detected', + }); + expect(mockGetDecimalChainId).toHaveBeenCalledWith('0x1'); + }); + + it('returns undefined when getDecimalChainId throws error', async () => { + mockGetDecimalChainId.mockImplementation(() => { + throw new Error('Invalid chainId'); + }); + + const { result } = renderHook(() => useNftDetection()); + + const mockNft = { + address: '0xNFT1', + tokenId: '1', + name: 'Test NFT', + description: 'Test NFT Description', + image: 'test-image.jpg', + chainId: 'invalid' as Hex, + standard: 'ERC721', + } as unknown as Nft; + + await act(async () => { + await result.current.detectNfts(); + }); + + // Extract the param builder from the mock call + const prepareEventsCall = mockPrepareNftDetectionEvents.mock.calls[0]; + expect(prepareEventsCall).toBeDefined(); + + const paramBuilder = prepareEventsCall[2]; + const params = paramBuilder(mockNft); + + expect(params).toBeUndefined(); + expect(mockLoggerError).toHaveBeenCalledWith( + expect.any(Error), + 'useNftDetection.getNftDetectionAnalyticsParams', + ); + }); + }); + + describe('return value', () => { + it('returns detectNfts function and chainIdsToDetectNftsFor', () => { + const { result } = renderHook(() => useNftDetection()); + + expect(result.current).toEqual({ + detectNfts: expect.any(Function), + chainIdsToDetectNftsFor: mockChainIds, + }); + }); + }); +}); diff --git a/app/components/hooks/useNftDetection.ts b/app/components/hooks/useNftDetection.ts new file mode 100644 index 00000000000..aa92709ddc8 --- /dev/null +++ b/app/components/hooks/useNftDetection.ts @@ -0,0 +1,104 @@ +import { useCallback } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { cloneDeep } from 'lodash'; +import Engine from '../../core/Engine'; +import { selectSelectedInternalAccountFormattedAddress } from '../../selectors/accountsController'; +import { endTrace, trace, TraceName } from '../../util/trace'; +import { MetaMetricsEvents, useMetrics } from './useMetrics'; +import { useNftDetectionChainIds } from './useNftDetectionChainIds'; +import { prepareNftDetectionEvents } from '../../util/assets'; +import { getDecimalChainId } from '../../util/networks'; +import { Nft } from '@metamask/assets-controllers'; +import Logger from '../../util/Logger'; +import { + hideNftFetchingLoadingIndicator, + showNftFetchingLoadingIndicator, +} from '../../reducers/collectibles'; + +/** + * Hook that provides NFT detection functionality + * Encapsulates the common logic used across the app for detecting NFTs + */ +export const useNftDetection = () => { + const dispatch = useDispatch(); + const { trackEvent, createEventBuilder } = useMetrics(); + + const selectedAddress = useSelector( + selectSelectedInternalAccountFormattedAddress, + ); + const chainIdsToDetectNftsFor = useNftDetectionChainIds(); + + const getNftDetectionAnalyticsParams = useCallback((nft: Nft) => { + try { + return { + chain_id: getDecimalChainId(nft.chainId), + source: 'detected' as const, + }; + } catch (error) { + Logger.error( + error as Error, + 'useNftDetection.getNftDetectionAnalyticsParams', + ); + return undefined; + } + }, []); + + const detectNfts = useCallback(async () => { + if (!selectedAddress) return; + + const { NftDetectionController, NftController, PreferencesController } = + Engine.context; + + // Read fresh state from the controller to avoid stale closure values + const isNftDetectionCurrentlyEnabled = + PreferencesController.state.useNftDetection; + + if (!isNftDetectionCurrentlyEnabled) return; + + const formattedSelectedAddress = selectedAddress.toLowerCase(); + + const previousNfts = cloneDeep( + NftController.state.allNfts[formattedSelectedAddress], + ); + + try { + trace({ name: TraceName.DetectNfts }); + dispatch(showNftFetchingLoadingIndicator()); + + await NftDetectionController.detectNfts(chainIdsToDetectNftsFor); + } finally { + endTrace({ name: TraceName.DetectNfts }); + dispatch(hideNftFetchingLoadingIndicator()); + } + + const newNfts = cloneDeep( + NftController.state.allNfts[formattedSelectedAddress], + ); + + const eventParams = prepareNftDetectionEvents( + previousNfts, + newNfts, + getNftDetectionAnalyticsParams, + ); + + eventParams.forEach((params) => { + trackEvent( + createEventBuilder(MetaMetricsEvents.COLLECTIBLE_ADDED) + .addProperties({ + chain_id: params.chain_id, + source: params.source, + }) + .build(), + ); + }); + }, [ + selectedAddress, + chainIdsToDetectNftsFor, + dispatch, + trackEvent, + createEventBuilder, + getNftDetectionAnalyticsParams, + ]); + + return { detectNfts, chainIdsToDetectNftsFor }; +}; From 7203db7145eedc04f1ea2fb724a1562113eb1480 Mon Sep 17 00:00:00 2001 From: Vince Howard Date: Wed, 10 Dec 2025 09:43:06 -0700 Subject: [PATCH 11/12] fix: improve import assets UI (#23735) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** improve import assets UI - Move network badge to the left of network picker - The close icon "X" no longer appears if no search string exists, but it will appear if they do - Footer button "Next" for Search Tab and Custom Token tab both are aligned and aren't being overlapped with the Android OS navigation bar Other changes that aren't related to the tickets - Improve multi selector layout by removing horizontal padding in parent container ## **Changelog** CHANGELOG entry: Improved import assets UI ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MDP-247 https://consensyssoftware.atlassian.net/browse/MDP-246 https://consensyssoftware.atlassian.net/browse/MDP-594 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** ### Main UI Changes https://consensyssoftware.atlassian.net/browse/MDP-246 | before | after | | -------- | ------- | | ![before](https://github.com/user-attachments/assets/0da6da9a-ac2e-4359-a502-513e9e139404) | ![after](https://github.com/user-attachments/assets/2d15c992-5941-4230-acbe-93d4d88dc49c) | ### Android Button Obstruction Fix - Search Tab https://consensyssoftware.atlassian.net/browse/MDP-594 | before | after | | -------- | ------- | | ![before](https://github.com/user-attachments/assets/89eec7c6-e031-43eb-871c-dc7f1f8ac7b2) | ![after](https://github.com/user-attachments/assets/7569690c-b3dd-433d-8383-dc05a9d0d096) | ### Android Button Obstruction Fix - Custom Token Tab https://consensyssoftware.atlassian.net/browse/MDP-594 | before | after | | -------- | ------- | | ![before](https://github.com/user-attachments/assets/74cf6edf-6b58-4b37-9159-cff4943c1249) | ![after](https://github.com/user-attachments/assets/0ea3ec3b-098e-4760-b073-79554e9a77e9) | ### Close icon disappearing and reappearing based on search string https://consensyssoftware.atlassian.net/browse/MDP-247 https://github.com/user-attachments/assets/1d5d5073-cad5-4d03-989a-b98e1f76e2d5 ### **Before** `~` ### **After** `~` ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- > [!NOTE] > Refines Import Assets screens with left-aligned network picker avatar, conditional clear icon, full-width bottom-aligned Next buttons that respect Android safe-area, and improved search field layout. > > - **Import Token (Custom Token)**: > - Apply safe-area insets (Android) to footer; wrap Next button in `buttonWrapper` with margins and full-width `Button`. > - Add `paddingHorizontal` to `wrapper` and refactor styles; inject `safeAreaInsets` via HOC. > - **Search (AssetSearch/SearchTokenAutocomplete)**: > - Make search input full-width with absolute `Search`/`Close` icons; show `Close` only when text exists. > - Add margins around search input and safe-area-aware padding for footer `Next` button. > - **Add Asset screen**: > - Rework network picker: move network avatar to the left of the label and reduce arrow icon size; add spacing style. > - Adjust container padding to align tabs/content; minor style cleanups. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit d679ae4432baf5fda44c3351efc676464deae520. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../__snapshots__/index.test.tsx.snap | 75 +++++----- app/components/UI/AddCustomToken/index.js | 66 ++++++--- .../__snapshots__/index.test.tsx.snap | 49 +------ app/components/UI/AssetSearch/index.tsx | 81 ++++++----- .../__snapshots__/index.test.tsx.snap | 53 +------ .../UI/SearchTokenAutocomplete/index.tsx | 12 +- .../Views/AddAsset/AddAsset.styles.ts | 4 +- app/components/Views/AddAsset/AddAsset.tsx | 24 ++-- .../__snapshots__/AddAsset.test.tsx.snap | 134 ++++++++++-------- 9 files changed, 237 insertions(+), 261 deletions(-) diff --git a/app/components/UI/AddCustomToken/__snapshots__/index.test.tsx.snap b/app/components/UI/AddCustomToken/__snapshots__/index.test.tsx.snap index da7c91c5bde..b3262832f51 100644 --- a/app/components/UI/AddCustomToken/__snapshots__/index.test.tsx.snap +++ b/app/components/UI/AddCustomToken/__snapshots__/index.test.tsx.snap @@ -6,6 +6,7 @@ exports[`AddCustomToken renders correctly with required props 1`] = ` { "backgroundColor": "#ffffff", "flex": 1, + "paddingHorizontal": 16, } } > @@ -186,50 +187,54 @@ exports[`AddCustomToken renders correctly with required props 1`] = ` - - - Next - - + + Next + + + `; diff --git a/app/components/UI/AddCustomToken/index.js b/app/components/UI/AddCustomToken/index.js index bdce7a1d18b..2a95c0d633c 100644 --- a/app/components/UI/AddCustomToken/index.js +++ b/app/components/UI/AddCustomToken/index.js @@ -1,5 +1,13 @@ import React, { PureComponent } from 'react'; -import { Text, TextInput, View, StyleSheet, ScrollView } from 'react-native'; +import { + Text, + TextInput, + View, + StyleSheet, + ScrollView, + Platform, +} from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { fontStyles } from '../../../styles/common'; import Engine from '../../../core/Engine'; import PropTypes from 'prop-types'; @@ -24,6 +32,7 @@ import { formatIconUrlWithProxy } from '@metamask/assets-controllers'; import Button, { ButtonSize, ButtonVariants, + ButtonWidthTypes, } from '../../../component-library/components/Buttons/Button'; import Icon, { IconName, @@ -37,11 +46,12 @@ import CLText from '../../../component-library/components/Texts/Text/Text'; import Logger from '../../../util/Logger'; import { endTrace, trace, TraceName } from '../../../util/trace'; -const createStyles = (colors) => +const createStyles = (colors, bottomInset = 0) => StyleSheet.create({ wrapper: { backgroundColor: colors.background.default, flex: 1, + paddingHorizontal: 16, }, overlappingAvatarsContainer: { flexDirection: 'row', @@ -56,7 +66,11 @@ const createStyles = (colors) => rowWrapper: { paddingHorizontal: 16, }, - buttonWrapper: {}, + buttonWrapper: { + paddingVertical: 16, + margin: 16, + paddingBottom: bottomInset, + }, textInput: { borderWidth: 1, borderRadius: 8, @@ -102,15 +116,6 @@ const createStyles = (colors) => paddingTop: 4, paddingRight: 8, }, - import: { - fontSize: 18, - color: colors.primary.default, - ...fontStyles.normal, - position: 'relative', - width: '100%', - alignSelf: 'center', - marginBottom: 16, - }, textWrapper: { padding: 0, }, @@ -172,6 +177,11 @@ class AddCustomToken extends PureComponent { * The network client ID */ networkClientId: PropTypes.string, + + /** + * Safe area insets from react-native-safe-area-context + */ + safeAreaInsets: PropTypes.object, }; getTokenAddedAnalyticsParams = () => { @@ -527,7 +537,9 @@ class AddCustomToken extends PureComponent { } = this.state; const colors = this.context.colors || mockTheme.colors; const themeAppearance = this.context.themeAppearance || 'light'; - const styles = createStyles(colors); + const bottomInset = + Platform.OS === 'ios' ? 0 : this.props.safeAreaInsets?.bottom || 0; + const styles = createStyles(colors, bottomInset); const isDisabled = !symbol || !decimals || !this.props.selectedNetwork; const addressInputStyle = onFocusAddress @@ -659,15 +671,17 @@ class AddCustomToken extends PureComponent { ) : null} -