Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

fix: disconnect error during transport reset (iOS devices only) #231

Merged
merged 5 commits into from
Jul 10, 2024

Conversation

dawnseeker8
Copy link
Contributor

@dawnseeker8 dawnseeker8 commented Jul 8, 2024

Currently, in iOS devices a disconnect error will be thrown when trying to reset the transport object. The reason for this is because the ETH app hasn't been recreated with the new transport object, even though the transport has the same properties as the previous one.

Note: We do not see this in Android devices, the setup and communication will work correctly without the need to reset the transport object.

…ort, and because eth app hasn't been recreated with new transport even that transport has exactly same property (android will not throw error, only ios did)
@dawnseeker8 dawnseeker8 requested a review from a team as a code owner July 8, 2024 01:44
@dawnseeker8 dawnseeker8 added team-hardware-wallets bug Something isn't working labels Jul 8, 2024
@gantunesr gantunesr changed the title fix: Current iOS will throw disconnect error when we reset the transp… fix: disconnect error during transport reset (iOS devices only) Jul 9, 2024
src/ledger-keyring.test.ts Outdated Show resolved Hide resolved
src/ledger-keyring.test.ts Outdated Show resolved Hide resolved
src/ledger-keyring.ts Outdated Show resolved Hide resolved
src/ledger-transport-middleware.test.ts Outdated Show resolved Hide resolved
…iption and follow metamask unit tests guideline
@gantunesr gantunesr merged commit 3a82a2b into main Jul 10, 2024
19 checks passed
@gantunesr gantunesr deleted the fix/ios-bug-ble-disconnect-issue branch July 10, 2024 15:26
@@ -60,9 +58,6 @@ export class LedgerTransportMiddleware implements TransportMiddleware {
* @returns An generic interface for communicating with a Ledger hardware wallet to perform operation.
*/
getEthApp(): MetaMaskLedgerHwAppEth {
if (!this.#app) {
Copy link
Member

Choose a reason for hiding this comment

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

The comment above is now inaccurate: "it create a new eth app instance if not exist."

We should fix that in a follow up PR

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working team-hardware-wallets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants