Improve NWC support (return response from send payment, get_balance, make_invoice/lookup_invoice)

[rolznz]

Hi, we added a mutiny wallet connector to Bitcoin Connect so you can connect to your wallet in any lightning webapp and seamlessly execute lightning functionality, as long as the wallet is open on a second device (e.g. keep the Mutiny wallet open on your PC and then use Bitcoin Connect to connect to it on your phone).

Unfortunately there are some issues:

• Send Payment works, but it seems Mutiny Wallet never sends a response event, or at least not within 60 seconds. I had the wallet open on my phone and it marked the payment as sent within 20 seconds or so.
• Get Balance and Make invoice just don't seem to be supported at all?

I created an issue here: getAlby/bitcoin-connect#150

[benthecarman]

The response event is a problem with our relay.

What is the filter you're using to try and get it?

[rolznz]

@benthecarman we use this code:

const event = await this.signEvent(unsignedEvent);
const sub = this.relay.sub([
  {
    kinds: [23195],
    authors: [this.walletServicePubkey], 
    "#e": [event.id],
  },
]);

[benthecarman]

Ah okay, looks like we were only allowing filters that had the authors and #p defined. Have a change ready to fix that: MutinyWallet/blastr#31

[benthecarman]

Get Balance and Make invoice just don't seem to be supported at all?

We haven't done get_balance because of the privacy concerns. I think we can add make_invoice without any issues though, probably only need to worry about rate limiting

[TonyGiorgio]

Is there any info or capabilities feature of NWC? Now that the new NWC extensions are going to add a ton more API calls, some we may or may not implement. I wonder what's the best way to indicate who supports what.

[rolznz]

@TonyGiorgio good point, it would be nice to add a methods property in the get_info RPC call at https://github.com/nostr-protocol/nips/pull/685/files

That is currently how WebLN does it: https://www.webln.guide/building-lightning-apps/webln-reference/webln.getinfo - I think this makes sense because it can be per-connection (not per-wallet-service as per the NIP-47 info event)

I will add a suggestion there.

[benthecarman]

There is the info event that you create when you create the NWC, but yeah adding to the get_info event makes sense

[rolznz]

We added a NWC connector to the Alby extension and it will be shipped soon. It uses the get_info event which unfortunately fails with Mutiny.

If you make any progress here please let us know. I think it's cool to use your Mutiny wallet and pay on desktop using Alby with WebLN 🚀

[TonyGiorgio]

We added a NWC connector to the Alby extension and it will be shipped soon. It uses the get_info event which unfortunately fails with Mutiny.

If you make any progress here please let us know. I think it's cool to use your Mutiny wallet and pay on desktop using Alby with WebLN 🚀

What is it that you need out of the get_info call? So far we haven't needed to support it so I'm curious what is needed out of it besides firing off an invoice.

[rolznz]

@TonyGiorgio if the get_info call was supported and returned a list of methods we could then know what Mutiny does and does not support (get_balance for example which Ben mentioned is not possible right now).

Currently we use get_info to test the connection. However, if the call times out we can fallback to just assuming pay_invoice is the only supported method. Unfortunately that means we cannot actually test the NWC URL is valid or not.

Adding extra support to Mutiny for the extension methods is not necessary, it'll just improve the UX in the Alby extension (and Bitcoin Connect).

[benthecarman]

We definitely want to support most of the functions. I don't think get_info for that purpose will work well with mutiny however. It can only send responses when the wallet is open so it won't be able to register what it supports. The info event is really meant to signal what the connection supports

[rolznz]

@benthecarman I think it's ok to require the user to have their wallet open while using the Alby extension. We can add instructions there.

Currently the WebLN spec assumes the web application connected to a wallet can get a response directly from that wallet (e.g. within 60 seconds).

[benthecarman]

Okay if that is fine with you guys I guess that's okay.

[rolznz]

By the way, was MutinyWallet/blastr#31 deployed? I still didn't seem to get a response event when I tested paying again today.

[benthecarman]

It is. I tested with the filter you gave me and was able to get a response

[rolznz]

@benthecarman thanks! it seems to be working now. The first payment I did failed with this error but then the next two succeeded. Can you check the error code?

{
  error: 'Failed to pay invoice: Payment timed out.',
  code: 'INSUFFICIENT_BALANCE'
}

[benthecarman]

It says payment timed out. That normally means the payment will fail. We send this if the payment hasn't completed after 30 seconds.

The insufficient balance code isn't the best but not sure of a better error code that'd fit.

[rolznz]

@benthecarman would it make sense to add one in the extensions PR? to me it just feels confusing because I had sufficient balance.

[rolznz]

Currently doing concurrent requests cause all the requests to time out (e.g. if there is an outstanding get_balance or get_info request, I never get a response for send_payment even though the payment is sent). Are there any restrictions on your side we need to be aware of?

[benthecarman]

Looks like PAYMENT_FAILED was defined just not in rust-nostr will add it to my PR.

[benthecarman]

Are there any restrictions on your side we need to be aware of?

Sounds like a bug on our side, I think I might know the cause

[benthecarman]

Currently doing concurrent requests cause all the requests to time out (e.g. if there is an outstanding get_balance or get_info request, I never get a response for send_payment even though the payment is sent). Are there any restrictions on your side we need to be aware of?

Added what I think is a fix in #905. We weren't responding to events with different commands instead of sending NOT_IMPLEMENTED so we would have to continually reprocess the events. If you left the wallet open for long enough it should eventually respond I believe unless we are running into limits on the relay side.

[rolznz]

@benthecarman thanks for looking into it. We currently set a timeout of 60 seconds so we also time out there, but I think more than 60 seconds is too long for a user to wait in our case. I will try again when the fix is merged