[Bug]: Unable to Send legacy transaction with Ledger #22118
Assignees
Labels
area-hardware
regression-RC
DEPRECATED: Please use "regresssion-RC-x.y.z" label instead
regression-RC-11.7.0
release-11.7.0
Issue or pull request that will be included in release 11.7.0
release-11.8.0
Issue or pull request that will be included in release 11.8.0
release-blocker
This bug is blocking the next release
team-accounts
team-wallet-framework
type-bug
Comments
plasmacorral
added
type-bug
team-accounts
regression-RC
|
This commit introduced a bug with legacy transactions in the Ledger Keyring, more information in the PR that fixed it. The fix on Will follow a PR on the extension to update the package version once it has been published. |
mikesposito
added a commit
that referenced
this issue
Dec 4, 2023
## **Description** This PR sets the minimum `@metamask/eth-ledger-bridge-keyring` version to `2.0.1`, which includes a bugfix for broken legacy transactions on all Ledger devices. ## **Related issues** Fixes: #22118 ## **Manual testing steps** 1. Connect a Ledger account to MM 2. Send a legacy transaction from the test dapp ## **Screenshots/Recordings** ### **Before**  ### **After** The above error should not be thrown ## **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 clearly explained what problem this PR is solving and how it is solved. - [ ] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] 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. - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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. Co-authored-by: legobeat <[email protected]>
metamaskbot
added
the
release-11.8.0
mikesposito
added a commit
that referenced
this issue
Dec 5, 2023
## **Description** This PR sets the minimum `@metamask/eth-ledger-bridge-keyring` version to `2.0.1`, which includes a bugfix for broken legacy transactions on all Ledger devices. ## **Related issues** Fixes: #22118 ## **Manual testing steps** 1. Connect a Ledger account to MM 2. Send a legacy transaction from the test dapp ## **Screenshots/Recordings** ### **Before**  ### **After** The above error should not be thrown ## **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 clearly explained what problem this PR is solving and how it is solved. - [ ] I've linked related issues - [ ] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [ ] 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. - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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. Co-authored-by: legobeat <[email protected]>
gauthierpetetin
removed
the
release-11.8.0
metamaskbot
added
the
release-11.8.0
|
Missing release label release-11.8.0 on issue. Adding release label release-11.8.0 on issue, as issue is linked to PR #22136 which has this release label. |
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Unable to send a legacy tx with the test dapp when using a Ledger Nano X on firmware 2.1.0 with Eth app 1.10.1 and blind signing enabled.
EIP1559 sends with the test dapp work fine with Ledger. Legacy send works with HD account, QR hardware, Trezor. Discovered this initially on a feature branch, confirmed the issue was present in Develop. Can confirm that the issue is NOT present in 11.5.2 or 11.6.2, but appears in the 11.7 RC commit 9787b7e
Expected behavior
Given I am connected to the test dapp with a Ledger address
And I am on a testnet (goerli)
When I Send Legacy Transaction in the test dapp
And I Confirm the transaction in extension
Then the tx should be sent to Ledger device
When I approve the tx on the hardware
Then the tx should be broadcast to the testnet
And eventually be confirmed
Screenshots/Recordings
Video: https://recordit.co/i5ImJNHdWp
Steps to reproduce
Error messages or log output
{name: 'Error', message: 'invalid rlp data (argument="data", value=Uint8Arra…20840), code=INVALID_ARGUMENT, version=rlp/5.5.0)', stack: 'Error: invalid rlp data (argument="data", value=Ui….io/eth-ledger-bridge-keyring/bundle.js:18253:12)', code: undefined, rpc: undefined}Version
11.7.0
Build type
None
Browser
Chrome
Operating system
MacOS
Hardware wallet
Ledger
Additional context
This appears to be introduced in 3713c18, and I can confirm that the issue is not present in the commit before #21149 was merged.
Severity
No response
The text was updated successfully, but these errors were encountered: