Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft NIP for filter by time seen #79

Closed
wants to merge 3 commits into from

Conversation

mikedilger
Copy link
Contributor

This relates to issue #78

@cameri I can add you as author too if you want

@fiatjaf
Copy link
Member

fiatjaf commented Nov 27, 2022

Will a client that uses this feature still work if the relay doesn't support it?

@fiatjaf
Copy link
Member

fiatjaf commented Nov 27, 2022

Even though this looks like a harmless change I think it is a slippery slope if we add new querying capabilities for every new app that decides it needs new querying primitives to provide a better UX. I think the current filters are already too big and confusing and bloated, adding a new one dislikes me very much -- although I understand the need for it and it looks like a good thing.

I've raised a point that wasn't addressed at #78 (comment), to which I want to add that Nostr cannot be expected to offer the same guarantees a centralized system can, so I think clients should embrace the looseness of everything and be prepared to deal with faulty relays, relays that go offline, relays that do not have all events you expect them to have, relays that delete events and relays that do not know how to count or order things. If clients are programmed to only work with pristine relays that work wonderfully that will seriously degrade the decentralization of the protocol and make it so in the end everybody will only be using the very good and fast and perfect Google relay.

I am not saying that this change will ruin Nostr or that this change will necessarily open the door for more changes that will ruin Nostr. I cannot even at this time think of new querying primitives that anyone would want to add (but I hadn't thought of this one either, so I am sure people will be creating and come up with new ideas very soon). All I am saying is that we should be more mindful of trading the simplicity and openness of the protocol for better UX.

@fiatjaf
Copy link
Member

fiatjaf commented Nov 27, 2022

Another small problem I see here: when implementing my JS and my Go libraries I did it such that the client library would automatically run events received from relays through the same filters it sent, so it would protect the application from faulty relays sending events invalid for the specified filter. With these clauses that same check can't be made. Again, it's a small thing, but just noting.

@fiatjaf
Copy link
Member

fiatjaf commented Nov 27, 2022

Here is one suggestion that would fix all my concerns: instead of adding two new filters, seen_since and seen_until, this NIP could instead be a recommendation that relays and clients use the since and until differently, in a way that addresses what this is trying to solve but it's still backwards-compatible.

For relays implementing this NIP (I refer here to my suggestion of NIP), a filter since=2022-01-01 would mean "return all events received after 2022-01-01 OR created after 2022-01-01". Since most events won't be received before they are created (unless someone is frontdating events) this will be mostly the same as seen_since.

On the client side, clients that choose to implement this NIP (or activate it on a query-by-query basis) will not filter out an event that has a created_at=2021 even though the filter specified since=2022 because they understand that that event was received by the relay after 2022.

For clients that do not implement this NIP, they will just ignore the 2021 event as they should normally if they're protecting themselves against faulty relays.

For frontdated event (e.g. I create an event today with created_at=2025) either clients or relays should already have something in place to either make that a "scheduled post" that is to be shown later or just exclude that since it is too odd. Nothing changes.

@eskema
Copy link
Collaborator

eskema commented Nov 27, 2022

I'd prefer this as well, if relays simply also forward events received after the since that also match the rest of the filter, then it's perfect and the new filters are not needed.

@eskema
Copy link
Collaborator

eskema commented Nov 27, 2022

Here is one suggestion that would fix all my concerns: instead of adding two new filters, seen_since and seen_until, this NIP could instead be a recommendation that relays and clients use the since and until differently, in a way that addresses what this is trying to solve but it's still backwards-compatible.

For relays implementing this NIP (I refer here to my suggestion of NIP), a filter since=2022-01-01 would mean "return all events received after 2022-01-01 OR created after 2022-01-01". Since most events won't be received before they are created (unless someone is frontdating events) this will be mostly the same as seen_since.

On the client side, clients that choose to implement this NIP (or activate it on a query-by-query basis) will not filter out an event that has a created_at=2021 even though the filter specified since=2022 because they understand that that event was received by the relay after 2022.

For clients that do not implement this NIP, they will just ignore the 2021 event as they should normally if they're protecting themselves against faulty relays.

For frontdated event (e.g. I create an event today with created_at=2025) either clients or relays should already have something in place to either make that a "scheduled post" that is to be shown later or just exclude that since it is too odd. Nothing changes.

after thinking about a little more, I think this will lead to worst experience, as you're no longer receiving events only for that timerange, forever from that relay. so I think this should be explicit, so the behaviour of current queries doesn't change.

@cameri
Copy link
Member

cameri commented Nov 27, 2022

Another small problem I see here: when implementing my JS and my Go libraries I did it such that the client library would automatically run events received from relays through the same filters it sent, so it would protect the application from faulty relays sending events invalid for the specified filter. With these clauses that same check can't be made. Again, it's a small thing, but just noting.

nostr-ts-relay works the same way but it's workable.

To me seen_since and seen_until seem to be the most explicit and backward compatible way of doing this.

That said I do agree that clients should not expect consistency and instead expect events showing up out of order, duplicated, missing references, significantly out of date, etc.

@mikedilger
Copy link
Contributor Author

Will a client that uses this feature still work if the relay doesn't support it?

Depends what the relay does. If relays ignore fields they don't understand, then the client might be querying far too many events that it didn't want. If relays error when they see fields they don't understand, then the client can fallback to something else (which I will elaborate on in my next reply to your next comment).

@mikedilger
Copy link
Contributor Author

I think clients should embrace the looseness of everything and be prepared to deal with faulty relays

You are right, and I've been thinking about that. If relays censors (or drops) posts, how would you know about it? You just won't see the post. Asking the more refined question does not fix this bigger issue. I think the right solution to this is to chain posts together by including the ID of your previous post. Then you can know that you are missing posts (according to the author of them), suspect the relay of censoring or at least being faulty, and tell the author to maybe find a different relay that works. Could be in an 'e' tag.

But maybe that is not simple enough for you.

@fiatjaf
Copy link
Member

fiatjaf commented Nov 27, 2022

Chaining posts is fine with me. I think we could standardize it, probably with a tag named parent or something, such that it doesn't get confused with a reply.

By looking at a parent tag a client should be able to find out if there were backdated events and ask for those specifically from a relay, and also detect if relays are dropping or censoring events and so on.

I personally think this approach comes with other UX problems (for example, a user cannot easily use multiple clients for multiple purposes without risking creating a fork in their own chain of events), but I can see myself using it for some things in the future and I wouldn't object to it being nipped at all.

@fiatjaf
Copy link
Member

fiatjaf commented Nov 27, 2022

I still maintain that my suggestion at #79 (comment) is a good and simple and backwards-compatible approach for the problem at hand. I would like to hear your opinions on it, @cameri and @mikedilger.

@eskema I've addressed your point already above:

For clients that do not implement this NIP, they will just ignore the 2021 event as they should normally if they're protecting themselves against faulty relays.

@eskema
Copy link
Collaborator

eskema commented Nov 28, 2022

For clients that do not implement this NIP, they will just ignore the 2021 event as they should normally if they're protecting themselves against faulty relays.

but the thing is, I don't know if I want to receive events outside of the range I asked, and that's what will happen if relays simply start to send backdated events I didn't ask for, just because they received them after... it would be a very different behaviour than right now.. for this to happen, I'd prefer to explicitly ask for it.

@mikedilger
Copy link
Contributor Author

@fiatjaf I think your suggestion is good, I'm fine with it.

After a bit of thought, I think I have to side with @fiatjaf in that these should probably be implicit. The reason is that if it is made explicit, and a client asks a relay which does not support these fields to filter on these fields, that relay is probably going to think the filter is empty and dump everything on them. Other than that concern, I would have preferred it to be explicit. If it was clear that relays receiving filter fields they don't understand should reject the request, then I would vote for explicit fields, but I don't think that is clear anywhere (unless I missed it). Implicit feels more complex and surprising that explicit so I don't feel great about it.

The post-chaining that I mentioned here would be a separate future nip.

@fiatjaf
Copy link
Member

fiatjaf commented Nov 28, 2022

My thought is that it would be bad if the relay rejected it too. Because the client would then not work since it was expecting the seen_since filter to work.

Or the client would limit itself to only talking to relays that support seen_since, which either

  1. turns the feature from an optional addition to a mandatory requirement for all relays, increases the complexity of the protocol (albeit slightly, and maybe it's worth it?) and thus indirectly centralizes the network; or
  2. makes it so some relays become better than others and start to dominate over the others, which very directly centralizes the network.

@fiatjaf
Copy link
Member

fiatjaf commented Nov 28, 2022

it would be a very different behaviour than right now

@eskema my point was that right now you can't really expect relays to send you pristine data, you have to filter the data locally too (otherwise if you ask for events from X and the relay send events from Y you will show those?).

So if you are already filtering that the behavior doesn't change. You'll get a very small number of backdated events and automatically ignore those.

@cameri
Copy link
Member

cameri commented Nov 29, 2022

I still maintain that my suggestion at #79 (comment) is a good and simple and backwards-compatible approach for the problem at hand. I would like to hear your opinions on it, @cameri and @mikedilger.

I am not happy about the surprise factor of returning events whose created_at date does not match the since and until filters because there's no way for a client to know why they are getting events they didn't ask for.
I'm not 100% happy with explicit filters either since the problem to be solved is "how to get all new events since the last time the client disconnected from the relay" not how do I get events seen between dates or after given date.

I think we can do much better.

If we agree that both clients and relay operators are incentivized to reduce the bandwidth used, most clients will eventually use this since it also guarantees they won't lose events.

I propose instead subscription hints. These hints are part of the subscription ID. It works similar to the If-Modified-Since of the HTTP protocol. Ref: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Modified-Since

Suppose the client knows it disconnected from the relay at 1669693970.

To resume their subscriptions, the client hints to the relay it is only interested in receiving events that have been seen AFTER 1669693970.

["REQ", "my-notifications if-first-seen:1669693970", { "#p": ["my pubkey"] }, { "#e": ["watched event"] }]
["REQ", "follows if-first-seen:1669693970", { "authors": ["Alice", "Bob"] }]

Supporting relays will only send the events that have been seen AFTER the given timestamp.
Non-supporting relays will ignore the hint, thus sending all of the events that match the filter.

A client wishing to use this would store the timestamp of the last received event from each relay and provide it the next time it subscribes. The client does not need to update the timestamp unless an event is received, EOSE can be ignored for this purpose.

Notice that the hint affects all filters equally so there's no need to repeat it.

@mikedilger
Copy link
Contributor Author

@cameri while that seems to solve the same problem as fiatjaf's solution and is more explicit (and more complex), it also fails to solve another problem as does fiatjaf's solution which is: The client has no way of knowing if it is talking to a relay that supports this or not, so it doesn't know what set of events it received. All that the relays reply with are events.

So I'm going to have to just keep asking relays the same questions over and over again because I'll never know if a well behaved relay picked up an old event recently and I had no way to know about it or ask for it. I hope relays are happy with all that traffic.

I'm beginning to think that the desire to make nostr simple has made it too simple to improve. The lack of version numbers or reserved fields was always kind of conspicuous to me.

Oh well, it was fun to believe in something for a few weeks. 🙁

@mikedilger
Copy link
Contributor Author

mikedilger commented Nov 29, 2022

Maybe this can be done with a new version negotiation message. Client sends ["VERSION", 2] relay responds with it's version... except when the relay doesn't respond because it doesn't know about versioning. Or is it just slow? Oops. Another thing we can't add.

(10 minutes pass) Maybe the relay can send a NOTICE to the client right when the client first connects in a predetermined format with protocol version number. While the client hasn't gotten one of these, it continues to presume the relay is protocol version 1. Clients of version 1 will just display the version like an error message as suggested in NIP-01.

@cameri
Copy link
Member

cameri commented Nov 29, 2022

@cameri while that seems to solve the same problem as fiatjaf's solution and is more explicit (and more complex), it also fails to solve another problem as does fiatjaf's solution which is: The client has no way of knowing if it is talking to a relay that supports this or not, so it doesn't know what set of events it received. All that the relays reply with are events.

So I'm going to have to just keep asking relays the same questions over and over again because I'll never know if a well behaved relay picked up an old event recently and I had no way to know about it or ask for it. I hope relays are happy with all that traffic.

I'm beginning to think that the desire to make nostr simple has made it too simple to improve. The lack of version numbers or reserved fields was always kind of conspicuous to me.

Oh well, it was fun to believe in something for a few weeks. 🙁

Have you read this?
https://github.com/nostr-protocol/nips/blob/master/11.md

@fiatjaf
Copy link
Member

fiatjaf commented Nov 29, 2022

The lack of version numbers or reserved fields was always kind of conspicuous to me.

This was a conscious decision that I hope never has to change. I believe version numbers and other cheap ways to introduce backwards-incompatible changes are pernicious to any open protocol.

@mikedilger
Copy link
Contributor Author

mikedilger commented Nov 29, 2022

Then you are painted into a corner.

This is where I exit. Enjoy your adventures.

EDIT: I'm wrong. I just got very frustrated. And I read your comment just after I had test implemented Sec-WebSocket-Protocol for last-resort versioning, which then obviously I didn't feel like sharing after your comment.

I have to think long and hard about how nostr solves problems by going out of protocol (e.g. NIP-11) as there are no in-protocol mechanisms. Maybe that's not wrong, maybe I'm wrong. I haven't thought about it enough.

@mikedilger mikedilger closed this Nov 29, 2022
@fiatjaf
Copy link
Member

fiatjaf commented Nov 29, 2022

This was a sad moment.

@cameri
Copy link
Member

cameri commented Nov 29, 2022

This was a sad moment.

Only if we let that happen. Let's continue the conversation and end in a happy note.

I think it's in our best interest to continue to find a way to reduce the bandwidth used by clients and relays.

@fiatjaf @AlphaAma what do you think of my proposal?

@eskema
Copy link
Collaborator

eskema commented Nov 29, 2022

that person is not me, it's mostly @eskema everywhere but telegram :P
I think your proposal is interesting, but wouldn't that require relays to parse subscription_ids ? not sure how the implementations work now and what impact that would make, other than that it would solve this and may open the door for more "smart" queries

@Semisol
Copy link
Collaborator

Semisol commented Nov 30, 2022

we may as well put subscription hints in filters, relays should ignore anything they do not know about.
also, some way to get the nip-11 info within the websocket would be great.

@fiatjaf
Copy link
Member

fiatjaf commented Nov 30, 2022

@cameri I like your proposal, but I like mine more.

I like my first proposal even more though.

@cameri
Copy link
Member

cameri commented Nov 30, 2022

@cameri I like your proposal, but I like mine more.

I like my first proposal even more though.

I like your first proposal too. I also think we could do something about the inefficiency of sending events that the client already has with one simple trick 😁

@fiatjaf
Copy link
Member

fiatjaf commented Dec 1, 2022

Oh, if you frame it like that I want to fix that too, specially if it can work across multiple relays.

In the context of a single relay the since filter solves that already -- albeit it neglects the rare occurrence of backdated events, which my first proposal suggests to ignore.

But across multiple relays I don't know, maybe we can get fancy and use this: https://github.com/sipa/minisketch
But I can't even read the README.

@fiatjaf
Copy link
Member

fiatjaf commented Dec 1, 2022

@mikedilger even if you decide Nostr is not good enough, please don't hold bad feelings about us here, we're trying to fix the entire world and that may be a sensitive topic sometimes.

@mikedilger
Copy link
Contributor Author

@fiatjaf I'm embarrassed at my behavior. I've been laying low, not sleeping well, feeling sick to my stomach.

I'm continuing to build Gossip, and also probably fork nostr-rs-relay so I can try new things on both ends. But I'm going to stay away from NIPs work. I'd rather just write code.

I'm going to build new features without NIP-ping them right away. They should all be backwards compatible and I won't pollute on any event.type numbers or single-letter tag names. If I feel like I need one of those, I'll file an issue. They can be NIP-ped later after proving themselves in the wild if they work out. And if later on after other people look at them they suggest it should have been done differently and I concur, I'll just change my code (which is way easier for me than debating things ahead of time).

Sorry I was an ass.

@cameri
Copy link
Member

cameri commented Dec 1, 2022

@fiatjaf I'm embarrassed at my behavior. I've been laying low, not sleeping well, feeling sick to my stomach.

I'm continuing to build Gossip, and also probably fork nostr-rs-relay so I can try new things on both ends. But I'm going to stay away from NIPs work. I'd rather just write code.

I'm going to build new features without NIP-ping them right away. They should all be backwards compatible and I won't pollute on any event.type numbers or single-letter tag names. If I feel like I need one of those, I'll file an issue. They can be NIP-ped later after proving themselves in the wild if they work out. And if later on after other people look at them they suggest it should have been done differently and I concur, I'll just change my code (which is way easier for me than debating things ahead of time).

Sorry I was an ass.

@mikedilger I knew you'd come around <3 'cause I know you believe in this project more than you'd like to admit :D Don't beat yourself up about what happened, you've acknowledged it and apologized and now what happened is behind us and we can focus on what we do best: building.
Personally I am seeing a psychotherapist because now I understand that being well also includes my mental health. Stress, anxiety, irritability, self-doubt have plagued me for too long, which are the precursors to my own outbursts and rough patches.
To be honest I'm really excited to see what you will be doing next and I can't wait.

Oh, if you frame it like that I want to fix that too, specially if it can work across multiple relays.

In the context of a single relay the since filter solves that already -- albeit it neglects the rare occurrence of backdated events, which my first proposal suggests to ignore.

But across multiple relays I don't know, maybe we can get fancy and use this: https://github.com/sipa/minisketch But I can't even read the README.

I'm gonna go down that rabbit hole and report back if I find something we can use.

@mikedilger
Copy link
Contributor Author

@cameri If the therapist works for you keep going. I never felt therapy did me any good. Generally time alone to think and plenty of sleep cures my ills. I'm not good with conflict. But I figured out a way to avoid it and also make peace with my issues with nostr at the same time: https://github.com/mikedilger/nostr-odyssey

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants