From 7c95671b9b1140a909be934d21b8b16f3a69798f Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Wed, 22 May 2024 13:39:43 -0230 Subject: [PATCH] fix: Send correct message action from ledger offscreen bridge to ledger bridge (#24622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Fixes a bug. Personal signatures were broken for ledger on MV3. The problem was that the offscreen bridge named those message actions incorrectly. The ledger bridge expects `'ledger-sign-personal-message'`: https://github.com/MetaMask/eth-ledger-bridge-keyring/blob/gh-pages/ledger-bridge.js#L35-L37 This is consistent with the implementation of `deviceSignMessage` on the current iframe bridge: https://github.com/MetaMask/eth-ledger-bridge-keyring/blob/main/src/ledger-iframe-bridge.ts#L219-L226 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24622?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/24588 ## **Manual testing steps** 1. Build MV3 2. Import a ledger 3. Go to the test dapp 4. Try a Personal Sign 5. Accept in MM 6. You should see the signature on your ledger and be able to successfully sign ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.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/develop/.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. --- app/scripts/lib/offscreen-bridge/ledger-offscreen-bridge.ts | 2 +- shared/constants/offscreen-communication.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/lib/offscreen-bridge/ledger-offscreen-bridge.ts b/app/scripts/lib/offscreen-bridge/ledger-offscreen-bridge.ts index fd622fac3b8a..e17178a76b25 100644 --- a/app/scripts/lib/offscreen-bridge/ledger-offscreen-bridge.ts +++ b/app/scripts/lib/offscreen-bridge/ledger-offscreen-bridge.ts @@ -131,7 +131,7 @@ export class LedgerOffscreenBridge implements LedgerBridge { chrome.runtime.sendMessage( { target: OffscreenCommunicationTarget.ledgerOffscreen, - action: LedgerAction.signMessage, + action: LedgerAction.signPersonalMessage, params, }, (response) => { diff --git a/shared/constants/offscreen-communication.ts b/shared/constants/offscreen-communication.ts index 3be4d5da32de..cab4232c09dc 100644 --- a/shared/constants/offscreen-communication.ts +++ b/shared/constants/offscreen-communication.ts @@ -39,7 +39,7 @@ export enum LedgerAction { unlock = 'ledger-unlock', getPublicKey = 'ledger-unlock', signTransaction = 'ledger-sign-transaction', - signMessage = 'ledger-sign-message', + signPersonalMessage = 'ledger-sign-personal-message', signTypedData = 'ledger-sign-typed-data', }