Skip to content

Commit

Permalink
refactor: remove global network usage from transaction confirmations (#…
Browse files Browse the repository at this point in the history
…12955)

## **Description**

Remove all global network references from the transaction confirmations
and confirmations in general.

Note that some exceptions remain in the send components as these
transactions are triggered internally so cannot rely on a dApp selected
network.

Specifically:

- Add component properties:
  - `AddressElement` > `chainId`
  - `AddressList` > `chainId`
  - `SendFlowAddressFrom` > `chainId`
- Add selectors:
  - `selectConversionRateByChainId`
  - `selectProviderTypeByChainId`
  - `selectRpcUrlByChainId`
  - `selectContractExchangeRatesByChainId`
- Update linting rule to apply to entire
`app/components/Views/confirmations` directory.

## **Related issues**

Fixes: [#2025](MetaMask/mobile-planning#2025)

## **Manual testing steps**

## **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.
  • Loading branch information
matthewwalsh0 authored Jan 24, 2025
1 parent 9c39bb6 commit 2ce6596
Show file tree
Hide file tree
Showing 43 changed files with 615 additions and 493 deletions.
12 changes: 6 additions & 6 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,18 @@ module.exports = {
'app/components/UI/Name/**/*.{js,ts,tsx}',
'app/components/UI/SimulationDetails/**/*.{js,ts,tsx}',
'app/components/hooks/DisplayName/**/*.{js,ts,tsx}',
'app/components/Views/confirmations/components/Confirm/DataTree/**/*.{js,ts,tsx}',
'app/components/Views/confirmations/components/Confirm/Info/**/*.{js,ts,tsx}',
'app/components/Views/confirmations/components/PersonalSign/**/*.{js,ts,tsx}',
'app/components/Views/confirmations/components/SignatureRequest/**/*.{js,ts,tsx}',
'app/components/Views/confirmations/components/TypedSign/**/*.{js,ts,tsx}',
'app/components/Views/confirmations/**/*.{js,ts,tsx}'
],
excludedFiles: [
'app/components/Views/confirmations/components/WatchAssetRequest/**/*.{js,ts,tsx}'],
rules: {
'no-restricted-syntax': [
'error',
{
selector: `ImportSpecifier[imported.name=/${[
'selectChainId',
'selectContractExchangeRates',
'selectConversionRate',
'selectNetworkClientId',
'selectNetworkStatus',
'selectNickname',
Expand All @@ -99,7 +99,7 @@ module.exports = {
'selectSelectedNetworkClientId',
'selectTicker',
]
.map((method) => `(${method})`)
.map((method) => `^${method}$`)
.join('|')}/]`,
message: 'Avoid using global network selectors in confirmations',
},
Expand Down
3 changes: 3 additions & 0 deletions app/components/Nav/Main/RootRPCMethodsUI.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ const RootRPCMethodsUI = (props) => {
autoSign(transactionMeta);
} else {
const {
chainId,
networkClientId,
txParams: { value, gas, gasPrice, data },
} = transactionMeta;
Expand Down Expand Up @@ -448,6 +449,7 @@ const RootRPCMethodsUI = (props) => {
origin: transactionMeta.origin,
securityAlertResponse: transactionMeta.securityAlertResponse,
networkClientId,
chainId,
...transactionMeta.txParams,
});
} else {
Expand All @@ -460,6 +462,7 @@ const RootRPCMethodsUI = (props) => {
id: transactionMeta.id,
origin: transactionMeta.origin,
securityAlertResponse: transactionMeta.securityAlertResponse,
chainId,
networkClientId,
...transactionMeta.txParams,
});
Expand Down
6 changes: 3 additions & 3 deletions app/components/UI/SimulationDetails/useBalanceChanges.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ import {
import { fetchTokenContractExchangeRates } from '@metamask/assets-controllers';

import { getTokenDetails } from '../../../util/address';
import { selectConversionRate } from '../../../selectors/currencyRateController';
import useBalanceChanges from './useBalanceChanges';
import { FIAT_UNAVAILABLE, AssetType } from './types';
import { selectConversionRateByChainId } from '../../../selectors/currencyRateController';

jest.mock('react-redux', () => ({
...jest.requireActual('react-redux'),
useSelector: jest.fn().mockImplementation((callback) => callback()),
}));

jest.mock('../../../selectors/currencyRateController', () => ({
selectConversionRate: jest.fn(),
selectConversionRateByChainId: jest.fn(),
selectCurrentCurrency: jest.fn(),
}));

Expand All @@ -37,7 +37,7 @@ jest.mock('@metamask/assets-controllers', () => ({

// TODO: Replace "any" with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const mockSelectConversionRate = selectConversionRate as any;
const mockSelectConversionRate = selectConversionRateByChainId as any;
const mockGetTokenDetails = getTokenDetails as jest.Mock;
const mockFetchTokenContractExchangeRates =
fetchTokenContractExchangeRates as jest.Mock;
Expand Down
5 changes: 3 additions & 2 deletions app/components/UI/SimulationDetails/useBalanceChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ import {
} from './types';
import { getTokenDetails } from '../../../util/address';
import {
selectConversionRate,
selectConversionRateByChainId,
selectCurrentCurrency,
} from '../../../selectors/currencyRateController';
import { useAsyncResultOrThrow } from '../../hooks/useAsyncResult';
import { RootState } from '../../../reducers';

const NATIVE_DECIMALS = 18;

Expand Down Expand Up @@ -186,7 +187,7 @@ export default function useBalanceChanges({
chainId: Hex;
simulationData?: SimulationData;
}): { pending: boolean; value: BalanceChange[] } {
const nativeFiatRate = useSelector(selectConversionRate) as number;
const nativeFiatRate = useSelector((state: RootState) => selectConversionRateByChainId(state, chainId)) as number;
const fiatCurrency = useSelector(selectCurrentCurrency);

const { nativeBalanceChange, tokenBalanceChanges = [] } =
Expand Down
2 changes: 2 additions & 0 deletions app/components/Views/Settings/Contacts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,15 @@ class Contacts extends PureComponent {
const colors = this.context.colors || mockTheme.colors;
const themeAppearance = this.context.themeAppearance;
const styles = createStyles(colors);
const { chainId } = this.props;

return (
<SafeAreaView
style={styles.wrapper}
testID={ContactsViewSelectorIDs.CONTAINER}
>
<AddressList
chainId={chainId}
onlyRenderAddressBook
reloadAddressList={reloadAddressList}
onAccountPress={this.onAddressPress}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,15 @@ import {
handleGetGasLimit,
} from '../../../../../../util/dappTransactions';
import {
selectChainId,
selectProviderType,
selectTicker,
} from '../../../../../../selectors/networkController';
import {
selectConversionRate,
selectConversionRateByChainId,
selectCurrentCurrency,
} from '../../../../../../selectors/currencyRateController';
import { selectAccounts } from '../../../../../../selectors/accountTrackerController';
import { selectContractBalances } from '../../../../../../selectors/tokenBalancesController';
import { selectSelectedInternalAccountFormattedAddress } from '../../../../../../selectors/accountsController';
import { selectGasFeeEstimates } from '../../../../../../selectors/confirmTransaction';
import { selectGasFeeControllerEstimateType } from '../../../../../../selectors/gasFeeController';
import { selectNativeCurrencyByChainId, selectProviderTypeByChainId } from '../../../../../../selectors/networkController';

const EDIT = 'edit';
const REVIEW = 'review';
Expand Down Expand Up @@ -128,7 +124,7 @@ class TransactionEditor extends PureComponent {
*/
primaryCurrency: PropTypes.string,
/**
* A string representing the network chainId
* ID of the associated chain
*/
chainId: PropTypes.string,
};
Expand Down Expand Up @@ -241,8 +237,8 @@ class TransactionEditor extends PureComponent {
dappSuggestedGasPrice
? fromWei(dappSuggestedGasPrice, 'gwei')
: gasEstimateType === GAS_ESTIMATE_TYPES.LEGACY
? this.props.gasFeeEstimates[selected]
: this.props.gasFeeEstimates.gasPrice;
? this.props.gasFeeEstimates[selected]
: this.props.gasFeeEstimates.gasPrice;

const LegacyGasData = this.parseTransactionDataLegacy(
{
Expand Down Expand Up @@ -520,9 +516,9 @@ class TransactionEditor extends PureComponent {
const tokenAmountToSend = selectedAsset && value && value.toString(16);
return to && tokenAmountToSend
? generateTransferData('transfer', {
toAddress: to,
amount: tokenAmountToSend,
})
toAddress: to,
amount: tokenAmountToSend,
})
: undefined;
},
ERC721: () => {
Expand Down Expand Up @@ -637,9 +633,9 @@ class TransactionEditor extends PureComponent {

const totalError = this.validateTotal(
EIP1559GasData?.totalMaxHex ||
this.state.EIP1559GasData.totalMaxHex ||
LegacyGasData?.totalHex ||
this.state.LegacyGasData.totalHex,
this.state.EIP1559GasData.totalMaxHex ||
LegacyGasData?.totalHex ||
this.state.LegacyGasData.totalHex,
);
const amountError = await validateAmount(
assetType,
Expand Down Expand Up @@ -815,9 +811,9 @@ class TransactionEditor extends PureComponent {
onModeChange,
gasFeeEstimates,
primaryCurrency,
chainId,
gasEstimateType,
transaction,
chainId,
} = this.props;
const {
ready,
Expand Down Expand Up @@ -907,6 +903,7 @@ class TransactionEditor extends PureComponent {
error={legacyGasTransaction.error}
onUpdatingValuesStart={this.onUpdatingValuesStart}
onUpdatingValuesEnd={this.onUpdatingValuesEnd}
chainId={chainId}
/>
) : (
<EditGasFee1559
Expand Down Expand Up @@ -935,7 +932,7 @@ class TransactionEditor extends PureComponent {
EIP1559GasDataTemp.renderableMaxFeePerGasConversion
}
primaryCurrency={primaryCurrency}
chainId={chainId}
chainId={transaction.chainId}
timeEstimate={EIP1559GasDataTemp.timeEstimate}
timeEstimateColor={EIP1559GasDataTemp.timeEstimateColor}
timeEstimateId={EIP1559GasDataTemp.timeEstimateId}
Expand Down Expand Up @@ -965,21 +962,26 @@ class TransactionEditor extends PureComponent {
};
}

const mapStateToProps = (state) => ({
accounts: selectAccounts(state),
contractBalances: selectContractBalances(state),
networkType: selectProviderType(state),
selectedAddress: selectSelectedInternalAccountFormattedAddress(state),
ticker: selectTicker(state),
transaction: getNormalizedTxState(state),
activeTabUrl: getActiveTabUrl(state),
gasFeeEstimates: selectGasFeeEstimates(state),
gasEstimateType: selectGasFeeControllerEstimateType(state),
conversionRate: selectConversionRate(state),
currentCurrency: selectCurrentCurrency(state),
primaryCurrency: state.settings.primaryCurrency,
chainId: selectChainId(state),
});
const mapStateToProps = (state) => {
const transaction = getNormalizedTxState(state);
const chainId = transaction?.chainId;

return {
accounts: selectAccounts(state),
contractBalances: selectContractBalances(state),
networkType: selectProviderTypeByChainId(state, chainId),
selectedAddress: selectSelectedInternalAccountFormattedAddress(state),
ticker: selectNativeCurrencyByChainId(state, chainId),
transaction,
activeTabUrl: getActiveTabUrl(state),
gasFeeEstimates: selectGasFeeEstimates(state),
gasEstimateType: selectGasFeeControllerEstimateType(state),
conversionRate: selectConversionRateByChainId(state, chainId),
currentCurrency: selectCurrentCurrency(state),
primaryCurrency: state.settings.primaryCurrency,
chainId,
};
}

Check warning on line 984 in app/components/Views/confirmations/Approval/components/TransactionEditor/index.js

View workflow job for this annotation

GitHub Actions / scripts (lint)

Missing semicolon

Check warning on line 984 in app/components/Views/confirmations/Approval/components/TransactionEditor/index.js

View workflow job for this annotation

GitHub Actions / scripts (lint)

Missing semicolon

const mapDispatchToProps = (dispatch) => ({
setTransactionObject: (transaction) =>
Expand Down
40 changes: 21 additions & 19 deletions app/components/Views/confirmations/Approval/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ import {
TX_SUBMITTED,
TX_REJECTED,
} from '../../../../constants/transaction';
import {
selectChainId,
selectProviderType,
} from '../../../../selectors/networkController';
import { selectSelectedInternalAccountFormattedAddress } from '../../../../selectors/accountsController';
import { providerErrors } from '@metamask/rpc-errors';
import { getDeviceId } from '../../../../core/Ledger/Ledger';
Expand All @@ -62,6 +58,7 @@ import { buildTransactionParams } from '../../../../util/confirmation/transactio
import DevLogger from '../../../../core/SDKConnect/utils/DevLogger';
import SDKConnect from '../../../../core/SDKConnect/SDKConnect';
import WC2Manager from '../../../../core/WalletConnect/WalletConnectV2';
import { selectProviderTypeByChainId } from '../../../../selectors/networkController';

const REVIEW = 'review';
const EDIT = 'edit';
Expand Down Expand Up @@ -382,8 +379,8 @@ class Approval extends PureComponent {
request_source: this.originIsMMSDKRemoteConn
? AppConstants.REQUEST_SOURCES.SDK_REMOTE_CONN
: this.originIsWalletConnect
? AppConstants.REQUEST_SOURCES.WC
: AppConstants.REQUEST_SOURCES.IN_APP_BROWSER,
? AppConstants.REQUEST_SOURCES.WC
: AppConstants.REQUEST_SOURCES.IN_APP_BROWSER,
};

try {
Expand Down Expand Up @@ -750,19 +747,24 @@ class Approval extends PureComponent {
};
}

const mapStateToProps = (state) => ({
transaction: getNormalizedTxState(state),
transactions: selectTransactions(state),
simulationData: selectCurrentTransactionMetadata(state)?.simulationData,
selectedAddress: selectSelectedInternalAccountFormattedAddress(state),
networkType: selectProviderType(state),
showCustomNonce: selectShowCustomNonce(state),
chainId: selectChainId(state),
activeTabUrl: getActiveTabUrl(state),
shouldUseSmartTransaction: selectShouldUseSmartTransaction(state),
transactionMetricsById: selectTransactionMetrics(state),
securityAlertResponse: selectCurrentTransactionSecurityAlertResponse(state),
});
const mapStateToProps = (state) => {
const transaction = getNormalizedTxState(state);
const chainId = transaction?.chainId;

return {
transaction,
transactions: selectTransactions(state),
simulationData: selectCurrentTransactionMetadata(state)?.simulationData,
selectedAddress: selectSelectedInternalAccountFormattedAddress(state),
networkType: selectProviderTypeByChainId(state, chainId),
showCustomNonce: selectShowCustomNonce(state),
chainId,
activeTabUrl: getActiveTabUrl(state),
shouldUseSmartTransaction: selectShouldUseSmartTransaction(state),
transactionMetricsById: selectTransactionMetrics(state),
securityAlertResponse: selectCurrentTransactionSecurityAlertResponse(state),
};
}

Check warning on line 767 in app/components/Views/confirmations/Approval/index.js

View workflow job for this annotation

GitHub Actions / scripts (lint)

Missing semicolon

Check warning on line 767 in app/components/Views/confirmations/Approval/index.js

View workflow job for this annotation

GitHub Actions / scripts (lint)

Missing semicolon

const mapDispatchToProps = (dispatch) => ({
resetTransaction: () => dispatch(resetTransaction()),
Expand Down
Loading

0 comments on commit 2ce6596

Please sign in to comment.