Skip to content

Fix: 8917 user unable to send funds using ledger#8930

Closed
dawnseeker8 wants to merge 1 commit into
release/7.18.0from
fix/8917-fix-send-funds-for-7.17.x
Closed

Fix: 8917 user unable to send funds using ledger#8930
dawnseeker8 wants to merge 1 commit into
release/7.18.0from
fix/8917-fix-send-funds-for-7.17.x

Conversation

@dawnseeker8

@dawnseeker8 dawnseeker8 commented Mar 14, 2024

Copy link
Copy Markdown
Contributor

Description

This PR will fix #8917 for all the release 7.17.0, 7.17.1 and 7.18.0.

The fix is quite simple to move the await this.persistTransactionParameters(transaction); above ledger code in Confirm/index.js file so that TransactionController can persist all gas relative tx parameter before ledger process signTransaction.

⚠️ : Due to main branch have TransactionController upgrade which use @ethereumjs/tx library, which cause another issue in ledger transaction flow, this PR will not fix the issue on main branch, however we should still merge this PR to main branch and then i will fix another ledger transaction issue due to @ethereumjs/tx upgrade.

Related issues

Fixes: #8917

Manual testing steps

  1. Connect Ledger device
  2. Switch network to Linea Testnet
  3. Send transaction from Ledger account to Account 2 (normal account)
  4. Confirm transaction on Ledger device
  5. Transaction should successfully submitted.

Screenshots/Recordings

Before

8917_iPhone.MP4

After

Screen_Recording_20240314_133247_MetaMask.mp4

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • 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 format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). 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.

@dawnseeker8 dawnseeker8 requested a review from a team as a code owner March 14, 2024 05:33
@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@dawnseeker8 dawnseeker8 added team-hardware-wallets-deprecated DEPRECATED: please use "team-accounts-framework" instead release-7.17.0 Issue or pull request that will be included in release 7.17.0 release-7.18.0 Issue or pull request that will be included in release 7.18.0 release-7.17.1 Issue or pull request that will be included in release 7.17.1 needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) Run Smoke E2E labels Mar 14, 2024
@github-actions

github-actions Bot commented Mar 14, 2024

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: c2d4bdf
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6fd24524-f38b-4849-8a7e-80e6c52195f9

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@dawnseeker8 dawnseeker8 changed the base branch from release/7.18.0 to release/7.17.1 March 14, 2024 09:43
@dawnseeker8 dawnseeker8 requested a review from a team as a code owner March 14, 2024 09:43
@dawnseeker8 dawnseeker8 changed the base branch from release/7.17.1 to release/7.18.0 March 14, 2024 09:43
@dawnseeker8

Copy link
Copy Markdown
Contributor Author

e2e tests pass even Check Bitrise E2E status pipeline failed.
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6fd24524-f38b-4849-8a7e-80e6c52195f9

@dawnseeker8

Copy link
Copy Markdown
Contributor Author

Close this PR because another cherry pick PR #8940 will be used instead.

@dawnseeker8 dawnseeker8 deleted the fix/8917-fix-send-funds-for-7.17.x branch March 14, 2024 15:59
@github-actions github-actions Bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Mar 14, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 14, 2024
@vivek-consensys vivek-consensys added the QA Passed QA testing has been completed and passed label Mar 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

QA Passed QA testing has been completed and passed release-7.17.0 Issue or pull request that will be included in release 7.17.0 release-7.17.1 Issue or pull request that will be included in release 7.17.1 release-7.18.0 Issue or pull request that will be included in release 7.18.0 team-hardware-wallets-deprecated DEPRECATED: please use "team-accounts-framework" instead

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants