`editRpc` returned a thunk that didn't return a Promise, despite doing
async work. It now returns a Promise.
In the one place where this is used, it didn't seem important to update
the callsite to block on this finishing. Only one call followed it in
the event handler, and it didn't seem to depend on this.
`updateAndSetCustomRpc` returned a thunk that didn't return a Promise,
despite doing async work. It now returns a Promise.
In the one place where this is used, it didn't seem important to update
the callsite to block on this finishing. Only one call followed it in
the event handler, and it didn't seem to depend on this.
The `getSelectedAddress` selector has a fallback of selecting the first
MetaMask account. This is not useful. The only time the
`selectedAddress` is not set is during onboarding, before any accounts
exist, so selecting the first account wouldn't be useful anyway.
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
`setRpcTarget` returned a thunk that didn't return a Promise, despite
doing async work. It now returns a Promise.
The callers of this action creator didn't need to be updated, as they
were all in event handlers that didn't require knowing when the
operation had completed.
`editRpc` returned a thunk that didn't return a Promise, despite doing
async work. It now returns a Promise.
In the one place where this is used, it didn't seem important to update
the callsite to block on this finishing. Only one call followed it in
the event handler, and it didn't seem to depend on this.
Changes to the background state were being detected in the `update`
event handler in `ui/index.js` that receives state updates from the
background. However this doesn't catch every update; some state
changes are received by the UI in-between these `update` events.
The background `getState` function is callable directly from the UI,
and many action creators call it via `forceUpdateMetamaskState` to
update the `metamask` state immediately without waiting for the next
`update` event. These state updates skip this change detection in
`ui/index.js`.
For example, if a 3Box state restoration resulted in a `currentLocale`
change, and then a `forceUpdateMetamaskState` call completed before the
next `update `event was received, then `updateCurrentLocale` wouldn't
be called, and the `locale` store would not be updated correctly with
the localized messages for the new locale.
We now check for background state changes in the `updateMetamaskState`
action creator. All `metamask` state updates go through this function,
so we aren't missing any changes anymore.
The page tested by the benchmark was changed from `notification` to
`home` in #8358, but the announce script was still expecting the
`notification` page to be in the results. It does collect results for
all pages, but the `notification` page was hard-coded to be used for
the benchmark summary.
The announce script now correctly looks for the `home` page results for
the benchmark summary. Variable names have been updated to make it more
clear what's going on here as well.
`setProviderType` returned a thunk that didn't return a Promise,
despite doing async work. It now returns a Promise.
None of the callers of this action creator needed to know when it
completed, so no changes to the call sites were made.
A simple default store of `{ metamask: {} }` is now used for the
actions tests.
While I would prefer that any expectations about the store be included
in each test, the mere existence of this `metamask` object seems like
a fairly reasonable default, as it's (hopefully) impossible for it to
be unset at runtime.
The `2-state.json` test state file was deleted as well, as it was no
longer used.
These action creators for the "message manager" controller
interactions have been updated to use `async/await`. There should be
almost no changes in behavior. The only things removed were a few debug
log statements, and a single `console.log`.
Many of the "message manager" background methods return a full copy of
the background state in their response; presumably to save us from
making a full round-trip to update the UI `metamask` state after it
changes. However, the action creators responsible for calling these
methods were calling `updateMetamaskState` even when the background
method failed. In effect, they were setting the UI `metamask` state to
`undefined`.
They have been updated to only set the UI `metamask` state if the
background method succeeded.
`setSelectedAddress` returned a thunk that didn't return a Promise,
despite doing async work. It now returns a Promise.
This action creator was only called in two places, and neither benefit
from using the Promise now returned. They were both event handlers. In
both cases there was an existing Promise chain, but the only thing
after this set was a `catch` block that displayed any error
encountered. I decided not to return the result of `setSelectedAddress`
to this chain, because all it would do is set the warning a second
time in the event of failure.
The `forceUpdateMetamaskState` function now uses `async/await` instead
of a Promise constructor. This was done to make an upcoming change
easier (making `updateMetamaskState` async).
`showAccountDetail` returned a thunk that didn't return a Promise,
despite doing async work. Now it returns a Promise.
This action is only called in one place, and it looks like the actions
dispatched alongside it were meant to be run in parallel, so no changes
were made there.
`forceUpdateMetamaskState` was being called in various action creators
without `await`. Each action creator now waits for the state update to
complete before continuing.
`setCurrentCurrency` returned a thunk that didn't return a Promise,
despite doing async work. It now returns a Promise.
The callers in this case never needed to know when this action had
completed, but at least this makes our tests more reliable. They were
already `await`-ing this action, despite the lack of Promise.
The action creators that use `forceUpdateMetamaskState` without
awaiting that task's completion have been updated to use `async/await`.
This was done in preparation for `await`-ing the completion of
`forceUpdateMetamaskState`, which will be done in a subsequent PR.
The `shift-list-item` component for displaying ShapeShift transactions
has been removed, along with three other components that were used
solely by that component (`copyButton`, `eth-balance`, and
`fiat-value`).
This component hasn't been used in some time, as ShapeShift
transactions no longer exist to display. The controller that ShapeShift
transactions originated from was removed in #8118, and it became
impossible to create new ShapeShift transactions from within MetaMask
in #6746
This state has been removed from the background. It was used for the
old UI, and has been unused for some time. A migration has been added
to delete this state as well.
The action creator responsible for updating this state has been removed
from the UI as well, along with the `callBackgroundThenUpdateNoSpinner`
convenience function, which was only used for this action.
The `transForward` app state is no longer used, so it has been removed.
Associated actions have been removed as well.
This state dates back a few years, so I was unable to determine when it
was made obsolete.
Keyrings are added either through the `getKeyringForDevice` background
method (as part of the hardware wallet connect flow), or via
`importAccountWithStrategy` (when importing an account). The
`addNewKeyring` action and corresponding background method has not been
used in a long time.
The `estimateGasMethod` function passed to `estimateGas` is now an
async function. It is now invoked using `async/await` rather than a
Promise constructor.
This was done as part of a broader effort to use Promises rather than
callbacks when interacting with the background.
The background connection used in `actions.js` was being promisified
in specific actions. Instead it's now promisified once. This was made
possible by changes in `pify` v5.0.0 that ensure the binding works
correctly when passing in an object to `pify` (e.g. the `this` value
is correctly set to the wrapped background connection).
This async background connection has been temporarily assigned to a
separate variable, until we can transition all of our actions to using
this. This was done to reduce the size of this PR. There are a lot of
actions.
This state hasn't been used since #5886. The nonce we display in our UI
is now from the background, rather than queried directly from the
front-end.
This also means we save making this network call each time a pending
transaction is added, and each time the transaction list is mounted.
Some of the unit tests for `actions.js` were calling async actions
without `await`-ing them. All async actions are now called with `await`
to ensure they've completed.
`pify` v5.0.0 will preserve `this` references correctly, so explicit
binding of objects passed to `pify` is no longer needed.
There are no breaking changes that affect us; the only breaking change
in v4 and v5 is to update the minimum Node.js version to v10.
The comment above this function was originally written for a different
function: `callBackgroundThenUpdate`. It was mistakenly left above
`callBackgroundThenUpdateNoSpinner` in #1603 when this function was
added.
The original `callBackgroundThenUpdate` function this was written for
was removed recently in #7675, as it was no longer used.
The format of the comment has also been updated to match our
conventions, and JSDoc params have been added.
`markPasswordForgotten` is an asynchronous function, but it was being
called synchronously. The page was redirected without waiting for the
operation to complete.
We now wait for the operation to complete before continuing. Failure is
still not being handled correctly, but that will be addressed in a
separate PR.
Mark Stacey
Whymarrh Whitby
Erik Marks
kumavis
Jenny Pollack