Skip to content

Commit

Permalink
Fully remove eth_sign (#4319)
Browse files Browse the repository at this point in the history
## Explanation
Months ago, because of phishing risk, we disabled the `eth_sign` API
method by default (users could manually enable it with a preference
toggle). Now because of additional risk associated with [potentially
malicious EIP-3074
invokers](https://ethereum-magicians.org/t/eip-3074-is-unsafe-unnecessary-puts-user-funds-at-risk-while-fragmenting-ux-liquidity-and-the-wallet-stack/19662)
we are fully removing support for this method.

## References

* Fixes MetaMask/MetaMask-planning#2371

## Changelog

### `@metamask/signature-controller`

- **REMOVED**: Methods related to `eth_sign` signatures -
`unapprovedMsgCount`, `messages`, `newUnsignedMessage` - have been
removed.
- **REMOVED**: constructor argument `isEthSignEnabled` is no longer
expected
- 
### `@metamask/preferences-controller`

- **REMOVED**: `disabledRpcMethodPreferences` removed from state
- **REMOVED**: `setDisabledRpcMethodPreference` removed

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
  • Loading branch information
adonesky1 authored May 30, 2024
1 parent 50efa9c commit e6ecd95
Show file tree
Hide file tree
Showing 15 changed files with 57 additions and 665 deletions.
1 change: 0 additions & 1 deletion packages/controller-utils/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ export enum ApprovalType {
ConnectAccounts = 'connect_accounts',
EthDecrypt = 'eth_decrypt',
EthGetEncryptionPublicKey = 'eth_getEncryptionPublicKey',
EthSign = 'eth_sign',
EthSignTypedData = 'eth_signTypedData',
PersonalSign = 'personal_sign',
ResultError = 'result_error',
Expand Down
12 changes: 6 additions & 6 deletions packages/logging-controller/src/LoggingController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ describe('LoggingController', () => {
await unrestricted.call('LoggingController:add', {
type: LogType.EthSignLog,
data: {
signingMethod: SigningMethod.EthSign,
signingMethod: SigningMethod.PersonalSign,
stage: SigningStage.Proposed,
signingData: '0x0000000000000',
signingData: 'hello',
},
}),
).toBeUndefined();
Expand All @@ -97,9 +97,9 @@ describe('LoggingController', () => {
log: expect.objectContaining({
type: LogType.EthSignLog,
data: {
signingMethod: SigningMethod.EthSign,
signingMethod: SigningMethod.PersonalSign,
stage: SigningStage.Proposed,
signingData: '0x0000000000000',
signingData: 'hello',
},
}),
});
Expand Down Expand Up @@ -167,9 +167,9 @@ describe('LoggingController', () => {
await unrestricted.call('LoggingController:add', {
type: LogType.EthSignLog,
data: {
signingMethod: SigningMethod.EthSign,
signingMethod: SigningMethod.PersonalSign,
stage: SigningStage.Proposed,
signingData: '0x0000000000000',
signingData: 'Heya',
},
}),
).toBeUndefined();
Expand Down
1 change: 0 additions & 1 deletion packages/logging-controller/src/logTypes/EthSignLog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type { LogType } from './LogType';
* An enum of the signing method types that we are interested in logging.
*/
export enum SigningMethod {
EthSign = 'eth_sign',
PersonalSign = 'personal_sign',
EthSignTypedData = 'eth_signTypedData',
EthSignTypedDataV3 = 'eth_signTypedData_v3',
Expand Down
2 changes: 1 addition & 1 deletion packages/message-manager/src/AbstractMessageManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export interface AbstractMessage {
}

/**
* @type MessageParams
* @type AbstractMessageParams
*
* Represents the parameters to pass to the signing method once the signature request is approved.
* @property from - Address from which the message is processed
Expand Down
156 changes: 0 additions & 156 deletions packages/message-manager/src/MessageManager.test.ts

This file was deleted.

128 changes: 0 additions & 128 deletions packages/message-manager/src/MessageManager.ts

This file was deleted.

1 change: 0 additions & 1 deletion packages/message-manager/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
export * from './AbstractMessageManager';
export * from './MessageManager';
export * from './PersonalMessageManager';
export * from './TypedMessageManager';
export * from './EncryptionPublicKeyManager';
Expand Down
7 changes: 2 additions & 5 deletions packages/message-manager/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { validate } from 'jsonschema';

import type { DecryptMessageParams } from './DecryptMessageManager';
import type { EncryptionPublicKeyParams } from './EncryptionPublicKeyManager';
import type { MessageParams } from './MessageManager';
import type { PersonalMessageParams } from './PersonalMessageManager';
import type { TypedMessageParams } from './TypedMessageManager';

Expand Down Expand Up @@ -48,14 +47,12 @@ export function normalizeMessageData(data: string) {
}

/**
* Validates a PersonalMessageParams and MessageParams objects for required properties and throws in
* Validates a PersonalMessageParams objects for required properties and throws in
* the event of any validation error.
*
* @param messageData - PersonalMessageParams object to validate.
*/
export function validateSignMessageData(
messageData: PersonalMessageParams | MessageParams,
) {
export function validateSignMessageData(messageData: PersonalMessageParams) {
const { from, data } = messageData;
validateAddress(from, 'from');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ export function testsForProviderType(providerType: ProviderType) {

{ name: 'eth_sendRawTransaction', numberOfParameters: 1 },
{ name: 'eth_sendTransaction', numberOfParameters: 1 },
{ name: 'eth_sign', numberOfParameters: 2 },

{ name: 'eth_createAccessList', numberOfParameters: 2 },
{ name: 'eth_getLogs', numberOfParameters: 1 },
{ name: 'eth_getProof', numberOfParameters: 3 },
Expand Down
Loading

0 comments on commit e6ecd95

Please sign in to comment.