Skip to content

Commit

Permalink
Merge branch 'develop' into caip-multichain
Browse files Browse the repository at this point in the history
  • Loading branch information
jiexi committed Oct 15, 2024
2 parents df11376 + 5814224 commit 71ed3aa
Show file tree
Hide file tree
Showing 20 changed files with 326 additions and 42 deletions.
12 changes: 3 additions & 9 deletions .yarnrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,9 @@ npmAuditIgnoreAdvisories:
# upon old versions of ethereumjs-utils.
- 'ethereum-cryptography (deprecation)'

# Currently only dependent on deprecated @metamask/types as it is brought in
# by @metamask/keyring-api. Updating the dependency in keyring-api will
# remove this.
- '@metamask/types (deprecation)'

# @metamask/keyring-api also depends on @metamask/snaps-ui which is
# deprecated. Replacing that dependency with @metamask/snaps-sdk will remove
# this.
- '@metamask/snaps-ui (deprecation)'
# Currently in use for the network list drag and drop functionality.
# Maintenance has stopped and the project will be archived in 2025.
- 'react-beautiful-dnd (deprecation)'

npmRegistries:
'https://npm.pkg.github.com':
Expand Down
28 changes: 23 additions & 5 deletions shared/lib/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ const ID_DEFAULT = 'default';
const OP_DEFAULT = 'custom';

const tracesByKey: Map<string, PendingTrace> = new Map();
const durationsByName: { [name: string]: number } = {};

if (process.env.IN_TEST && globalThis.stateHooks) {
globalThis.stateHooks.getCustomTraces = () => durationsByName;
}

type PendingTrace = {
end: (timestamp?: number) => void;
Expand Down Expand Up @@ -155,9 +160,8 @@ export function endTrace(request: EndTraceRequest) {

const { request: pendingRequest, startTime } = pendingTrace;
const endTime = timestamp ?? getPerformanceTimestamp();
const duration = endTime - startTime;

log('Finished trace', name, id, duration, { request: pendingRequest });
logTrace(pendingRequest, startTime, endTime);
}

function traceCallback<T>(request: TraceRequest, fn: TraceCallback<T>): T {
Expand All @@ -181,9 +185,7 @@ function traceCallback<T>(request: TraceRequest, fn: TraceCallback<T>): T {
},
() => {
const end = Date.now();
const duration = end - start;

log('Finished trace', name, duration, { error, request });
logTrace(request, start, end, error);
},
) as T;
};
Expand Down Expand Up @@ -242,6 +244,22 @@ function startSpan<T>(
});
}

function logTrace(
request: TraceRequest,
startTime: number,
endTime: number,
error?: unknown,
) {
const duration = endTime - startTime;
const { name } = request;

if (process.env.IN_TEST) {
durationsByName[name] = duration;
}

log('Finished trace', name, duration, { request, error });
}

function getTraceId(request: TraceRequest) {
return request.id ?? ID_DEFAULT;
}
Expand Down
17 changes: 16 additions & 1 deletion test/e2e/benchmark.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ const FixtureBuilder = require('./fixture-builder');
const DEFAULT_NUM_SAMPLES = 20;
const ALL_PAGES = Object.values(PAGES);

const CUSTOM_TRACES = {
backgroundConnect: 'Background Connect',
firstReactRender: 'First Render',
getState: 'Get State',
initialActions: 'Initial Actions',
loadScripts: 'Load Scripts',
setupStore: 'Setup Store',
uiStartup: 'UI Startup',
};

