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

eng-5301-sats-connect-disable-automatic-detection-for-wbip-compatible-wallets #45

Conversation

m-aboelenein
Copy link
Member

No description provided.

@m-aboelenein m-aboelenein self-assigned this Oct 7, 2024
@m-aboelenein m-aboelenein merged commit 1349889 into main Oct 7, 2024
1 check passed
@m-aboelenein m-aboelenein deleted the mahmoud/eng-5301-sats-connect-disable-automatic-detection-for-wbip-compatible branch October 7, 2024 07:14
@orenyomtov
Copy link

Hey, what's the reasoning for this change? I'm working on a WBIP004 compatible wallet and this breaks the dApps' automatic detection of it. @m-aboelenein @victorkirov

@orenyomtov
Copy link

It's deviating from what the documentation says it does, and is a breaking change
#47

@aryzing
Copy link
Contributor

aryzing commented Oct 15, 2024

Hi @orenyomtov, we disabled support at this time since we don't have the bandwidth to cater to all providers' particularities. The docs are being updated to reflect this. We may enable support for other providers again in the future

@aryzing
Copy link
Contributor

aryzing commented Oct 15, 2024

@orenyomtov A bit more context: the WBIP004 standard doesn't seem to be actively maintained or agreed upon by all wallets in the space, which consumes a lot of resources when trying to make sats-connect work for all wallets. When the standard is more stable and better implemented by wallets in the space, we may enable support for it again.

If you absolutely need a connection kit that supports multiple wallets, you may want to check out dynamic's solution.

@orenyomtov
Copy link

Okay I see, thanks for the update

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.

4 participants