-
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: create default shard info, update tests #2085
Changes from 2 commits
d045898
1887f4f
0ec507b
00040b0
73a335a
7931791
5a6061d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,14 @@ | ||
/** | ||
* DefaultPubsubTopic is the default gossipsub topic to use for Waku. | ||
*/ | ||
export const DefaultPubsubTopic = "/waku/2/rs/0/0"; | ||
import { ShardInfo } from "./enr"; | ||
|
||
/** | ||
* The default cluster ID for The Waku Network | ||
*/ | ||
export const DEFAULT_CLUSTER_ID = 1; | ||
|
||
/** | ||
* DefaultShardInfo is default configuration for The Waku Network. | ||
*/ | ||
export const DefaultShardInfo: ShardInfo = { | ||
clusterId: DEFAULT_CLUSTER_ID, | ||
shards: [0, 1, 2, 3, 4, 5, 6, 7, 8] | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ | |
import { wakuMetadata } from "@waku/core"; | ||
import { | ||
type CreateLibp2pOptions, | ||
DefaultPubsubTopic, | ||
DefaultShardInfo, | ||
type IMetadata, | ||
type Libp2p, | ||
type Libp2pComponents, | ||
|
@@ -86,7 +86,7 @@ | |
...pubsubService, | ||
...options?.services | ||
} | ||
}) as any as Libp2p; // TODO: make libp2p include it; | ||
Check warning on line 89 in packages/sdk/src/create/libp2p.ts GitHub Actions / check
|
||
} | ||
|
||
export async function createLibp2pAndUpdateOptions( | ||
|
@@ -138,12 +138,15 @@ | |
options.shardInfo = { contentTopics: options.contentTopics }; | ||
} | ||
|
||
if (!options.shardInfo) { | ||
options.shardInfo = DefaultShardInfo; | ||
} | ||
|
||
const shardInfo = options.shardInfo | ||
? ensureShardingConfigured(options.shardInfo) | ||
: undefined; | ||
|
||
options.pubsubTopics = shardInfo?.pubsubTopics ?? | ||
options.pubsubTopics ?? [DefaultPubsubTopic]; | ||
options.pubsubTopics = options.pubsubTopics ?? shardInfo?.pubsubTopics; | ||
Comment on lines
+141
to
+149
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here in case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a good point but I don't think we should get rid of
|
||
|
||
return shardInfo?.shardInfo; | ||
} | ||
|
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'd vote to remove this completely as well. We are assuming that developers might still want to use named sharding, but a complete deprecation looks like the right step ahead.
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 was also thinking the same - but I changed my mind when recalled scenarios of some of our consumers that do not use any of our fleets, not to mention TWN.
So we need to continue providing support for
pubsubTopics
in order to allow them soft transition (or for some not to use our fleets at all).