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!: set peer-exchange with default bootstrap #1469

Merged
merged 75 commits into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from 71 commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
29cb6a8
set peer-exchange with default bootstrap
danisharora099 Aug 9, 2023
3646fdc
only initialise protocols with bootstrap peers
danisharora099 Aug 11, 2023
8d48380
Merge branch 'master' into feat/px-default
danisharora099 Aug 11, 2023
573fb8d
update package
danisharora099 Aug 11, 2023
4894091
Merge branch 'master' into feat/px-default
danisharora099 Aug 11, 2023
a5c1372
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 Aug 13, 2023
6c1ec84
update package-lock
danisharora099 Aug 13, 2023
a0f0668
refactor `getPeers` while setting up a protocol
danisharora099 Aug 15, 2023
f376334
move codecs to `@waku/interfaces`
danisharora099 Aug 15, 2023
f7f383b
lightpush: send messages to multiple peers
danisharora099 Aug 15, 2023
6807888
only use multiple peers for LP and Filter
danisharora099 Aug 15, 2023
f956f7f
fix: ts warnings
danisharora099 Aug 15, 2023
b43fe35
lightpush: tests pass
danisharora099 Aug 15, 2023
d712e8a
update breaking changes for new API
danisharora099 Aug 15, 2023
8bf969a
move codecs back into protocol files
danisharora099 Aug 17, 2023
3204267
refactor: `getPeers()`
danisharora099 Aug 17, 2023
645d7c1
rm: log as an arg
danisharora099 Aug 17, 2023
9d70d55
add tsdoc for getPeers
danisharora099 Aug 17, 2023
12cec4a
Merge branch 'master' into feat/px-default
danisharora099 Aug 17, 2023
773d1ef
add import
danisharora099 Aug 17, 2023
f345496
add prettier rule to eslint
danisharora099 Aug 17, 2023
f2c2fd0
Merge branch 'feat/fix-prettier-eslint' of github.com:waku-org/js-wak…
danisharora099 Aug 17, 2023
efef7a2
add: peer exchange to sdk as a dep
danisharora099 Aug 17, 2023
6a17cc3
fix eslint error
danisharora099 Aug 17, 2023
befbd37
add try catch
danisharora099 Aug 17, 2023
aa87190
Merge branch 'master' into feat/px-default
danisharora099 Aug 21, 2023
83d5103
revert unecessary diff
danisharora099 Aug 21, 2023
63796db
Merge branch 'feat/px-default' of github.com:waku-org/js-waku into fe…
danisharora099 Aug 21, 2023
9583bda
revert unecessary diff
danisharora099 Aug 21, 2023
6082ffb
Merge branch 'feat/px-default' of github.com:waku-org/js-waku into fe…
danisharora099 Aug 21, 2023
c63d146
fix imports
danisharora099 Aug 21, 2023
6fdcf61
convert relaycodecs to array
danisharora099 Aug 22, 2023
916af18
Merge branch 'master' into feat/px-default
danisharora099 Aug 22, 2023
5dff864
remove: peerId as an arg for protocol methods
danisharora099 Aug 29, 2023
f75b2d9
keep peerId as an arg for peer-exchange
danisharora099 Aug 29, 2023
7d14956
remove: peerId from getPeers()
danisharora099 Aug 29, 2023
1c93c12
lightpush: extract hardcoded numPeers as a constant
danisharora099 Aug 29, 2023
016965b
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 Aug 29, 2023
04b9bb5
return all peers if numPeers is 0 and increase readability for random…
danisharora099 Aug 29, 2023
1f81ae1
refactor considering more than 1 bootstrap peers can exist
danisharora099 Aug 29, 2023
dce7b3b
use `getPeers`
danisharora099 Aug 29, 2023
a376bdd
change arg for `getPeers` to object
danisharora099 Aug 29, 2023
ec3bcc8
address comments
danisharora099 Aug 31, 2023
c697c15
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 Aug 31, 2023
1b0e1fe
refactor tests for new API
danisharora099 Aug 31, 2023
c7fca3e
lightpush: make constant the class variable
danisharora099 Aug 31, 2023
191355b
use `maxBootstrapPeers` instead of `includeBootstrap`
danisharora099 Sep 1, 2023
6820ebe
refactor protocols for new API
danisharora099 Sep 1, 2023
e968d4d
add tests for `getPeers`
danisharora099 Sep 1, 2023
401265b
skip getPeers test
danisharora099 Sep 2, 2023
1b91278
rm: only from test
danisharora099 Sep 2, 2023
75ee463
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 Sep 4, 2023
56d8b84
move tests to `base_protocol.spec.ts`
danisharora099 Sep 5, 2023
dcb4fbf
break down `getPeers` into a `filter` method
danisharora099 Sep 5, 2023
2cd8fa0
return all bootstrap peers if arg is 0
danisharora099 Sep 5, 2023
ad518af
refactor test without stubbing
danisharora099 Sep 5, 2023
f24e8c0
address comments
danisharora099 Sep 5, 2023
7354b09
update test title
danisharora099 Sep 5, 2023
c1d2e1a
move `filterPeers` to a separate file
danisharora099 Sep 5, 2023
919f10b
address comments & add more test
danisharora099 Sep 5, 2023
e5f1331
Merge branch 'master' into feat/px-default
danisharora099 Sep 5, 2023
6a1a4ad
make test title more verbose
danisharora099 Sep 5, 2023
a6cad7a
Merge branch 'feat/px-default' of github.com:waku-org/js-waku into fe…
danisharora099 Sep 5, 2023
0f629cb
address comments
danisharora099 Sep 5, 2023
9302a86
remove ProtocolOptions
danisharora099 Sep 5, 2023
50df227
Merge branch 'master' into feat/px-default
danisharora099 Sep 5, 2023
053c9da
chore: refactor tests for new API
danisharora099 Sep 5, 2023
5aa4fcb
add defaults for getPeers
danisharora099 Sep 5, 2023
afbcaae
address comments
danisharora099 Sep 5, 2023
78114c7
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 Sep 5, 2023
d44670d
rm unneeded comment
danisharora099 Sep 5, 2023
9439bb3
address comment: add diversity of node tags to test
danisharora099 Sep 7, 2023
1e2a9d2
address comments
danisharora099 Sep 7, 2023
7e49625
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 Sep 7, 2023
c90f88b
fix: imports
danisharora099 Sep 7, 2023
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
3 changes: 3 additions & 0 deletions package-lock.json

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

