Skip to content

Commit

Permalink
fix: Send correct message action from ledger offscreen bridge to ledg…
Browse files Browse the repository at this point in the history
…er bridge (#24622)

## **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: #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.
  • Loading branch information
danjm authored May 22, 2024
1 parent 7c36209 commit 7baba13
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export class LedgerOffscreenBridge implements LedgerBridge {
chrome.runtime.sendMessage(
{
target: OffscreenCommunicationTarget.ledgerOffscreen,
action: LedgerAction.signMessage,
action: LedgerAction.signPersonalMessage,
params,
},
(response) => {
Expand Down
2 changes: 1 addition & 1 deletion shared/constants/offscreen-communication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}

Expand Down

0 comments on commit 7baba13

Please sign in to comment.