fix fake relay detection for push.services.mozilla.com#906
Conversation
| && result.read.message.includes('NIP-01 compatible'); | ||
| const isFakeRelay = wasOnline && readCheckEnabled && readFailedProtocol; | ||
| const paymentRequired = result.info?.data?.limitation?.payment_required === true; | ||
| const isFakeRelay = wasOnline && readCheckEnabled && result.read?.data !== true && !paymentRequired; |
There was a problem hiding this comment.
I don't understand this conditional. !paymentRequired has nothing to do with detecting a NIP-01 non-compliant websocket connection. NIP-11 fields really cannot be used for this, because NIP-11 is optional, and not all NIP-11s include payment required, etiher. There would be too many false positives.
There was a problem hiding this comment.
The intent was "either you should be available, or you should have a good reason for not being available (eg payment required)". If you can't think of any good alternatives I'll turn this into an issue.
There was a problem hiding this comment.
This was probably a regression, @nostrwatch/nocap was patched almost a year ago to harden this case by using the read check to catch websockets that did not respond with a NIP-01 message, and then I am pretty sure the line in relaymon was a safeguard (the one you found). So my guess is that something in nocap is short-citcuiting or otherwise, the failure isn't being caught.
There was a problem hiding this comment.
Okay yeah this is actually a nocap thing. I got my agent to adjust it so if the server closes the connection it hard fails as not being NIP-01 compatible, does it look right to you?
There was a problem hiding this comment.
It's quite a bit more subtle than your implementation. As written, it assumes onError which is a websocket level error, which this case wouldn't throw. And an unexpected close doesn't mean that the relay isn't NIP-01 compatible, there are several such cases where this could happen. Also, wss://echo.websocket.org wouldn't throw in either of these scenarios.
On read or write the relay should return some kind of NIP-01 compatible message respective of the request. Relays will return different messages for ["REQ", ... ] and ["EVENT", ...] . Generic websockets won't do this, a websocket like wss://echo.websocket.org will return the same message that was sent back and other generic websocket endpoints will return any number of things. They might even return an array/tuple but not a NIP-01 compatible tuple, so the checks need to be very strictly typed to every possible valid relay response, even those outside of NIP-01 (other nips MAY define custom return formats.... NIP-42 for example returns ["AUTH", ...])
Tests should probably be written as well to validate the behavior in the various test scenarios against mocked relays.
If you continue down this path you will see why these edge cases are not something i have focused on extensively, it may seem trivial at first glance, but the problem-space is subtle and many footguns lurk in the shadows; particularly at scale.
push.services.mozilla.com may not have a specific message but it's still not actually a nostr relay