Skip to content

Commit

Permalink
fix: fall back to bundled chainlist (#23392)
Browse files Browse the repository at this point in the history
## **Description**

The list of known chains is fetched at runtime from
`https://chainid.network/chains.json` and cached. There are some issues
with the way this works:
- MetaMask will not have a list of chains until online (if ever)
- When the 24h cache timeout expires, the chainlist becomes unavailable

This PR addresses this by:
- Refactoring out `https://chainid.network/chains.json` into constant
`CHAIN_SPEC_URL`
- Add new optional option `allowStale` to `fetchWithCache`. If set to
`true`, it will falling back to return any entry instead of throwing an
error when a request fail.
- Set `allowStale` to `true` for all requests to `CHAIN_SPEC_URL`
- Seed the fetch cache for `CHAIN_SPEC_URL` with
[`eth-chainlist`](https://www.npmjs.com/package/eth-chainlist), which is
the same data exposed via a published npm package.
- Open for suggestions on if this should be bundled differently - maybe
we want our own equivalent mirror?

While an improvement, this could still be further improved.

- The bundled result could be used immediately in all cases without
waiting for response
- The cached data could be updated asynchronously in the background,
without being prompted by user action

I currently consider these out-of-scope for this PR.

Or put more generally: Decoupling the fetching of the data from its use
would be even better.
[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/PR?quickstart=1)

## **Related issues**

Fixes:

## **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**

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

### **After**

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

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained what problem this PR is solving and how it
is solved.
- [ ] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [ ] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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
legobeat authored Oct 17, 2024
1 parent 55d0972 commit 01ea106
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 3 deletions.
25 changes: 25 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
LedgerIframeBridge,
} from '@metamask/eth-ledger-bridge-keyring';
import LatticeKeyring from 'eth-lattice-keyring';
import { rawChainData } from 'eth-chainlist';
import { MetaMaskKeyring as QRHardwareKeyring } from '@keystonehq/metamask-airgapped-keyring';
import EthQuery from '@metamask/eth-query';
import EthJSQuery from '@metamask/ethjs-query';
Expand Down Expand Up @@ -169,6 +170,7 @@ import {
} from '../../shared/constants/swaps';
import {
CHAIN_IDS,
CHAIN_SPEC_URL,
NETWORK_TYPES,
NetworkStatus,
MAINNET_DISPLAY_NAME,
Expand Down Expand Up @@ -200,6 +202,10 @@ import {
} from '../../shared/constants/metametrics';
import { LOG_EVENT } from '../../shared/constants/logs';

import {
getStorageItem,
setStorageItem,
} from '../../shared/lib/storage-helpers';
import {
getTokenIdParam,
fetchTokenBalance,
Expand Down Expand Up @@ -413,6 +419,8 @@ export default class MetamaskController extends EventEmitter {
this.getRequestAccountTabIds = opts.getRequestAccountTabIds;
this.getOpenMetamaskTabsIds = opts.getOpenMetamaskTabsIds;

this.initializeChainlist();

this.controllerMessenger = new ControllerMessenger();

this.loggingController = new LoggingController({
Expand Down Expand Up @@ -6304,6 +6312,23 @@ export default class MetamaskController extends EventEmitter {
});
}

/**
* The chain list is fetched live at runtime, falling back to a cache.
* This preseeds the cache at startup with a static list provided at build.
*/
async initializeChainlist() {
const cacheKey = `cachedFetch:${CHAIN_SPEC_URL}`;
const { cachedResponse } = (await getStorageItem(cacheKey)) || {};
if (cachedResponse) {
return;
}
await setStorageItem(cacheKey, {
cachedResponse: rawChainData(),
// Cached value is immediately invalidated
cachedTime: 0,
});
}

/**
* Returns the nonce that will be associated with a transaction once approved
*
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@
"currency-formatter": "^1.4.2",
"debounce-stream": "^2.0.0",
"deep-freeze-strict": "1.1.1",
"eth-chainlist": "~0.0.498",
"eth-ens-namehash": "^2.0.8",
"eth-lattice-keyring": "^0.12.4",
"eth-method-registry": "^4.0.0",
Expand Down
1 change: 1 addition & 0 deletions shared/constants/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export const NETWORK_NAMES = {
HOMESTEAD: 'homestead',
};

export const CHAIN_SPEC_URL = 'https://chainid.network/chains.json';
/**
* An object containing all of the chain ids for networks both built in and
* those that we have added custom code to support our feature set.
Expand Down
7 changes: 7 additions & 0 deletions shared/lib/fetch-with-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const fetchWithCache = async ({
fetchOptions = {},
cacheOptions: { cacheRefreshTime = MINUTE * 6, timeout = SECOND * 30 } = {},
functionName = '',
allowStale = false,
}: {
url: string;
// TODO: Replace `any` with type
Expand All @@ -16,6 +17,7 @@ const fetchWithCache = async ({
// eslint-disable-next-line @typescript-eslint/no-explicit-any
cacheOptions?: Record<string, any>;
functionName: string;
allowStale?: boolean;
}) => {
if (
fetchOptions.body ||
Expand Down Expand Up @@ -49,6 +51,11 @@ const fetchWithCache = async ({
...fetchOptions,
});
if (!response.ok) {
const message = `Fetch with cache failed within function ${functionName} with status'${response.status}': '${response.statusText}'`;
if (allowStale) {
console.debug(`${message}. Returning cached result`);
return cachedResponse;
}
throw new Error(
`Fetch with cache failed within function ${functionName} with status'${response.status}': '${response.statusText}'`,
);
Expand Down
4 changes: 3 additions & 1 deletion ui/hooks/useIsOriginalNativeTokenSymbol.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import fetchWithCache from '../../shared/lib/fetch-with-cache';
import {
CHAIN_ID_TO_CURRENCY_SYMBOL_MAP,
CHAIN_ID_TO_CURRENCY_SYMBOL_MAP_NETWORK_COLLISION,
CHAIN_SPEC_URL,
} from '../../shared/constants/network';
import { DAY } from '../../shared/constants/time';
import { useSafeChainsListValidationSelector } from '../selectors';
Expand Down Expand Up @@ -78,7 +79,8 @@ export function useIsOriginalNativeTokenSymbol(
}

const safeChainsList = await fetchWithCache({
url: 'https://chainid.network/chains.json',
url: CHAIN_SPEC_URL,
allowStale: true,
cacheOptions: { cacheRefreshTime: DAY },
functionName: 'getSafeChainsList',
});
Expand Down
4 changes: 3 additions & 1 deletion ui/pages/confirmations/confirmation/confirmation.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { produce } from 'immer';
import log from 'loglevel';
import { ApprovalType } from '@metamask/controller-utils';
import { DIALOG_APPROVAL_TYPES } from '@metamask/snaps-rpc-methods';
import { CHAIN_SPEC_URL } from '../../../../shared/constants/network';
import fetchWithCache from '../../../../shared/lib/fetch-with-cache';
import {
MetaMetricsEventCategory,
Expand Down Expand Up @@ -372,7 +373,8 @@ export default function ConfirmationPage({
try {
if (useSafeChainsListValidation) {
const response = await fetchWithCache({
url: 'https://chainid.network/chains.json',
url: CHAIN_SPEC_URL,
allowStale: true,
cacheOptions: { cacheRefreshTime: DAY },
functionName: 'getSafeChainsList',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { useSelector } from 'react-redux';

import { useSafeChainsListValidationSelector } from '../../../../selectors';
import fetchWithCache from '../../../../../shared/lib/fetch-with-cache';
import { CHAIN_SPEC_URL } from '../../../../../shared/constants/network';
import { DAY } from '../../../../../shared/constants/time';

export type SafeChain = {
Expand All @@ -25,8 +26,9 @@ export const useSafeChains = () => {
if (useSafeChainsListValidation) {
useEffect(() => {
fetchWithCache({
url: 'https://chainid.network/chains.json',
url: CHAIN_SPEC_URL,
functionName: 'getSafeChainsList',
allowStale: true,
cacheOptions: { cacheRefreshTime: DAY },
})
.then((response) => {
Expand Down
8 changes: 8 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -18431,6 +18431,13 @@ __metadata:
languageName: node
linkType: hard

"eth-chainlist@npm:~0.0.498":
version: 0.0.498
resolution: "eth-chainlist@npm:0.0.498"
checksum: 10/a414c0e1f0a877f9ab8bf1cf775556308ddbb66618e368666d4dea9a0b949febedf8ca5440cf57419413404e7661f1e3d040802faf532d0e1618c40ecd334cbf
languageName: node
linkType: hard

"eth-eip712-util-browser@npm:^0.0.3":
version: 0.0.3
resolution: "eth-eip712-util-browser@npm:0.0.3"
Expand Down Expand Up @@ -26313,6 +26320,7 @@ __metadata:
eslint-plugin-react-hooks: "npm:^4.2.0"
eslint-plugin-storybook: "npm:^0.6.15"
eta: "npm:^3.2.0"
eth-chainlist: "npm:~0.0.498"
eth-ens-namehash: "npm:^2.0.8"
eth-lattice-keyring: "npm:^0.12.4"
eth-method-registry: "npm:^4.0.0"
Expand Down

0 comments on commit 01ea106

Please sign in to comment.