Skip to content

Commit

Permalink
feat: Move CAIP-25 permission validation logic into caveat validator (#…
Browse files Browse the repository at this point in the history
…29166)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Adopts `@metamask/mutlichain` changes that move validation logic out of
the CAIP-25 permission and into the caveat itself.

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

## **Related issues**

Core: MetaMask/core#5064

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] 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/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.
  • Loading branch information
jiexi authored Dec 16, 2024
1 parent 18af503 commit d000725
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 35 deletions.
37 changes: 18 additions & 19 deletions app/scripts/controllers/permissions/specifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
createCaip25Caveat,
Caip25CaveatType,
caip25EndowmentBuilder,
caip25CaveatBuilder,
} from '@metamask/multichain';
import {
EndowmentTypes,
Expand Down Expand Up @@ -38,12 +39,23 @@ export const CaveatFactories = Object.freeze({
/**
* Gets the specifications for all caveats that will be recognized by the
* PermissionController.
*
* @param options - The options object.
* @param options.listAccounts - A function that returns the
* `AccountsController` internalAccount objects for all evm accounts.
* @param options.findNetworkClientIdByChainId - A function that
* returns the networkClientId given a chainId.
* @returns the caveat specifications to construct the PermissionController.
*/
export const getCaveatSpecifications = () => {
export const getCaveatSpecifications = ({
listAccounts,
findNetworkClientIdByChainId,
}) => {
return {
[Caip25CaveatType]: {
type: Caip25CaveatType,
},
[Caip25CaveatType]: caip25CaveatBuilder({
listAccounts,
findNetworkClientIdByChainId,
}),
...snapsCaveatsSpecifications,
...snapsEndowmentCaveatSpecifications,
};
Expand All @@ -53,25 +65,12 @@ export const getCaveatSpecifications = () => {
* Gets the specifications for all permissions that will be recognized by the
* PermissionController.
*
* @param options - The options object.
* @param options.listAccounts - A function that returns the
* `AccountsController` internalAccount objects for all evm accounts.
* @param options.findNetworkClientIdByChainId - A function that
* returns the networkClientId given a chainId.
* @returns the permission specifications to construct the PermissionController.
*/
export const getPermissionSpecifications = ({
listAccounts,
findNetworkClientIdByChainId,
}) => {
export const getPermissionSpecifications = () => {
return {
[caip25EndowmentBuilder.targetName]:
caip25EndowmentBuilder.specificationBuilder({
methodHooks: {
findNetworkClientIdByChainId,
listAccounts,
},
}),
caip25EndowmentBuilder.specificationBuilder({}),
};
};

Expand Down
20 changes: 10 additions & 10 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -1268,17 +1268,17 @@ export default class MetamaskController extends EventEmitter {
],
}),
state: initState.PermissionController,
caveatSpecifications: getCaveatSpecifications(),
permissionSpecifications: {
...getPermissionSpecifications({
listAccounts: this.accountsController.listAccounts.bind(
this.accountsController,
caveatSpecifications: getCaveatSpecifications({
listAccounts: this.accountsController.listAccounts.bind(
this.accountsController,
),
findNetworkClientIdByChainId:
this.networkController.findNetworkClientIdByChainId.bind(
this.networkController,
),
findNetworkClientIdByChainId:
this.networkController.findNetworkClientIdByChainId.bind(
this.networkController,
),
}),
}),
permissionSpecifications: {
...getPermissionSpecifications(),
...this.getSnapPermissionSpecifications(),
},
unrestrictedMethods,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@
"@metamask/message-manager": "^11.0.0",
"@metamask/message-signing-snap": "^0.6.0",
"@metamask/metamask-eth-abis": "^3.1.1",
"@metamask/multichain": "^1.1.2",
"@metamask/multichain": "^2.0.0",
"@metamask/name-controller": "^8.0.0",
"@metamask/network-controller": "patch:@metamask/network-controller@npm%3A22.1.0#~/.yarn/patches/@metamask-network-controller-npm-22.1.0-621c281f70.patch",
"@metamask/notification-services-controller": "^0.15.0",
Expand Down
10 changes: 5 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5760,9 +5760,9 @@ __metadata:
languageName: node
linkType: hard

"@metamask/multichain@npm:^1.1.2":
version: 1.1.2
resolution: "@metamask/multichain@npm:1.1.2"
"@metamask/multichain@npm:^2.0.0":
version: 2.0.0
resolution: "@metamask/multichain@npm:2.0.0"
dependencies:
"@metamask/api-specs": "npm:^0.10.12"
"@metamask/controller-utils": "npm:^11.4.4"
Expand All @@ -5773,7 +5773,7 @@ __metadata:
peerDependencies:
"@metamask/network-controller": ^22.0.0
"@metamask/permission-controller": ^11.0.0
checksum: 10/ae2f9be92dc3d5e68a8a93f65f7dba30844c0ba98877005ad228f587a43fa06f45b441f3cb10260db95febdc405f06f3fb3b65d3545efb4c634cdecea846e5fb
checksum: 10/3ae5a1b76070f06b952c1781d36b075a11cc7e94cb3dec35f93e20ed29c5e356cec320079e583e92e3cce52613c125c740bdd020fae0da30a2503b2eb3a2dca0
languageName: node
linkType: hard

Expand Down Expand Up @@ -26599,7 +26599,7 @@ __metadata:
"@metamask/message-manager": "npm:^11.0.0"
"@metamask/message-signing-snap": "npm:^0.6.0"
"@metamask/metamask-eth-abis": "npm:^3.1.1"
"@metamask/multichain": "npm:^1.1.2"
"@metamask/multichain": "npm:^2.0.0"
"@metamask/name-controller": "npm:^8.0.0"
"@metamask/network-controller": "patch:@metamask/network-controller@npm%3A22.1.0#~/.yarn/patches/@metamask-network-controller-npm-22.1.0-621c281f70.patch"
"@metamask/notification-services-controller": "npm:^0.15.0"
Expand Down

0 comments on commit d000725

Please sign in to comment.