nostr-protocol / nips

Nostr Implementation Possibilities
2.39k stars 582 forks source link

Nip46 upgrade part2: remove nip05 and create_account, clarify nostrconnect, cleanup, add nip05 signer metadata #1553

Closed nostrband closed 1 week ago

nostrband commented 3 weeks ago

A continuation of https://github.com/nostr-protocol/nips/pull/1545, where we agreed on:

removing NIP-05, updating nostrconnect:// and moving create_account elsewhere

Also added Overview, shifted signing flow example below request/response format description, added nip05 signer metadata description to appendix, compressed nip89 use for signer discovery.

vitorpamplona commented 3 weeks ago

Way better

Can we clarify what the get_relays method does? Where are these relays coming from? the signer service? or the user's NIP-65 relays? Both? Or maybe a hardcoded relay set on the service?

nostrband commented 3 weeks ago

Can we clarify what the get_relays method does? Where are these relays coming from? the signer service? or the user's NIP-65 relays? Both? Or maybe a hardcoded relay set on the service?

I guess it mirrors get_relays of nip-07 where it's supposed to return relays similar to nip65. I'm not sure any signer supports it though. I don't like the idea of asking signer to fetch nip65 list and forward it, and don't see a point of specifying them manually by user in the signer, so maybe we could deprecate it?

nostrband commented 3 weeks ago

Inviting @mikedilger and @greenart7c3

vitorpamplona commented 3 weeks ago

so maybe we could deprecate it?

Yeah, delete everything that is not being used.

nostrband commented 3 weeks ago

Yeah, delete everything that is not being used.

How about ping? Anyone using it or at least have the rationale for it?

alexgleason commented 3 weeks ago

I think we shouldn't delete get_relays unless it's also deleted from NIP-07. The signers should be consistent with each other.

Therefore I think it should be a separate PR to delete it from both instead of including it in this PR.

I'm torn on it. It's really good for the client if we can know your relays as you are logging in. NIP-65 is a chicken and egg problem. Where do I get your NIP-65 relay list from? If the signer provides an initial set of relays, I can use that to look up your NIP-65 list. But does this actually ever work? Do people actually ever fill in these relays? To be clear it's not supposed to forward your NIP-65 relays, it's supposed to be entered manually by the user in either the extension or the bunker. It works the same way for both. There is clear value to it... in theory. In practice, I don't know.

greenart7c3 commented 3 weeks ago

Looks good to me. ping I never implemented and I don't remember anyone asking for it. The get_relays doesn't make sense to me, specially in this nip.

Why would a signer have my nip 65 relays? It would just make things even more complicated for signers since it would need to keep track of user relays and not only the relays from this nip If it's the relays from the connection them how would the client know if a user replaces relay1 with relay2

staab commented 3 weeks ago

get_relays should return, if anything, relays where the client can immediately find the user's 10002 (and maybe 0, 3, 10050). This could be an indexer, rather than the user's actual relay selections, which would make it easier for a signer to maintain. Also, it's germane to the signer's value prop, because a signer with good relay hints will result in a better sign-in experience. That's not to say that I use these anywhere. I agree with Alex, it's a good idea in theory. Maybe in practice, as the network partitions.

fiatjaf commented 3 weeks ago

get_relays should return, if anything, relays where the client can immediately find the user's 10002 (and maybe 0, 3, 10050).

This is how I came to think of get_relays too. I think it's safe to leave it there for now. Clients should probably call it and if it doesn't work fall back to hardcoded indexers like purplepag.es, while bunkers may hardcode the same indexers or, if there is some other complex setup involved, return something different.

Maybe in a future PR we could spell out these things.

nostrband commented 3 weeks ago

Brought get_relays back. I'm not convinced signer can magically find better relays than client itself, but let's discuss it elsewhere.

fiatjaf commented 2 weeks ago

I didn't inspect every line change carefully, but if this is just what it says then I think we should be good to merge, right?

mikedilger commented 2 weeks ago

Overall it looks good to me.