Skip to content

Commit

Permalink
fix: disable unsupported swap+send networks (#25474)
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**

Swap+Send relies on a function in the newest swap+send contracts (i.e.
once Swap+Send is deployed on all current swap networks, Swap+Send
network support should be 1-to-1 with that of Swaps). However, we are
still waiting on the API to be updated for these new contracts so we
cannot operate under the assumption that all of these networks will be
deployed by the time Swap+Send begins rolling out in production.

To mitigate this concern, we can utilize a property in Swaps' feature
flag endpoint (see: https://swap.dev-api.cx.metamask.io/featureFlags)
when the send flow is initialized to determine if that network should be
disabled despite having swaps support.

<!--
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/25474?quickstart=1)

## **Related issues**

Fixes: #25107 

## **Manual testing steps**

1. Set `SWAPS_USE_DEV_APIS=true` in `.metamaskrc` (the endpoint update
hasn't been deployed to prod as of writing)
2. Switch to an unsupported network (e.g., zkSync)
3. Verify that, when entering the swaps flow, that network is disabled
(i.e., the destination asset picker is disabled)
4. Switch to a supported network (e.g., BNB)
5. Verify that the network is not disabled in the send flow

## **Screenshots/Recordings**

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

### **Before**

#### zkSync (network enabled despite no support)

<img width="405" alt="Screenshot 2024-06-21 at 2 01 27 PM"
src="https://github.com/MetaMask/metamask-extension/assets/44588480/fbbac5d5-860f-49a4-9bf3-1f4722a975f0">


#### BNB (network enabled)

<img width="405" alt="Screenshot 2024-06-21 at 2 00 48 PM"
src="https://github.com/MetaMask/metamask-extension/assets/44588480/06fbc608-829e-443f-80d2-8bd4e152843c">

<!-- [screenshots/recordings] -->

### **After**

#### zkSync (network disabled)

<img width="405" alt="Screenshot 2024-06-21 at 1 59 48 PM"
src="https://github.com/MetaMask/metamask-extension/assets/44588480/7910a56c-3744-4aef-9964-085c83f6c391">

#### BNB (network enabled)

<img width="405" alt="Screenshot 2024-06-21 at 2 00 11 PM"
src="https://github.com/MetaMask/metamask-extension/assets/44588480/e3fd781f-e3f0-4e7d-bc1b-ed3cc2148dc8">

<!-- [screenshots/recordings] -->

## **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/develop/.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/develop/.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
BZahory authored and micaelae committed Jun 21, 2024
1 parent b733dc4 commit f7b4771
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 1 deletion.
1 change: 1 addition & 0 deletions test/data/mock-send-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,7 @@
"send": {
"amountMode": "INPUT",
"currentTransactionUUID": "1-tx",
"disabledSwapAndSendNetworks": [],
"draftTransactions": {
"1-tx": {
"amount": {
Expand Down
1 change: 1 addition & 0 deletions test/data/mock-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -1947,6 +1947,7 @@
"send": {
"amountMode": "INPUT",
"currentTransactionUUID": null,
"disabledSwapAndSendNetworks": [],
"draftTransactions": {},
"eip1559support": false,
"gasEstimateIsLoading": true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
acknowledgeRecipientWarning,
getBestQuote,
getCurrentDraftTransaction,
getIsSwapAndSendDisabledForNetwork,
getSendAsset,
getSwapsBlockedTokens,
} from '../../../../../ducks/send';
Expand Down Expand Up @@ -46,12 +47,16 @@ export const SendPageRecipientContent = ({

const isBasicFunctionality = useSelector(getUseExternalServices);
const isSwapsChain = useSelector(getIsSwapsChain);
const isSwapAndSendDisabledForNetwork = useSelector(
getIsSwapAndSendDisabledForNetwork,
);
const swapsBlockedTokens = useSelector(getSwapsBlockedTokens);
const memoizedSwapsBlockedTokens = useMemo(() => {
return new Set(swapsBlockedTokens);
}, [swapsBlockedTokens]);
const isSwapAllowed =
isSwapsChain &&
!isSwapAndSendDisabledForNetwork &&
[AssetType.token, AssetType.native].includes(sendAsset.type) &&
isBasicFunctionality &&
!memoizedSwapsBlockedTokens.has(sendAsset.details?.address?.toLowerCase());
Expand Down
21 changes: 20 additions & 1 deletion ui/ducks/send/send.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,10 @@ import {
DEFAULT_ROUTE,
} from '../../helpers/constants/routes';
import { fetchBlockedTokens } from '../../pages/swaps/swaps.util';
import { getSwapAndSendQuotes } from './swap-and-send-utils';
import {
getDisabledSwapAndSendNetworksFromAPI,
getSwapAndSendQuotes,
} from './swap-and-send-utils';
import {
estimateGasLimitForSend,
generateTransactionParams,
Expand Down Expand Up @@ -463,6 +466,7 @@ export const draftTransactionInitialState = {
* clean up AND during initialization. When a transaction is edited a new UUID
* is generated for it and the state of that transaction is copied into a new
* entry in the draftTransactions object.
* @property {string[]} disabledSwapAndSendNetworks - list of networks that are disabled for swap and send
* @property {{[key: string]: DraftTransaction}} draftTransactions - An object keyed
* by UUID with draftTransactions as the values.
* @property {boolean} eip1559support - tracks whether the current network
Expand Down Expand Up @@ -505,6 +509,7 @@ export const draftTransactionInitialState = {
export const initialState = {
amountMode: AMOUNT_MODES.INPUT,
currentTransactionUUID: null,
disabledSwapAndSendNetworks: [],
draftTransactions: {},
eip1559support: false,
gasEstimateIsLoading: true,
Expand Down Expand Up @@ -763,11 +768,15 @@ export const initializeSendState = createAsyncThunk(
? (await fetchBlockedTokens(chainId)).map((t) => t.toLowerCase())
: [];

const disabledSwapAndSendNetworks =
await getDisabledSwapAndSendNetworksFromAPI();

return {
account,
chainId: getCurrentChainId(state),
tokens: getTokens(state),
chainHasChanged,
disabledSwapAndSendNetworks,
gasFeeEstimates,
gasEstimateType,
gasLimit,
Expand Down Expand Up @@ -1980,6 +1989,8 @@ const slice = createSlice({
});
}
state.swapsBlockedTokens = action.payload.swapsBlockedTokens;
state.disabledSwapAndSendNetworks =
action.payload.disabledSwapAndSendNetworks;
if (state.amountMode === AMOUNT_MODES.MAX) {
slice.caseReducers.updateAmountToMax(state);
}
Expand Down Expand Up @@ -3520,6 +3531,14 @@ export function getSwapsBlockedTokens(state) {
return state[name].swapsBlockedTokens;
}

export const getIsSwapAndSendDisabledForNetwork = createSelector(
(state) => state.metamask.providerConfig,
(state) => state[name]?.disabledSwapAndSendNetworks ?? [],
({ chainId }, disabledSwapAndSendNetworks) => {
return disabledSwapAndSendNetworks.includes(chainId);
},
);

export const getSendAnalyticProperties = createSelector(
(state) => state.metamask.providerConfig,
getCurrentDraftTransaction,
Expand Down
35 changes: 35 additions & 0 deletions ui/ducks/send/send.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
INITIAL_SEND_STATE_FOR_EXISTING_DRAFT,
} from '../../../test/jest/mocks';
import { ETH_EOA_METHODS } from '../../../shared/constants/eth-methods';
import * as Utils from './swap-and-send-utils';
import sendReducer, {
initialState,
initializeSendState,
Expand Down Expand Up @@ -81,6 +82,7 @@ import sendReducer, {
getSender,
getSwapsBlockedTokens,
updateSendQuote,
getIsSwapAndSendDisabledForNetwork,
} from './send';
import { draftTransactionInitialState, editExistingTransaction } from '.';

Expand Down Expand Up @@ -171,6 +173,9 @@ describe('Send Slice', () => {
jest
.spyOn(Actions, 'getLayer1GasFee')
.mockReturnValue({ type: 'GET_LAYER_1_GAS_FEE' });
jest
.spyOn(Utils, 'getDisabledSwapAndSendNetworksFromAPI')
.mockReturnValue([]);
});

describe('Reducers', () => {
Expand Down Expand Up @@ -4485,6 +4490,36 @@ describe('Send Slice', () => {
}),
).toStrictEqual(['target']);
});

it('has a selector to get if swap+send is disabled for that network', () => {
expect(
getIsSwapAndSendDisabledForNetwork({
metamask: {
providerConfig: {
chainId: 'disabled network',
},
},
send: {
...INITIAL_SEND_STATE_FOR_EXISTING_DRAFT,
disabledSwapAndSendNetworks: ['disabled network'],
},
}),
).toStrictEqual(true);

expect(
getIsSwapAndSendDisabledForNetwork({
metamask: {
providerConfig: {
chainId: 'enabled network',
},
},
send: {
...INITIAL_SEND_STATE_FOR_EXISTING_DRAFT,
disabledSwapAndSendNetworks: ['disabled network'],
},
}),
).toStrictEqual(false);
});
});
});
});
30 changes: 30 additions & 0 deletions ui/ducks/send/swap-and-send-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { isNumber } from 'lodash';
import {
ALLOWED_PROD_SWAPS_CHAIN_IDS,
SWAPS_API_V2_BASE_URL,
SWAPS_CLIENT_ID,
SWAPS_DEV_API_V2_BASE_URL,
Expand All @@ -18,6 +19,10 @@ import {
hexToDecimal,
} from '../../../shared/modules/conversion.utils';
import { isValidHexAddress } from '../../../shared/modules/hexstring-utils';
import {
fetchSwapsFeatureFlags,
getNetworkNameByChainId,
} from '../../pages/swaps/swaps.util';

type Address = `0x${string}`;

Expand Down Expand Up @@ -208,3 +213,28 @@ export async function getSwapAndSendQuotes(request: Request): Promise<Quote[]> {

return newQuotes;
}

export async function getDisabledSwapAndSendNetworksFromAPI(): Promise<
string[]
> {
try {
const blockedChains: string[] = [];

const featureFlagResponse = await fetchSwapsFeatureFlags();

ALLOWED_PROD_SWAPS_CHAIN_IDS.forEach((chainId) => {
// explicitly look for disabled so that chains aren't turned off accidentally
if (
featureFlagResponse[getNetworkNameByChainId(chainId)]?.v2?.swapAndSend
?.enabled === false
) {
blockedChains.push(chainId);
}
});

return blockedChains;
} catch (error) {
// assume no networks are blocked since the quotes will not be fetched on an unavailable network anyways
return [];
}
}

0 comments on commit f7b4771

Please sign in to comment.