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

feat: unsubscribe in case invalid push requests are received from server #1124

Closed
chaitanyaprem opened this issue Jun 12, 2024 · 3 comments · Fixed by #1160
Closed

feat: unsubscribe in case invalid push requests are received from server #1124

chaitanyaprem opened this issue Jun 12, 2024 · 3 comments · Fixed by #1160
Assignees

Comments

@chaitanyaprem
Copy link
Collaborator

chaitanyaprem commented Jun 12, 2024

Problem

There are few scenarios noticed during dogfooding that Filter-Push is received for subscriptions LightNode no longer has.

One scenario where this could happen is

  1. Filter Subscribe for contentTopics C1, C2 and C3 to node1.
  2. Internet is disconnected and reconnected again.
  3. Now C1 and C2 are subscribed to node2 whereas C3 is still subscribed to node1
  4. Now node1 thinks we are subscribed to C1 and c2 still as we keep sending Filter-Ping.

Suggested solution

When we receive an invalid push request, send a unsubscribe request to that peer for that specific contentTopic.

Alternatives considered

When we regain connectivity, send unsubscribeAll to all peers with which we had filter subscriptions before connectivityLoss.

Acceptance criteria

Have a test to validate this scenario.

@chaitanyaprem
Copy link
Collaborator Author

cc @waku-org/js-waku-developers @danisharora099.
This is something that will have to be implemented in Filter SDK.

Happy to hear if there are any other alternative solutions for this.

@chaitanyaprem chaitanyaprem self-assigned this Jun 12, 2024
@chaitanyaprem chaitanyaprem changed the title feat: Unsubscribe if Filter ping returns 404 feat: unsubscribe if filter ping returns 404 Jun 12, 2024
@chaitanyaprem chaitanyaprem changed the title feat: unsubscribe if filter ping returns 404 feat: unsubscribe in case invalid push requests are received from server Jun 12, 2024
@danisharora099
Copy link

danisharora099 commented Jun 28, 2024

Internet is disconnected and reconnected again.
Now C1 and C2 are subscribed to node2 whereas C3 is still subscribed to node1

I'm curious to understand how this happens where when the internet connectivity is lost, two content topics have subscriptions with the node, but subscription with one content topic is still left?
Another perspective: why does the connected and subscribed node lose subscriptions with certain content topics, while we regain connectivity?

@chaitanyaprem
Copy link
Collaborator Author

Internet is disconnected and reconnected again.
Now C1 and C2 are subscribed to node2 whereas C3 is still subscribed to node1

I'm curious to understand how this happens where when the internet connectivity is lost, two content topics have subscriptions with the node, but subscription with one content topic is still left? Another perspective: why does the connected and subscribed node lose subscriptions with certain content topics, while we regain connectivity?

Well, this could happen if after reconnection the same peer(which before disconnection was used for all 3) but was selected for only C3. So serviceNode/peer thinks we still have subscription for all 3 as we keep pinging the node.
Hence it keeps forwarding messages for C1 and C2 as well.

Only way i can think of to handle this is to send unsubscribe to those contentTopics for which we are receiving messages and we are not subscribed to for that peer. i.e in this case on receiving messages for C2 from the peer, we have to unsubscribe C2 towards that peer.

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

Successfully merging a pull request may close this issue.

2 participants