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

Issues with Wallet Connection and the "Connected Websites" Feature #1355

Open
Crypto-Druide opened this issue Dec 1, 2024 · 3 comments
Open

Comments

@Crypto-Druide
Copy link

Hi,

There are multiple problems with wallet connection to a website and the "Connected Websites" feature in the extension:

Case 1: When I want to connect my wallet to Namadillo, a pop-up asks me to connect my wallet.

  • Problem: The name of the wallet I am trying to connect is not specified.
  • Solution: Display the name of the wallet being connected.

Case 2: When I click the button to approve the connection.

  • Problem: All wallets present in the extension are automatically connected, which is not the desired behavior.
  • Solution: Only the wallet selected as active in the extension should be connected.

Case 3: When I access the "Connected Websites" section and try to disconnect a site from my wallet.

  • Problem 1: Websites are not displayed per wallet.
  • Problem 2: The disconnect button removes the website connection from all wallets.
  • Solution: The "Connected Sites" screen should only display sites connected to the active account, and disconnection should only affect that account.

Let me know if you need any additional information.

@Crypto-Druide
Copy link
Author

@jurevans

@jurevans
Copy link
Collaborator

jurevans commented Dec 2, 2024

@Crypto-Druide Thanks for reporting this! This isn't a bug, this is how it is currently designed. I think we need to think more about what should happen when a user connects, and when a user adds a wallet to the keychain. I think it might be tedious to only allow one wallet to be connected at a time, as when we sync shielded balances, we would need to re-sync any time you switch the active account. Should the user maybe see a list of accounts they can authorize for any domain they connect to? This would allow us to keep the existing functionality (for shielded sync), and still gives the user complete control over which accounts they share with the connected app.

I have a somewhat related PR that establishes additional levels of permissions #1142 - this is kind of on the back-burner at the moment, but I think the whole UI with Connected Sites could be approved, where we have Domain-level access, with enabled signing chains under that (so we could revoke signing permission for a specific chain, or revoke the entire domain/app). I think this could be extended to support per-wallet permissions for each Connected Site - Does this make sense? We will need to get some design input on this as well.

@Crypto-Druide
Copy link
Author

Hey @jurevans ,

Thanks for your feedback.
I understand it wasn’t built that way, but connecting all the wallets at once sounds like a security issue to me.
I’m open to discussing the other points (ways to manage connected websites), but connecting all the wallets is something nobody would prefer.

Here’s a basic example:
I have two wallets: one for degen testing and another holding my assets (the piggy bank wallet).
What would happen if I intend to connect only my degen wallet to a shady interface but accidentally approve a wrong transaction on my "precious" wallet?
Also, if I’m correct, since the wallet name isn’t displayed on the approval transaction screen, there’s no way for me to identify it.

I'll let you discuss that point with the team but that seems important to me to find a solution.

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

No branches or pull requests

2 participants