Skip to content

Commit

Permalink
feat: remove PermissionLogController in favor of core implementation (#…
Browse files Browse the repository at this point in the history
…23182)

## **Description**

Following the successful migration of PermissionLogController to the
core monorepo (MetaMask/core#1826), this
commit removes the redundant PermissionLogController logic from the
extension. All future developments and maintenance will be concentrated
on the core implementation to streamline efforts and enhance
functionality coherence across the platform.


[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23182?quickstart=1)

## **Related issues**

- Fixes: 23181

## **Changes**

The transition of this controller from the extension repo to the core
monorepo unfolded in three phases:

1. The controller was integrated into Core, with more information
available at MetaMask/core#1871
2. The logic of the controller was streamlined, with additional details
at MetaMask/core#3662
3. The tests for the controller were overhauled, with further
information at MetaMask/core#3937

## **Manual testing steps**

These instructions outline the process for conducting manual testing
locally.

1. Launch the extension from the latest development branch.
2. Navigate to the [test-dapp](https://metamask.github.io/test-dapp/).
3. Initiate the REQUEST_PERMISSIONS action from the Permissions Actions
menu.
4. Open the background.html inspect window.
5. Execute the script `chrome.storage.local.get(null, ({data}) =>
console.log(data.PermissionLogController))` in the console.
6. Record the output from the previous step.
7. Switch to the branch named
`feature/23181-remove-Permissionlogcontroller`.
8. Repeat steps 2 through 6 for this branch.
9. Compare the outputs from step 6 for both the development and feature
branches. Look for matching entries in `permissionHistory` and
`permissionActivityLog` from the initial run in the second run's output.
Note that the log history is limited to 100 entries.


## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [ ] I've included screenshots/recordings if applicable
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: MetaMask Bot <[email protected]>
  • Loading branch information
cryptodev-2s and metamaskbot authored Mar 1, 2024
1 parent 8d6f356 commit 61a79e0
Show file tree
Hide file tree
Showing 13 changed files with 50 additions and 1,433 deletions.
17 changes: 0 additions & 17 deletions app/scripts/controllers/permissions/enums.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,5 @@
export const WALLET_PREFIX = 'wallet_';

export const NOTIFICATION_NAMES = {
accountsChanged: 'metamask_accountsChanged',
unlockStateChanged: 'metamask_unlockStateChanged',
chainChanged: 'metamask_chainChanged',
};

export const LOG_IGNORE_METHODS = [
'wallet_registerOnboarding',
'wallet_watchAsset',
];

export const LOG_METHOD_TYPES = {
restricted: 'restricted',
internal: 'internal',
};

/**
* The permission activity log size limit.
*/
export const LOG_LIMIT = 100;
1 change: 0 additions & 1 deletion app/scripts/controllers/permissions/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export * from './caveat-mutators';
export * from './background-api';
export * from './enums';
export * from './permission-log';
export * from './specifications';
export * from './selectors';
Loading

0 comments on commit 61a79e0

Please sign in to comment.