[NWC] Add an encryption tag to negotiate upgrading to NIP44.

[jklein24]

This is an alternative proposal to #1531 to upgrade to nip44 without introducing a versioning system.

[jklein24]

cc @vitorpamplona @kumulynja @rolznz @ekzyis

[kumulynja]

Thanks a lot @jklein24 ! This feels like the most simple and flexible way 🥇

[ekzyis]

Looks good to me! But I don't have high stakes in nostr development, I only want to use NIP-44 instead of NIP-04 for NWC on Stacker News.

Btw, thanks to @kumulynja for pushing for alternatives, I can see the value now in not using semantic versioning. I just didn't agree with the initial arguments against it ❤️

Something to consider with this "feature flag" approach is though that some combination of feature flags might be incompatible but I don't think this is going to be a significant problem and we can deal with this when we actually add more feature flags.

[jklein24]

Something to consider with this "feature flag" approach is though that some combination of feature flags might be incompatible but I don't think this is going to be a significant problem and we can deal with this when we actually add more feature flags.

Yeah this is one of the benefits of explicit versioning, but I agree we can cross that bridge if/when we get to it.

[jklein24]

Updated the UMA Auth NWC container in this PR

[vitorpamplona]

Can we make it more obvious that the encryption options are only NIP-44v2 and NIP-04 and that the NIP-04 is deprecated and only there for backward compatibility? Right now there are these notes for backward compatibility in each section which feel disjoint and verbose.

Maybe instead of the encryption documentation being explained throughout the text, a section at the bottom can make it more clear?

Encryption

The encryption tag can contain either nip44v2 or nip04. The absence of this tag implies that the wallet only supports nip04.

Encryption code	Use	Notes
nip44v2	NIP-44	Required
nip04	NIP-04	Deprecated and only here for backward compatibility
<not present>	NIP-04	Deprecated and only here for backward compatibility

[jklein24]

vitorpamplona

Good callout. Added this table in the Encryption section at the bottom.

[im-adithya]

Will this be used? If yes, how will this error be encrypted? 😄

I think it's better to not respond to such requests to reduce complexity...

[jklein24]

Good catch on the encryption point! I'd kind of lost the fact that errors also need to be encrypted. Thinking through this a bit, I'm not sure I love not responding to these events because that's not very debuggable on the client app side and will leave client apps hanging until they time out, so it's not a great ux either. We can't encrypt the error using the client's encryption type because by definition in this case the wallet service doesn't support it.

I think the best behavior here is actually to just encrypt the error content using the wallet service's default encryption scheme. Of course the client won't be able to decrypt it, but at least they'll get a decryption error while debugging, which will tip them in the right direction. There may even be some cases where it was actually just the client's encryption tag which was wrong, and so they will see the error message. For example:

1. Client forgets to populate the encryption tag in a request or sends something like nip44 or nip44v2 instead of nip44_v2
2. Wallet service sees this and doesn't understand, so it responds with a nip44 encrypted UNSUPPORTED_ENCRYPTION error
3. Client app can actually still decrypt this and see the right error type to nudge them to re-check their encryption tags.

Does that make sense to you? If so, I can add some clarification to this PR.

[rolznz]

On the other hand, building a lot of extra logic to try to catch all the cases where a developer doesn't implement the spec properly is a slippery slope and makes more complicated code to account for all these cases as Adithya mentioned. But it's a good point that developers who cannot see the wallet service logs will find it harder to debug. In Alby Hub the developer can see the logs, but this won't be the case for services where the developer is not the owner of the backend.

[jklein24]

After thinking about this more, I still feel pretty strongly that the debugging experience would be pretty painful if servers just ignore these messages entirely. As a client app developer, it's pretty likely that I didn't write the software for my wallet and that I'm not running it locally. It feels like a pretty easy foot-gun to write nip44 instead of nip44_v2 and spend the next several hours confused why your app doesn't work. IMO being defensive here is just good devX.

I do understand your point about not trying to catch every case where a developer doesn't implement the spec correctly, but in this case, even doing nothing is a choice in special handling. Even then you have already looked at the message, seen an invalid encryption scheme, and then decided to do nothing instead of returning an error. I don't think either case is really much extra handling tbh.