-
Notifications
You must be signed in to change notification settings - Fork 43
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(static-sharding)!: allow multiple pubSubTopics #1586
Conversation
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preliminary review. blocking on the multiple store requests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing various tests:
- initiate with multiple pubsub topic
- try to use any protocol with a pubsub topic that is not configured.
7d5e2a1
to
76208e2
Compare
cda719b
to
2036d1f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think relay could have been done with a 2nd PR to keep reviewing easy.
@@ -41,12 +42,12 @@ type PreparePushMessageResult = | |||
* Implements the [Waku v2 Light Push protocol](https://rfc.vac.dev/spec/19/). | |||
*/ | |||
class LightPush extends BaseProtocol implements ILightPush { | |||
options: ProtocolCreateOptions; | |||
private readonly pubsubTopics: PubSubTopic[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private readonly pubsubTopics: PubSubTopic[]; | |
private readonly pubSubTopics: PubSubTopic[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored this, thanks for the spot.
perhaps this change should be part of a larger code-wide refactor? tracking here: https://github.com/waku-org/js-waku/compare/chore/pubSubTopic-typo?expand=1
Co-authored-by: fryorcraken <110212804+fryorcraken@users.noreply.github.com>
* refactor: simplify handling of observers * refactor: Remove redundant PubSubTopic from Observer
…o feat/staticsharding
looking back in hindsight, would definitely make things simpler! 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice.
This PR, part of the #1310 milestone:
PubSubTopic
during node creationDefaultPubSubTopic
still as defaultPubSubTopic
as an argument for theencoder
/decoder
creation APIlightpush
,filter
,store
andrelay
) to confirm that the request can only be sent to one of thePubSubTopic
that were used during initialisationNotes
Next
ConnectionManager
Discovery
rs
/rsv
values for peers