Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix the mobile crash due to ledger bluetooth relative exception #11769

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

dawnseeker8
Copy link
Contributor

Description

This PR will fix the mobile crash issue when user disable the blind signing and reject the transaction in ledger devices.
This PR create a global exception handler to handle the bluetooth relative exception throw directly from native code, which can't be handled by our javascript code, (useLedgerBluetooth.ts has code handle those exception but shomehow, the exception still bubble up to ExceptionHandler in react native core which cause the red screen and app crash).

The global exception will specially handle the Ledger relative exception and stop exception bubble to react native core to stop app crash.

Related issues

Fixes: #11166
#11216

Manual testing steps

  1. Have Ledger with Eth app 1.11.2 installed and have eth app settings on Ledger set to blind signing: disabled
  2. Connect to test dapp on MM mobile browser
  3. Scroll down to ERC1155 section and tap "deploy"
  4. Note "Confirm transaction on your Ledger" shown on mobile client
  5. Note error "Unexpected error occurred. An unexpected error occurred. Please try again"
  6. Note MM closes on android and presents crash error on iOS

Screenshots/Recordings

Before

https://api.zenhub.com/attachedFiles/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBBenN6Qnc9PSIsImV4cCI6bnVsbCwicHVyIjoiYmxvYl9pZCJ9fQ==--8d54ab43db845febde47fa78240c0aff20dbbff9/RPReplay_Final1728461873.MP4

image

After

Pre-merge author checklist

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.

Copy link
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 the Run Smoke E2E Triggers smoke e2e on Bitrise label Oct 14, 2024
Copy link
Contributor

github-actions bot commented Oct 14, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 959d553
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/53490e11-c14b-438b-8666-50751297264f

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 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 15, 2024
Copy link
Contributor

github-actions bot commented Oct 15, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: e897d88
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/56b59a3b-9b74-496f-af33-d59f08ae3f86

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 added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Oct 15, 2024
Copy link
Contributor

@Daniel-Cross Daniel-Cross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@vivek-consensys vivek-consensys added the QA in Progress QA has started on the feature. label Oct 16, 2024
@dawnseeker8 dawnseeker8 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 17, 2024
Copy link
Contributor

github-actions bot commented Oct 17, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 4cb6176
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/72e705f5-f8af-4db5-ae5e-8357a4630189

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

Copy link

sonarcloud bot commented Oct 17, 2024

@vivek-consensys vivek-consensys added QA Passed A successful QA run through has been done and removed QA in Progress QA has started on the feature. labels Oct 17, 2024
@vivek-consensys
Copy link
Contributor

Tested on Bitrise build Android and iOS, working as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hardware-ledger needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) QA Passed A successful QA run through has been done Run Smoke E2E Triggers smoke e2e on Bitrise team-accounts
Projects
Status: Review finalised - Ready to be merged
Development

Successfully merging this pull request may close these issues.

[Bug]: Contract deployment with Ledger blind signing off causes MetaMask to crash
3 participants