Our automatic token detection was hard-coded to only work on our built-
in Infura Mainnet endpoint. It now works with custom Mainnet RPC
endpoints as well.
Relates to #6992
The `disable-console` script introduced in #10040 used an arrow-
function no-op function to replace `console.log` and `console.info`.
This replacement function was early-bound to the `this` context of the
`disable-console` script, because that's how arrow functions work.
This violates an assumption baked into Sentry, which also replaces the
`console` functions. It wraps them in a function it uses to track
console logs as breadcrumbs. This wrapper function blows up for some
reason if the "original" `console` function is early-bound to a `this`
value of `undefined`.
This resulted in various UI freezes. One example is during onboarding,
when using Firefox with Enhanced Tracking Protection set in "strict"
mode. After submitting a password in the 'Create wallet' flow, the
Sentry `console` wrapper would throw and leave the user stuck on the
loading screen.
By replacing the no-op arrow function with a no-op function
declaration, the problem has been resolved.
Relates to #10097
Failure to persist state will now only report to Sentry if the last
attempt to save state succeeded. This ensures that if anyone is stuck
in a state where state can't be saved (e.g. low disk space), we aren't
flooded with repeated errors on Sentry.
This update comes with a breaking change to the Approval controller. It
now requires a `defaultApprovalType` parameter.
I don't think we have any use for a default approval type, but I've
added a "NO_TYPE" one for now because it's a strict requirement. We
should consider making this parameter optional in the future, for cases
like this where it's not needed.
This update will hopefully address some caching issues we've been
seeing with our phishing configuration. See here for more details:
https://github.com/MetaMask/controllers/pull/297
`eth_getProof` is an unpermissioned, read-only RPC method for getting account-related Merkle proofs, specified here: https://eips.ethereum.org/EIPS/eip-1186
It's been supported by major Ethereum clients, and Infura, for some time. By adding it to the safe methods list, we enable this method for our users.
* Maintain console logging in dev mode
Co-authored-by: kumavis <aaron@kumavis.me>
Co-authored-by: Erik Marks <rekmarks@protonmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
This PR introduces the new approval controller to the extension codebase. We use it for the permissions controller's pending approval functionality.
The approval controller sets us up for a new pattern of requesting and managing user confirmations in RPC methods. Along with the generic RPC method middleware, the approval controller will allow us to eliminate our message managers, and decouple various method handlers from our provider stack, making the implementations more portable between the extension and mobile.
On Firefox 56 and Waterfox Classic, our `runLockdown.js` script throws
an error. This is fine on the HTML pages, as the next script tags still
get run without issue (though they don't benefit from the SES lockdown
sadly). But in the `contentscript`, an exception thrown here appears to
halt the execution of subsequent scripts.
To prevent the `contentscript` from crashing completely, lockdown
errors are now caught and logged. They are also logged to Sentry on the
pages where Sentry is setup.
From a behavioral standpoint this PR fixes the issue with tracking, and persisting, tokens that the user hides. Whether we can/should optimize this to prevent duplicates of the accountHiddenTokens and hiddenToken is a point of contention, but it acts similiarly to how we track tokens and accountTokens.
Also to note, for tokens under a custom network there is no way to distinguish two different custom network sets of hidden tokens, they are all under the `rpc` property, same as accountTokens.
* @metamask/inpage-provider@^8.0.0
* Replace public config store with JSON-RPC notifications
* Encapsulate notification permissioning in permissions controller
* Update prefix of certain internal RPC methods and notifications
* Add accounts to getProviderState
* Send accounts with isUnlocked notification (#10007)
* Rename provider streams, notify provider of stream failures (#10006)
The new metrics controller has a `trackEvent` function that was being
called unbound, so `this` references were undefined. It is now bound
early in both places where it is passed in as a parameter.
The SES lockdown added in #9729 had the effect of obfuscating our error
messages. Any messages printed to the console would have the error
message replaced with the string "Error #" followed by a number. The
stack was also updated to point at `lockdown.cjs`, though the original
stack was preserved beneath the top stack frame.
Marking the `console` API as untamed seems to have fixed both issues.
The original error message is now printed to the console, along with
the original stack.
* Migration to remove legacy local storage keys from localStorage
* Update app/scripts/migrations/050.js
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* Update app/scripts/migrations/050.js
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* Fix unit tests for migration 50
* Fixing stubbing and localstorage reference in migration 50
* Update test/helper.js
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Attempts to send metrics would fail when no `options` were used. This
was because when the options parameter was not set, it was often sent
over our RPC connection as `undefined`, which gets serialized to `null`
when the message is converted to JSON. This `null` parameter didn't
trigger the default parameter set in the metametrics controller, as
default parameters are only used for `undefined`.
Instead the `options` parameter is now treated as fully optional, with
no default value set. The optional chaining operator is used to ensure
it won't blow up if it's not set. A fallback of `{}` was used for the
one destructure case as well.
If a `gasPrice` was specified in a transaction sent via a dapp, we
would include it in our `eth_estimateGas` call, causing it to fail if
the user had insufficient balance (for either the transaction amount or
the gas fee). This resulted in the fallback gas estimate being used;
the block gas limit. The block gas limit is quite a bit larger than
most transactions need, so this resulted in wildly inflated gas costs
being shown on our confirmation screen.
The `gasPrice` has been removed from the `txParams` object we pass to
`eth_estimateGas`, so now it won't perform any balance checks anymore.
This ensures that we'll get a valid gas estimate, as long as geth is
able to simulate the contract execution properly.
Fixes#9967
* 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>
* Freezeglobals: remove Promise freezing, add lockdown
* background & UI: temp disable sentry
* add loose-envify, dedupe symbol-observable
* use loose envify
* add symbol-observable patch
* run freezeGlobals after sentry init
* use require instead of import
* add lockdown to contentscript
* add error code in message
* try increasing node env heap size to 2048
* change back circe CI option
* make freezeGlobals an exported function
* make freezeGlobals an exported function
* use freezeIntrinsics
* pass down env to child process
* fix unknown module
* fix tests
* change back to 2048
* fix import error
* attempt to fix memory error
* fix lint
* fix lint
* fix mem gain
* use lockdown in phishing detect
* fix lint
* move sentry init into freezeIntrinsics to run lockdown before other imports
* lint fix
* custom lockdown modules per context
* lint fix
* fix global test
* remove run in child process
* remove lavamoat-core, use ses, require lockdown directly
* revert childprocess
* patch package postinstall
* revert back child process
* add postinstall to ci
* revert node max space size to 1024
* put back loose-envify
* Disable sentry to see if e2e tetss pass
* use runLockdown, add as script in manifest
* remove global and require from runlockdown
* add more memory to tests
* upgrade resource class for prep-build & prep-build-test
* fix lint
* lint fix
* upgrade remote-redux-devtools
* skillfully re-add sentry
* lintfix
* fix lint
* put back beep
* remove envify, add loose-envify and patch-package in dev deps
* Replace patch with Yarn resolution (#9923)
Instead of patching `symbol-observable`, this ensures that all
versions of `symbol-observable` are resolved to the given range, even
if it contradicts the requested range.
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
The `matomo` option passed to the send metrics function is invalid. The
intent was to set the `matomoEvent` option, but instead of rectifying
that, we've decide to keep sending this event to the production Segment
project for now. The invalid option has been removed.
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.
The `seedPhraseBackedUp` now tracks whether or not the seed phrase has
been backed up. Previously this defaulted to `true`, which left no way
to distinguish whether it had been backed up or not during onboarding.
The default is now `null`, and the UI logic has been updated to account
for this, so that "existing users" (i.e. users that have a backup that
is years old) aren't mistakenly considered to have not backed up their
seed phrase. This value is already set explicitly to `true` or `false`
during onboarding, in both the create and import flow.
This change was made primarily to make it easier to fix the onboarding
library integration, which will be done in a subsequent PR.
* Alternative savings fix
* Further required changes to savings fix
* Further fix to savings calculations that properly accounts for metamask fees
* metaMaskFeeInEth property on quotes to decimal string
* Fix swaps controller unit tests
* Improve documentation in swaps controller
* Prevent getMedianEthValueQuote from mutation passed quotes array with .sort() call
* Another fix and refactor to savings calculations in _findTopQuoteAndCalculateSavings
Cleaner structuring of conditionals for setting tokenValueOfQuoteForSorting, ethValueOfQuote and metaMaskFeeInEth in swaps controller
Stop subtracting medianMetaMaskFee from savings, but include it in savings data
Another fix and refactor to savings calculations in _findTopQuoteAndCalculateSavings
* Add and update unit tests for _findTopQuoteAndCalculateSavings
* Improve calculation of overallValueOfQuoteForSorting for case where ETH is the source token
* Clean up getMedianEthValueQuote code, test and comments
* Clean up _findTopQuoteAndCalculateSavings, create test input and expected results helper functions
* Update getMedianEthValueQuote to account for multiple quotes with overall values equal to the median
* Add jsdoc comment for meansOfQuotesFeesAndValue
* Fix jsdoc comment for getMedianEthValueQuote
* create custom addHexPrefix function
* switch to custom addHexPrefix
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Co-authored-by: Erik Marks <rekmarks@protonmail.com>
* Log web3 usage for functions and nested properties only
* Change web3 metrics source to legacy
* Update web3 metrics properties and event name
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
The incoming transactions controller now uses the `chainId` for the
current network instead of the `networkId`. This ensures that custom
RPC endpoints for the built-in supported networks do correctly receive
incoming transactions.
As part of this change, the incoming transactions controller will also
cease keeping track of the "last block fetched" for networks that are
not supported. This piece of state never really represented the last
block fetched, as _no_ blocks were fetched for any such networks. It
been removed.
If the swaps state is cleared in between the initial quote fetch and
the subsequent poll fetch, a `TypeError` will be thrown due to
`fetchParams` being set to `null`.
This is of no functional consequence, as `fetchParams` _should_ be
`null` in this case, and and no further action should be taken.
The optional chaining operator is now used to ensure the call no longer
throws.
This is a continuation of #9726, which did not fix the problem
described.
If the initial network when the extension is started is something other
than Mainnet, the swaps controller will never successfully retrieve
swap quotes. This is because `ethers` will continue to communicate
with whichever network the provider was initially on.
We tried fixing this by hard-coding the `chainId` to Mainnet's
`chainId` when constructing the Ethers provider, but this did not work.
I suspect this failed because the `provider` we pass to `ethers` is not
compliant with EIP 1193, as `ethers` doubtless expects it to be.
Instead the entire `ethers` provider is now reconstructed each time the
network changes. This mirrors the approach we take in some other
controllers.
If the initial network when the extension is started is something other
than Mainnet, the swaps controller will never successfully retrieve
swap quotes. This is because the `ethers` provider used by the swaps
controller doesn't allow network changes by default - it assumes that
the network remains the same as when the provider was initialized.
This was fixed by hard-coding Mainnet as the initial chain ID for this
`ethers` provider used by the swaps controller.
Some adjustments needed to be made to the `provider` stub to allow
setting `1` as the network ID and chain ID in unit tests.
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.
Our ENS resolver for the browser address bar was incorrectly resolving
addresses that included query strings. We were concatenating the `path`
property with the `search` property, despite the fact that the `path`
property already contains `search`. As a result, `search` was
duplicated in the resolved addresses.
For example, if an IPFS content ID was found for this address, the
resolved address for `metamask.eth/?foo=bar` would have the path
`/?foo=bar?foo=bar`
The original intent was likely to use `pathname` in place of `path`.
The resolver has been updated to use `pathname`, and the query string
now appears only once in the resolved address.
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>
`@metamask/eslint-config` has been updated to v4.1.0. This update
requires that we update `eslint` to v7 as well, which in turn requires
updating most `eslint`-related packages.
Most notably, `babel-eslint` was replaced with `@babel/eslint-parser`,
and `babel-eslint-plugin` was replaced by `@babel/eslint-plugin`. This
required renaming all the `babel/*` rules to `@babel/*`.
Most new or updated rules that resulted in lint errors have been
temporarily disabled. They will be fixed and re-enabled in subsequent
PRs.
* Calculate savings per swap relative to median values
* Update test mock quotes, add getMedian tests
* Identify assets by sourceToken and destinationToken
The account tracker had one doc comment above the constructor that
partially served to document the constructor, but mostly contained a
type definition for the class itself. It has been split into two
blocks; one for the class, one for the constructor. The constructor doc
comment has also been expanded to document all constructor options.
The `chainId` is now used by the account tracker to identify the
current network, instead of the `networkId`. This should have no
functional impact, aside from that different chains with the same
`networkId` will now be correctly distinguished from each other.
An attempt to safely release the `nonceLock` upon failure has instead
made failure worse by masking it with a new error. If the call to get
the `nonceLock` throws an exception, then the `finally` block here
would attempt to call `releaseLock` on the `nonceLock` variable, which
is guaranteed to be `undefined` if the previous call failed. The
attempt to call a method on `undefined` throws another error, masking
the original error.
It is safer to obtain the `nonceLock` and release it without using any
`try` or `finally` block. The `nonceLock` is synchronously released
immediately after it is obtained, and any errors bubble up correctly
without being masked. There is no case where the lock is left
unreleased.
If the `signTypedData` background function threw an exception, it would
return `undefined` to the UI, which would throw another exception in
the UI. It now re-throws the error if an error is thrown, which
allows the UI to handle the error.
I'm not sure why this might fail, and I'm not sure we're handling this
failure well, but this is an improvement at least.
* Add data point to 'Swaps Completed' segment event: estimated vs used gas
* Linted
* Correct property name for estimated gas on swapMetaData in _trackSwapsMetrics()
* Set estimated_gas property on swapMetaData to a hex string
* Correct base when dividing by estimated_gas
Co-authored-by: Dan Finlay <dan@danfinlay.com>
* Delete CachedBalancesController.cachedBalances
* Migrate provider to Rinkeby instead of deleting it
* Convert hex transaction metamaskNetworkId values to decimal
* Don't migrate provider state in e2e tests
* Don't kick custom RPC users to Rinkeby unnecessarily
* Use provider.chainId for address book chainId values
* Add address book migration
* Fix failing unit test
* fixup! Merge branch 'develop' into address-book-use-chainId
* Select address book entries for display by chainId
* Merge all address book entry keys
* fixup! Merge all address book entry keys
* 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
* Update txMeta after postTxBalance has been retrieved
* Use gas used from txReceipt to calculate eth received
* Return null from getSwapsTokensReceivedFromTxMeta in tokenSymbol is ETH and txReceipt is missing
* Get latest txMeta before updating it with postTxBalance in case of a swaps tx in confirmTransaction
* Lint fix
* Use initial transaction for settings swap transaction title params, and remove addition of swap properties to cancel transcations
* Update unit test data
* Use token symbol properties from initial transaction for filitering in transaction list
* call this.txStateManager.setTxStatusConfirmed before async call in confirmTransaction in the transactions controller
* Clone txMeta before setTxStatusConfirmed in confirmTransaction
* Correctly updateTx in confirmTransaction
* Track swaps event only if it is a swap transaction
* Create wrapper function for segment events
* Extract transaction controller metrics calls into own function
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* Stop passing a gas param to the estimateGas call initiated in the swaps controller timedoutGasReturn
* Stop passing gas params to timedoutGasReturn
* Lint fix
* Stop passing no longer used param to setInitialGasEstimate
When the `chainId` for a custom RPC endpoint is edited, we now migrate
the corresponding address book entries to ensure they are not orphaned.
The address book entries are grouped by the `metamask.network` state,
which unfortunately was sometimes the `chainId`, and sometimes the
`networkId`. It was always the `networkId` for built-in Infura
networks, but for custom RPC endpoints it would be set to the user-set
`chainId` field, with a fallback to the `networkId` of the network.
A recent change will force users to enter valid `chainId`s on all
custom networks, which will be normalized to be hex-prefixed. As a
result, address book contacts will now be keyed by a different string.
The contact entries are now migrated when this edit takes place.
There are some edge cases where two separate entries share the same set
of contacts. For example, if two entries have the same `chainId`, or if
they had the same `networkId` and had no `chainId` set. When the
`chainId` is edited in such cases, the contacts are duplicated on both
networks. This is the best we can do, as we don't have any way to know
which network the contacts _should_ be on.
The `typed-message-manager` unit tests have also been updated as part
of this commit because the addition of `sinon.restore()` to the
preferences controller tests ended up clearing a test object in-between
individual tests in that file. The test object is now re-constructed
before each individual test.
The `_fetchAndSetSwapsLiveness` was accidentally passed to
`setInterval` without being bound first, so the `this` reference was
not defined when it was called. It is now bound before being passed to
`setInterval`.
* Remove network config store
* Remove inline networks variable in network controller
* Re-key network controller 'rpcTarget' to 'rpcUrl'
* Require chainId in lookupNetwork, implement eth_chainId
* Require chain ID in network form
* Add alert, migrations, and tests
* Add chainId validation to addToFrequentRpcList
* Update public config state selector to match new network controller
state
* Use network enums in networks-tab.constants
* Ensure chainId in provider config is current
* Update tests
Right now when editing an address in "Settings > Contact", the contact
is lost after saving. This is because the code awaits
`removeFromAddressBook()` before creating the new contact but
`removeFromAddressBook()` never resolves. This change fixes this bug.
The web3 usage metrics added in #9144 assumed that all web3 properties
were strings. When a `Symbol` property is accessed, our `inpage.js`
script crashes because the `Symbol` cannot be serialized correctly.
A check has been added for non-string property access. The metric event
in these cases is set to the string "typeof ", followed by the type of
the key. (e.g. `typeof symbol` for a `Symbol` property).
Fixes#9234
The usage metrics for the injected web3 instance were being sent upon
each use, which exceeded the limits of our Matomo plan. These metrics
are now only being sent upon the first usage, for each origin and
property.
* Fix require-unicode-regexp issues
See [`require-unicode-regexp`](https://eslint.org/docs/rules/require-unicode-regexp) for more information.
This change enables `require-unicode-regexp` and fixes the issues raised by the rule.
* Remove case-insensitive flag from regexps
The "Background" metrics category was being set in the
`backgroundMetaMetricsEvent` function. This function might not
necessarily include any event at all though, so setting it here seemed
inappropriate. It would also crash if `eventData.eventOpts` was not
set, which is not great since that property is optional.
The background category is now set in the `sendBackgroundMetaMetrics`
function in `metamask-controller`. This method is used solely for event
data, so it would make sense for this category to be always set.
There is no functional difference, since `backgroundMetaMetricsEvent`
is called solely by `sendBackgroundMetaMetrics`.
The `currentPath` parameter passed to our metrics utility had been
passed the full URL rather than just the path, contrary to what the
name would imply. We only used the path portion, so passing the full
URL did lead to complications.
Now just the `pathname` is passed in, rather than the full URL. This
simplifies the metrics logic, and it incidentally fixes two bugs.
The main bug fixed is regarding Firefox metrics. Previously we had
assumed the `currentPath` would start with `chrome-extension://`, which
of course was not true on Firefox. This lead to us incorrectly parsing
the `currentPath`, so path tracking was broken for Firefox events.
This broken parsing is now bypassed entirely, so metrics should now
work the same on Firefox as on Chrome.
The second bug was that we were incorrectly setting the tracking URL
for background events during tests. As a result, we were incorrectly
detecting ourselves as an internal site that had referred the user to
us. But this was not of major concern, since it only affected test
metrics (which get sent to the development Matomo project).
Lastly, this change let us discard the `pathname` parameter used in
the `overrides` parameter of the `metricsEvent` function. Now that
`currentPath` is equivalent to `pathname`, the `pathname` parameter is
redundant.
* Remove `url` parameter from `metricsEvent`
The `url` parameter was used to override the `currentPath`, but it
never worked correctly. It was supposed to be used for setting the
`url` query parameter that was sent to Matomo, but `currentPath` was
always used even if it `url` was set and `currentPath` was empty.
Instead, `currentPath` is now always used. There was never a need to
provide an "override" for `currentPath` when it can be set directly.
The metrics provider does set `currentPath` automatically by default,
but this can be overwritten already by passing a second parameter to
`metricsEvent`.
There were two places this `url` parameter was being used: background
events, and path changes. Background events were submitted with no
`currentPath`, so because of the bug with the `url` parameter, the
metrics utility would crash upon each event. So those were never
actually sent. This commit will fix that crash.
The `currentPath` parameter was supplied as an empty string for the
path change events, so those never crashed. They just had the `url`
query string parameter set incorrectly (to an empty string). It should
now be correctly populated, which should mean we'll be capturing all
path changes now. Previously we were only capturing path changes to
pages that happened to include an event, because of this blank `url`
problem.
* Use `url` query parameter as fallback for generating `pv_id`
The `pv_id` parameter currently isn't generated correctly on Firefox,
as the generation assumes that the current URL starts with
`chrome-extension://`. The `url` query parameter is still unique for
each path, so it's probably good enough for generating an id for each
page.
This is just a temporary fix; it will be removed in a future PR, where
Firefox will be properly supported.
Background events are now sent in the `Background` category, rather
than `backend`. Conventionally we use the term "background" over
"backend", as it's not really a "backend" in the normal sense since
it's a client background process. Also it's capitalized because all of
the other event categories are capitalized as well.
The metrics URL has been updated to use `background` instead of
`backend` as well, for consistency.
Luckily we don't have to worry about our metrics being disjointed due
to this name change, because the background metrics never worked to
begin with! So there will be none under the old name. The metrics will
be made functional in a separate PR.
The Sentry DSN is now expected to be provided via environment variable
for production builds. The build script will fail if it is missing, and
an error will be thrown at runtime if it is missing.
The `SENTRY_DSN` environment variable has been set in CI to the old
value for `SENTRY_PROD_DSN`. We can migrate to a new DSN at some point
in the future.
In a non-production environment, Sentry was configured to send error
reports to a "test" MetaMask project. It will still do this during e2e
tests, but in development Sentry is now disabled completely.
In practice this was never useful in development.
* Fix popup/notification when browser is in fullscreen, primarily on OSX.
The issue was reported internally via Slack. User was running Mac OSX Chrome in fullscreen mode where Chrome is created in a new Desktop workspace.
The issue reproduced on OSX Chrome in fullscreen/maximized view overrides the explicitly set width and height for `windows.create()`. Possibly not overrides, but creates a window based off of the window that it was created from. Found a related [Chromium bug](https://bugs.chromium.org/p/chromium/issues/detail?id=263092&q=window%20create%20width%20os%3DMac&can=2).
The fullscreen `popup.left` pixel will calculate the window position incorrectly since we set and assume the width of the created window. The incorrect `left` position the window and transition the focus Desktop/Workspace incorrectly and make is seem to lose focus of the new window/workspace. Incidentally this will make the popup full width/height, and create a new workspace for the view, which we have no control over until Chrome
fixes it.
This will check if the popup is 'fullscreen', which it gets passed from the origin window, if so then don't reposition the window. If Chrome fixes the issue we can revert this change.
* Feedback commit
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
The `extra` property of errors sent to Sentry is sometimes not
initialized when we add the application state. A check has been added
to initialize it if it's missing.
I suspect that this changed with v5 of `@sentry/browser`, though I
can't find any explicit confirmation of this in their changelog.
The state snapshot that was attached to Sentry errors was removed
recently in #8794 because it had become too large. The snapshot has
now been restored and reduced in size.
A utility function has been written to reduce the state object to just
the requested properties. This seemed safer than filtering out state
that is known to be large or to contain identifiable information.
This is not a great solution, as now knowledge about the state shape
resides in this large constant, but it will suffice for now. I am
hopeful that we can decorate our controllers with this metadata in the
future instead, as part of the upcoming background controller refactor.
A separate `getSentryState` global function has been added to get the
reduced state, so that the old `getCleanAppState` function that we used
to use could remain unchanged. It's still useful to get that full state
copy while debugging, and in e2e tests.
This migration had referred to the non-existent
`TransactionsController` instead of `TransactionController`, so it
effectively did nothing. Now it should work.
This migration hasn't been included in any release yet, so we can fix
it in-place instead of adding an additional corrected migration.
The migration comment has also been updated, as it was inaccurate.
This method was accidentally broken with the introduction of the
permissions controller, as this was missing from the list of safe
methods.
It is now included in the list of safe methods.
Fixes#8993
The currency rate controller is updated upon each network change, as
the "native currency" is network-dependent and might have changed.
However, any thrown errors were being caught and passed to an empty
callback.
The errors are now re-thrown in the callback. As a result, the errors
will now be printed to the console and sent to Sentry.
It seems that this blocklist checker never worked correctly. Ever since
the initial commit, it was comparing the Number `1` to the `networkId`,
which is a string. Additionally, even if it did throw, the transaction
continued unhindered. The user could still approve it, and there was no
indication shown to the user that anything went wrong. Also some of the
blocklist entries were incorrectly mixed-case, and were never hit.
We can remove this for now, and re-add it later on after we rewrite the
transaction controller.
The `metamaskNetworkId` property in the `txMeta` for incoming
transactions was incorrectly set as a Number instead of a String. This
change was made accidentally as part of #8627.
As a result incoming transactions were being excluded from the
transaction list, as they didn't have a matching network ID.
`metamaskNetworkId` is now set to a string, and a migration has been
added to ensure `metamaskNetworkId` is converted to a string for any
incoming transactions in state.
An optimization in `account-tracker.js` was being skipped consistently
due to a type error (a number was being compared to a string).
The optimization in this case was to update the balances for all
accounts with a single request, rather than one request per account.
The `activeTab.id` property is relied upon in the connected sites modal
to prevent the user from manually connecting to the MetaMask extension
itself. Unfortunately the `id` property was never set.
`id` is now set on the `activeTab` state, so manually connecting to the
extension UI is now impossible.
The `activeTab` state is now set to an empty object if the `origin` of
the active tab is missing or invalid. It can be invalid if the URL
passed to the `URL` constructor is missing a scheme (e.g.
`about: blank`).
There are currently no cases where the rest of the data in `activeTab`
is useful in the absence of an `origin`. This will make upcoming UI
logic changes a bit simpler than they would be otherwise. Now we can
assume that if any property is set on `activeTab`, it must have a valid
`origin`.
There were three cases where execution unintentionally continued after
an error was encountered. These cases likely are impossible to
encounter in practice due to recent validation improvements in the
`eth-json-rpc-middleware/wallet` module, but they were broken
nonetheless.
Execution inside the Promise constructor now halts immediately after
`reject` is called.
* Use over the whole stringified error object which doesn't show the actual error message that is set as the
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* Feedback commit
The code for checking whether a transaction was dropped or not was
refactored in #8398, but in the process an off-by-one error was
introduced.
The old version of `_checkIfTxWasDropped` would query for an updated
transaction count from the network, and would consider the pending
transaction to be dropped if the count was above the nonce. However,
the version introduced in #8398 considers the transaction to be dropped
if the count is above *or equal to* the nonce.
The pending transaction nonce is expected to be equal to the
transaction count, because the nonce starts at zero. The transaction
count is equal to the expected next nonce.
The variable name has been updated to make this more clear
(`networkNextNonce` is how the `nonce-tracker` refers to this value).
`parseInt` is now called with an explicit radix of `16` as well, to
ensure both nonce strings are always parsed as hex. In all cases I am
aware of, these nonce strings were prefixed by `0x`, meaning that
`parseInt` would default to a radix of `16`, so this likely doesn't
constitute a functional change.
Fixes#8688
* update connected accounts appearance
* consolidate account alerts
* UnconnectedAccountAlert: use ConnectedAccountsList
* move switch account action out of menu in all views
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Imported accounts can be removed, but the permissions controller is not
informed when this happens. Permissions are now removed as part of the
account removal process.
Additionally, the `getPermittedIdentitiesForCurrentTab` selector now
filters out any non-existent accounts, in case a render occurs in the
middle of an account removal.
This was resulting in a render crash upon opening the popup on a site
that was connected to the removed account.
The state snapshot we were attaching to Sentry errors was too large.
As a temporary solution, it has been removed completely. We can re-add
it later after reducing its size.
* restore and enhance the time est feature
background: we had a feature for showing a time estimate on pending txs
that was accidently removed during the redesign implementation. This PR
restores that feature and also enhances it:
1. Displays the time estimate on all views instead of just fullscreen
2. Uses Intl.RelativeTimeFormat to format the time
3. Adds a way to toggle the feature flag.
4. Uses a hook to calculate the time remaining instead of a component
* Update app/_locales/en/messages.json
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* do not display on test nets
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
The `TokenRatesController` was accidentally broken in #8744, when the
logic for starting and stopping polling was moved from the `isActive`
property to start/stop functions.
A reference to the now-obsolete `isActive` property was accidentally
left behind, resulting in no exchange rate updates.
