feat: waku sync shard matching check #3259
Conversation
|
You can find the image built from this PR at Built from 4ebb5e8 |
SionoiS
force-pushed
the
feat--waku-sync-shard-support
branch
from
23ac2a0 to
4b1fa1a
Compare
| serverSwitch, idsChannel, localWants, remoteNeeds, @[0.uint16, 1, 2, 3] | ||
| ) | ||
| client = await newTestWakuRecon( |
There was a problem hiding this comment.
Sorry but I can't fully follow why we pass both the localWants and remoteNeeds to both client and server. Somehow I would expect passing either nil or an empty seq in some param, f.e. client with empty remoteNeeds, etc. But I might be missing something
There was a problem hiding this comment.
Since the protocol is symmetrical both peers (client & server) have the same information about who wants what.
Client:
local want -> 0xA
remote need -> 0xB
Server:
remote need -> 0xA
local want -> 0xB
Because of the differences in hashes and peerId you don't have to create 2 for each the result is the same.
| @@ -26,6 +26,8 @@ type | |||
| ItemSet = 2 | |||
|
|
|||
| RangesData* = object | |||
| shards*: seq[uint16] | |||
There was a problem hiding this comment.
I think we'd need the network ID too to fully qualify the shard.
I'm confused... Why add this to the spec? It's just a quick hack to prevent sync in some cases. |
Afaict this changes what is serialised to the wire, which IMO is good to specify for other implementers/maintainers/testers - even if we expect a future version of the protocol to replace this logic. :) |
Add a check so that peers supporting different shards don't sync.