Skip to content

Commit

Permalink
refactor: shared bridge types (#29254)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
There is a lint rule that flags lines in which controllers and ui
components import types/variables/methods from restricted directories.
This change moves the imported elements to the `shared` directory to
satisfy the linter. Also uses `import type` to remove type definitions
from runtime bundles when possible


<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29254?quickstart=1)

## **Related issues**

Fixes: https://consensyssoftware.atlassian.net/browse/MMS-1829

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

N/A

### **After**

N/A - no functional changes

## **Pre-merge author checklist**

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/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-extension/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.
  • Loading branch information
micaelae authored Dec 19, 2024
1 parent b030ba9 commit b114f88
Show file tree
Hide file tree
Showing 40 changed files with 355 additions and 401 deletions.
4 changes: 1 addition & 3 deletions app/scripts/controllers/bridge-status/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import {
StatusRequestWithSrcTxHash,
StatusRequestDto,
} from '../../../../shared/types/bridge-status';
// TODO fix this
// eslint-disable-next-line import/no-restricted-paths
import { Quote } from '../../../../ui/pages/bridge/types';
import type { Quote } from '../../../../shared/types/bridge';
import { validateResponse, validators } from './validators';

const CLIENT_ID_HEADER = { 'X-Client-Id': BRIDGE_CLIENT_ID };
Expand Down
13 changes: 6 additions & 7 deletions app/scripts/controllers/bridge/bridge-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,19 @@ import { BRIDGE_API_BASE_URL } from '../../../../shared/constants/bridge';
import { CHAIN_IDS } from '../../../../shared/constants/network';
import { SWAPS_API_V2_BASE_URL } from '../../../../shared/constants/swaps';
import { flushPromises } from '../../../../test/lib/timer-helpers';
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
import * as bridgeUtil from '../../../../ui/pages/bridge/bridge.util';
import * as bridgeUtil from '../../../../shared/modules/bridge-utils/bridge.util';
import * as balanceUtils from '../../../../shared/modules/bridge-utils/balance';
import mockBridgeQuotesErc20Native from '../../../../test/data/bridge/mock-quotes-erc20-native.json';
import mockBridgeQuotesNativeErc20 from '../../../../test/data/bridge/mock-quotes-native-erc20.json';
import mockBridgeQuotesNativeErc20Eth from '../../../../test/data/bridge/mock-quotes-native-erc20-eth.json';
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
import { QuoteResponse } from '../../../../ui/pages/bridge/types';
import {
type QuoteResponse,
RequestStatus,
} from '../../../../shared/types/bridge';
import { decimalToHex } from '../../../../shared/modules/conversion.utils';
import BridgeController from './bridge-controller';
import { BridgeControllerMessenger } from './types';
import { DEFAULT_BRIDGE_CONTROLLER_STATE, RequestStatus } from './constants';
import { DEFAULT_BRIDGE_CONTROLLER_STATE } from './constants';

const EMPTY_INIT_STATE = {
bridgeState: DEFAULT_BRIDGE_CONTROLLER_STATE,
Expand Down
32 changes: 12 additions & 20 deletions app/scripts/controllers/bridge/bridge-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import {
fetchBridgeFeatureFlags,
fetchBridgeQuotes,
fetchBridgeTokens,
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
} from '../../../../ui/pages/bridge/bridge.util';
} from '../../../../shared/modules/bridge-utils/bridge.util';
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
import { fetchTopAssetsList } from '../../../../ui/pages/swaps/swaps.util';
Expand All @@ -23,30 +21,24 @@ import {
sumHexes,
} from '../../../../shared/modules/conversion.utils';
import {
L1GasFees,
QuoteRequest,
QuoteResponse,
TxData,
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
} from '../../../../ui/pages/bridge/types';
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
import { isValidQuoteRequest } from '../../../../ui/pages/bridge/utils/quote';
type L1GasFees,
type QuoteRequest,
type QuoteResponse,
type TxData,
type BridgeControllerState,
BridgeFeatureFlagsKey,
RequestStatus,
} from '../../../../shared/types/bridge';
import { isValidQuoteRequest } from '../../../../shared/modules/bridge-utils/quote';
import { hasSufficientBalance } from '../../../../shared/modules/bridge-utils/balance';
import { CHAIN_IDS } from '../../../../shared/constants/network';
import { REFRESH_INTERVAL_MS } from '../../../../shared/constants/bridge';
import {
BRIDGE_CONTROLLER_NAME,
DEFAULT_BRIDGE_CONTROLLER_STATE,
REFRESH_INTERVAL_MS,
RequestStatus,
METABRIDGE_CHAIN_TO_ADDRESS_MAP,
} from './constants';
import {
BridgeControllerState,
BridgeControllerMessenger,
BridgeFeatureFlagsKey,
} from './types';
import type { BridgeControllerMessenger } from './types';

const metadata: StateMetadata<{ bridgeState: BridgeControllerState }> = {
bridgeState: {
Expand Down
14 changes: 4 additions & 10 deletions app/scripts/controllers/bridge/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,15 @@ import { zeroAddress } from 'ethereumjs-util';
import { Hex } from '@metamask/utils';
import {
BRIDGE_DEFAULT_SLIPPAGE,
DEFAULT_MAX_REFRESH_COUNT,
METABRIDGE_ETHEREUM_ADDRESS,
REFRESH_INTERVAL_MS,
} from '../../../../shared/constants/bridge';
import { CHAIN_IDS } from '../../../../shared/constants/network';
import { BridgeControllerState, BridgeFeatureFlagsKey } from './types';
import { BridgeFeatureFlagsKey } from '../../../../shared/types/bridge';
import type { BridgeControllerState } from '../../../../shared/types/bridge';

export const BRIDGE_CONTROLLER_NAME = 'BridgeController';
export const REFRESH_INTERVAL_MS = 30 * 1000;
const DEFAULT_MAX_REFRESH_COUNT = 5;

export enum RequestStatus {
LOADING,
FETCHED,
ERROR,
}

export const DEFAULT_BRIDGE_CONTROLLER_STATE: BridgeControllerState = {
bridgeFeatureFlags: {
[BridgeFeatureFlagsKey.EXTENSION_CONFIG]: {
Expand Down
58 changes: 6 additions & 52 deletions app/scripts/controllers/bridge/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,64 +2,18 @@ import {
ControllerStateChangeEvent,
RestrictedControllerMessenger,
} from '@metamask/base-controller';
import { Hex } from '@metamask/utils';
import { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller';
import {
NetworkControllerFindNetworkClientIdByChainIdAction,
NetworkControllerGetSelectedNetworkClientAction,
} from '@metamask/network-controller';
import { SwapsTokenObject } from '../../../../shared/constants/swaps';
import {
L1GasFees,
QuoteRequest,
QuoteResponse,
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
} from '../../../../ui/pages/bridge/types';
import { ChainConfiguration } from '../../../../shared/types/bridge';
import type {
BridgeBackgroundAction,
BridgeControllerState,
BridgeUserAction,
} from '../../../../shared/types/bridge';
import BridgeController from './bridge-controller';
import { BRIDGE_CONTROLLER_NAME, RequestStatus } from './constants';

export enum BridgeFeatureFlagsKey {
EXTENSION_CONFIG = 'extensionConfig',
}

export type BridgeFeatureFlags = {
[BridgeFeatureFlagsKey.EXTENSION_CONFIG]: {
refreshRate: number;
maxRefreshCount: number;
support: boolean;
chains: Record<Hex, ChainConfiguration>;
};
};

export type BridgeControllerState = {
bridgeFeatureFlags: BridgeFeatureFlags;
srcTokens: Record<string, SwapsTokenObject>;
srcTopAssets: { address: string }[];
srcTokensLoadingStatus?: RequestStatus;
destTokensLoadingStatus?: RequestStatus;
destTokens: Record<string, SwapsTokenObject>;
destTopAssets: { address: string }[];
quoteRequest: Partial<QuoteRequest>;
quotes: (QuoteResponse & L1GasFees)[];
quotesInitialLoadTime?: number;
quotesLastFetched?: number;
quotesLoadingStatus?: RequestStatus;
quoteFetchError?: string;
quotesRefreshCount: number;
};

export enum BridgeUserAction {
SELECT_SRC_NETWORK = 'selectSrcNetwork',
SELECT_DEST_NETWORK = 'selectDestNetwork',
UPDATE_QUOTE_PARAMS = 'updateBridgeQuoteRequestParams',
}
export enum BridgeBackgroundAction {
SET_FEATURE_FLAGS = 'setBridgeFeatureFlags',
RESET_STATE = 'resetState',
GET_BRIDGE_ERC20_ALLOWANCE = 'getBridgeERC20Allowance',
}
import { BRIDGE_CONTROLLER_NAME } from './constants';

type BridgeControllerAction<FunctionName extends keyof BridgeController> = {
type: `${typeof BRIDGE_CONTROLLER_NAME}:${FunctionName}`;
Expand Down
8 changes: 4 additions & 4 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ import { isSnapId } from '../../ui/helpers/utils/snaps';
import { BridgeStatusAction } from '../../shared/types/bridge-status';
import { ENVIRONMENT } from '../../development/build/constants';
import fetchWithCache from '../../shared/lib/fetch-with-cache';
import {
BridgeUserAction,
BridgeBackgroundAction,
} from '../../shared/types/bridge';
import { BalancesController as MultichainBalancesController } from './lib/accounts/BalancesController';
import {
///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
Expand Down Expand Up @@ -365,10 +369,6 @@ import { updateSecurityAlertResponse } from './lib/ppom/ppom-util';
import createEvmMethodsToNonEvmAccountReqFilterMiddleware from './lib/createEvmMethodsToNonEvmAccountReqFilterMiddleware';
import { isEthAddress } from './lib/multichain/address';
import { decodeTransactionData } from './lib/transaction/decode/util';
import {
BridgeUserAction,
BridgeBackgroundAction,
} from './controllers/bridge/types';
import BridgeController from './controllers/bridge/bridge-controller';
import { BRIDGE_CONTROLLER_NAME } from './controllers/bridge/constants';
import {
Expand Down
2 changes: 2 additions & 0 deletions shared/constants/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,5 @@ export const NETWORK_TO_SHORT_NETWORK_NAME_MAP: Record<
[CHAIN_IDS.BASE]: 'Base',
};
export const BRIDGE_MM_FEE_RATE = 0.875;
export const REFRESH_INTERVAL_MS = 30 * 1000;
export const DEFAULT_MAX_REFRESH_COUNT = 5;
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import fetchWithCache from '../../../shared/lib/fetch-with-cache';
import { CHAIN_IDS } from '../../../shared/constants/network';
import { zeroAddress } from 'ethereumjs-util';
import fetchWithCache from '../../lib/fetch-with-cache';
import { CHAIN_IDS } from '../../constants/network';
import mockBridgeQuotesErc20Erc20 from '../../../test/data/bridge/mock-quotes-erc20-erc20.json';
import mockBridgeQuotesNativeErc20 from '../../../test/data/bridge/mock-quotes-native-erc20.json';
import { zeroAddress } from '../../__mocks__/ethereumjs-util';
import {
fetchBridgeFeatureFlags,
fetchBridgeQuotes,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,36 +1,25 @@
import { Contract } from '@ethersproject/contracts';
import { Hex, add0x } from '@metamask/utils';
import { abiERC20 } from '@metamask/metamask-eth-abis';
import {
BridgeFeatureFlagsKey,
BridgeFeatureFlags,
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
} from '../../../app/scripts/controllers/bridge/types';
import {
BRIDGE_API_BASE_URL,
BRIDGE_CLIENT_ID,
ETH_USDT_ADDRESS,
METABRIDGE_ETHEREUM_ADDRESS,
} from '../../../shared/constants/bridge';
import { MINUTE } from '../../../shared/constants/time';
import fetchWithCache from '../../../shared/lib/fetch-with-cache';
import {
decimalToHex,
hexToDecimal,
} from '../../../shared/modules/conversion.utils';
REFRESH_INTERVAL_MS,
} from '../../constants/bridge';
import { MINUTE } from '../../constants/time';
import fetchWithCache from '../../lib/fetch-with-cache';
import { decimalToHex, hexToDecimal } from '../conversion.utils';
import {
SWAPS_CHAINID_DEFAULT_TOKEN_MAP,
SwapsTokenObject,
} from '../../../shared/constants/swaps';
} from '../../constants/swaps';
import {
isSwapsDefaultTokenAddress,
isSwapsDefaultTokenSymbol,
} from '../../../shared/modules/swaps.utils';
// TODO: Remove restricted import
// eslint-disable-next-line import/no-restricted-paths
import { REFRESH_INTERVAL_MS } from '../../../app/scripts/controllers/bridge/constants';
import { CHAIN_IDS } from '../../../shared/constants/network';
} from '../swaps.utils';
import { CHAIN_IDS } from '../../constants/network';
import {
BridgeAsset,
BridgeFlag,
Expand All @@ -41,7 +30,9 @@ import {
QuoteRequest,
QuoteResponse,
TxData,
} from './types';
BridgeFeatureFlagsKey,
BridgeFeatureFlags,
} from '../../types/bridge';
import {
FEATURE_FLAG_VALIDATORS,
QUOTE_VALIDATORS,
Expand All @@ -50,7 +41,7 @@ import {
validateResponse,
QUOTE_RESPONSE_VALIDATORS,
FEE_DATA_VALIDATORS,
} from './utils/validators';
} from './validators';

const CLIENT_ID_HEADER = { 'X-Client-Id': BRIDGE_CLIENT_ID };
const CACHE_REFRESH_TEN_MINUTES = 10 * MINUTE;
Expand Down
36 changes: 36 additions & 0 deletions shared/modules/bridge-utils/quote.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import type { QuoteRequest } from '../../types/bridge';

export const isValidQuoteRequest = (
partialRequest: Partial<QuoteRequest>,
requireAmount = true,
): partialRequest is QuoteRequest => {
const STRING_FIELDS = ['srcTokenAddress', 'destTokenAddress'];
if (requireAmount) {
STRING_FIELDS.push('srcTokenAmount');
}
const NUMBER_FIELDS = ['srcChainId', 'destChainId', 'slippage'];

return (
STRING_FIELDS.every(
(field) =>
field in partialRequest &&
typeof partialRequest[field as keyof typeof partialRequest] ===
'string' &&
partialRequest[field as keyof typeof partialRequest] !== undefined &&
partialRequest[field as keyof typeof partialRequest] !== '' &&
partialRequest[field as keyof typeof partialRequest] !== null,
) &&
NUMBER_FIELDS.every(
(field) =>
field in partialRequest &&
typeof partialRequest[field as keyof typeof partialRequest] ===
'number' &&
partialRequest[field as keyof typeof partialRequest] !== undefined &&
!isNaN(Number(partialRequest[field as keyof typeof partialRequest])) &&
partialRequest[field as keyof typeof partialRequest] !== null,
) &&
(requireAmount
? Boolean((partialRequest.srcTokenAmount ?? '').match(/^[1-9]\d*$/u))
: true)
);
};
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import { isStrictHexString } from '@metamask/utils';
import { isValidHexAddress as isValidHexAddress_ } from '@metamask/controller-utils';
import {
truthyDigitString,
validateData,
} from '../../../../shared/lib/swaps-utils';
import { BridgeFlag, FeatureFlagResponse } from '../types';
import { truthyDigitString, validateData } from '../../lib/swaps-utils';
import { BridgeFlag, FeatureFlagResponse } from '../../types/bridge';

type Validator<ExpectedResponse> = {
property: keyof ExpectedResponse | string;
Expand Down
9 changes: 1 addition & 8 deletions shared/types/bridge-status.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
import { TransactionMeta } from '@metamask/transaction-controller';
// TODO fix this
import {
ChainId,
Quote,
QuoteMetadata,
QuoteResponse,
// eslint-disable-next-line import/no-restricted-paths
} from '../../ui/pages/bridge/types';
import type { ChainId, Quote, QuoteMetadata, QuoteResponse } from './bridge';

// All fields need to be types not interfaces, same with their children fields
// o/w you get a type error
Expand Down
Loading

0 comments on commit b114f88

Please sign in to comment.