Fix Damus Mutiny NWC

[alltheseas]

Problem confirmed as originating from Damus by @benthecarman

nostrability/nostrability#13

[benthecarman]

Doing some testing and mutiny never gets the request, this is how we construct our filter. The timestamp is the timestamp of the last event we processed

https://github.com/MutinyWallet/mutiny-node/blob/master/mutiny-core/src/nostr/nwc.rs#L269

[jb55]

damus works with many NWC implementations, why do we have to make an exception for mutiny? I'm confused.

[benthecarman]

damus works with many NWC implementations, why do we have to make an exception for mutiny? I'm confused.

same with mutiny, just want to figure out why damus <> mutiny doesn't work when damus seems to work with everything else and mutiny seems to work with everything else

[alltheseas]

Received a NWC failure report. Tracking down which wallet

https://damus.io/nevent1qqsw8ckv64n0ax3a82a6crrrfn47856y7trlevs7e5jw2f4gz98ducqpzfmhxue69uhk7enxvd5xz6tw9ec82cspr4mhxue69uhkummnw3ez6ur4vgh8wetvd3hhyer9wghxuet5qydhwumn8ghj7mn0wd68ytfj9eax2cn9v3jk2tnrd3hh2eqpzamhxue69uhkv6tvw3jhytnwdaehgu3wwa5kueg7e93pa

[jb55]

On Mon, Feb 05, 2024 at 07:12:04AM -0800, alltheseas wrote: Received a NWC failure report. Tracking down which wallet https://damus.io/nevent1qqsw8ckv64n0ax3a82a6crrrfn47856y7trlevs7e5jw2f4gz98ducqpzfmhxue69uhk7enxvd5xz6tw9ec82cspr4mhxue69uhkummnw3ez6ur4vgh8wetvd3hhyer9wghxuet5qydhwumn8ghj7mn0wd68ytfj9eax2cn9v3jk2tnrd3hh2eqpzamhxue69uhkv6tvw3jhytnwdaehgu3wwa5kueg7e93pa

This is probably someone just trying to scan a lightning invoice thinking this will somehow attach the wallet, this seems to be a very common problem. maybe we need to hide this under advanced or something.

[alltheseas]

On Mon, Feb 05, 2024 at 07:12:04AM -0800, alltheseas wrote:

Received a NWC failure report. Tracking down which wallet

https://damus.io/nevent1qqsw8ckv64n0ax3a82a6crrrfn47856y7trlevs7e5jw2f4gz98ducqpzfmhxue69uhk7enxvd5xz6tw9ec82cspr4mhxue69uhkummnw3ez6ur4vgh8wetvd3hhyer9wghxuet5qydhwumn8ghj7mn0wd68ytfj9eax2cn9v3jk2tnrd3hh2eqpzamhxue69uhkv6tvw3jhytnwdaehgu3wwa5kueg7e93pa

This is probably someone just trying to scan a lightning invoice
thinking this will somehow attach the wallet, this seems to be a very
common problem.

maybe we need to hide this under advanced or something.

Confirmed this one was a newbie trying to set up a wallet that does not support NWC (strike). Maybe a list of compatible wallets fixes this #1976

[benthecarman]

i found the issue here

I can get it to work with the NWC URI:

nostr+walletconnect://975e6b43d84057d134e1e1a5ca5205157d0bc3b1e76f54ddfe8b1a7862db67fd?relay=wss://relay.mutinywallet.com&secret=507f0f3fb5e2397723d57466dc9ef99d7dd511dc24967d24ddaa5371151210a0

but mutiny/rust-nostr produces them like

nostr+walletconnect://975e6b43d84057d134e1e1a5ca5205157d0bc3b1e76f54ddfe8b1a7862db67fd?relay=wss%3A%2F%2Frelay.mutinywallet.com%2F&secret=507f0f3fb5e2397723d57466dc9ef99d7dd511dc24967d24ddaa5371151210a0

the difference being that the relay url is uri encoded

the spec does have the url as uri encoded so I would say this is a damus bug

https://github.com/nostr-protocol/nips/blob/master/47.md#example-connection-string

Maybe a good sanity check would be to have it try to connect to the relay when configuring NWC

[alltheseas]

https://damus.io/nevent1qqs89z4zq9xmwfjd2ffzatlr3r2f0ver8s9x7c7ha9xsfnqjtl5rxmqpz4mhxue69uhk2er9dchxummnw3ezumrpdejqzrthwden5te0dehhxtnvdakqz9rhwden5te0wfjkccte9ejxzmt4wvhxjmcpzpmhxue69uhkummnw3ezuamfdejs3z6t3y

Still doesn’t make sense to me because we use the standard ios url parser and i find it hard to believe it doesn’t handle url decoding

@jb55

[ynniv]

Hah, I thought I was just doing it wrong. This might fix the issue: #2037

[alltheseas]

Hah, I thought I was just doing it wrong. This might fix the issue: #2037

@ynniv what specifically was the issue, and what does your patch do?

[ynniv]

RelayPool caches connections to relays. Since you don't want w.xyz and w.xyz/ to create two connections, it normalizes URLs by trimming any trailing slashes. Unfortunately, it isn't always normalizing the URL when asking for an existing connection, so it ends up thinking that relay isn't available. My PR normalizes the requested URL so that doesn't happen.

This might actually be causing a lot of issues beyond NWC

[ynniv]

@benthecarman 's example isn't what it seems. The encoded parameter isn't confusing Damus, it's confusing us:

I can get it to work with the NWC URI:

[...]relay=wss://relay.mutinywallet.com&[...]

but mutiny/rust-nostr produces them like

[...]relay=wss%3A%2F%2Frelay.mutinywallet.com%2F&[...]

Notice the %2F (slash) before the ampersand.

[TonyGiorgio]

So is this fixed or a PR fixing it? I see the problem is tracked down but no reference PR and the issue is still opened.

[alltheseas]

#2037

PR is started. @jb55 asked for modifications to the proposed PR

[ericholguin]

Daniel's patch does what Will was asking for, as discussed in #2072

[alltheseas]

Thanks @ericholguin.

Did not notice the nwc test cases @danieldaquino ran on the relay slash issue. If this pr fixes nwc 🔥

[alltheseas]

@danieldaquino advised we might have to do more in depth testing to confirm that the relay slash issue fix, also fixes Mutiny NWC

[alltheseas]

Adding as bonus points to sprint 12, as this is related to the recently fixed relay slash / cannot remove relay issue, and affects user experience negatively @danieldaquino @jb55

[benthecarman]

Our latest release no longer has the trailing slash

[ericholguin]

Can confirm this is fixed in both existing apple store version and upcoming release

[jb55]

Really? People say it's not working

[ericholguin]

Looks like Damus needs to be restarted for the relay connection to go through. Not sure why.

[alltheseas]

@ericholguin advised that there is something bigger than Mutiny outstanding - Alby also affected

[alltheseas]

How might we communicate NWC relay is different than a note relay?