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: map correct fleet according to the pubsub topic configured #2015

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
78 changes: 39 additions & 39 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 12 additions & 2 deletions packages/sdk/src/utils/discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import {
wakuLocalPeerCacheDiscovery,
wakuPeerExchangeDiscovery
} from "@waku/discovery";
import { type Libp2pComponents, PubsubTopic } from "@waku/interfaces";
import {
DefaultPubsubTopic,
type Libp2pComponents,
PubsubTopic
} from "@waku/interfaces";

const DEFAULT_NODE_REQUIREMENTS = {
lightPush: 1,
Expand All @@ -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
danisharora099 marked this conversation as resolved.
Show resolved Hide resolved
// https://github.com/waku-org/js-waku/issues/2014
const dnsFleet = pubsubTopics.includes(DefaultPubsubTopic)
Copy link
Collaborator

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?

Copy link
Collaborator

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)

Copy link
Collaborator Author

@danisharora099 danisharora099 May 29, 2024

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?

Copy link
Collaborator

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.

Copy link
Collaborator Author

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

? enrTree["TEST"]
: enrTree["SANDBOX"];

const discoveries = [
wakuDnsDiscovery([enrTree["SANDBOX"]], DEFAULT_NODE_REQUIREMENTS),
wakuDnsDiscovery([dnsFleet], DEFAULT_NODE_REQUIREMENTS),
wakuLocalPeerCacheDiscovery(),
wakuPeerExchangeDiscovery(pubsubTopics)
];
Expand Down
Loading