* Migration 51: ensure chainId is set in network controller provider state for all infura/default networks
* Clean up
* Migrate incorrect as well as falsy chainIds
Co-authored-by: Erik Marks <rekmarks@protonmail.com>
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
Ensures that `hideLoadingIndication` is always called in all actions that call `showLoadingIndication`. It's unclear how many of these actions were failing to hide the loading indication, because other actions superset `hideLoadingIndication`.
At the very least, `updateTransaction` was probably failing to hide the loading indication in the error case.
This PR also refactors a lot of actions to call `hideLoadingIndication` once in `finally` blocks as opposed to multiple times across `try` and `catch` blocks. We avoided making changes to functions using `Promise` methods, because `Promise.finally` is not supported by Waterfox, and it's not properly transpiled by Babel.
Ensures that `hideLoadingIndication` is always called in all actions that call `showLoadingIndication`. It's unclear how many of these actions were failing to hide the loading indication, because other actions superset `hideLoadingIndication`.
At the very least, `updateTransaction` was probably failing to hide the loading indication in the error case.
This PR also refactors a lot of actions to call `hideLoadingIndication` once in `finally` blocks as opposed to multiple times across `try` and `catch` blocks. We avoided making changes to functions using `Promise` methods, because `Promise.finally` is not supported by Waterfox, and it's not properly transpiled by Babel.
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
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.
The `waitUntilCalled` utility now has a timeout. It will now throw an
error if the stub is not called enough times, rather than blocking
forever.
The return type had to be changed to a function, so that we could throw
when the timeout is triggered. I tried returning an error that rejected
first, but if you don't handle the error synchronously Node.js will
consider it to be an unhandled Promise rejected (even if it _is_
handled later on).
I worked around this by resolving in the timeout case as well, so that
there is never a "deferred" Promise exception in the timeout case. The
returned function re-throws the error if it's given. That way there is
never any unhandled Promise rejection.
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 `waitUntilCalled` utility now has a timeout. It will now throw an
error if the stub is not called enough times, rather than blocking
forever.
The return type had to be changed to a function, so that we could throw
when the timeout is triggered. I tried returning an error that rejected
first, but if you don't handle the error synchronously Node.js will
consider it to be an unhandled Promise rejected (even if it _is_
handled later on).
I worked around this by resolving in the timeout case as well, so that
there is never a "deferred" Promise exception in the timeout case. The
returned function re-throws the error if it's given. That way there is
never any unhandled Promise rejection.
* 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>
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.
* 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>
Some of the unit tests for the incoming transaction controller included
a 1 second wait. The wait was to ensure that a state update did not
occur, as it happens asynchronously.
The tests work equally well using a `setTimeout` with a zero second
wait, because the asynchronous block update is guaranteed to have been
queued up by the time this timeout function is called. The timeout has
been reduced to `0` to speed up the tests.
Additionally, `undefined` has been added to the list of network names
used to construct the fake API responses. This is to ensure that the
API returns a valid response, so that the test fails when it should.
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.
Unit tests have been added to the incoming transactions controller to
ensure that block updates are correctly resulting in state updates when
incoming transactions are enabled. All other events that trigger state
updates are tested as well.
The tests were written to be minimally dependent upon implementation
details of the controller itself. `nock` was used to mock the API
response from Etherscan. Each event is triggered asynchronously by
`sinon`, as in production they are likely only triggered
asynchronously.
This was extracted from #9583
This PR includes a new `wait-until-called` module meant to help with
writing asynchronous tests. It allows you to wait until a stub has been
called.
The shared mocks used previously in the incoming transaction controller
tests have been replaced with functions that can generate a new mock
for each test.
We should avoid ever sharing mocks between tests. It's quite easy for
a mock to get accidentally mutated or not correctly "reset" for the
next test, leading to test inter-dependencies and misleading results.
In particular, it is unsafe to share a `sinon` fake (e.g. a spy or
stub) because they can't be fully reset between tests. Or at least it's
difficult to reset them property, and it can't be done while also
following their recommendations for preventing memory leaks.
The spy API and all related state can be reset with `resetHistory`,
which can be called between each test. However `sinon` also recommends
calling `restore` after each test, and this will cause `sinon` to drop
its internal reference to the fake object, meaning any subsequent call
to `resetHistory` would fail. This is intentional, as it's required to
prevent memory from building up during the test run, but it also means
that sharing `sinon` fakes is particularly difficult to do safely.
Instead we should never share mocks in the first place, which has other
benefits anyway.
This was discovered while writing tests for #9583. I mistakenly
believed that `sinon.restore()` would reset the spy state, and this was
responsible for many hours of debugging test failures.
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.
`@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
* 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
Thomas Huang
Erik Marks
Dan J Miller
Mark Stacey
David Walsh
Patryk Łucka
Brad Decker