-
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!: set peer-exchange with default bootstrap #1469
Merged
Merged
Changes from all commits
Commits
Show all changes
75 commits
Select commit
Hold shift + click to select a range
29cb6a8
set peer-exchange with default bootstrap
danisharora099 3646fdc
only initialise protocols with bootstrap peers
danisharora099 8d48380
Merge branch 'master' into feat/px-default
danisharora099 573fb8d
update package
danisharora099 4894091
Merge branch 'master' into feat/px-default
danisharora099 a5c1372
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 6c1ec84
update package-lock
danisharora099 a0f0668
refactor `getPeers` while setting up a protocol
danisharora099 f376334
move codecs to `@waku/interfaces`
danisharora099 f7f383b
lightpush: send messages to multiple peers
danisharora099 6807888
only use multiple peers for LP and Filter
danisharora099 f956f7f
fix: ts warnings
danisharora099 b43fe35
lightpush: tests pass
danisharora099 d712e8a
update breaking changes for new API
danisharora099 8bf969a
move codecs back into protocol files
danisharora099 3204267
refactor: `getPeers()`
danisharora099 645d7c1
rm: log as an arg
danisharora099 9d70d55
add tsdoc for getPeers
danisharora099 12cec4a
Merge branch 'master' into feat/px-default
danisharora099 773d1ef
add import
danisharora099 f345496
add prettier rule to eslint
danisharora099 f2c2fd0
Merge branch 'feat/fix-prettier-eslint' of github.com:waku-org/js-wak…
danisharora099 efef7a2
add: peer exchange to sdk as a dep
danisharora099 6a17cc3
fix eslint error
danisharora099 befbd37
add try catch
danisharora099 aa87190
Merge branch 'master' into feat/px-default
danisharora099 83d5103
revert unecessary diff
danisharora099 63796db
Merge branch 'feat/px-default' of github.com:waku-org/js-waku into fe…
danisharora099 9583bda
revert unecessary diff
danisharora099 6082ffb
Merge branch 'feat/px-default' of github.com:waku-org/js-waku into fe…
danisharora099 c63d146
fix imports
danisharora099 6fdcf61
convert relaycodecs to array
danisharora099 916af18
Merge branch 'master' into feat/px-default
danisharora099 5dff864
remove: peerId as an arg for protocol methods
danisharora099 f75b2d9
keep peerId as an arg for peer-exchange
danisharora099 7d14956
remove: peerId from getPeers()
danisharora099 1c93c12
lightpush: extract hardcoded numPeers as a constant
danisharora099 016965b
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 04b9bb5
return all peers if numPeers is 0 and increase readability for random…
danisharora099 1f81ae1
refactor considering more than 1 bootstrap peers can exist
danisharora099 dce7b3b
use `getPeers`
danisharora099 a376bdd
change arg for `getPeers` to object
danisharora099 ec3bcc8
address comments
danisharora099 c697c15
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 1b0e1fe
refactor tests for new API
danisharora099 c7fca3e
lightpush: make constant the class variable
danisharora099 191355b
use `maxBootstrapPeers` instead of `includeBootstrap`
danisharora099 6820ebe
refactor protocols for new API
danisharora099 e968d4d
add tests for `getPeers`
danisharora099 401265b
skip getPeers test
danisharora099 1b91278
rm: only from test
danisharora099 75ee463
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 56d8b84
move tests to `base_protocol.spec.ts`
danisharora099 dcb4fbf
break down `getPeers` into a `filter` method
danisharora099 2cd8fa0
return all bootstrap peers if arg is 0
danisharora099 ad518af
refactor test without stubbing
danisharora099 f24e8c0
address comments
danisharora099 7354b09
update test title
danisharora099 c1d2e1a
move `filterPeers` to a separate file
danisharora099 919f10b
address comments & add more test
danisharora099 e5f1331
Merge branch 'master' into feat/px-default
danisharora099 6a1a4ad
make test title more verbose
danisharora099 a6cad7a
Merge branch 'feat/px-default' of github.com:waku-org/js-waku into fe…
danisharora099 0f629cb
address comments
danisharora099 9302a86
remove ProtocolOptions
danisharora099 50df227
Merge branch 'master' into feat/px-default
danisharora099 053c9da
chore: refactor tests for new API
danisharora099 5aa4fcb
add defaults for getPeers
danisharora099 afbcaae
address comments
danisharora099 78114c7
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 d44670d
rm unneeded comment
danisharora099 9439bb3
address comment: add diversity of node tags to test
danisharora099 1e2a9d2
address comments
danisharora099 7e49625
Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…
danisharora099 c90f88b
fix: imports
danisharora099 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 { | ||
|
@@ -14,7 +13,6 @@ import type { | |
Libp2p, | ||
PeerIdStr, | ||
ProtocolCreateOptions, | ||
ProtocolOptions, | ||
PubSubTopic, | ||
Unsubscribe | ||
} from "@waku/interfaces"; | ||
|
@@ -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; | ||
|
||
private getActiveSubscription( | ||
pubSubTopic: PubSubTopic, | ||
|
@@ -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({ | ||
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. uses the new API but only uses one peer for filter |
||
maxBootstrapPeers: 1, | ||
numPeers: this.NUM_PEERS_PROTOCOL | ||
}) | ||
)[0]; | ||
|
||
const subscription = | ||
this.getActiveSubscription(_pubSubTopic, peer.id.toString()) ?? | ||
|
@@ -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); | ||
} | ||
|
||
/** | ||
|
@@ -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); | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,144 @@ | ||
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 { expect } from "chai"; | ||
|
||
import { filterPeers } from "./filterPeers.js"; | ||
|
||
describe("filterPeers 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, | ||
tags: new Map<string, Tag>([[Tags.PEER_EXCHANGE, { 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); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
import { Peer } from "@libp2p/interface/peer-store"; | ||
import { Tags } from "@waku/interfaces"; | ||
|
||
/** | ||
* Retrieves a list of peers based on the specified criteria. | ||
* | ||
* @param peers - The list of peers to filter from. | ||
* @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. | ||
*/ | ||
export async function filterPeers( | ||
peers: Peer[], | ||
numPeers: number, | ||
maxBootstrapPeers: number | ||
): Promise<Peer[]> { | ||
// Collect the bootstrap peers up to the specified maximum | ||
const bootstrapPeers = peers | ||
.filter((peer) => peer.tags.has(Tags.BOOTSTRAP)) | ||
.slice(0, maxBootstrapPeers); | ||
|
||
// Collect non-bootstrap peers | ||
const nonBootstrapPeers = peers.filter( | ||
(peer) => !peer.tags.has(Tags.BOOTSTRAP) | ||
); | ||
|
||
// If numPeers is 0, return all peers | ||
if (numPeers === 0) { | ||
return [...bootstrapPeers, ...nonBootstrapPeers]; | ||
} | ||
|
||
// Initialize the list of selected peers with the bootstrap peers | ||
const selectedPeers: Peer[] = [...bootstrapPeers]; | ||
|
||
// Fill up to numPeers with remaining random peers if needed | ||
while (selectedPeers.length < numPeers && nonBootstrapPeers.length > 0) { | ||
const randomIndex = Math.floor(Math.random() * nonBootstrapPeers.length); | ||
const randomPeer = nonBootstrapPeers.splice(randomIndex, 1)[0]; | ||
selectedPeers.push(randomPeer); | ||
} | ||
|
||
return selectedPeers; | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 see that we always define
constants
in a file scope, likeFilterCodecs
up there.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.
Maybe we can place in
core/constants
or something?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.
this is a design decision to keep
Codecs
tightly coupled with the protocol classThere 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.
ref: #1469 (comment)
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.
no problems with codecs, we have other places with constants defined on top
js-waku/packages/core/src/lib/connection_manager.ts
Line 21 in 9fc79f6
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 agree with you and incline towards moving constants to a dedicated package
perhaps we can track in an issue?
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.
oh, I don't really think it should be a separate package, to me it seems good enough that we have
constants.ts
filesfor local constans to classes I think current structure is good that we have
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.
gotcha. tracked here: #1535