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

Solution for multiple wallet connect instances #412

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

Conversation

tmctl
Copy link
Contributor

@tmctl tmctl commented Jan 14, 2025

Description:
This offers a solution in the cases where there are multiple browser tabs open.

In testing by @deligrim :

I have reproduced the problem both on the current version of hedera-wallet-connect and in the latest version of AppKit test applications.

I think the problem is in the wallet-connect kernel \ relay server. The relay server sends a result message to the only last connected sign client for a given topic id, while causing it to throw an exception because it can't find the original request record.

I provided a workaround to this bug in PR by force reconnecting the requesting dApp instance to relay if it was not the last connected instance (among browser tabs).

Related issue(s):

Fixes #387

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@tmctl tmctl requested review from a team as code owners January 14, 2025 22:43
@tmctl tmctl requested a review from Ashe-Oro January 14, 2025 22:43
@tmctl
Copy link
Contributor Author

tmctl commented Jan 14, 2025

@teacoat does this seem like a viable solution for dApps for having multiple browser tabs open?

@franfernandez20
Copy link
Contributor

That's a fantastic improvement! However, this doesn't allow the user to interact with multiple tabs, it only enables interaction with the most recent tab they connected to. I believe it remains essential to address the issue of using multiple tabs simultaneously.

@franfernandez20 franfernandez20 self-requested a review January 15, 2025 08:57
@kantorcodes
Copy link
Contributor

Hmmm, having to close all other tabs would be similar to the current experience, I am also worried about maintaining any code that touches the relay directly in this way and would suggest end to end tests to keep track of this in case the base functionality changes again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Session breaks with multiple tabs open
4 participants