A data race was introduced in #9919 when the old synchronous storage
API was replaced with an async storage API. The problem arises when
`fetchWithCache` is called a second time while it's still processing
another call. In this case, the `cachedFetch` object can become
stale while blocked waiting for a fetch response, and result in a cache
being overwritten unintentionally.
See this example (options omitted for simplicity, and assuming an empty
initial cache):
```
await Promise.all([
fetchWithCache('https://metamask.io/foo'),
fetchWithCache('https://metamask.io/bar'),
]
```
The order of events could be as follows:
1. Empty cache retrieved for `/foo` route
2. Empty cache retrieved for `/bar` route
3. Call made to `/foo` route
4. Call made to `/bar` route
5. `/foo` response is added to the empty cache object retrieved in
step 1, then is saved in the cache.
6. `/bar` response is added to the empty cache object retrieved in
step 2, then is saved in the cache.
In step 6, the cache object saved would not contain the `/foo`
response set in step 5. As a result, `/foo` would never be cached.
This problem was resolved by embedding the URL being cached directly in
the cache key. This prevents simultaneous responses from overwriting
each others caches.
Technically a data race still exists when handing simultaneous
responses to the same route, but the result would be that the last call
to finish would overwrite the previous. This seems acceptable.
A data race was introduced in #9919 when the old synchronous storage
API was replaced with an async storage API. The problem arises when
`fetchWithCache` is called a second time while it's still processing
another call. In this case, the `cachedFetch` object can become
stale while blocked waiting for a fetch response, and result in a cache
being overwritten unintentionally.
See this example (options omitted for simplicity, and assuming an empty
initial cache):
```
await Promise.all([
fetchWithCache('https://metamask.io/foo'),
fetchWithCache('https://metamask.io/bar'),
]
```
The order of events could be as follows:
1. Empty cache retrieved for `/foo` route
2. Empty cache retrieved for `/bar` route
3. Call made to `/foo` route
4. Call made to `/bar` route
5. `/foo` response is added to the empty cache object retrieved in
step 1, then is saved in the cache.
6. `/bar` response is added to the empty cache object retrieved in
step 2, then is saved in the cache.
In step 6, the cache object saved would not contain the `/foo`
response set in step 5. As a result, `/foo` would never be cached.
This problem was resolved by embedding the URL being cached directly in
the cache key. This prevents simultaneous responses from overwriting
each others caches.
Technically a data race still exists when handing simultaneous
responses to the same route, but the result would be that the last call
to finish would overwrite the previous. This seems acceptable.
* Remove use of ethgassthat; use metaswap /gasPrices api for gas price estimates
* Remove references to ethgasstation
* Pass base to BigNumber constructor in fetchExternalBasicGasEstimates
* Update ui/app/hooks/useTokenTracker.js
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
* Delete gas price chart
* Remove price chart css import
* Delete additional fee chart code
* Lint fix
* Delete more code no longer used after ethgasstation removal
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
* Document where we need BigNumber-related changes
* Fix 1 unit test
* Debug progress
* Add required values for each upstream usage of getBigNumber
* Switch to base 10
* Address feedback
A few inconsistencies in JSDoc formatting have been fixed throughout
the project. Many issues remain; these were just the few things that
were easy to fix with a regular expression.
The changes include:
* Using lower-case for primitive types, but capitalizing non-primitive
types
* Separating the parameter identifier and the description with a dash
* Omitting a dash between the return type and the return description
* Ensuring the parameter type is first and the identifier is second (in
a few places it was backwards)
* Using square brackets to denote when a parameter is optional, rather
than putting "(optional)" in the parameter description
* Including a type and identifier with every parameter
* Fixing inconsistent spacing, except where it's used for alignment
* Remove incorrectly formatted `@deprecated` tags that reference non-
existent properties
* Remove lone comment block without accompanying function
Additionally, one parameter was renamed for clarity.
This reverts commit f5265c24ab.
Apparently it wasn't unnecessary after all. The Lock page served a few
different purposes. First, it was used to safeguard the seed phrase, in
case the user was interrupted after setting a password. Otherwise
anyone could open MetaMask and see the seed phrase without verifying
the password. Second, the submit function for the initialization unlock
screen also returned the seed phrase, so that it could be set in React
state for the confirmation step. Third, the submit function was also
responsible for navigating back to the seed phrase reveal page.
Removing the lock page had the effect of causing an infinite render
loop if onboarding was interrupted in the "Create" flow after setting
a password but before seed phrase confirmation. That redirect loop has
now been fixed.
* create custom addHexPrefix function
* switch to custom addHexPrefix
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Co-authored-by: Erik Marks <rekmarks@protonmail.com>
This page appears to serve the sole purpose of locking the extension
and redirecting back to the base route if the page is refreshed during
the onboarding flow. This ineffectual before the vault has been
initialized, and it's a barrier to resuming interrupted onboarding
flows when done after initialization.
Refs #9663
See [`node/no-deprecated-api`][1] for more information.
This change enables `node/no-deprecated-api` and fixes the issues raised by the rule.
[1]:https://github.com/mysticatea/eslint-plugin-node/blob/v11.1.0/docs/rules/no-deprecated-api.md
The change to the way that `punycode` is imported is to address the fact that
third-party module is hidden by the built-in. This is a silly hack but it works.
Consolidates the background and UI segment implementations into a shared solution.
This results in the introduction of our first shared module.
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
* Delete localhost provider type
* Use ganache-cli default chain ID for tests
* Delete unused test firstTimeState variable
* Migrate default ganache-cli network to frequentRpcListDetail
* Add default test provider state
* Add test functionality to createJsonRpcClient
* Lint locales
* Update test middleware creation
* fixup! Update test middleware creation
This updated error message shows the key of the message instead of the
message text itself, followed by the locale code.
This will make it easier to filter messages we want to ignore in
Sentry. Currently it's difficult to filter because the message is
radically different for each locale. With this change, we can add a
filter and use a glob for the last part to filter it for all locales.
It's still important to include the locale code in the message, so that
we can catch cases where the message has the wrong number of
substitutions for a specific locale.
Mark Stacey
David Walsh
Dan J Miller
Erik Marks
Brad Decker
jimthematrix
Patryk Łucka
Erik Marks
Whymarrh Whitby