-
Notifications
You must be signed in to change notification settings - Fork 42
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: map correct fleet according to the pubsub topic configured #2015
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.
did you check that it works?
I am a bit confused with https://discord.com/channels/1110799176264056863/1236947977252110346/1239894482585911318
Yes! wakuv2.test has |
@@ -16,8 +20,14 @@ const DEFAULT_NODE_REQUIREMENTS = { | |||
export function defaultPeerDiscoveries( | |||
pubsubTopics: PubsubTopic[] | |||
): ((components: Libp2pComponents) => PeerDiscovery)[] { | |||
// TODO: add a check to see if it is indeed TWN or if it is a custom network | |||
// https://github.com/waku-org/js-waku/issues/2014 | |||
const dnsFleet = pubsubTopics.includes(DefaultPubsubTopic) |
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.
@adklempner would this help with the issue we were looking at yesterday in the testing app?
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 guess this PR doesn't work as it didn't introduce any ENR to https://github.com/waku-org/js-waku/blob/master/packages/core/src/lib/predefined_bootstrap_nodes.ts (noticed by Jakub)
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 guess this PR doesn't work as it didn't introduce any ENR to https://github.com/waku-org/js-waku/blob/master/packages/core/src/lib/predefined_bootstrap_nodes.ts (noticed by Jakub)
@weboko Not sure I follow. Can you explain? What/where do you think an ENR is to be introduced?
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.
All good, I forgot the fact that predefined nodes are not used and only ENR is used for discovery, which I added and it should work.
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.
yes, following up with deprecation of predefined addresses with #2025
d8ed83f
to
00c77c6
Compare
superseded by #2031 |
Problem
#2011
Solution
Use the
TEST
fleet ifDefaultPubsubTopic
is usedNotes
Contribution checklist:
!
in title if breaks public API;