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

chore: throw if more than one network config is passed #2056

Merged
merged 10 commits into from
Jul 19, 2024
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import { type Libp2pComponents, type LightNode } from "@waku/interfaces";
import { type LightNode } from "@waku/interfaces";

import { createLibp2pAndUpdateOptions } from "../utils/libp2p.js";
import { CreateWakuNodeOptions, WakuNode, WakuOptions } from "../waku.js";

export { Libp2pComponents };
import { createLibp2pAndUpdateOptions } from "./libp2p.js";

/**
* Create a Waku node that uses Waku Light Push, Filter and Store to send and
Expand Down
2 changes: 2 additions & 0 deletions packages/sdk/src/create/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { createLightNode } from "./create.js";
export { defaultLibp2p } from "./libp2p.js";
Original file line number Diff line number Diff line change
Expand Up @@ -86,30 +86,19 @@
...pubsubService,
...options?.services
}
}) as any as Libp2p; // TODO: make libp2p include it;

Check warning on line 89 in packages/sdk/src/create/libp2p.ts

View workflow job for this annotation

GitHub Actions / check

Unexpected any. Specify a different type

Check warning on line 89 in packages/sdk/src/create/libp2p.ts

View workflow job for this annotation

GitHub Actions / proto

Unexpected any. Specify a different type
}

export async function createLibp2pAndUpdateOptions(
options: CreateWakuNodeOptions
): Promise<Libp2p> {
logWhichShardInfoIsUsed(options);

if (options.contentTopics) {
options.shardInfo = { contentTopics: options.contentTopics };
}

const shardInfo = options.shardInfo
? ensureShardingConfigured(options.shardInfo)
: undefined;

options.pubsubTopics = shardInfo?.pubsubTopics ??
options.pubsubTopics ?? [DefaultPubsubTopic];
const shardInfo = configureNetworkOptions(options);

const libp2pOptions = options?.libp2p ?? {};
const peerDiscovery = libp2pOptions.peerDiscovery ?? [];

if (options?.defaultBootstrap) {
peerDiscovery.push(...defaultPeerDiscoveries(options.pubsubTopics));
peerDiscovery.push(...defaultPeerDiscoveries(options.pubsubTopics!));
}

if (options?.bootstrapPeers) {
Expand All @@ -119,7 +108,7 @@
libp2pOptions.peerDiscovery = peerDiscovery;

const libp2p = await defaultLibp2p(
shardInfo?.shardInfo,
shardInfo,
wakuGossipSub(options),
libp2pOptions,
options?.userAgent
Expand All @@ -128,6 +117,37 @@
return libp2p;
}

function configureNetworkOptions(
options: CreateWakuNodeOptions
): ShardInfo | undefined {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this return undefined optionally?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because this is how types are defined and shardInfo can be missing in some cases

const flags = [
options.contentTopics,
options.pubsubTopics,
options.shardInfo
].filter((v) => !!v);

if (flags.length > 1) {
throw Error(
"Too many network configurations provided. Pass only one of: pubsubTopic, contentTopics or shardInfo."
);
}

logWhichShardInfoIsUsed(options);

if (options.contentTopics) {
options.shardInfo = { contentTopics: options.contentTopics };
}

const shardInfo = options.shardInfo
? ensureShardingConfigured(options.shardInfo)
: undefined;

options.pubsubTopics = shardInfo?.pubsubTopics ??
options.pubsubTopics ?? [DefaultPubsubTopic];

return shardInfo?.shardInfo;
Comment on lines +129 to +148
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use the same if-else syntax for shardInfo and pubsubTopics as well, as used for contentTopics

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as I mentioned here - I don't think it is improvement for readability

if (shardInfo?.pubsubTopics) {
  options.pubsubTopics = shardInfo.pubsubTopics;
} else if (!options.pubsubTopics) {
  options.pubsubTopics = [DefaultPubsubTopic];
}

if (shardInfo?.shardInfo) {
  return shardInfo.shardInfo;
} 

return undefined;

again, I think this area should be re-worked and here is the task #2073

}

function logWhichShardInfoIsUsed(options: CreateWakuNodeOptions): void {
if (options.pubsubTopics) {
logger.info("Using pubsubTopics array to bootstrap the node.");
Expand Down
3 changes: 1 addition & 2 deletions packages/sdk/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ export {

export { utf8ToBytes, bytesToUtf8 } from "@waku/utils/bytes";

export { defaultLibp2p } from "./utils/libp2p.js";
export * from "./utils/content_topic.js";
export * from "./waku.js";

export { createLightNode } from "./light-node/index.js";
export { createLightNode, defaultLibp2p } from "./create/index.js";
export { wakuLightPush } from "./protocols/light_push.js";
export { wakuFilter } from "./protocols/filter.js";
export { wakuStore } from "./protocols/store.js";
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk/src/relay-node/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { type FullNode, type RelayNode } from "@waku/interfaces";
import { RelayCreateOptions } from "@waku/relay";

import { createLibp2pAndUpdateOptions } from "../utils/libp2p.js";
import { createLibp2pAndUpdateOptions } from "../create/libp2p.js";
import { CreateWakuNodeOptions, WakuNode, WakuOptions } from "../waku.js";

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk/src/utils/content_topic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
shardInfoToPubsubTopics
} from "@waku/utils";

import { createLightNode } from "../light-node/index.js";
import { createLightNode } from "../create/index.js";

interface CreateTopicOptions {
waku?: LightNode;
Expand Down
16 changes: 10 additions & 6 deletions packages/tests/tests/filter/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,23 @@ export async function runMultipleNodes(
withoutFilter
);

const waku_options: ProtocolCreateOptions = {
const wakuOptions: ProtocolCreateOptions = {
staticNoiseKey: NOISE_KEY_1,
libp2p: {
addresses: { listen: ["/ip4/0.0.0.0/tcp/0/ws"] }
},
pubsubTopics,
shardInfo
}
};

log.info("Starting js waku node with :", JSON.stringify(waku_options));
if (shardInfo) {
wakuOptions.shardInfo = shardInfo;
} else {
wakuOptions.pubsubTopics = pubsubTopics;
}

log.info("Starting js waku node with :", JSON.stringify(wakuOptions));
let waku: LightNode | undefined;
try {
waku = await createLightNode(waku_options);
waku = await createLightNode(wakuOptions);
await waku.start();
} catch (error) {
log.error("jswaku node failed to start:", error);
Expand Down
Loading