The simple send gas estimation has been moved out of the gas estimation
module, and into the transaction controller. This was done in an effort
to limit the number of places where `txMeta` is mutated while the
default gas parameters are being set.
The test for receiving ETH from a contract had been clicking on the
first transaction list item, assuming it was pending. This is not
necessarily true; if the pending transaction hadn't yet been rendered,
this could select the first confirmed transaction instead.
The test has been updated to look for the first _pending_ transaction,
rather than just the first transaction.
Note that this likely does not fix the intermittent failure we've been
experiencing. The failure has been observed with this fix in place.
This test would occasionally fail due to a fluke of timing, where a
pending transaction would take slightly longer than expected to
be rendered in the "confirmed transactions" list. This `wait` block
ensures the test will try again until it has confirmed.
The test artifact directory for failed test "verbose reports" was
mistakenly being set to `[browser]/undefined`. This was broken during
the refactor in #7798, when the `driver` parameter was mistakenly left
in after the `verboseReportOnFailure` function was converted to a
method being called on `driver`.
Backport #8445 to v7.7.9. Original commit description:
* Don't updatePendingTxs outside of block updates
Refs #8377
Reverts 507397f6c (#5431)
* Check for new block data on unlock
Co-authored-by: Whymarrh Whitby <whymarrh.whitby@gmail.com>
Backport #8200 to v7.7.9. Original commit description:
The method registry was being initialized with the global variable
`ethereumProvider` before that variable was set. As a result, the
method registry was falling back to an internally constructed provider
that used the wrong provider URL (an obsolete Infura API). This was
resulting in an error with the message "Project ID not found".
The method registry is now initialized lazily, when it's first needed.
This should be well after the initialization of `ethereumProvider`,
which occurs during the UI initialization.
Backport #8363 to v7.7.9. Note that this uses `clone` instead of
`cloneDeep`, because `clone` hadn't yet been replaced by `cloneDeep` on
`master`.
Backporting that change as well would have been very disruptive, so
I've updated this to use `clone` instead to minimize conflicts. It is
functionally equivalent.
Co-authored-by: Whymarrh Whitby <whymarrh.whitby@gmail.com>
An alert is now shown when the user switches from an account that is
connected to the active tab to an account that is not connected. The
alert prompts the user to dismiss the alert or connect the account
they're switching to.
The "loading" state is handled by disabling the buttons, and the error
state is handled by displaying a generic error message and disabling
the connect button.
The new reducer for this alert has been created with `createSlice` from
the Redux Toolkit. This utility is recommended by the Redux team, and
represents a new style of writing reducers that I hope we will use more
in the future (or at least something similar). `createSlice` constructs
a reducer, actions, and action creators automatically. The reducer is
constructed using their `createReducer` helper, which uses Immer to
allow directly mutating the state in the reducer but exposing these
changes as immutable.
This backports the deletion of the Dai/Sai migration notification
(#8418). Note that the migration to delete the now unused background
state has not been included, as it is non-essential and would have been
more difficult to backport. The migration to delete the unused state
will be included in the next major release instead.
`removeFromAddressBook` returned a thunk that didn't return a Promise,
despite doing async work. It now returns a Promise.
The callers were updated to `await` the completion of this operation.
`addToAddressBook` returned a thunk that didn't return a Promise,
despite doing async work. It now returns a Promise.
The callers of this action creator were updated to `await` the
completion of the operation. It was called just before redirecting the
user to a different page or closing a modal, and it seemed appropriate
to wait before doing those things.
This is a backport of #8314. Here's the original description:
MetaMask would sometimes get into a state where the notification popup
would never open. This could happen if the notification window was
closed shortly after being opened. After this happened, no popups would
show up until after the extension was reset.
This was happening because the background thought the popup was already
open. The variable it uses to track whether the popup was open or not
was being set to `true` immediately after the background asked the
browser to open a new window, before a handler was attached that could
respond to the window being closed.
Removing this line seems to solve the problem.
This line was added originally in #5437, which dealt with batch
transactions. Batches of transactions seem to work just fine without
this line though (from local testing), and I can't think of why this
would be required.
Closes#7051
`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.
Mark Stacey
Erik Marks
Brad Decker
Whymarrh Whitby
MetaMask Bot