Skip to content

Conversation

mikesposito
Copy link
Member

@mikesposito mikesposito commented Apr 1, 2025

The Ledger iframe messaging system is being refactored to be promise based instead of using callbacks. Moreover, #sendMessage now throws a timeout error when the message takes to long to get replied by the iframe

Related to #258

@mikesposito mikesposito marked this pull request as ready for review April 1, 2025 13:38
@mikesposito mikesposito requested review from a team as code owners April 1, 2025 13:38
@mikesposito mikesposito changed the title feat: ledger iframe messages timeout fix: ledger iframe messages timeout Apr 1, 2025
mcmire
mcmire previously approved these changes Apr 1, 2025
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Makes sense to me!

@mikesposito mikesposito enabled auto-merge April 1, 2025 14:44
@mikesposito mikesposito added this pull request to the merge queue Apr 1, 2025
Merged via the queue into main with commit 86007bd Apr 1, 2025
31 checks passed
@mikesposito mikesposito deleted the mikesposito/fix-iframe-onload branch April 1, 2025 14:51
github-merge-queue bot pushed a commit that referenced this pull request Apr 15, 2025
<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?

Are there any issues or other links reviewers should consult to
understand this pull request better? For instance:

* Fixes #12345
* See: #67890
-->

#271 introduced timeouts for
all messages between the Ledger bridge and the iframe. Though a
20-second timeout may not be enough for the user to grab the device and
accept the request - especially if the user wants to look closely at the
signature request on the hardware device.

For this reason, the timeout is being removed for messages requiring
user intervention, while keeping it for messages that do not expect user
actions (i.e. `updateTransportMethod` and `attemptMakeApp`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants