Skip to content

Commit

Permalink
Merge branch 'main' into hm/add-metamaskbot-codeowner-announcement
Browse files Browse the repository at this point in the history
  • Loading branch information
hmalik88 committed Mar 3, 2025
2 parents 5110889 + 2e5a244 commit 76b9f1e
Show file tree
Hide file tree
Showing 25 changed files with 1,522 additions and 1,757 deletions.
26 changes: 17 additions & 9 deletions .madgerc
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
{
"excludeRegExp": [
"/.[^/]*\/test\/.[^/]*/u",
"/.[^/]*\/tests\/.[^/]*/u",
"/.[^/]*\/[^/]*.test.[^/]*/u",
"/.[^/]*\/[^/]*.spec.[^/]*/u",
"/.[^/]*\/stories\/.[^/]*/u",
"/.[^/]*\/storybook\/.[^/]*/u",
"/.[^/]*\/[^/]*.stories.[^/]*/u"
"/.[^/]*\/test\/.[^/]*/u"
],
"fileExtensions": ["js", "jsx", "ts", "tsx"],
"tsConfig": "tsconfig.json",
"webpackConfig": "webpack.config.js",
"detectiveOptions": {
"es6": {
"skipTypeImports": true
Expand All @@ -23,5 +16,20 @@
"skipTypeImports": true,
"skipAsyncImports": true
}
}
},
"allowedCircularGlob": [
"ui/pages/confirmations/**",
"ui/pages/notifications/**",
"ui/ducks/**",
"ui/selectors/**",
"ui/hooks/**",
"ui/store/**",
"ui/helpers/utils/token-util.js",
"ui/components/multichain/**",
"ui/components/app/**",
"ui/components/component-library/**",
"shared/modules/selectors/**",
"app/scripts/metamask-controller.js",
"app/scripts/lib/snap-keyring/**"
]
}
69 changes: 0 additions & 69 deletions app/scripts/controllers/permissions/specifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,72 +193,3 @@ export const unrestrictedMethods = Object.freeze([
'metamaskinstitutional_openAddHardwareWallet',
///: END:ONLY_INCLUDE_IF
]);

/**
* Validates the accounts associated with a caveat. In essence, ensures that
* the accounts value is an array of non-empty strings, and that each string
* corresponds to a PreferencesController identity.
*
* @param {string[]} accounts - The accounts associated with the caveat.
* @param {() => Record<string, import('@metamask/keyring-internal-api').InternalAccount>} getInternalAccounts -
* Gets all AccountsController InternalAccounts.
* TODO: Remove this function once the CAIP-25 permission refactor/factory differ work is merged into main
*/
export function validateCaveatAccounts(accounts, getInternalAccounts) {
if (!Array.isArray(accounts) || accounts.length === 0) {
throw new Error(
`${PermissionNames.eth_accounts} error: Expected non-empty array of Ethereum addresses.`,
);
}

const internalAccounts = getInternalAccounts();
accounts.forEach((address) => {
if (!address || typeof address !== 'string') {
throw new Error(
`${PermissionNames.eth_accounts} error: Expected an array of Ethereum addresses. Received: "${address}".`,
);
}

if (
!internalAccounts.some(
(internalAccount) =>
internalAccount.address.toLowerCase() === address.toLowerCase(),
)
) {
throw new Error(
`${PermissionNames.eth_accounts} error: Received unrecognized address: "${address}".`,
);
}
});
}

/**
* Validates the networks associated with a caveat. Ensures that
* the networks value is an array of valid chain IDs.
*
* @param {string[]} chainIdsForCaveat - The list of chain IDs to validate.
* @param {function(string): string} findNetworkClientIdByChainId - Function to find network client ID by chain ID.
* @throws {Error} If the chainIdsForCaveat is not a non-empty array of valid chain IDs.
* TODO: Remove this function once the CAIP-25 permission refactor/factory differ work is merged into main
*/
export function validateCaveatNetworks(
chainIdsForCaveat,
findNetworkClientIdByChainId,
) {
if (!Array.isArray(chainIdsForCaveat) || chainIdsForCaveat.length === 0) {
throw new Error(
`${PermissionNames.permittedChains} error: Expected non-empty array of chainIds.`,
);
}

chainIdsForCaveat.forEach((chainId) => {
try {
findNetworkClientIdByChainId(chainId);
} catch (e) {
console.error(e);
throw new Error(
`${PermissionNames.permittedChains} error: Received unrecognized chainId: "${chainId}". Please try adding the network first via wallet_addEthereumChain.`,
);
}
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const addEthereumChain = {
requestUserApproval: true,
getCurrentChainIdForDomain: true,
getCaveat: true,
requestPermittedChainsPermissionForOrigin: true,
requestPermittedChainsPermissionIncrementalForOrigin: true,
},
};
Expand All @@ -40,7 +39,6 @@ async function addEthereumChainHandler(
requestUserApproval,
getCurrentChainIdForDomain,
getCaveat,
requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin,
},
) {
Expand Down Expand Up @@ -189,7 +187,6 @@ async function addEthereumChainHandler(
autoApprove: shouldAddOrUpdateNetwork,
setActiveNetwork,
getCaveat,
requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin,
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ const createMockedHandler = () => {
defaultRpcEndpointIndex: 0,
rpcEndpoints: [{ networkClientId: 123 }],
}),
requestPermittedChainsPermissionForOrigin: jest.fn(),
requestPermittedChainsPermissionIncrementalForOrigin: jest.fn(),
};
const response = {};
Expand Down Expand Up @@ -187,8 +186,6 @@ describe('addEthereumChainHandler', () => {
autoApprove: true,
getCaveat: mocks.getCaveat,
setActiveNetwork: mocks.setActiveNetwork,
requestPermittedChainsPermissionForOrigin:
mocks.requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin:
mocks.requestPermittedChainsPermissionIncrementalForOrigin,
},
Expand Down Expand Up @@ -254,8 +251,6 @@ describe('addEthereumChainHandler', () => {
autoApprove: true,
getCaveat: mocks.getCaveat,
setActiveNetwork: mocks.setActiveNetwork,
requestPermittedChainsPermissionForOrigin:
mocks.requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin:
mocks.requestPermittedChainsPermissionIncrementalForOrigin,
},
Expand Down Expand Up @@ -301,8 +296,6 @@ describe('addEthereumChainHandler', () => {
autoApprove: false,
getCaveat: mocks.getCaveat,
setActiveNetwork: mocks.setActiveNetwork,
requestPermittedChainsPermissionForOrigin:
mocks.requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin:
mocks.requestPermittedChainsPermissionIncrementalForOrigin,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ export function validateAddEthereumChainParams(params) {
* @param {boolean} [hooks.autoApprove] - A boolean indicating whether the request should prompt the user or be automatically approved.
* @param {Function} hooks.setActiveNetwork - The callback to change the current network for the origin.
* @param {Function} hooks.getCaveat - The callback to get the CAIP-25 caveat for the origin.
* @param {Function} hooks.requestPermittedChainsPermissionForOrigin - The callback to request a new permittedChains-equivalent CAIP-25 permission.
* @param {Function} hooks.requestPermittedChainsPermissionIncrementalForOrigin - The callback to add a new chain to the permittedChains-equivalent CAIP-25 permission.
* @param {Function} hooks.setTokenNetworkFilter - The callback to set the token network filter.
* @returns a null response on success or an error if user rejects an approval when autoApprove is false or on unexpected errors.
Expand All @@ -182,7 +181,6 @@ export async function switchChain(
autoApprove,
setActiveNetwork,
getCaveat,
requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin,
setTokenNetworkFilter,
},
Expand All @@ -203,7 +201,7 @@ export async function switchChain(
});
}
} else {
await requestPermittedChainsPermissionForOrigin({
await requestPermittedChainsPermissionIncrementalForOrigin({
chainId,
autoApprove,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ describe('Ethereum Chain Utils', () => {
autoApprove: false,
setActiveNetwork: jest.fn(),
getCaveat: jest.fn(),
requestPermittedChainsPermissionForOrigin: jest.fn(),
requestPermittedChainsPermissionIncrementalForOrigin: jest.fn(),
setTokenNetworkFilter: jest.fn(),
};
Expand Down Expand Up @@ -47,7 +46,7 @@ describe('Ethereum Chain Utils', () => {
await switchChain('0x1', 'mainnet');

expect(
mocks.requestPermittedChainsPermissionForOrigin,
mocks.requestPermittedChainsPermissionIncrementalForOrigin,
).toHaveBeenCalledWith({ chainId: '0x1', autoApprove: false });
});

Expand All @@ -61,14 +60,16 @@ describe('Ethereum Chain Utils', () => {

it('should throw an error if the switch chain approval is rejected', async () => {
const { mocks, end, switchChain } = createMockedSwitchChain();
mocks.requestPermittedChainsPermissionForOrigin.mockRejectedValueOnce({
code: errorCodes.provider.userRejectedRequest,
});
mocks.requestPermittedChainsPermissionIncrementalForOrigin.mockRejectedValueOnce(
{
code: errorCodes.provider.userRejectedRequest,
},
);

await switchChain('0x1', 'mainnet');

expect(
mocks.requestPermittedChainsPermissionForOrigin,
mocks.requestPermittedChainsPermissionIncrementalForOrigin,
).toHaveBeenCalled();
expect(mocks.setActiveNetwork).not.toHaveBeenCalled();
expect(end).toHaveBeenCalledWith({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ const requestEthereumAccounts = {
getUnlockPromise: true,
sendMetrics: true,
metamaskState: true,
requestCaip25ApprovalForOrigin: true,
grantPermissionsForOrigin: true,
getCaip25PermissionFromLegacyPermissionsForOrigin: true,
requestPermissionsForOrigin: true,
},
};
export default requestEthereumAccounts;
Expand All @@ -39,8 +39,8 @@ const locks = new Set();
* @param options.getUnlockPromise - A hook that resolves when the wallet is unlocked.
* @param options.sendMetrics - A hook that helps track metric events.
* @param options.metamaskState - The MetaMask app state.
* @param options.requestCaip25ApprovalForOrigin - A hook that requests approval for the CAIP-25 permission for the origin.
* @param options.grantPermissionsForOrigin - A hook that grants permission for the approved permissions for the origin.
* @param options.getCaip25PermissionFromLegacyPermissionsForOrigin - A hook that returns a CAIP-25 permission from a legacy `eth_accounts` and `endowment:permitted-chains` permission.
* @param options.requestPermissionsForOrigin - A hook that requests CAIP-25 permissions for the origin.
* @returns A promise that resolves to nothing
*/
async function requestEthereumAccountsHandler(
Expand All @@ -53,8 +53,8 @@ async function requestEthereumAccountsHandler(
getUnlockPromise,
sendMetrics,
metamaskState,
requestCaip25ApprovalForOrigin,
grantPermissionsForOrigin,
getCaip25PermissionFromLegacyPermissionsForOrigin,
requestPermissionsForOrigin,
},
) {
const { origin } = req;
Expand Down Expand Up @@ -84,8 +84,9 @@ async function requestEthereumAccountsHandler(
}

try {
const caip25Approval = await requestCaip25ApprovalForOrigin();
await grantPermissionsForOrigin(caip25Approval);
const caip25Permission =
getCaip25PermissionFromLegacyPermissionsForOrigin();
await requestPermissionsForOrigin(caip25Permission);
} catch (error) {
return end(error);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ const createMockedHandler = () => {
'0x3': {},
},
};
const requestCaip25ApprovalForOrigin = jest.fn().mockResolvedValue({});
const grantPermissionsForOrigin = jest.fn().mockReturnValue({});
const getCaip25PermissionFromLegacyPermissionsForOrigin = jest
.fn()
.mockResolvedValue({});
const requestPermissionsForOrigin = jest.fn().mockReturnValue({});
const response: PendingJsonRpcResponse<string[]> = {
jsonrpc: '2.0' as const,
id: 0,
Expand All @@ -54,8 +56,8 @@ const createMockedHandler = () => {
getUnlockPromise,
sendMetrics,
metamaskState,
requestCaip25ApprovalForOrigin,
grantPermissionsForOrigin,
getCaip25PermissionFromLegacyPermissionsForOrigin,
requestPermissionsForOrigin,
});

return {
Expand All @@ -66,8 +68,8 @@ const createMockedHandler = () => {
getUnlockPromise,
sendMetrics,
metamaskState,
requestCaip25ApprovalForOrigin,
grantPermissionsForOrigin,
getCaip25PermissionFromLegacyPermissionsForOrigin,
requestPermissionsForOrigin,
handler,
};
};
Expand Down Expand Up @@ -130,17 +132,20 @@ describe('requestEthereumAccountsHandler', () => {
});

describe('eip155 account permissions do not exist', () => {
it('requests the CAIP-25 approval', async () => {
const { handler, requestCaip25ApprovalForOrigin } = createMockedHandler();
it('gets the CAIP-25 permission object to request approval for', async () => {
const { handler, getCaip25PermissionFromLegacyPermissionsForOrigin } =
createMockedHandler();

await handler({ ...baseRequest, origin: 'http://test.com' });
expect(requestCaip25ApprovalForOrigin).toHaveBeenCalledWith();
expect(
getCaip25PermissionFromLegacyPermissionsForOrigin,
).toHaveBeenCalledWith();
});

it('throws an error if the CAIP-25 approval is rejected', async () => {
const { handler, requestCaip25ApprovalForOrigin, end } =
const { handler, requestPermissionsForOrigin, end } =
createMockedHandler();
requestCaip25ApprovalForOrigin.mockRejectedValue(
requestPermissionsForOrigin.mockRejectedValue(
new Error('approval rejected'),
);

Expand All @@ -151,14 +156,16 @@ describe('requestEthereumAccountsHandler', () => {
it('grants the CAIP-25 approval', async () => {
const {
handler,
requestCaip25ApprovalForOrigin,
grantPermissionsForOrigin,
getCaip25PermissionFromLegacyPermissionsForOrigin,
requestPermissionsForOrigin,
} = createMockedHandler();

requestCaip25ApprovalForOrigin.mockResolvedValue({ foo: 'bar' });
getCaip25PermissionFromLegacyPermissionsForOrigin.mockReturnValue({
foo: 'bar',
});

await handler({ ...baseRequest, origin: 'http://test.com' });
expect(grantPermissionsForOrigin).toHaveBeenCalledWith({ foo: 'bar' });
expect(requestPermissionsForOrigin).toHaveBeenCalledWith({ foo: 'bar' });
});

it('returns the newly granted and properly ordered eth accounts', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const switchEthereumChain = {
setActiveNetwork: true,
getCaveat: true,
getCurrentChainIdForDomain: true,
requestPermittedChainsPermissionForOrigin: true,
requestPermittedChainsPermissionIncrementalForOrigin: true,
setTokenNetworkFilter: true,
},
Expand All @@ -31,7 +30,6 @@ async function switchEthereumChainHandler(
setActiveNetwork,
getCaveat,
getCurrentChainIdForDomain,
requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin,
setTokenNetworkFilter,
},
Expand Down Expand Up @@ -69,7 +67,6 @@ async function switchEthereumChainHandler(
return switchChain(res, end, chainId, networkClientIdToSwitchTo, {
setActiveNetwork,
getCaveat,
requestPermittedChainsPermissionForOrigin,
requestPermittedChainsPermissionIncrementalForOrigin,
setTokenNetworkFilter,
});
Expand Down
Loading

0 comments on commit 76b9f1e

Please sign in to comment.