feat!: set peer-exchange with default bootstrap #1469

danisharora099 · 2023-08-11T08:46:38Z

This PR addresses #1429 (comment):

sets peer-exchange with the default bootstrap flag
adds a configurable method to be able to fetch bootstrap peers and number of peers according to the protocol:
- sets up redundant usage with LP while adding functionality that we can extend on (getPeers()) to directly use multiple nodes for other protocols

Notes:

~~the tests are added but skip as our current infrastructure does not support stubbing standalone functions~~
- ~~tests to be un-skipped once test suite lacks mocking capabilities for standalone functions #1523 is done~~
- - this is now not required since we are directly using a function to test it instead of stubbing/mocking it; will leave the above issue open however to future proof our systems

TODO:

~~use proxyquire/rewire or alternatives to stub standalone functions~~
- ~~explored; unfeasible~~
- tracked here: test suite lacks mocking capabilities for standalone functions #1523

github-actions · 2023-08-11T08:51:18Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
Waku core	28.73 KB (+0.2% 🔺)	575 ms (+0.2% 🔺)	511 ms (+218.84% 🔺)	1.1 s
Waku Simple Light Node	309.61 KB (+0.44% 🔺)	6.2 s (+0.44% 🔺)	1.7 s (+101.07% 🔺)	7.9 s
ECIES encryption	28.68 KB (0%)	574 ms (0%)	451 ms (+44.79% 🔺)	1.1 s
Symmetric encryption	28.68 KB (0%)	574 ms (0%)	313 ms (-5.79% 🔽)	887 ms
DNS discovery	118.59 KB (0%)	2.4 s (0%)	795 ms (+34.21% 🔺)	3.2 s
Privacy preserving protocols	122.61 KB (+0.01% 🔺)	2.5 s (+0.01% 🔺)	784 ms (+37.94% 🔺)	3.3 s
Light protocols	28.95 KB (+0.9% 🔺)	579 ms (+0.9% 🔺)	379 ms (-1.51% 🔽)	958 ms
History retrieval protocols	28.08 KB (+0.57% 🔺)	562 ms (+0.57% 🔺)	354 ms (+15.53% 🔺)	915 ms
Deterministic Message Hashing	5.64 KB (0%)	113 ms (0%)	128 ms (+10% 🔺)	241 ms

…ault

packages/core/src/lib/base_protocol.ts

packages/interfaces/src/protocols.ts

packages/core/src/lib/light_push/index.ts

…u into feat/px-default

packages/core/src/lib/base_protocol.ts

packages/core/src/lib/light_push/index.ts

weboko

still some comments to address but looks neat

weboko · 2023-09-05T14:52:15Z

packages/core/src/lib/light_push/index.ts

-        log("Failed to decode push reply", err);
-        error = SendError.DECODE_FAILED;
+        log("Failed to send waku light push request", err);
+        error = SendError.GENERIC_FAIL;


Why it's GENERIC_FAIL now?

because there's another try-catch block for decoding attempt (line 134)

this try-catch block is purely for the pipe() operation

…ault

danisharora099 · 2023-09-05T20:38:01Z

packages/core/src/lib/filter/index.ts

    const _pubSubTopic =
      pubSubTopic ?? this.options.pubSubTopic ?? DefaultPubSubTopic;

-    const peer = await this.getPeer(peerId);
+    const peer = (
+      await this.getPeers({


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

danisharora099 · 2023-09-05T20:39:31Z

packages/core/src/lib/store/index.ts


-    const peer = await this.getPeer(options?.peerId);
+    const peer = (
+      await this.getPeers({


uses the new API is above with Filter but fetches one peers -- extendable to using multiple peers for the Store protocol

danisharora099 · 2023-09-05T20:41:08Z

packages/peer-exchange/src/waku_peer_exchange.ts

  /**
   * @param components - libp2p components
   */
  constructor(components: Libp2pComponents) {
    super(PeerExchangeCodec, components);
-    this.multicodec = PeerExchangeCodec;


this is already set/accessible with this.multicodec as we initialise BaseProtocol with super()

fryorcraken

A number of software engineer principles that would make sense to follow:

single-responsibility principle in regards to the filterPeers function
tests are close to the code they test (filterPeers is in @waku/utils but the test are in @waku/core
relevant/minimal scope: keep the scope of variables and functions minimal: filterPeers is in @waku/utils but only used by @waku/core

Approving because business logic looks correct but refactoring is needed to keep code maintainable.

fryorcraken · 2023-09-07T05:55:42Z

packages/core/src/lib/base_protocol.spec.ts

+        tags: new Map<string, Tag>([[Tags.BOOTSTRAP, { value: 100 }]])
+      },
+      {
+        id: peer3,


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

fryorcraken · 2023-09-07T05:59:03Z

packages/utils/src/common/filterPeers.ts

+ * @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(


Why is this function in a different package? is it used by another other package but @waku/core?

it is not - fair point. wanted it to be similar to other standalone functions we use for base_protocol that stay in @waku/utils like selectPeerForProtocol.
personally inclining to have these standalone functions not in the root of core as it's a helper function; perhaps this issue is part of a potential larger proposal to move helpers into a subdir within the same package to increase readability?

move helpers into a subdir within the same package to increase readability?

Sounds good

move helpers into a subdir within the same package to increase readability?

Sounds good

tracking here: #1570

fryorcraken · 2023-09-07T05:59:51Z

packages/core/src/lib/base_protocol.spec.ts

+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";


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

renamed this to be a filterPeers spec test instead

…ault

danisharora099 · 2023-09-07T07:44:39Z

A number of software engineer principles that would make sense to follow:

single-responsibility principle in regards to the filterPeers function

tests are close to the code they test (filterPeers is in @waku/utils but the test are in @waku/core

relevant/minimal scope: keep the scope of variables and functions minimal: filterPeers is in @waku/utils but only used by @waku/core

Approving because business logic looks correct but refactoring is needed to keep code maintainable.

Thanks for the feedback!!
I have addressed the comments. Please let me know if there's anything else that you think needs to be addressed re this PR and I can followup!

danisharora099 added 2 commits August 9, 2023 19:19

set peer-exchange with default bootstrap

29cb6a8

only initialise protocols with bootstrap peers

3646fdc

danisharora099 requested a review from a team as a code owner August 11, 2023 08:46

danisharora099 mentioned this pull request Aug 11, 2023

[Epic] Peer Exchange is supported and used by default #1429

Closed

10 tasks

danisharora099 and others added 12 commits August 11, 2023 14:23

Merge branch 'master' into feat/px-default

8d48380

update package

573fb8d

Merge branch 'master' into feat/px-default

4894091

Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…

a5c1372

…ault

update package-lock

6c1ec84

refactor getPeers while setting up a protocol

a0f0668

move codecs to @waku/interfaces

f376334

lightpush: send messages to multiple peers

f7f383b

only use multiple peers for LP and Filter

6807888

fix: ts warnings

f956f7f

lightpush: tests pass

b43fe35

update breaking changes for new API

d712e8a

fryorcraken requested changes Aug 16, 2023

View reviewed changes

danisharora099 added 4 commits August 17, 2023 16:33

move codecs back into protocol files

8bf969a

refactor: getPeers()

3204267

rm: log as an arg

645d7c1

add tsdoc for getPeers

9d70d55

danisharora099 mentioned this pull request Aug 17, 2023

chore: refactor LightPush send #1487

Merged

danisharora099 and others added 6 commits August 17, 2023 17:02

Merge branch 'master' into feat/px-default

12cec4a

add import

773d1ef

add prettier rule to eslint

f345496

Merge branch 'feat/fix-prettier-eslint' of github.com:waku-org/js-wak…

f2c2fd0

…u into feat/px-default

add: peer exchange to sdk as a dep

efef7a2

fix eslint error

6a17cc3

danisharora099 requested a review from fryorcraken August 17, 2023 11:47

danisharora099 changed the title ~~feat: set peer-exchange with default bootstrap~~ feat!: set peer-exchange with default bootstrap Sep 5, 2023

danisharora099 and others added 2 commits September 5, 2023 17:28

Merge branch 'master' into feat/px-default

50df227

chore: refactor tests for new API

053c9da

weboko requested changes Sep 5, 2023

View reviewed changes

packages/core/src/lib/base_protocol.ts Outdated Show resolved Hide resolved

packages/core/src/lib/light_push/index.ts Outdated Show resolved Hide resolved

danisharora099 added 2 commits September 5, 2023 18:02

add defaults for getPeers

5aa4fcb

address comments

afbcaae

weboko approved these changes Sep 5, 2023

View reviewed changes

weboko reviewed Sep 5, 2023

View reviewed changes

Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…

78114c7

…ault

danisharora099 mentioned this pull request Sep 5, 2023

chore: move constants to a dedicated file constants.ts #1535

Closed

danisharora099 commented Sep 5, 2023

View reviewed changes

rm unneeded comment

d44670d

fryorcraken approved these changes Sep 7, 2023

View reviewed changes

danisharora099 added 4 commits September 7, 2023 11:59

address comment: add diversity of node tags to test

9439bb3

address comments

1e2a9d2

Merge branch 'master' of github.com:waku-org/js-waku into feat/px-def…

7e49625

…ault

fix: imports

c90f88b

danisharora099 force-pushed the feat/px-default branch from 4a9e241 to c90f88b Compare September 7, 2023 07:33

danisharora099 merged commit 81a52a8 into master Sep 7, 2023
9 of 10 checks passed

danisharora099 deleted the feat/px-default branch September 7, 2023 07:45

This was referenced Sep 7, 2023

chore: release master #1452

Closed

chore: release master #1554

Closed

chore: release master #1555

Closed

chore: release master #1556

Closed

chore: release master #1558

Merged

danisharora099 mentioned this pull request Sep 15, 2023

moving standlone functions into a subdir helpers instead of having them in root #1570

Open

danisharora099 mentioned this pull request Sep 13, 2024

[Epic: js-waku] Reliability Protocol for Resource-Restricted Clients #2154

Closed

39 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: set peer-exchange with default bootstrap #1469

feat!: set peer-exchange with default bootstrap #1469

danisharora099 commented Aug 11, 2023 •

edited

Loading

github-actions bot commented Aug 11, 2023 •

edited

Loading

weboko left a comment

weboko Sep 5, 2023

danisharora099 Sep 5, 2023

danisharora099 Sep 5, 2023

danisharora099 Sep 5, 2023 •

edited

Loading

danisharora099 Sep 5, 2023 •

edited

Loading

fryorcraken left a comment

fryorcraken Sep 7, 2023

fryorcraken Sep 7, 2023

danisharora099 Sep 7, 2023 •

edited

Loading

fryorcraken Sep 15, 2023

danisharora099 Sep 15, 2023 •

edited

Loading

fryorcraken Sep 7, 2023

danisharora099 Sep 7, 2023

danisharora099 commented Sep 7, 2023

feat!: set peer-exchange with default bootstrap #1469

feat!: set peer-exchange with default bootstrap #1469

Conversation

danisharora099 commented Aug 11, 2023 • edited Loading

Notes:

TODO:

github-actions bot commented Aug 11, 2023 • edited Loading

size-limit report 📦

weboko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danisharora099 Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

danisharora099 Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

fryorcraken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danisharora099 Sep 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danisharora099 Sep 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danisharora099 commented Sep 7, 2023

danisharora099 commented Aug 11, 2023 •

edited

Loading

github-actions bot commented Aug 11, 2023 •

edited

Loading

danisharora099 Sep 5, 2023 •

edited

Loading

danisharora099 Sep 5, 2023 •

edited

Loading

danisharora099 Sep 7, 2023 •

edited

Loading

danisharora099 Sep 15, 2023 •

edited

Loading