Skip to content

refactor(activity): migrate non-EVM transaction mapping - #34682

Open
n3ps wants to merge 9 commits into
mainfrom
n3ps/client-utils-keyring-transaction-mapper-947d
Open

refactor(activity): migrate non-EVM transaction mapping#34682
n3ps wants to merge 9 commits into
mainfrom
n3ps/client-utils-keyring-transaction-mapper-947d

Conversation

@n3ps

@n3ps n3ps commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Description

Migrate call sites to import mapKeyringTransaction directly from @metamask/client-utils (same as extension)

Call sites apply the bridge quote for cross-chain bridge history as needed.

Related PRs:

Changelog

CHANGELOG entry: null

Related issues

Refs: #34655

Manual testing steps

Feature: non-EVM activity mapping via client-utils

  Scenario: Solana send appears in activity
    Given a wallet with Solana activity

    When the user opens the Activity list
    Then Solana sends/receives/swaps render via mapKeyringTransaction from @metamask/client-utils
    And cross-chain bridges show bridge enrichment from bridge history

Screenshots/Recordings

N/A — mapper refactor; no UI design changes

Before

N/A

After

image

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
  • I've tested with a power user scenario
  • I've instrumented key operations with Sentry traces for production performance metrics

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

Medium Risk
Touches unified and multichain activity pipelines and bridge pending/success semantics; dependency bump on client-utils, but behavior is covered by new and updated tests.

Overview
Non-EVM activity rows now use mapKeyringTransaction from @metamask/client-utils (v2.0.1) instead of the removed mobile vendored mapper. ActivityListItem / ActivityKind are aligned with client-utils types, with a thin mobile layer for perps/stake and similar kinds.

Cross-chain bridge enrichment is no longer inside the keyring mapper. A new applyBridgeQuote helper rewrites mapped items to bridge with quote-derived tokens and status (pending until the dest leg completes). mapNonEvmTransactions, ActivityList, and MultichainTransactionsView call it when bridge history is cross-chain; same-chain swaps stay swaps.

Multichain asset-detail rows take a pre-built ActivityListItem and resolve the raw keyring tx from Redux only for analytics and legacy sheets. rampBuy / rampSell get buy/sell titles and icons. Token display treats missing amounts with a symbol as 0 (e.g. -0 ETH) in list/details formatting.

Reviewed by Cursor Bugbot for commit 812cb3a. Bugbot is set up for automated code reviews on this repo. Configure here.

@n3ps n3ps added team-core-extension-ux Core Extension UX team no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed labels Aug 12, 2026
@github-actions github-actions Bot added the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label Aug 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@socket-security

socket-security Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatednpm/​@​metamask/​client-utils@​1.2.1 ⏵ 2.0.177 +110079 +297 +3100

View full report

@n3ps n3ps changed the title refactor(activity): use client-utils for keyring/non-EVM transaction mapping refactor(activity): migrate non-evm transaction mapping Aug 12, 2026
n3ps and others added 3 commits August 12, 2026 16:39
Pick up shared mapper exports and treat missing token amounts with
symbol/assetId as 0 in getHumanReadableTokenAmount.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ctly

Remove the mobile keyring mapper wrapper. Call sites import
mapKeyringTransaction from @metamask/client-utils and attach mobile raw
plus bridge enrichment at the call site, matching extension.

Co-authored-by: Cursor <cursoragent@cursor.com>
Drop stale account-tree-controller descriptor range so yarn --immutable
succeeds in CI.

Co-authored-by: Francis Nepomuceno <n3ps@users.noreply.github.com>
@cursor
cursor Bot force-pushed the n3ps/client-utils-keyring-transaction-mapper-947d branch from 24bf7c0 to 2429c81 Compare August 12, 2026 16:40
cursoragent and others added 3 commits August 12, 2026 16:57
Prettier-fix call sites, allow optional bridge.from for enrichment, and
default missing transaction.fees before mapKeyringTransaction so tests and
incomplete fixtures do not crash in client-utils getFees.

Co-authored-by: Francis Nepomuceno <n3ps@users.noreply.github.com>
Address flaky-test bot feedback on MultichainTransactionsView by resetting
fake timers after each test, and align activityTokenFormat with the
client-utils missing-amount-as-zero display.

Co-authored-by: Francis Nepomuceno <n3ps@users.noreply.github.com>
Match extension: re-export Status/Fee/FiatAmount and base ActivityItem from
client-utils, keep only mobile transition extras (raw, isEarliestNonce,
isUnlimitedApproval, stake/unstake/swapIncomplete, perps order kinds).

Co-authored-by: Francis Nepomuceno <n3ps@users.noreply.github.com>
* Produces ActivityListItem[] from API EVM and non-EVM transaction sources.
* Local EVM transactions are handled separately by useLocalActivityItems.
*/
import { mapKeyringTransaction } from '@metamask/client-utils';

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Point of interest

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed this