async function measurePage(pageName) {
let metrics;
await withFixtures(
Expand All @@ -32,6 +42,7 @@ async function measurePage(pageName) {
await driver.findElement('[data-testid="account-menu-icon"]');
await driver.navigate(pageName);
await driver.delay(1000);

metrics = await driver.collectMetrics();
},
);
Expand Down Expand Up @@ -79,7 +90,7 @@ async function profilePageLoad(pages, numSamples, retries) {
runResults.push(result);
}

if (runResults.some((result) => result.navigation.lenth > 1)) {
if (runResults.some((result) => result.navigation.length > 1)) {
throw new Error(`Multiple navigations not supported`);
} else if (
runResults.some((result) => result.navigation[0].type !== 'navigate')
Expand Down Expand Up @@ -107,6 +118,10 @@ async function profilePageLoad(pages, numSamples, retries) {
),
};

for (const [key, name] of Object.entries(CUSTOM_TRACES)) {
result[key] = runResults.map((metrics) => metrics[name]);
}

results[pageName] = {
min: minResult(result),
max: maxResult(result),
Expand Down
27 changes: 15 additions & 12 deletions test/e2e/tests/network/multi-rpc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,12 +396,10 @@ describe('MultiRpc:', function (this: Suite) {
await driver.delay(regularDelayMs);

// go to advanced settigns
await driver.clickElement({
await driver.clickElementAndWaitToDisappear({
text: 'Manage default settings',
});

await driver.delay(regularDelayMs);

await driver.clickElement({
text: 'General',
});
Expand All @@ -420,23 +418,18 @@ describe('MultiRpc:', function (this: Suite) {
tag: 'button',
});

await driver.clickElement({
await driver.clickElementAndWaitToDisappear({
text: 'Save',
tag: 'button',
});

await driver.delay(regularDelayMs);
await driver.waitForSelector('[data-testid="category-back-button"]');
await driver.clickElement('[data-testid="category-back-button"]');

await driver.waitForSelector(
'[data-testid="privacy-settings-back-button"]',
);
await driver.clickElement(
'[data-testid="privacy-settings-back-button"]',
);

await driver.clickElement({
await driver.clickElementAndWaitToDisappear({
text: 'Done',
tag: 'button',
});
Expand All @@ -446,7 +439,7 @@ describe('MultiRpc:', function (this: Suite) {
tag: 'button',
});

await driver.clickElement({
await driver.clickElementAndWaitToDisappear({
text: 'Done',
tag: 'button',
});
Expand All @@ -461,7 +454,17 @@ describe('MultiRpc:', function (this: Suite) {
'“Arbitrum One” was successfully edited!',
);
// Ensures popover backround doesn't kill test
await driver.delay(regularDelayMs);
await driver.assertElementNotPresent('.popover-bg');

// We need to use clickElementSafe + assertElementNotPresent as sometimes the network dialog doesn't appear, as per this issue (#27870)
// TODO: change the 2 actions for clickElementAndWaitToDisappear, once the issue is fixed
await driver.clickElementSafe({ tag: 'h6', text: 'Got it' });

await driver.assertElementNotPresent({
tag: 'h6',
text: 'Got it',
});

await driver.clickElement('[data-testid="network-display"]');

const arbitrumRpcUsed = await driver.findElement({
Expand Down
5 changes: 4 additions & 1 deletion test/e2e/webdriver/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,10 @@ function collectMetrics() {
});
});

return results;
return {
...results,
...window.stateHooks.getCustomTraces(),
};
}

module.exports = { Driver, PAGES };
3 changes: 3 additions & 0 deletions test/jest/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,14 @@ export function createMockInternalAccount({
address = MOCK_DEFAULT_ADDRESS,
type = EthAccountType.Eoa,
keyringType = KeyringTypes.hd,
lastSelected = 0,
snapOptions = undefined,
}: {
name?: string;
address?: string;
type?: string;
keyringType?: string;
lastSelected?: number;
snapOptions?: {
enabled: boolean;
name: string;
Expand Down Expand Up @@ -228,6 +230,7 @@ export function createMockInternalAccount({
type: keyringType,
},
snap: snapOptions,
lastSelected,
},
options: {},
methods,
Expand Down
1 change: 1 addition & 0 deletions types/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ type HttpProvider = {
};

type StateHooks = {
getCustomTraces?: () => { [name: string]: number };
getCleanAppState?: () => Promise<any>;
getLogs?: () => any[];
getMostRecentPersistedState?: () => any;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { InternalAccount } from '@metamask/keyring-api';
import type { CurrencyDisplayProps } from '../../ui/currency-display/currency-display.component';
import type { PRIMARY, SECONDARY } from '../../../helpers/constants/common';

export type UserPrefrencedCurrencyDisplayProps = OverridingUnion<
CurrencyDisplayProps,
{
type?: PRIMARY | SECONDARY;
account?: InternalAccount;
currency?: string;
showEthLogo?: boolean;
ethNumberOfDecimals?: string | number;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, { useMemo } from 'react';
import PropTypes from 'prop-types';
import { useSelector } from 'react-redux';
import { EtherDenomination } from '../../../../shared/constants/common';
import { PRIMARY, SECONDARY } from '../../../helpers/constants/common';
import CurrencyDisplay from '../../ui/currency-display';
Expand All @@ -10,13 +11,14 @@ import {
getMultichainCurrentNetwork,
} from '../../../selectors/multichain';
import { useMultichainSelector } from '../../../hooks/useMultichainSelector';
import { getSelectedEvmInternalAccount } from '../../../selectors';

/* eslint-disable jsdoc/require-param-name */
// eslint-disable-next-line jsdoc/require-param
/** @param {PropTypes.InferProps<typeof UserPreferencedCurrencyDisplayPropTypes>>} */
export default function UserPreferencedCurrencyDisplay({
'data-testid': dataTestId,
account,
account: multichainAccount,
ethNumberOfDecimals,
fiatNumberOfDecimals,
numberOfDecimals: propsNumberOfDecimals,
Expand All @@ -28,6 +30,15 @@ export default function UserPreferencedCurrencyDisplay({
shouldCheckShowNativeToken,
...restProps
}) {
// NOTE: When displaying currencies, we need the actual account to detect whether we're in a
// multichain world or EVM-only world.
// To preserve the original behavior of this component, we default to the lastly selected
// EVM accounts (when used in an EVM-only context).
// The caller has to pass the account in a multichain context to properly display the currency
// here (e.g for Bitcoin).
const evmAccount = useSelector(getSelectedEvmInternalAccount);
const account = multichainAccount ?? evmAccount;

const currentNetwork = useMultichainSelector(
getMultichainCurrentNetwork,
account,
Expand Down
2 changes: 2 additions & 0 deletions ui/components/app/wallet-overview/coin-overview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export const CoinOverview = ({

///: END:ONLY_INCLUDE_IF

const account = useSelector(getSelectedAccount);
const showNativeTokenAsMainBalanceRoute = getSpecificSettingsRoute(
t,
t('general'),
Expand Down Expand Up @@ -254,6 +255,7 @@ export const CoinOverview = ({
{balanceToDisplay ? (
<UserPreferencedCurrencyDisplay
style={{ display: 'contents' }}
account={account}
className={classnames(
`${classPrefix}-overview__primary-balance`,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ const mockSetShowTestNetworks = jest.fn();
const mockToggleNetworkMenu = jest.fn();
const mockSetNetworkClientIdForDomain = jest.fn();
const mockSetActiveNetwork = jest.fn();
const mockUpdateCustomNonce = jest.fn();
const mockSetNextNonce = jest.fn();

jest.mock('../../../store/actions.ts', () => ({
setShowTestNetworks: () => mockSetShowTestNetworks,
setActiveNetwork: () => mockSetActiveNetwork,
toggleNetworkMenu: () => mockToggleNetworkMenu,
updateCustomNonce: () => mockUpdateCustomNonce,
setNextNonce: () => mockSetNextNonce,
setNetworkClientIdForDomain: (network, id) =>
mockSetNetworkClientIdForDomain(network, id),
}));
Expand Down Expand Up @@ -206,6 +210,8 @@ describe('NetworkListMenu', () => {
fireEvent.click(getByText(MAINNET_DISPLAY_NAME));
expect(mockToggleNetworkMenu).toHaveBeenCalled();
expect(mockSetActiveNetwork).toHaveBeenCalled();
expect(mockUpdateCustomNonce).toHaveBeenCalled();
expect(mockSetNextNonce).toHaveBeenCalled();
});

it('shows the correct selected network when networks share the same chain ID', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import {
setEditedNetwork,
grantPermittedChain,
showPermittedNetworkToast,
updateCustomNonce,
setNextNonce,
} from '../../../store/actions';
import {
CHAIN_ID_TO_NETWORK_IMAGE_URL_MAP,
Expand Down Expand Up @@ -277,6 +279,8 @@ export const NetworkListMenu = ({ onClose }: { onClose: () => void }) => {
network.rpcEndpoints[network.defaultRpcEndpointIndex];
dispatch(setActiveNetwork(networkClientId));
dispatch(toggleNetworkMenu());
dispatch(updateCustomNonce(''));
dispatch(setNextNonce(''));

if (permittedAccountAddresses.length > 0) {
grantPermittedChain(selectedTabOrigin, network.chainId);
Expand Down
16 changes: 16 additions & 0 deletions ui/ducks/swaps/swaps.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { captureMessage } from '@sentry/browser';

import { TransactionType } from '@metamask/transaction-controller';
import { createProjectLogger } from '@metamask/utils';
import { CHAIN_IDS } from '../../../shared/constants/network';
import {
addToken,
addTransactionAndWaitForPublish,
Expand Down Expand Up @@ -1284,6 +1285,21 @@ export const signAndSendTransactions = (
},
},
);
if (
[
CHAIN_IDS.LINEA_MAINNET,
CHAIN_IDS.LINEA_GOERLI,
CHAIN_IDS.LINEA_SEPOLIA,
].includes(chainId)
) {
debugLog(
'Delaying submitting trade tx to make Linea confirmation more likely',
);
const waitPromise = new Promise((resolve) =>
setTimeout(resolve, 5000),
);
await waitPromise;
}
} catch (e) {
debugLog('Approve transaction failed', e);
await dispatch(setSwapsErrorKey(SWAP_FAILED_ERROR));
Expand Down
19 changes: 19 additions & 0 deletions ui/helpers/utils/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,25 @@ export function getAccountByAddress(accounts = [], targetAddress) {
return accounts.find(({ address }) => address === targetAddress);
}

/**
* Sort the given list of account their selecting order (descending). Meaning the
* first account of the sorted list will be the last selected account.
*
* @param {import('@metamask/keyring-api').InternalAccount[]} accounts - The internal accounts list.
* @returns {import('@metamask/keyring-api').InternalAccount[]} The sorted internal account list.
*/
export function sortSelectedInternalAccounts(accounts) {
// This logic comes from the `AccountsController`:
// TODO: Expose a free function from this controller and use it here
return accounts.sort((accountA, accountB) => {
// Sort by `.lastSelected` in descending order
return (
(accountB.metadata.lastSelected ?? 0) -
(accountA.metadata.lastSelected ?? 0)
);
});
}

/**
* Strips the following schemes from URL strings:
* - http
Expand Down
Loading

0 comments on commit 71ed3aa

Please sign in to comment.