A race-condition exists between setting of the (unprotected) status and the callback which sets the status upon receiving an OK.
The message is sent which can receive an OK in separate goroutine (setting status) prior to the status being set to 'sent.'
The OK can be received prior to the status being set.
This fix also sets the status to PublishStatusFailed if the WriteJSON call fails.
- if `receivedEvent.ID` not match `event.ID`, may trigger an error `fatal error: sync: unlock of unlocked mutex`.
- if context cancled, it does not needs mutex.
the client never reported a failed status, for example when a relay
responds with a:
["OK", event-id, false, "blocked"]
this was due to Relay.statusChans map missing a channel for an event
when a nip-20 status command is reported by a relay. the reason this
happened is due to the method's receiver, which was copied instead of
referenced by a pointer:
func (r Relay) Publish(event Event) chan Status {
// uses a **copy** of statusChans here:
r.statusChans.Store(event.ID, statusChan)
...
}
the bugfix is a one character change:
func (r *Relay) Publish(event Event) chan Status
but while there, spotted another bug where an ok variable was shadowed
and the status chan would've reported incorrect value:
// ok, which is a command status from earlier, is shadowed here:
if statusChan, ok := r.statusChans.Load(eventId); ok {
statusChan <- ...
}
as a side effect, Relay.Publish now reports PublishStatusSucceeded
twice for relays which implement nip-20: once from an OK command status
and the other one from its adhoc subscription to observe whether the
event has been seen. added a todo to address it in the future.
A websocket dial may hand for an unreasonably long time and a nostr client
has no control over this when trying to connect to a relay.
Go started introducing context in networking since 2014 -
see https://go.dev/blog/context - and by now many net functions have
XxxContext equivalent, such as DialContext.
Example usage of the change introduced by this commit:
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()
r, err := nostr.RelayConnectContext(ctx, "ws://relay.example.org")
The code above makes RelayConnectContext last at most 3 sec, returning
an error if a connection cannot be established in the given time.
This helps whenever a tight control over connection latency is required,
such as distributed systems.
The change is backwards-compatible except the case where RelayPool.Add
sent an error over the returned channel without actually closing said
channel. I believe it was a bug.
fiatjaf
Marc Tarnutzer
Patrick Bennett
Jasper Rädisch
Wayback Archiver
Dylan Cant
barkyq
alex
mlctrez