publish: correctly report failed command statuses from nip-20 relays

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.
This commit is contained in:
alex 2022-12-26 17:20:56 +01:00 committed by fiatjaf
parent 5bfb398f4d
commit 435579dc75
2 changed files with 139 additions and 2 deletions

View File

@ -176,7 +176,7 @@ func (r *Relay) ConnectContext(ctx context.Context) error {
json.Unmarshal(jsonMessage[1], &eventId)
json.Unmarshal(jsonMessage[2], &ok)
if statusChan, ok := r.statusChans.Load(eventId); ok {
if statusChan, exist := r.statusChans.Load(eventId); exist {
if ok {
statusChan <- PublishStatusSucceeded
} else {
@ -190,7 +190,7 @@ func (r *Relay) ConnectContext(ctx context.Context) error {
return nil
}
func (r Relay) Publish(event Event) chan Status {
func (r *Relay) Publish(event Event) chan Status {
statusChan := make(chan Status, 4)
go func() {
@ -206,6 +206,9 @@ func (r Relay) Publish(event Event) chan Status {
}
statusChan <- PublishStatusSent
// TODO: there's no reason to sub if the relay supports nip-20 (command results).
// in fact, subscribing here with nip20-enabled relays makes it send PublishStatusSucceded
// twice: once here, and the other on "OK" command result.
sub := r.Subscribe(Filters{Filter{IDs: []string{event.ID}}})
for {
select {

View File

@ -1,7 +1,9 @@
package nostr
import (
"bytes"
"context"
"encoding/json"
"errors"
"io"
"net/http"
@ -13,6 +15,83 @@ import (
"golang.org/x/net/websocket"
)
func TestPublish(t *testing.T) {
// test note to be sent over websocket
priv, pub := makeKeyPair(t)
textNote := Event{
Kind: 1,
Content: "hello",
CreatedAt: time.Unix(1672068534, 0), // random fixed timestamp
Tags: Tags{[]string{"foo", "bar"}},
PubKey: pub,
}
if err := textNote.Sign(priv); err != nil {
t.Fatalf("textNote.Sign: %v", err)
}
// fake relay server
var mu sync.Mutex // guards published to satisfy go test -race
var published bool
ws := newWebsocketServer(func(conn *websocket.Conn) {
mu.Lock()
published = true
mu.Unlock()
// verify the client sent exactly the textNote
var raw []json.RawMessage
if err := websocket.JSON.Receive(conn, &raw); err != nil {
t.Errorf("websocket.JSON.Receive: %v", err)
}
event := parseEventMessage(t, raw)
if !bytes.Equal(event.Serialize(), textNote.Serialize()) {
t.Errorf("received event:\n%+v\nwant:\n%+v", event, textNote)
}
// send back an ok nip-20 command result
res := []any{"OK", textNote.ID, true, ""}
if err := websocket.JSON.Send(conn, res); err != nil {
t.Errorf("websocket.JSON.Send: %v", err)
}
})
defer ws.Close()
// connect a client and send the text note
rl := mustRelayConnect(ws.URL)
want := map[Status]bool{
PublishStatusSent: true,
PublishStatusSucceeded: true,
}
testPublishStatus(t, rl.Publish(textNote), want)
if !published {
t.Errorf("fake relay server saw no event")
}
}
func TestPublishBlocked(t *testing.T) {
// test note to be sent over websocket
textNote := Event{Kind: 1, Content: "hello"}
textNote.ID = textNote.GetID()
// fake relay server
ws := newWebsocketServer(func(conn *websocket.Conn) {
// discard received message; not interested
var raw []json.RawMessage
if err := websocket.JSON.Receive(conn, &raw); err != nil {
t.Errorf("websocket.JSON.Receive: %v", err)
}
// send back a not ok nip-20 command result
res := []any{"OK", textNote.ID, false, "blocked"}
websocket.JSON.Send(conn, res)
})
defer ws.Close()
// connect a client and send a text note
rl := mustRelayConnect(ws.URL)
want := map[Status]bool{
PublishStatusSent: true,
PublishStatusFailed: true,
}
testPublishStatus(t, rl.Publish(textNote), want)
}
func TestConnectContext(t *testing.T) {
// fake relay server
var mu sync.Mutex // guards connected to satisfy go test -race
@ -70,3 +149,58 @@ func newWebsocketServer(handler func(*websocket.Conn)) *httptest.Server {
var anyOriginHandshake = func(conf *websocket.Config, r *http.Request) error {
return nil
}
func makeKeyPair(t *testing.T) (priv, pub string) {
t.Helper()
privkey := GeneratePrivateKey()
pubkey, err := GetPublicKey(privkey)
if err != nil {
t.Fatalf("GetPublicKey(%q): %v", privkey, err)
}
return privkey, pubkey
}
func mustRelayConnect(url string) *Relay {
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
rl, err := RelayConnectContext(ctx, url)
if err != nil {
panic(err.Error())
}
return rl
}
func parseEventMessage(t *testing.T, raw []json.RawMessage) Event {
t.Helper()
if len(raw) < 2 {
t.Fatalf("len(raw) = %d; want at least 2", len(raw))
}
var typ string
json.Unmarshal(raw[0], &typ)
if typ != "EVENT" {
t.Errorf("typ = %q; want EVENT", typ)
}
var event Event
if err := json.Unmarshal(raw[1], &event); err != nil {
t.Errorf("json.Unmarshal: %v", err)
}
return event
}
func testPublishStatus(t *testing.T, ch <-chan Status, want map[Status]bool) {
for stat := range ch {
if !want[stat] {
t.Errorf("client reported %q status", stat)
}
delete(want, stat)
// stop early to speed up tests
if len(want) == 0 {
break
}
}
for stat, missed := range want {
if missed {
t.Errorf("client didn't report %q", stat)
}
}
}