fiatjaf / eventstore

an abstraction layer over databases for nostr events
The Unlicense
29 stars 16 forks source link

NIP-40 support #22

Open mattn opened 3 months ago

mattn commented 3 months ago

Currently, eventstore does not support NIP-40 (events with expiration), so it is not too difficult to add code to check the expiration tag in the part of QueryEvents that send events into ch. However, COUNT will ignore the tag. @fiatjaf Do you have plan to support NIP-40 ?

fiatjaf commented 3 months ago

What would support look like? I always thought this had to be supported at the relay level, not here.

I once tried to implement it but it was too cumbersome, like every time you received an event with the expiration tag that would be added to some other table and then every now and we would check what events in that other table were expired and then call DeleteEvent() on the respective eventstore. It's doable though.

mattn commented 3 months ago

Deletion timers can be overloaded if run at high frequency, so it is necessary to first exclude expired events in QueryEvents. This is PoC code to remove expired events.

https://github.com/mattn/eventstore/commit/b32fd4a5f80bba7d5ee63ca9eec3ca99d3d7cac8

fiatjaf commented 3 months ago

I see, this is an elegant solution, but isn't this going to make queries slower for everybody always and everywhere just for supporting this extra thing that's not very important? And then the events are not even deleted so they still match over and over in future queries?

I think I prefer doing it at the relay level still.

fiatjaf commented 3 months ago

Also couldn't you do exactly this, but on the side that reads from the channel returned by QueryEvents()?

mattn commented 3 months ago

Also couldn't you do exactly this, but on the side that reads from the channel returned by QueryEvents()?

Right. It can add into relayer.

mattn commented 3 months ago
diff --git a/handlers.go b/handlers.go
index 7aff834..be230eb 100644
--- a/handlers.go
+++ b/handlers.go
@@ -8,6 +8,7 @@ import (
    "encoding/json"
    "fmt"
    "net/http"
+   "strconv"
    "time"

    "github.com/fasthttp/websocket"
@@ -244,8 +245,18 @@ func (s *Server) doReq(ctx context.Context, ws *WebSocket, request []json.RawMes
        if filter.Limit == 0 {
            filter.Limit = 9999999999
        }
+
+       now := nostr.Now()
        i := 0
+
+   loop_events:
        for event := range events {
+           for _, ex := range event.Tags.GetAll([]string{"expiration"}) {
+               v, err := strconv.ParseUint(ex.Value(), 10, 64)
+               if err != nil || nostr.Timestamp(v) <= now {
+                   continue loop_events
+               }
+           }
            ws.WriteJSON(nostr.EventEnvelope{SubscriptionID: &id, Event: *event})
            i++
            if i > filter.Limit {

However we have to add code to eventstore to delete expired events. expiration tag can not be used to match as filter.

fiatjaf commented 3 months ago

Maybe some options for configuring what extra tags to index (or not index) instead. I have some situations in which I want to skip indexing some tags and I'm sure people may want to index other stuff besides expiration.

But I guess this would have to be done differently in each package, right?

mattn commented 3 months ago

Do you mean we should add field like below?

server.SkipFunc = func(e *nostr.Event) bool {
    return e.CreatedAt != 0
}
mattn commented 3 months ago

FYI, I found a way to query expired events in postgresql.

select id from (select id id, to_number(jsonb_path_query(e.tags, '$?(@[0]=="expiration")[1]')::text, '99') b from event e) as c where c.b
 < extract(epoch from now());
mattn commented 3 months ago

https://github.com/fiatjaf/relayer/pull/117

mattn commented 3 months ago

Do you think that eventstore have to add new method Clean() error that delete events expired or should be deleted?

fiatjaf commented 3 months ago

I don't know. I think this is a more high-level feature. I wanted to keep eventstores more low-level. They don't even implement replacing events.

Maybe that would be a better fit for the RelayStore interface.

But eventstores could have dedicated internal methods for doing it -- and when they don't exist RelayWrapper would fallback to looping through all the events and deleting stuff. Does that work? I don't know.

I guess we need to create another interface for that, ExpiredCleaner or something?