# Permission System 2.0
## Background
This PR migrates the extension permission system to [the new `PermissionController`](https://github.com/MetaMask/snaps-skunkworks/tree/main/packages/controllers/src/permissions).
The original permission system, based on [`rpc-cap`](https://github.com/MetaMask/rpc-cap), introduced [`ZCAP-LD`](https://w3c-ccg.github.io/zcap-ld/)-like permissions to our JSON-RPC stack.
We used it to [implement](https://github.com/MetaMask/metamask-extension/pull/7004) what we called "LoginPerSite" in [version 7.7.0](https://github.com/MetaMask/metamask-extension/releases/tag/v7.7.0) of the extension, which enabled the user to choose which accounts, if any, should be exposed to each dapp.
While that was a worthwhile feature in and of itself, we wanted a permission _system_ in order to enable everything we are going to with Snaps.
Unfortunately, the original permission system was difficult to use, and necessitated the creation of the original `PermissionsController` (note the "s"), which was more or less a wrapper for `rpc-cap`.
With this PR, we shake off the yoke of the original permission system, in favor of the modular, self-contained, ergonomic, and more mature permission system 2.0.
Note that [the `PermissionController` readme](https://github.com/MetaMask/snaps-skunkworks/tree/main/packages/controllers/src/permissions/README.md) explains how the new permission system works.
The `PermissionController` and `SubjectMetadataController` are currently shipped via `@metamask/snap-controllers`. This is a temporary state of affairs, and we'll move them to `@metamask/controllers` once they've landed in prod.
## Changes in Detail
First, the changes in this PR are not as big as they seem. Roughly half of the additions in this PR are fixtures in the test for the new migration (number 68), and a significant portion of the remaining ~2500 lines are due to find-and-replace changes in other test fixtures and UI files.
- The extension `PermissionsController` has been deleted, and completely replaced with the new `PermissionController` from [`@metamask/snap-controllers`](https://www.npmjs.com/package/@metamask/snap-controllers).
- The original `PermissionsController` "domain metadata" functionality is now managed by the new `SubjectMetadataController`, also from [`@metamask/snap-controllers`](https://www.npmjs.com/package/@metamask/snap-controllers).
- The permission activity and history log controller has been renamed `PermissionLogController` and has its own top-level state key, but is otherwise functionally equivalent to the existing implementation.
- Migration number 68 has been added to account for the new state changes.
- The tests in `app/scripts/controllers/permissions` have been migrated from `mocha` to `jest`.
Reviewers should focus their attention on the following files:
- `app/scripts/`
- `metamask-controller.js`
- This is where most of the integration work for the new `PermissionController` occurs.
Some functions that were internal to the original controller were moved here.
- `controllers/permissions/`
- `selectors.js`
- These selectors are for `ControllerMessenger` selector subscriptions. The actual subscriptions occur in `metamask-controller.js`. See the `ControllerMessenger` implementation for details.
- `specifications.js`
- The caveat and permission specifications are required by the new `PermissionController`, and are used to specify the `eth_accounts` permission and its JSON-RPC method implementation.
See the `PermissionController` readme for details.
- `migrations/068.js`
- The new state should be cross-referenced with the controllers that manage it.
The accompanying tests should also be thoroughly reviewed.
Some files may appear new but have just moved and/or been renamed:
- `app/scripts/lib/rpc-method-middleware/handlers/request-accounts.js`
- This was previously implemented in `controllers/permissions/permissionsMethodMiddleware.js`.
- `test/mocks/permissions.js`
- A truncated version of `test/mocks/permission-controller.js`.
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* integration for tx decoding confirmation and history view
* upgrading @truffle/decoder to latest release 5.1.0
* Update acorn and colors patches
* feat: remove redundant styling
* feat: basic integration for nickname components
* feat: wiring functionality of adding new nickname
* feat: wire functionality of showing nickname modal
* feat: link the nickname popover with add/update popover
* feat: moving forward with address nicknames integration
* feat: fixing a bug related to passing chainId in addressBook
* feat: populating memo prop in addressbook entry
* feat: add explorer link
* feat: bug fixing update nickname component
* feat: fix proptypes
* feat: adding tooltip for copying nickname address
* featL fix styling for tx-details page
* feat: optimize code for error handling
* feat: limiting transaction decoding to tx with data
* feat: remove tree UI component
* feat: adding request to check for tx decoding supported networks
* feat: showing data hex component
* feat: fix react warnings
* feat: remove extra margin in tx decoding
* Remove unused package @truffle/source-map-utils
* Ensure messages get translated
* feat: link tx-decoding addresses with nicknames
* Omit value for boolean attributes
* Fix props reading in CopyRawData
* fix: fixing issue with transaltion
* Fix lint errors in TransactionDecoding
- Remove unused import
- Reorder imports
- Address conflict between caught `error` and error state flag by
renaming state flag to `hasError`
- Fix requestUrl identifier casing and use of template string
- Ensure `useEffect` gets passed the deps it needs
- Add scope braces around case statement where it's needed
- Omit literal `true` for boolean jsx attribute
- Refactor nested ternary as `if` statements
* fix: revert fetchWithCache modifications
* Fix linting for TransactionListItemDetails
- Remove unused import
- Fix import spacing
- Remove unused prop dereference
- Fix string interpolation for translated From/To
* Moving to popover pattern
* fix: sass color variable
* Omit value for boolean attribute
* Remove changes from modal.js
* fix: refactor nickname popovers
* Ensure const gets declared before it's used
* Fix linting for ConfirmTransactionBase
- Remove unused prop chainId
- Stop destructuring an unused field
* fix: refactor usage of nicknames popovers in send-content-container
* fix: remove extra prop updateAccountNicknameModal
* fix: refactor code for address.component
* fix: remove extra tooltip
* Ensure NicknamePopovers always returns component
* Fix linting for NicknamePopover component
- Fix useCallback deps
- Switch ternary to logical-or
* Fix linting for SenderToRecipient
... by fixing import order
* Remove unused addressCopied state
* Delete empty file
* fix: remove sender-to-recipient.container
* fix: refactor usage of nickname popovers in confirm-page-container
* fix: bug related to state variable
* Stylelint fix
* Lint fix
* Change "Total Amount" to "Total"
* Lint fix locales
* Update address-book.spec.js
* e2e test update
* Update e2e tests
* Fix issue where absence of function params in data hex tab would result in rendering a string
* Fix border radius, and width and height in small notification windows, of the update-nickname-popover
* Remove fake await
* Clean up
* Clean up
Co-authored-by: Alaa Hadad <alaahd@Alaas-MacBook-M1-Pro-14-inch.local>
Co-authored-by: Dan Miller <danjm.com@gmail.com>
Co-authored-by: g. nicholas d'andrea <gnidan@trufflesuite.com>
* Ignore sentry server errors in e2e tests
* Update test/e2e/webdriver/driver.js
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Co-authored-by: Dan J Miller <danjm.com@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
We're bumping from `^6` to `^8`. All imports are now named, and they have been updated. This is a breaking change, in that support for `eth_signTransaction` is added in `^8.0.0`. We do not support this method in our UI, so our middleware stack has been instrumented to reject.
In addition, there are some non-breaking behavioral changes in this version that reviewers should be aware of, see the [7.0.0 release](https://github.com/MetaMask/eth-json-rpc-middleware/releases).
* Upgrade style from Enzyme to React Testing Library
* Use real i18n provider instead of a fake one so that we can look for
English text instead of just i18n strings, improving readability
* Add delay to fix flaky account removal e2e test
* Await for element to update instead of awaiting an arbitrary delay
* Update test/e2e/tests/from-import-ui.spec.js
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* code formatting
* Remove redundant code
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Co-authored-by: PeterYinusa <peter.yinusa@consensys.net>
* Add notification for ledger live users about how they can switch to WebHID
* Add action button so that users can go right to settings from the what's new popup
* Fix
* Add notification 8 to e2e fixtures
* Lint fix
* Update ledger webhid notification wording
* Update app/_locales/en/messages.json
* Update ui/selectors/selectors.js
This PR adds an e2e test to ensure that the background and UI environments are locked down. It reuses the logic from the `protect-intrinsics.test.js`, and runs in both Chrome and Firefox.
* Turn off all old notifications
* Remove unnecessary attempt to close whats new popup in e2e test
* Remove unneeded whats new popup closes in e2e tests
* Lint fix
* Show test networks toggle button in settings/advanced tab.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Apply toggle testnet settings and show/hide testnets when on/off
Add localhost to testnet.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Lint fixes.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Show add network button
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Open full screen when add network is called.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Show custonm rpc before testnet rpcs
lint fixes.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Test cases for network dropdown.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Test cases for toggle test networks in advanced tab component.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Lint fixes.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Fix Locales.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* E2E Tests: Custom RPC is now called Add Network
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Lint fix
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* E2E: When Add Network button is clicked, wait for the full screen window to
be visible
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* findVisibleElement should use a class. i.e start with a dot
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Hide Dropdown when Add Netwok is clicked.
Only show full screen if it's not already showing.
E2E tests passing.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Lint fixes
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Fix tests for jest
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Testnets are not being shown by default anymore, tests should use
Mainnet instead.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Import Button from ui
Change selector name to getShowTestnetworks
Fix button to show full width
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Fix e2e tests
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Remove localhost from INFURA provider types.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Fix errors in Advanced Tab Component tests
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Lint fixes
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Fix unit tests for advanced tab component.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Remove deleted elements from e2e tests
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Make sure all tests passed.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Lint fixes
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
The fixture server now supports state substitutions. This allows us to
embed dynamic values in our fixtures.
The `custom-token` fixture has been updated to include such a fixture.
The date that the seed phrase reminder was last shown has been updated
to always be the current date, to prevent the reminder from showing up
during e2e tests. This fixes the e2e test failure for the test
"add-hide-token.spec.js" that we've been seeing on CI lately.
The npm scripts used to run Mocha scripts have been greatly simplified.
As we transition more tests from Mocha to Jest it was becoming
increasingly difficult to update the CLI arguments to keep all of these
scripts working correctly. This reorganization should make that process
much simpler.
The base Mocha options are in `.mocharc.js` - all except for the target
tests to run. Those are still given via the CLI. There is a second
config file specifically for the `test:unit:lax` tests (i.e. the Mocha
tests that have no coverage requirements) because it requires a change
to the `ignored` configuration property. We can create an additional
configuration file for each test script we add that needs further
configuration changes.
The `test:unit:path` script used to be used to run Mocha tests at a
given path. Now that can be done using `yarn mocha` instead, so this
script has been removed.
The `yarn watch` command has been broken for some time now, so it has
been removed as well. Mocha tests can still be run with a file watcher
using `yarn mocha --watch <path>` or `yarn test:unit:mocha --watch`.
The README has been updated to remove references about the `watch`
command that was removed. I considered explaining the other test
scripts there as well, but they were difficult to explain I will
attempt to update the README after making further simplifications
instead.
This PR fixes our local unit test package scripts. When the state migration unit tests were migrated to Jest in #12106, it left the `test:unit` script in a broken state, because it didn't tell `mocha` to ignore the state migration tests.
Arguably, that script was already broken, since the most reasonably expectation from its name is that it runs _all_ unit tests. The PR makes it so that it does just that, by means of `concurrently`.
Unfortunately, `concurrently` only outputs errors from child processes once (at the time when they exit, https://github.com/open-cli-tools/concurrently/issues/134). This means that we have to search/navigate the output for this combined script to identify the failure. That said, it's better than the status quo.
* lavamoat - add lavamoat to webapp background
* test:e2e - add delay to resolve failure
* test:e2e - add delay to resolve failure
* build - add a switch for applying lavamoat, currently off for all
* test/e2e - remove delays added for lavamoat
* Revert "test/e2e - remove delays added for lavamoat"
This reverts commit 79c3479f15c072ed362ba1d4f1af41ea11a17d63.
* lockdown - breakout making globalThis properties non-writable into lockdown-more.js
* Update app/scripts/lockdown-more.js
Co-authored-by: David Walsh <davidwalsh83@gmail.com>
* Update app/scripts/lockdown-more.js
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
Co-authored-by: David Walsh <davidwalsh83@gmail.com>
Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
* Jestify migrations/
* Lint exclude migrations from mocha config, and add inclusion to jest config
* Add migration tests to jest config
* Exclude/ignore migration tests
* Set process.env.IN_TEST to true when running tests locally
* Replace hardcoded sent ether label on confirm screen
* replace transaction type SENT_ETHER with network agnostic SENDING_NATIVE_ASSET
* remove sentEther translation base
* make backwards compatible with lingering transaction of legacy sentEther type
* update localalization files
* fixup legacy sentEther transaction type
* changing new transaction type away from localization string
* revert migration tests
* update fixtures and test data
* update name of new transaction type
* add migration
* remove legacy SENT_ETHER from transaction types enum object
This PR adds build-time code exclusion by means of code fencing. For details, please see the README in `./development/build/transforms`. Note that linting of transformed files as a form of validation is added in a follow-up, #12075.
Hopefully exhaustive tests are added to ensure that the transform works according to its specification. Since these tests are Node-only, they required their own Jest config. The recommended way to work with multiple Jest configs is using the `projects` field in the Jest config, however [that feature breaks coverage collection](https://github.com/facebook/jest/issues/9628). That being the case, I had to set up two separate Jest configs. In order to get both test suites to run in parallel, Jest is now invoked via a script, `./test/run-jest.sh`.
By way of example, this build system feature allows us to add fences like this:
```javascript
this.store.updateStructure({
...,
GasFeeController: this.gasFeeController,
TokenListController: this.tokenListController,
///: BEGIN:ONLY_INCLUDE_IN(beta)
PluginController: this.pluginController,
///: END:ONLY_INCLUDE_IN
});
```
Which at build time are transformed to the following if the build type is not `beta`:
```javascript
this.store.updateStructure({
...,
GasFeeController: this.gasFeeController,
TokenListController: this.tokenListController,
});
```
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* bump @metamask/controllers to v15.0.1 and remove AbortController workaround in e2e tests
* remove old abortcontroller polyfill
* bump @metamask/controllers to v15.0.2
There are a few issues encountered when running `yarn setup` on new
Apple Silicon (aka M1, aka arm64) Macs:
* The script halts when attempting to run the install step for
the `chromedriver` package with the message "Only Mac 64 bits
supported". This is somewhat misleading as it seems to indicate that
chromedriver can only be installed on a 64-bit Mac. However, what I
think is happening is that the installation script for `chromedriver`
is not able to detect that an arm64 CPU *is* a 64-bit CPU. After
looking through the `chromedriver` repo, it appears that 87.0.1 is the
first version that adds a proper check ([1]).
Note that upgrading chromedriver caused the Chrome-specific tests to
fail intermittently on CI. I was not able to 100% work out the reason
for this, but ensuring that X (which provides a way for Chrome to run
in a GUI setting from the command line) is available seems to fix
these issues.
* The script also halts when attempting to run the install step for
the `electron` package. This happens because for the version of
`electron` we are using (9.4.2), there is no available binary for
arm64. It appears that Electron 11.x was the first version to support
arm64 Macs ([2]). This is a bit trickier to resolve because we don't
explicitly rely on `electron` — that's brought in by `react-devtools`.
The first version of `react-devtools` that relies on `electron` 11.x
is 4.11.0 ([3]).
[1]: 469dd0a6ee
[2]: https://www.electronjs.org/blog/apple-silicon
[3]: https://github.com/facebook/react/blob/main/packages/react-devtools/CHANGELOG.md#4110-april-9-2021
Adds the latest version of `@metamask/controllers`, and updates our usage of the `ApprovalController`, which has been migrated to `BaseControllerV2`. Of [the new `controllers` release](https://github.com/MetaMask/controllers/releases/tag/v15.0.0), only the `ApprovalController` migration should be breaking.
This is the first time we use events on the `ControllerMessenger` to update the badge, so I turned the messenger into a property on the main `MetaMaskController` in order to subscribe to events on it in `background.js`. I confirmed that the badge does indeed update during local QA.
As it turns out, [MetaMask/controllers#571](https://github.com/MetaMask/controllers/pull/571) was breaking for a single unit test case, which is now handled during setup and teardown for the related test suite (`metamask-controller.test.js`).
This PR makes ~all named intrinsics in all of our JavaScript processes non-modifiable. A named intrinsic is any property specified by the ECMAScript specification that exists on `globalThis` when the JavaScript process starts. We say that a property is non-modifiable if it is non-configurable and non-writable. We make exceptions for properties that meet any of the following criteria:
1. Properties that are non-configurable by the time `lockdown-run.js` is executed are not modified, because they can't be.
2. Properties that have accessor properties (`get` or `set`) are made non-configurable, but their writability cannot be modified, and is therefore left unchanged. It's unclear how many of the named intrinsics this applies to, if any, but it's good defensive programming, regardless.
* Remove button group for non-EIP-1559 networks
* Fix tests...maybe
* Remove unnecessary props, as well as gas display
* Remove unused string
* test progress
* fix test
* fix test
* add customizes gas block to improve e2e pass rate
Co-authored-by: Alex <adonesky@gmail.com>
Fixing up tests and add back old custom gas modal for non-eip1559 compliant networks
Remove unnecessary props from send-gas-row.component
fix breaking test
Fix primary and secondary title overrides
fix rebase issue
Fix rebase conflict
Co-authored-by: David Walsh <davidwalsh83@gmail.com>
The benchmark script can now be set to retry upon failure, like the E2E
tests do. The default is zero, just as with the E2E tests. A retry of 2
has been set in CI to match the E2E tests as well.
The `retry` module had to be adjusted to throw an error in the case of
failure. Previously it just set the exit code, but that only worked
because it was the last thing called before the process ended. That is
no longer the case.
Our benchmark script now uses `yargs`. Functionally it should be nearly
the same as before, except that now it has more documentation and
validation. The one functional difference aside from that is that the
`--pages` flag now takes space-separated arguments rather than comma-
separated.
Previously the benchmark script would throw an error if asked to take
just 1 sample. Now it works, though the stats returned are of
dubious use.
The problem was that it was impossible to calculate the standard
deviation or margin of error of a set of 1. Instead it now returns
zero for both of those values in the single-sample case, which is what
it would return for two identical samples.
* Add `--leave-running` flag to E2E test script
The `--leave-running` flag has been added to the E2E test runner. This
ensures the browser, ganache, and everything else stays running upon
test failure. This is useful for local debugging, for investigating
what state the extension was in when it failed.
* Add `--leave-running` support to `metamask-ui.spec.js`
This script makes it easier to run an individual E2E test. In the past
I've run individual scripts by editing `run-all.sh` manually, but now
that can be done more easily with this script. It also allows setting
the number of retries to use and the browser to use from the CLI.
This script has been added as an npm script as well, called
'test:e2e:single'.
The `run-all.sh` script was rewritten in JavaScript to make it easier
to pass through a `--retries` argument.
The default number of retries has been set to zero to make local
testing easier. It has been set to 2 on CI.
This was mainly done to consolidate the code used to run an E2E test in
one place, to make later improvements easier.
Chrome logs are now enabled for E2E tests when the 'ENABLE_CHROME_LOGS'
environment variable is set to anything other than `false`.
This was helpful to me in debugging Chrome crashes on CI, the ones with
the error "unknown error: DevToolsActivePort file doesn't exist". This
was the only way to discover the cause of the error. It's also useful
for discovering console errors from the background process or from the
UI.
It's disabled by default because it makes the test output quite noisy
and difficult to read.
The dapp is now started directly from the `metamask-ui.spec.js` test
module. This makes it easier to run independently, and brings it in-
line with our other E2E tests.
The `--no-timeouts` flag is now used as well, rather than setting the
timeout to `0` within the test. This also brings it in-line with our
other tests.
Mainly this was done to facilitate further refactors which will come in
later PRs.
* warn users when they attempt to add a network that is already configured
* clean up validation logic
* fixing up e2e tests
* Update test/e2e/helpers.js
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
The version field is now stored in the main `package.json` file rather
than in the base manifest. It is built into the final manifest during
the build script.
This makes it easier to communicate what the current version should be
to our `auto-changelog` script. It's also generally a more conventional
place to keep track of the version, even considering that we're not
publishing to npm.
The `assert` module has two modes: "Legacy" and "strict". When using
strict mode, the "strict" version of each assertion method is implied.
Whereas in legacy mode, by default it will use the deprecated, "loose"
version of each assertion.
We now use strict mode everywhere. A few tests required updates where
they were asserting the wrong thing, and it was passing beforehand due
to the loose matching.
* Add 'What's New' notification popup
* Move selectors from shared/notifications into ui/ directory
* Use keys for localized message in whats new notifications objects, to ensure notifications will be translated.
* Remove unused swaps intro popup locale messages
* Fix keys of whats new notification locales
* Remove notifications messages and descriptions from comment in shared/notifications
* Move notifcationActionFunctions to shared/notifications and make it stateless
* Get notification data from constants instead of state in whats-new-popup
* Code cleanup
* Fix build quote reference to swapsEthToken, broken during rebase
* Rename notificationFilters to notificationToExclude to clarify its purpose
* Documentation for getSortedNotificationsToShow
* Move notification action functions from shared/ to whats-new-popup.js
* Stop setting swapsWelcomeMessageHasBeenShown to state in app-state controller
* Update e2e tests for whats new popup changes
* Updating migration files
* Addressing feedback part 1
* Addressing feedback part 2
* Remove unnecessary div in whats-new-popup
* Change getNotificationsToExclude to getNotificationsToInclude for use in the getSortedNotificationsToShow selector
* Delete intro-popup directory and test files
* Lint fix
* Add notifiction state to address-entry fixture
* Use two separate functions for rendering first and subsequent notifications in the whats-new-popup
* Ensure that string literals are passed to t for whats new popup text
* Update import-ui fixtures to include notificaiton controller state
* Remove unnecessary, accidental change confirm-approve
* Remove swaps notification in favour of mobile swaps as first notifcation and TBD 3rd notification
* Update whats-new-popup to use intersection observer api to detect if notification has been seen
* Add notifications to send-edit and threebox e2e test fixtures
* Update ui/app/selectors/selectors.js
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* Update ui/app/selectors/selectors.js
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* Clean up locale code for whats-new-popup notifications
* Disconnect observers in whats-new-popup when their callback is first called
* Add test case for migration 58 for when the AppStateController does not exist
* Rename popover components containerRef to popoverWrapRef
* Fix messages.json
* Update notification messages and images
* Rename popoverWrapRef -> popoverRef in whats-new-popup and popover.component
* Only create one observer, and only after images have loaded, in whats-new-popup
* Set width and height on whats-new-popup image, instead of setting state on img load
* Update ui/app/components/app/whats-new-popup/whats-new-popup.js
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* Code clean up in whats new popup re: notification rendering and action functions
* Code cleanup in render notification functions of whats-new-popup
* Update ui/app/components/app/whats-new-popup/whats-new-popup.js
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* lint fix
* Update and localize notification dates
* Clean up date code in shred/notifications/index.js
Co-authored-by: ryanml <ryanlanese@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* Swaps: Show a network name dynamically in a tooltip
* Replace “Ethereum” with “$1”, change “Test” to “Testnet”
* Replace 이더리움 with $1
* Translate network names, use ‘Ethereum’ by default if a translation is not available yet
* Reorder messages to resolve ESLint issues
* Add a snapshot test for the FeeCard component, increase Jest threshold
* Enable snapshot testing into external .snap files in ESLint
* Add the “networkNameEthereum” key in ko/messages.json, remove default “Ethereum” value
* Throw an error if chain ID is not supported by the Swaps feature
* Use string literals when calling the `t` fn,
* Watch Jest tests silently (no React warnings in terminal, only errors)
* Add @testing-library/jest-dom, import it before running Jest tests
* Add snapshot testing of Swaps’ React components for happy paths, increase minimum threshold for Jest
* Add the test/jest folder for Jest setup and shared functions, use it in Swaps Jest tests
* Fix ESLint issues, update linting config
* Enable ESLint for .snap files (Jest snapshots), throw an error if a snapshot is bigger than 50 lines
* Don’t run lint:fix for .snap files
* Move `createProps` outside of `describe` blocks, move store creation inside tests
* Use translations instead of keys, update a rendering function to load translations
* Make sure all Jest snapshots are shorter than 50 lines (default limit)
* Add / update props for Swaps tests
* Fix React warnings when running tests for Swaps
* Unifies the filename suffix to .test.js
* Display @babel/no-invalid-this rule for tx-controller.test.js
* Add test file extension to test:unit:global
This fixes a bug where the network menu would remain present after a
second click on the network menu button. The bug was caused by the
click being handled _twice_, by two separate handlers. First it was
caught by the external click handler of the dropdown menu, which closed
the menu. Second, it was caught by the network button itself, which re-
opened the menu. This all happens quickly enough that to the user it
appears to stay open.
The external click handler of the menu now only fires if the menu is
open. Additionally, any click that is caught by the network menu is
stopped from propagating further, so that it can't trigger additional
click handlers.
* 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.
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 Firefox e2e tests now use the `.zip` file for testing the
extension. We've found this to produce more similar results to
production, compared to the old method of loading the unzipped
directory.
Passing in a `.zip` file to the Chrome driver didn't seem to work. I
didn't investigate this further to see if it was possible, but I'm not
sure it makes a difference on Chrome anyway.
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>
The metrics e2e test would fail if the segment events still weren't
dispatched when the page loaded. The Segment events are sent on a set
interval, so it isn't abnormal for them to lag behind the page load
itself. The `waitUntilCalled` utility has been used to wait until all
required events have been dispatched.
The `wait-until-called` module was converted to an ES5 module, so that
it could be used from an e2e test. The optional `callCount` parameter
has also been added, to allow waiting for more than one call.
The `segmentSpy` had to be converted to a `segmentStub`, to allow the
`waitUntilCalled` utility to be used.
The e2e test driver used to perform the initial navigation
automatically within the `buildWebDriver` function, so that that step
wouldn't need to be repeated at the beginning of each test. However
this prevented you from doing any setup in the test before the first
navigation.
The navigation has now been moved into each individual test. It should
be functionally equivalent, except now it's possible to control exactly
when the first navigation occurs.
A 1 second delay was also removed, as it didn't seem to be necessary
when testing this. It was initially added as an attempted fix to an
intermittent failure. It did not fix that failure.
* 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>
The assertion ensuring that there were at least 3 metrics received
didn't end up being useful. If this assertion fails, it doesn't explain
what segment events _were_ received.
By removing this assertion and letting the later assertions catch this
case, we at least learn which of the three expected events were
present.
An e2e test has been added that uses the new mock Segment server to
verify that the three initial page metric events are sent correctly.
Using the mock Segment server requires a special build with this mock
Segment server hostname embedded, so a distinct job for building and
running this test was required. As such, it was left out of the
`run-all.sh` script.
* 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>
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.
The e2e test for the contract deposit action was unnecessarily reliant
upon timing. After initiating a deposit, it would grab the first
transaction in the transaction list and assume it was the deposit that
it had just initiated. If it looked prior to the unapproved transaction
being added to the list, it would grab the wrong transaction.
It now looks specifically for _unconfirmed_ transactions, meaning it
will block until the deposit transaction is rendered.
This was discovered in testing a test-dapp PR:
https://github.com/MetaMask/test-dapp/pull/76
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.
`@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