10 changes: 3 additions & 7 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,11 @@ export * as waku_filter from "./lib/filter/index.js";
export { wakuFilter, FilterCodecs } from "./lib/filter/index.js";

export * as waku_light_push from "./lib/light_push/index.js";
export { wakuLightPush, LightPushCodec } from "./lib/light_push/index.js";
export { wakuLightPush } from "./lib/light_push/index.js";

export * as waku_store from "./lib/store/index.js";
export {
PageDirection,
wakuStore,
StoreCodec,
createCursor
} from "./lib/store/index.js";

export { PageDirection, wakuStore, createCursor } from "./lib/store/index.js";

export { waitForRemotePeer } from "./lib/wait_for_remote_peer.js";

Expand Down
143 changes: 143 additions & 0 deletions packages/core/src/lib/base_protocol.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
import { Peer } from "@libp2p/interface/peer-store";
import type { Tag } from "@libp2p/interface/peer-store";
import { createSecp256k1PeerId } from "@libp2p/peer-id-factory";
import { Tags } from "@waku/interfaces";
import { filterPeers } from "@waku/utils";
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test file only tests filterPeers which is in a different package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

renamed this to be a filterPeers spec test instead

import { expect } from "chai";

describe("getPeers function", function () {
it("should return all peers when numPeers is 0", async function () {
const peer1 = await createSecp256k1PeerId();
const peer2 = await createSecp256k1PeerId();
const peer3 = await createSecp256k1PeerId();

const mockPeers = [
{
id: peer1,
tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
},
{
id: peer2,
tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
},
{
id: peer3,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd include non-boostrap peers in this test to ensure they don't get filtered out.

tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
}
] as unknown as Peer[];

const result = await filterPeers(mockPeers, 0, 10);
expect(result.length).to.deep.equal(mockPeers.length);
});

it("should return all non-bootstrap peers and no bootstrap peer when numPeers is 0 and maxBootstrapPeers is 0", async function () {
const peer1 = await createSecp256k1PeerId();
const peer2 = await createSecp256k1PeerId();
const peer3 = await createSecp256k1PeerId();
const peer4 = await createSecp256k1PeerId();

const mockPeers = [
{
id: peer1,
tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
},
{
id: peer2,
tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
},
{
id: peer3,
tags: new Map<string, Tag>([[Tags.PEER_EXCHANGE, { value: 100 }]])
},
{
id: peer4,
tags: new Map<string, Tag>([[Tags.PEER_EXCHANGE, { value: 100 }]])
}
] as unknown as Peer[];

const result = await filterPeers(mockPeers, 0, 0);

// result should have no bootstrap peers, and a total of 2 peers
expect(result.length).to.equal(2);
expect(
result.filter((peer: Peer) => peer.tags.has(Tags.BOOTSTRAP)).length
).to.equal(0);
});

it("should return one bootstrap peer, and all non-boostrap peers, when numPeers is 0 & maxBootstrap is 1", async function () {
const peer1 = await createSecp256k1PeerId();
const peer2 = await createSecp256k1PeerId();
const peer3 = await createSecp256k1PeerId();
const peer4 = await createSecp256k1PeerId();
const peer5 = await createSecp256k1PeerId();

const mockPeers = [
{
id: peer1,
tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
},
{
id: peer2,
tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
},
{
id: peer3,
tags: new Map<string, Tag>([[Tags.PEER_EXCHANGE, { value: 100 }]])
},
{
id: peer4,
tags: new Map<string, Tag>([[Tags.PEER_EXCHANGE, { value: 100 }]])
},
{
id: peer5,
tags: new Map<string, Tag>([[Tags.PEER_EXCHANGE, { value: 100 }]])
}
] as unknown as Peer[];

const result = await filterPeers(mockPeers, 0, 1);

// result should have 1 bootstrap peers, and a total of 4 peers
expect(result.length).to.equal(4);
expect(
result.filter((peer: Peer) => peer.tags.has(Tags.BOOTSTRAP)).length
).to.equal(1);
});

it("should return only bootstrap peers up to maxBootstrapPeers", async function () {
const peer1 = await createSecp256k1PeerId();
const peer2 = await createSecp256k1PeerId();
const peer3 = await createSecp256k1PeerId();
const peer4 = await createSecp256k1PeerId();
const peer5 = await createSecp256k1PeerId();

const mockPeers = [
{
id: peer1,
tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
},
{
id: peer2,
tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
},
{
id: peer3,
tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
},
{
id: peer4,
tags: new Map<string, Tag>([[Tags.PEER_EXCHANGE, { value: 100 }]])
},
{
id: peer5,
tags: new Map<string, Tag>([[Tags.PEER_EXCHANGE, { value: 100 }]])
}
] as unknown as Peer[];

const result = await filterPeers(mockPeers, 5, 2);

// check that result has at least 2 bootstrap peers and no more than 5 peers
expect(result.length).to.be.at.least(2);
expect(result.length).to.be.at.most(5);
expect(result.filter((peer: Peer) => peer.tags.has(Tags.BOOTSTRAP)).length);
});
});
29 changes: 29 additions & 0 deletions packages/core/src/lib/base_protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { Stream } from "@libp2p/interface/connection";
import type { PeerId } from "@libp2p/interface/peer-id";
import { Peer, PeerStore } from "@libp2p/interface/peer-store";
import type { IBaseProtocol, Libp2pComponents } from "@waku/interfaces";
import { filterPeers } from "@waku/utils";
import { getPeersForProtocol, selectPeerForProtocol } from "@waku/utils/libp2p";

import { StreamManager } from "./stream_manager.js";
Expand Down Expand Up @@ -60,4 +61,32 @@ export class BaseProtocol implements IBaseProtocol {
);
return peer;
}

/**
* Retrieves a list of peers based on the specified criteria.
*
* @param numPeers - The total number of peers to retrieve. If 0, all peers are returned.
* @param maxBootstrapPeers - The maximum number of bootstrap peers to retrieve.
* @returns A Promise that resolves to an array of peers based on the specified criteria.
*/
protected async getPeers(
{
numPeers,
maxBootstrapPeers
}: {
numPeers: number;
maxBootstrapPeers: number;
} = {
maxBootstrapPeers: 1,
numPeers: 0
}
): Promise<Peer[]> {
// Retrieve all peers that support the protocol
const allPeersForProtocol = await getPeersForProtocol(this.peerStore, [
this.multicodec
]);

// Filter the peers based on the specified criteria
return filterPeers(allPeersForProtocol, numPeers, maxBootstrapPeers);
}
}
25 changes: 12 additions & 13 deletions packages/core/src/lib/filter/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Stream } from "@libp2p/interface/connection";
import type { PeerId } from "@libp2p/interface/peer-id";
import type { Peer } from "@libp2p/interface/peer-store";
import type { IncomingStreamData } from "@libp2p/interface-internal/registrar";
import type {
Expand All @@ -14,7 +13,6 @@ import type {
Libp2p,
PeerIdStr,
ProtocolCreateOptions,
ProtocolOptions,
PubSubTopic,
Unsubscribe
} from "@waku/interfaces";
Expand Down Expand Up @@ -228,6 +226,7 @@ class Subscription {
class Filter extends BaseProtocol implements IReceiver {
private readonly options: ProtocolCreateOptions;
private activeSubscriptions = new Map<string, Subscription>();
private readonly NUM_PEERS_PROTOCOL = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see that we always define constants in a file scope, like FilterCodecs up there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can place in core/constants or something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is a design decision to keep Codecs tightly coupled with the protocol class

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

no problems with codecs, we have other places with constants defined on top

export const DEFAULT_MAX_BOOTSTRAP_PEERS_ALLOWED = 1;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i agree with you and incline towards moving constants to a dedicated package
perhaps we can track in an issue?

Copy link
Collaborator

@weboko weboko Sep 5, 2023

Choose a reason for hiding this comment

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

oh, I don't really think it should be a separate package, to me it seems good enough that we have constants.ts files

for local constans to classes I think current structure is good that we have

import { Foo } from "./foo";
...
const CONST_1 = 1;
const CONST_2 = 2;
...
export class A {}
...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

gotcha. tracked here: #1535


private getActiveSubscription(
pubSubTopic: PubSubTopic,
Expand Down Expand Up @@ -257,14 +256,16 @@ class Filter extends BaseProtocol implements IReceiver {
this.options = options ?? {};
}

async createSubscription(
pubSubTopic?: string,
peerId?: PeerId
): Promise<Subscription> {
async createSubscription(pubSubTopic?: string): Promise<Subscription> {
const _pubSubTopic =
pubSubTopic ?? this.options.pubSubTopic ?? DefaultPubSubTopic;

const peer = await this.getPeer(peerId);
const peer = (
await this.getPeers({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

uses the new API but only uses one peer for filter
with #1463, we can use multiple peers to subscribe to the message

maxBootstrapPeers: 1,
numPeers: this.NUM_PEERS_PROTOCOL
})
)[0];

const subscription =
this.getActiveSubscription(_pubSubTopic, peer.id.toString()) ??
Expand All @@ -278,10 +279,9 @@ class Filter extends BaseProtocol implements IReceiver {
}

public toSubscriptionIterator<T extends IDecodedMessage>(
decoders: IDecoder<T> | IDecoder<T>[],
opts?: ProtocolOptions | undefined
decoders: IDecoder<T> | IDecoder<T>[]
): Promise<IAsyncIterator<T>> {
return toAsyncIterator(this, decoders, opts);
return toAsyncIterator(this, decoders);
}

/**
Expand All @@ -301,10 +301,9 @@ class Filter extends BaseProtocol implements IReceiver {
*/
async subscribe<T extends IDecodedMessage>(
decoders: IDecoder<T> | IDecoder<T>[],
callback: Callback<T>,
opts?: ProtocolOptions
callback: Callback<T>
): Promise<Unsubscribe> {
const subscription = await this.createSubscription(undefined, opts?.peerId);
const subscription = await this.createSubscription();

await subscription.subscribe(decoders, callback);

Expand Down
Loading
Loading