From 524ab017c74c7c8ebd20223d9ede733c96668fbd Mon Sep 17 00:00:00 2001 From: Nicholas Ellul Date: Thu, 6 Jul 2023 19:48:38 -0400 Subject: [PATCH 1/7] Add edge case handling for tx utilities (#19907) --- shared/modules/transaction.utils.js | 2 +- shared/modules/transaction.utils.test.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/shared/modules/transaction.utils.js b/shared/modules/transaction.utils.js index b07d1f869..44a681519 100644 --- a/shared/modules/transaction.utils.js +++ b/shared/modules/transaction.utils.js @@ -176,7 +176,7 @@ export async function determineTransactionType(txParams, query) { contractCode = resultCode; if (isContractAddress) { - const hasValue = txParams.value && txParams.value !== '0x0'; + const hasValue = txParams.value && Number(txParams.value) !== 0; const tokenMethodName = [ TransactionType.tokenMethodApprove, diff --git a/shared/modules/transaction.utils.test.js b/shared/modules/transaction.utils.test.js index 615fda2e3..242d10007 100644 --- a/shared/modules/transaction.utils.test.js +++ b/shared/modules/transaction.utils.test.js @@ -186,6 +186,20 @@ describe('Transaction.utils', function () { getCodeResponse: '0xab', }); + const resultWithEmptyValue2 = await determineTransactionType( + { + value: '0x0000', + to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9', + data: '0xa9059cbb0000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C970000000000000000000000000000000000000000000000000000000000000000a', + }, + new EthQuery(_provider), + ); + + expect(resultWithEmptyValue2).toMatchObject({ + type: TransactionType.tokenMethodTransfer, + getCodeResponse: '0xab', + }); + const resultWithValue = await determineTransactionType( { value: '0x12345', From 64cbe1f2f83820d2e497c544ad8ffc8eeb67645c Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Tue, 20 Jun 2023 15:37:09 +0200 Subject: [PATCH 2/7] Accept SignController approval request from frontend (#19184) --- ...ture-controller-npm-3.0.0-8771b6885e.patch | 18 - app/scripts/controllers/decrypt-message.ts | 2 +- .../controllers/encryption-public-key.ts | 2 +- app/scripts/metamask-controller.js | 27 +- lavamoat/browserify/beta/policy.json | 3 +- lavamoat/browserify/desktop/policy.json | 3 +- lavamoat/browserify/flask/policy.json | 3 +- lavamoat/browserify/main/policy.json | 3 +- package.json | 7 +- .../qr-hardware-popover.js | 32 +- .../signature-request-original-warning.js | 1 + .../signature-request-original.component.js | 60 ++-- .../signature-request-original.container.js | 52 ++- .../signature-request-original.test.js | 32 +- .../signature-request-siwe.js | 67 ++-- .../signature-request.component.js | 44 ++- .../signature-request.container.js | 82 ++--- .../signature-request.container.test.js | 32 +- ui/pages/confirm-signature-request/index.js | 46 --- ui/store/actions.test.js | 225 ------------ ui/store/actions.ts | 321 ++---------------- .../institutional/institution-actions.test.js | 8 +- ui/store/institutional/institution-actions.ts | 12 +- ui/store/store.ts | 2 +- yarn.lock | 45 +-- 25 files changed, 281 insertions(+), 848 deletions(-) delete mode 100644 .yarn/patches/@metamask-signature-controller-npm-3.0.0-8771b6885e.patch diff --git a/.yarn/patches/@metamask-signature-controller-npm-3.0.0-8771b6885e.patch b/.yarn/patches/@metamask-signature-controller-npm-3.0.0-8771b6885e.patch deleted file mode 100644 index 4a2682995..000000000 --- a/.yarn/patches/@metamask-signature-controller-npm-3.0.0-8771b6885e.patch +++ /dev/null @@ -1,18 +0,0 @@ -diff --git a/dist/SignatureController.js b/dist/SignatureController.js -index b39d274f4547ab4e8b647293199ec21c4a9e38ca..288e55c97c3e4a234874dd8b8986ba77576b0dc4 100644 ---- a/dist/SignatureController.js -+++ b/dist/SignatureController.js -@@ -308,12 +308,12 @@ _SignatureController_keyringController = new WeakMap(), _SignatureController_isE - const messageId = msgParams.metamaskId; - try { - const cleanMessageParams = yield messageManager.approveMessage(msgParams); -+ __classPrivateFieldGet(this, _SignatureController_instances, "m", _SignatureController_acceptApproval).call(this, messageId); - const signature = yield getSignature(cleanMessageParams); - this.hub.emit(`${methodName}:signed`, { signature, messageId }); - if (!cleanMessageParams.deferSetAsSigned) { - messageManager.setMessageStatusSigned(messageId, signature); - } -- __classPrivateFieldGet(this, _SignatureController_instances, "m", _SignatureController_acceptApproval).call(this, messageId); - return __classPrivateFieldGet(this, _SignatureController_getAllState, "f").call(this); - } - catch (error) { diff --git a/app/scripts/controllers/decrypt-message.ts b/app/scripts/controllers/decrypt-message.ts index b9f37fea2..fee01257f 100644 --- a/app/scripts/controllers/decrypt-message.ts +++ b/app/scripts/controllers/decrypt-message.ts @@ -45,7 +45,7 @@ export type CoreMessage = AbstractMessage & { }; export type StateMessage = Required< - Omit + Omit >; export type DecryptMessageControllerState = { diff --git a/app/scripts/controllers/encryption-public-key.ts b/app/scripts/controllers/encryption-public-key.ts index 904bed476..da50f1afe 100644 --- a/app/scripts/controllers/encryption-public-key.ts +++ b/app/scripts/controllers/encryption-public-key.ts @@ -45,7 +45,7 @@ export type CoreMessage = AbstractMessage & { }; export type StateMessage = Required< - Omit + Omit > & { msgParams: string; }; diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 1c848efae..187143677 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1258,11 +1258,7 @@ export default class MetamaskController extends EventEmitter { this.signatureController = new SignatureController({ messenger: this.controllerMessenger.getRestricted({ name: 'SignatureController', - allowedActions: [ - `${this.approvalController.name}:addRequest`, - `${this.approvalController.name}:acceptRequest`, - `${this.approvalController.name}:rejectRequest`, - ], + allowedActions: [`${this.approvalController.name}:addRequest`], }), keyringController: this.keyringController, isEthSignEnabled: () => @@ -2248,27 +2244,6 @@ export default class MetamaskController extends EventEmitter { updatePreviousGasParams: txController.updatePreviousGasParams.bind(txController), - // signatureController - signMessage: this.signatureController.signMessage.bind( - this.signatureController, - ), - cancelMessage: this.signatureController.cancelMessage.bind( - this.signatureController, - ), - signPersonalMessage: this.signatureController.signPersonalMessage.bind( - this.signatureController, - ), - cancelPersonalMessage: - this.signatureController.cancelPersonalMessage.bind( - this.signatureController, - ), - signTypedMessage: this.signatureController.signTypedMessage.bind( - this.signatureController, - ), - cancelTypedMessage: this.signatureController.cancelTypedMessage.bind( - this.signatureController, - ), - // decryptMessageController decryptMessage: this.decryptMessageController.decryptMessage.bind( this.decryptMessageController, diff --git a/lavamoat/browserify/beta/policy.json b/lavamoat/browserify/beta/policy.json index c43c1c97b..929c69e1b 100644 --- a/lavamoat/browserify/beta/policy.json +++ b/lavamoat/browserify/beta/policy.json @@ -1718,7 +1718,8 @@ "browserify>buffer": true, "browserify>events": true, "eth-rpc-errors": true, - "ethereumjs-util": true + "ethereumjs-util": true, + "lodash": true } }, "@metamask/smart-transactions-controller": { diff --git a/lavamoat/browserify/desktop/policy.json b/lavamoat/browserify/desktop/policy.json index f80fbb328..d470098a1 100644 --- a/lavamoat/browserify/desktop/policy.json +++ b/lavamoat/browserify/desktop/policy.json @@ -1908,7 +1908,8 @@ "browserify>buffer": true, "browserify>events": true, "eth-rpc-errors": true, - "ethereumjs-util": true + "ethereumjs-util": true, + "lodash": true } }, "@metamask/smart-transactions-controller": { diff --git a/lavamoat/browserify/flask/policy.json b/lavamoat/browserify/flask/policy.json index f80fbb328..d470098a1 100644 --- a/lavamoat/browserify/flask/policy.json +++ b/lavamoat/browserify/flask/policy.json @@ -1908,7 +1908,8 @@ "browserify>buffer": true, "browserify>events": true, "eth-rpc-errors": true, - "ethereumjs-util": true + "ethereumjs-util": true, + "lodash": true } }, "@metamask/smart-transactions-controller": { diff --git a/lavamoat/browserify/main/policy.json b/lavamoat/browserify/main/policy.json index c43c1c97b..929c69e1b 100644 --- a/lavamoat/browserify/main/policy.json +++ b/lavamoat/browserify/main/policy.json @@ -1718,7 +1718,8 @@ "browserify>buffer": true, "browserify>events": true, "eth-rpc-errors": true, - "ethereumjs-util": true + "ethereumjs-util": true, + "lodash": true } }, "@metamask/smart-transactions-controller": { diff --git a/package.json b/package.json index 9793ffcc5..2bd1e481e 100644 --- a/package.json +++ b/package.json @@ -196,8 +196,7 @@ "fast-json-patch@^3.1.1": "patch:fast-json-patch@npm%3A3.1.1#./.yarn/patches/fast-json-patch-npm-3.1.1-7e8bb70a45.patch", "request@^2.83.0": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch", "request@^2.88.2": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch", - "request@^2.85.0": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch", - "@metamask/signature-controller@^3.0.0": "patch:@metamask/signature-controller@npm%3A3.0.0#./.yarn/patches/@metamask-signature-controller-npm-3.0.0-8771b6885e.patch" + "request@^2.85.0": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch" }, "dependencies": { "@actions/core": "^1.10.0", @@ -248,7 +247,7 @@ "@metamask/jazzicon": "^2.0.0", "@metamask/key-tree": "^7.0.0", "@metamask/logo": "^3.1.1", - "@metamask/message-manager": "^6.0.0", + "@metamask/message-manager": "^7.0.0", "@metamask/metamask-eth-abis": "^3.0.0", "@metamask/notification-controller": "^3.0.0", "@metamask/obs-store": "^8.1.0", @@ -261,7 +260,7 @@ "@metamask/rpc-methods-flask": "npm:@metamask/rpc-methods@0.34.0-flask.1", "@metamask/safe-event-emitter": "^2.0.0", "@metamask/scure-bip39": "^2.0.3", - "@metamask/signature-controller": "^3.0.0", + "@metamask/signature-controller": "^4.0.1", "@metamask/slip44": "^3.0.0", "@metamask/smart-transactions-controller": "^3.1.0", "@metamask/snaps-controllers": "^0.32.2", diff --git a/ui/components/app/qr-hardware-popover/qr-hardware-popover.js b/ui/components/app/qr-hardware-popover/qr-hardware-popover.js index c3875c4e8..40d577380 100644 --- a/ui/components/app/qr-hardware-popover/qr-hardware-popover.js +++ b/ui/components/app/qr-hardware-popover/qr-hardware-popover.js @@ -1,5 +1,6 @@ import React, { useCallback, useMemo, useState } from 'react'; import { useDispatch, useSelector } from 'react-redux'; +import { ethErrors, serializeError } from 'eth-rpc-errors'; import { getCurrentQRHardwareState } from '../../../selectors'; import Popover from '../../ui/popover'; import { useI18nContext } from '../../../hooks/useI18nContext'; @@ -7,11 +8,8 @@ import { cancelSyncQRHardware as cancelSyncQRHardwareAction, cancelQRHardwareSignRequest as cancelQRHardwareSignRequestAction, cancelTx, - cancelPersonalMsg, - cancelMsg, - cancelTypedMsg, + rejectPendingApproval, } from '../../../store/actions'; -import { MESSAGE_TYPE } from '../../../../shared/constants/app'; import QRHardwareWalletImporter from './qr-hardware-wallet-importer'; import QRHardwareSignRequest from './qr-hardware-sign-request'; @@ -43,25 +41,13 @@ const QRHardwarePopover = () => { ); const signRequestCancel = useCallback(() => { - let action = cancelTx; - switch (_txData.type) { - case MESSAGE_TYPE.PERSONAL_SIGN: { - action = cancelPersonalMsg; - break; - } - case MESSAGE_TYPE.ETH_SIGN: { - action = cancelMsg; - break; - } - case MESSAGE_TYPE.ETH_SIGN_TYPED_DATA: { - action = cancelTypedMsg; - break; - } - default: { - action = cancelTx; - } - } - dispatch(action(_txData)); + dispatch( + rejectPendingApproval( + _txData.id, + serializeError(ethErrors.provider.userRejectedRequest()), + ), + ); + dispatch(cancelTx(_txData)); dispatch(cancelQRHardwareSignRequestAction()); }, [dispatch, _txData]); diff --git a/ui/components/app/signature-request-original/signature-request-original-warning/signature-request-original-warning.js b/ui/components/app/signature-request-original/signature-request-original-warning/signature-request-original-warning.js index 6de7757e9..13ac14b76 100644 --- a/ui/components/app/signature-request-original/signature-request-original-warning/signature-request-original-warning.js +++ b/ui/components/app/signature-request-original/signature-request-original-warning/signature-request-original-warning.js @@ -96,6 +96,7 @@ const SignatureRequestOriginalWarning = ({