@n3ps
n3ps force-pushed the n3ps/client-utils-keyring-transaction-mapper-947d branch 3 times, most recently from 75cd117 to c45456a Compare August 15, 2026 01:37
@n3ps
n3ps marked this pull request as ready for review August 15, 2026 01:39
@n3ps
n3ps requested a review from a team as a code owner August 15, 2026 01:39
@n3ps n3ps changed the title refactor(activity): migrate non-evm transaction mapping refactor(activity): migrate non-EVM transaction mapping Aug 15, 2026
@github-actions github-actions Bot added the risk:high AI analysis: high risk label Aug 15, 2026
Comment thread app/components/Views/ActivityList/helpers/apply-bridge-quote.ts
@n3ps
n3ps force-pushed the n3ps/client-utils-keyring-transaction-mapper-947d branch from c45456a to 04724d3 Compare August 15, 2026 02:01
@MetaMask MetaMask deleted a comment from github-actions Bot Aug 15, 2026
Comment thread app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx Outdated
@MetaMask MetaMask deleted a comment from github-actions Bot Aug 15, 2026
@sonarqubecloud

Copy link
Copy Markdown

@n3ps
n3ps force-pushed the n3ps/client-utils-keyring-transaction-mapper-947d branch from ff90a94 to b7914ce Compare August 15, 2026 15:01

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b7914ce. Configure here.

@MetaMask MetaMask deleted a comment from github-actions Bot Aug 15, 2026
@n3ps
n3ps force-pushed the n3ps/client-utils-keyring-transaction-mapper-947d branch from b7914ce to 812cb3a Compare August 15, 2026 15:22
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx 0/138 0/178 0/378

AI-detected flaky patterns

app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx

  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The 'ActivityListItemRow — display currency conversion' describe block uses afterEach (not beforeEach) to call jest.clearAllMocks() and reset mock return values. This means the very first test in this block runs with whatever mock state was left by the last test in the preceding 'ActivityListItemRow — row content' describe block. If Jest runs tests in a different order (e.g. with --randomize) or if a test in the previous block sets a mock return value that affects currency/rate selectors, the first test in this block will see stale mock state and may produce a different result than expected. The fix is to move the mock reset into a beforeEach so every test — including the first — starts from a known baseline.
    • Suggested fix in app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx:1:
      -describe('ActivityListItemRow — display currency conversion', () => {
      -  const mockCurrency = jest.mocked(selectCurrentCurrency);
      -  const mockConversionRate = jest.mocked(selectConversionRateByChainId);
      -  const mockUsdConversionRate = jest.mocked(selectUSDConversionRateByChainId);
      -  afterEach(() => {
      -    jest.clearAllMocks();
      -    mockCurrency.mockReturnValue('usd');
      -    mockConversionRate.mockReturnValue(2500);
      -    mockUsdConversionRate.mockReturnValue(2500);
      -    // ...
      -  });
      +describe('ActivityListItemRow — display currency conversion', () => {
      +  const mockCurrency = jest.mocked(selectCurrentCurrency);
      +  const mockConversionRate = jest.mocked(selectConversionRateByChainId);
      +  const mockUsdConversionRate = jest.mocked(selectUSDConversionRateByChainId);
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +    mockCurrency.mockReturnValue('usd');
      +    mockConversionRate.mockReturnValue(2500);
      +    mockUsdConversionRate.mockReturnValue(2500);
      +  });
      +  afterEach(() => {
      +    jest.clearAllMocks();
      +  });
  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The 'ActivityListItemRow — ERC-20 fiat address casing' describe block also uses afterEach (not beforeEach) to clear mocks and reset the contract exchange rates mock. The first test in this block runs with whatever mock state was left by the last test in the preceding describe block ('ActivityListItemRow — amount display'), which has no clearAllMocks at all. If the preceding block sets selectContractExchangeRatesByChainId to a value that differs from the expected default, the first test here will see stale state. Moving the reset to beforeEach guarantees a clean baseline for every test including the first.
    • Suggested fix in app/components/UI/ActivityListItemRow/ActivityListItemRow.test.tsx:1:
      -describe('ActivityListItemRow — ERC-20 fiat address casing (TMCU-937)', () => {
      -  const mockContractExchangeRates = jest.mocked(
      -    selectContractExchangeRatesByChainId,
      -  );
      -  // ...
      -  afterEach(() => {
      -    jest.clearAllMocks();
      -    mockContractExchangeRates.mockReturnValue(ratesFor(LINEA_MUSD_ADDRESS));
      -  });
      -  it('renders fiat for an ERC-20 when market data is keyed by a checksummed address', () => {
      +describe('ActivityListItemRow — ERC-20 fiat address casing (TMCU-937)', () => {
      +  const mockContractExchangeRates = jest.mocked(
      +    selectContractExchangeRatesByChainId,
      +  );
      +  const ratesFor = (address: string) =>
      +    ({ [address]: { price: 0.0004 } }) as ReturnType<
      +      typeof selectContractExchangeRatesByChainId
      +    >;
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +    mockContractExchangeRates.mockReturnValue(ratesFor(LINEA_MUSD_ADDRESS));
      +  });
      +  afterEach(() => {
      +    jest.clearAllMocks();
      +  });

This check is informational only and does not block merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. risk:high AI analysis: high risk size-XL team-core-extension-ux Core Extension UX team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants