Conversation
Dayvvo
left a comment
There was a problem hiding this comment.
Hey @rolznz. Great that you've gotten around to this PR. Exactly what we need for self custodial wallet like flash and alby go. I haven't been able to test it successfully with alby-go but the flow looks like exactly what we need.
| const {unsub} = await nwaClient.subscribe({ | ||
| onSuccess: async (nwcClient) => { | ||
| nwcClient.close(); | ||
| // TODO: it makes no sense to connect again |
There was a problem hiding this comment.
Hey @rolznz. Great work defining the flow for custodial wallet. this makes it easier to add connectors for self custodial walets! Haven't been able to successfully test with alby-go yet but the flow looks solid. Looking forward to implementing and testing a flash connector.
One question though, Regarding the todo you left as a comment here. I'm assuming you are using connect right now cuz the current functionaliy uses that so save the nwcurl and oher config to the store. and also calls 'getInfo'. If we're looking at removing an unnecessary connect in the future, perhaps a good approach would be to call 'getInfo' outside of connect and then save the connect info?
I'm also thinking maybe wallet service could post the allowed method, budget and other config along with the walletpubkey and lud16? We can't just use the existing config that we set in the nwa string because they are optional and could be ignored by the user when setting his preffered options for nwc string that's being created. Let me know what you think about this.
There was a problem hiding this comment.
Than you!
One question though, Regarding the todo you left as a comment here. I'm assuming you are using connect right now cuz the current functionaliy uses that so save the nwcurl and oher config to the store. and also calls 'getInfo'. If we're looking at removing an unnecessary connect in the future, perhaps a good approach would be to call 'getInfo' outside of connect and then save the connect info?
The TODO is a bit hard to solve, because the connect function in the store only knows it needs to call init on a specific connector.
I'm also thinking maybe wallet service could post the allowed method, budget and other config along with the walletpubkey and lud16? We can't just use the existing config that we set in the nwa string because they are optional and could be ignored by the user when setting his preffered options for nwc string that's being created. Let me know what you think about this.
Unfortunately I had to remove the lud16 tag from the info event as it is a privacy issue - allowing someone to fetch all info events by a lightning address and from there subscribe to wallet service events for each app. It's the same issue with the other things you mentioned - it's better to call get_info because the response is end-to-end encrypted. I have a PR to move the lud16 field into get_info instead: getAlby/hub#1128
There was a problem hiding this comment.
@Dayvvo the Alby Go PR has been merged to master, so you should be able to test there and scan the QR code from bitcoin connect. For testing with Alby Hub it requires using Alby Hub from the master branch and creating a new Alby Go app with 1-click connections.
We plan to merge this but are happy to adapt to feedback - have you guys started working on the Flash wallet implementation of NWA?
@Dayvvo thanks! Alby Go is still a work in progress. I will let you know when it is ready. For now I tested just using the example in the JS SDK:
|
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Co-authored-by: Adithya Vardhan <imadithyavardhan@gmail.com>
Closes #278
Uses Alby JS SDK to initialize an NWAClient to get an auth string: getAlby/js-sdk#298 and subscribes to wait for the user to confirm the new connection. Once the auth string is confirmed, a new
NWCClientis available which allows Bitcoin Connect to finish the connection setup.TODO:
(Needs some styling improvements, but this is the general idea)
To test it in Alby Hub, you need to create an app connection that supports
create_connection, then use theexamples/nwc/client/nwa-accept.jsexample inside the JS SDK.