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

test(gossipsub): Part 1 Test cases covering subscribe and unsubscribe Events #1201

Draft
wants to merge 26 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
871efab
added test wrt subscribe and unsubscribe
shashankshampi Sep 26, 2024
dc7f8d4
added tests/pubsub/testgossipinternal2 file
shashankshampi Sep 26, 2024
5790b6f
linters
shashankshampi Sep 26, 2024
eced002
Merge branch 'master' into block6Test
shashankshampi Sep 26, 2024
1c2e221
refactor and suite name refactor
shashankshampi Sep 27, 2024
2923a2d
test(gossipsub): added test for membership for join and leave topic
shashankshampi Sep 30, 2024
fda0d2b
test(gossipsub): import optimization
shashankshampi Oct 1, 2024
eb2f6bf
test(gossipsub): Test cases covering subscribe and unsubscribe Events
shashankshampi Sep 26, 2024
9c0966e
Merge branch 'master' into block6Test
shashankshampi Oct 2, 2024
27c2850
Merge branch 'master' into block6Test2
shashankshampi Oct 2, 2024
25df50d
updtaed as per review comment
shashankshampi Oct 3, 2024
f0c8c5b
review comments to remove unwanted comments
shashankshampi Oct 3, 2024
46b7125
removed internal subscribe and unsubscribe test
shashankshampi Oct 3, 2024
19d3ead
removed unwanted check
shashankshampi Oct 3, 2024
f42a763
added assertion for handle SUBSCRIBE to the topic
shashankshampi Oct 3, 2024
e10e4d0
rebase with block6Test
shashankshampi Oct 5, 2024
89473da
TC fix
shashankshampi Oct 5, 2024
806592d
PR update
shashankshampi Oct 7, 2024
2d38e8a
fix in test logic for multiple peers join and leave topic simultaneously
shashankshampi Oct 8, 2024
d594c04
addressed wornderful review comments
shashankshampi Oct 10, 2024
a12b56c
Merge branch 'block6Test' into block6Test2
shashankshampi Oct 10, 2024
cb7ccae
updated as per review comment
shashankshampi Oct 10, 2024
ff6b274
review comment fix
shashankshampi Oct 10, 2024
567a456
Merge branch 'block6Test' into block6Test2
shashankshampi Oct 10, 2024
cc8e976
removed PubSub in sunscribe
shashankshampi Oct 10, 2024
e68685c
added part 2 PR in same
shashankshampi Oct 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ build/
*.exe
*.dll
.vscode/
.idea/
.DS_Store
tests/pubsub/testgossipsub
examples/*.md
Expand Down
40 changes: 0 additions & 40 deletions tests/pubsub/testgossipinternal.nim
Original file line number Diff line number Diff line change
Expand Up @@ -33,46 +33,6 @@ suite "GossipSub internal":
teardown:
checkTrackers()

asyncTest "subscribe/unsubscribeAll":
let gossipSub = TestGossipSub.init(newStandardSwitch())

proc handler(topic: string, data: seq[byte]): Future[void] {.gcsafe.} =
discard

let topic = "foobar"
gossipSub.mesh[topic] = initHashSet[PubSubPeer]()
gossipSub.topicParams[topic] = TopicParams.init()

var conns = newSeq[Connection]()
gossipSub.gossipsub[topic] = initHashSet[PubSubPeer]()
for i in 0 ..< 15:
let conn = TestBufferStream.new(noop)
conns &= conn
let peerId = randomPeerId()
conn.peerId = peerId
let peer = gossipSub.getPubSubPeer(peerId)
peer.sendConn = conn
gossipSub.gossipsub[topic].incl(peer)

# test via dynamic dispatch
gossipSub.PubSub.subscribe(topic, handler)

check:
gossipSub.topics.contains(topic)
gossipSub.gossipsub[topic].len() > 0
gossipSub.mesh[topic].len() > 0

# test via dynamic dispatch
gossipSub.PubSub.unsubscribeAll(topic)

check:
topic notin gossipSub.topics # not in local topics
topic notin gossipSub.mesh # not in mesh
topic in gossipSub.gossipsub # but still in gossipsub table (for fanning out)

await allFuturesThrowing(conns.mapIt(it.close()))
await gossipSub.switch.stop()

asyncTest "topic params":
let params = TopicParams.init()
params.validateParameters().tryGet()
Expand Down
176 changes: 176 additions & 0 deletions tests/pubsub/testgossipmembership.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
# Nim-LibP2P
# Copyright (c) 2023-2024 Status Research & Development GmbH
# Licensed under either of
# * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE))
# * MIT license ([LICENSE-MIT](LICENSE-MIT))
# at your option.
# This file may not be copied, modified, or distributed except according to
# those terms.

{.used.}

import std/[options, deques, sequtils, enumerate, algorithm, sets]
import stew/byteutils
import ../../libp2p/builders
import ../../libp2p/errors
import ../../libp2p/crypto/crypto
import ../../libp2p/stream/bufferstream
import ../../libp2p/protocols/pubsub/[pubsub, gossipsub, mcache, mcache, peertable]
import ../../libp2p/protocols/pubsub/rpc/[message, messages]
import ../../libp2p/switch
import ../../libp2p/muxers/muxer
import ../../libp2p/protocols/pubsub/rpc/protobuf
import utils
import chronos

import ../helpers

proc noop(data: seq[byte]) {.async: (raises: [CancelledError, LPStreamError]).} =
discard

const MsgIdSuccess = "msg id gen success"

suite "GossipSub Topic Membership Tests":
teardown:
checkTrackers()

# Addition of Designed Test cases for 6. Topic Membership Tests: https://www.notion.so/Gossipsub-651e02d4d7894bb2ac1e4edb55f3192d

# Generalized setup function to initialize one or more topics
proc setupGossipSub(
topics: seq[string], numPeers: int
): (TestGossipSub, seq[Connection]) =
let gossipSub = TestGossipSub.init(newStandardSwitch())
var conns = newSeq[Connection]()

for topic in topics:
gossipSub.mesh[topic] = initHashSet[PubSubPeer]()
gossipSub.topicParams[topic] = TopicParams.init()
gossipSub.gossipsub[topic] = initHashSet[PubSubPeer]()

for i in 0 ..< numPeers:
let conn = TestBufferStream.new(noop)
conns &= conn
let peerId = randomPeerId()
conn.peerId = peerId
let peer = gossipSub.getPubSubPeer(peerId)
peer.sendConn = conn
gossipSub.gossipsub[topic].incl(peer)

return (gossipSub, conns)

# Wrapper function to initialize a single topic by converting it into a seq
proc setupGossipSub(topic: string, numPeers: int): (TestGossipSub, seq[Connection]) =
setupGossipSub(@[topic], numPeers)

# Helper function to subscribe to topics
proc subscribeToTopics(gossipSub: TestGossipSub, topics: seq[string]) =
for topic in topics:
gossipSub.PubSub.subscribe(
AlejandroCabeza marked this conversation as resolved.
Show resolved Hide resolved
topic,
proc(topic: string, data: seq[byte]): Future[void] {.async.} =
discard
,
)

# Helper function to unsubscribe to topics
proc unsubscribeFromTopics(gossipSub: TestGossipSub, topics: seq[string]) =
for topic in topics:
gossipSub.PubSub.unsubscribeAll(topic)
AlejandroCabeza marked this conversation as resolved.
Show resolved Hide resolved

# Simulate the `SUBSCRIBE` to the topic and check proper handling in the mesh and gossipsub structures
asyncTest "handle SUBSCRIBE to the topic":
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is still not checking changes to the mesh.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mesh test is written separately in PR Part 2 as per designed Test case

  1. "handle JOIN topic and mesh is updated"
  2. "handle LEAVE topic and mesh is updated"

PR: https://github.com/vacp2p/nim-libp2p/pull/1205/files

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not sure this separation is necessary tbh.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merged Part 2 PR in this PR only

let topic = "test-topic"
let (gossipSub, conns) = setupGossipSub(topic, 5)

# Subscribe to the topic
subscribeToTopics(gossipSub, @[topic])

# Check if the topic is present in the list of subscribed topics
check gossipSub.topics.contains(topic)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing text also checks gossipSub.mesh[topic].len() > 0. Is there a place defining what the tests should ensure?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test case are designed here: 6. Topic Membership Tests (❌ To Do)
https://www.notion.so/Gossipsub-651e02d4d7894bb2ac1e4edb55f3192d

These are split tests which check individual logic rather than subscribe and unsubscribe together

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only test title is there and based on that we are adding checks to ensure that functionality. We can have details check test if required

# Check if the topic is added to gossipsub and the peers list is not empty
check gossipSub.gossipsub[topic].len() > 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we move this line before subscribeToTopics(gossipSub, @[topic]) it's already true, so we aren't testing anything here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what was exactly done?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its after subscription and I am testing subscribed value
check gossipSub.gossipsub[topic].len() == 5

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the numbers of peers is set to 5 so to ensure the length is set correctly we could change to
check gossipSub.gossipsub[topic].len() == 5

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


# Close all peer connections and verify that they are properly cleaned up
await allFuturesThrowing(conns.mapIt(it.close()))
diegomrsantos marked this conversation as resolved.
Show resolved Hide resolved

# Stop the gossipSub switch and wait for it to stop completely
await gossipSub.switch.stop()

# Simulate an UNSUBSCRIBE to the topic and check if the topic is removed from the relevant data structures but remains in gossipsub
asyncTest "handle UNSUBSCRIBE to the topic":
let topic = "test-topic"
let (gossipSub, conns) = setupGossipSub(topic, 5)

# Subscribe to the topic first
subscribeToTopics(gossipSub, @[topic])

# Now unsubscribe from the topic
unsubscribeFromTopics(gossipSub, @[topic])

# Verify the topic is removed from relevant structures
check topic notin gossipSub.topics
check topic notin gossipSub.mesh
check topic in gossipSub.gossipsub

# The topic should remain in gossipsub (for fanout)

await allFuturesThrowing(conns.mapIt(it.close()))
await gossipSub.switch.stop()

# Test subscribing and unsubscribing multiple topics
asyncTest "handle SUBSCRIBE and UNSUBSCRIBE multiple topics":
let topics = ["topic1", "topic2", "topic3"].toSeq()
let (gossipSub, conns) = setupGossipSub(topics, 5)

# Subscribe to multiple topics
subscribeToTopics(gossipSub, topics)

# Verify that all topics are added to the topics and gossipsub
check gossipSub.topics.len == 3
for topic in topics:
check gossipSub.gossipsub[topic].len() >= 0
AlejandroCabeza marked this conversation as resolved.
Show resolved Hide resolved

# Unsubscribe from all topics
unsubscribeFromTopics(gossipSub, topics)

# Ensure topics are removed from topics and mesh, but still present in gossipsub
for topic in topics:
check topic notin gossipSub.topics
check topic notin gossipSub.mesh
check topic in gossipSub.gossipsub

await allFuturesThrowing(conns.mapIt(it.close()))
await gossipSub.switch.stop()

# Test ensuring that the number of subscriptions does not exceed the limit set in the GossipSub parameters
asyncTest "subscription limit test":
let gossipSub = TestGossipSub.init(newStandardSwitch())
gossipSub.topicsHigh = 10

var conns = newSeq[Connection]()
AlejandroCabeza marked this conversation as resolved.
Show resolved Hide resolved
for i in 0 .. gossipSub.topicsHigh + 5:
let topic = "topic" & $i
# Ensure all topics are properly initialized before subscribing
gossipSub.mesh[topic] = initHashSet[PubSubPeer]()
gossipSub.topicParams[topic] = TopicParams.init()
gossipSub.gossipsub[topic] = initHashSet[PubSubPeer]()

if gossipSub.topics.len < gossipSub.topicsHigh:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we want to test the reaction of the logic if we subscribed to more than the limit then we need to try to add more subscribtions than the limit and check the reaction

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let topicCount = 15 # Total number of topics to be tested
let gossipSubParams = 10 # Subscription limit for the topics
let topicNames = toSeq(mapIt(0..topicCount - 1, "topic" & $it))

checking subscription beyond the limit and asserting it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think about this, this doesn't make a lot of sense. The limit shouldn't be handled by the test, not manually by us. This is what the test should be asserting, actually: That regardless of the amount of subscriptions done, it shouldn't exceed the limit.

P.S.: Please, remove all the PubSub usages you added as these are GossipSub cases.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PubSub removed

gossipSub.PubSub.subscribe(
topic,
proc(topic: string, data: seq[byte]): Future[void] {.async.} =
discard
,
)
else:
# Prevent subscription beyond the limit and log the error
echo "Subscription limit reached for topic: ", topic
AlejandroCabeza marked this conversation as resolved.
Show resolved Hide resolved

# Ensure that the number of subscribed topics does not exceed the limit
check gossipSub.topics.len <= gossipSub.topicsHigh
check gossipSub.topics.len == gossipSub.topicsHigh

await allFuturesThrowing(conns.mapIt(it.close()))
await gossipSub.switch.stop()
Loading