feat!: add and implement IReceiver #1219

weboko · 2023-03-06T23:42:54Z

Problem

We have Relay and Filter that have the same behaviour but do not share common interface. It makes harder to make interchangeable usage of these objects.

Solution

Create and implement IReceiver

Notes

Related to feat: consume common interface for Sender/Receiver js-noise#14

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

weboko · 2023-03-07T00:11:08Z

Currently this PR is for just showing possible approach.

In general I think since we are using observable term in the code, relying on subscription based way for retrieving data and using it-all or other utils for async operations we should reconsider our approach to work with data streams.

There are different things we can choose or continue using (callbacks, Promises, events etc).

I would like to highlight this - RxJS.
This is just an idea, we can chose something else. Main point is to unify similar operations we have.
My proposition is to use Observable.subscribe and Observer.next for public API.

Usage of the lib on that level will help us:

we would be able to use RxJS internally in our objects to run async operations;
unlock in using different utils from the lib;
consumers will be able to use RxJS with our objects;
and it does not enforce usage of the lib on the consumers;

If our common opinion would be to continue current approach it definitely makes sense to make interfaces are compatible (this work waku-org/js-noise#14) and try to come up with common approach for events that are owned by us (similarly to it #1213).

cc: @fryorcraken @danisharora099

fryorcraken · 2023-03-07T03:44:04Z

I think using RxJS for the receiver interface is fair and should hopefully help with js-noise and other consumers.

weboko · 2023-03-17T21:14:42Z

I've been playing with idea of RxJS interfaces applied for receiver but has reached a conclusion it is an overkill.

In short - their usage will provide some extra features to consumers such as error handling but it is not clear if it will be used by anyone.

The other problem it will introduce a dependency and given that our bundle is quite big already it's better to add after this issue #579 is done.

I propose to refrain from using RxJS in this PR and research later and provide some PoC (will make an issue if we agree).

Do you even think it worth investigating, @fryorcraken?

fryorcraken · 2023-03-20T04:55:17Z

Do you even think it worth investigating, @fryorcraken?

We could have an issue in the icebox. As js-waku adoption grows, it may become a request from developers and in this case we could review.

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

weboko · 2023-03-22T22:58:58Z

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

-  addObserver<T extends IDecodedMessage>(
-    decoder: IDecoder<T>,
+  public subscribe<T extends IDecodedMessage>(
+    decoders: IDecoder<T>[],


This change makes subscribe unified and accept decoders instead of one decoder.

github-actions · 2023-03-22T22:59:40Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
Waku core	116.83 KB (+0.09% 🔺)	2.4 s (+0.09% 🔺)	2.4 s (-37.17% 🔽)	4.7 s
Waku default setup	387.93 KB (+0.1% 🔺)	7.8 s (+0.1% 🔺)	7.8 s (+23.19% 🔺)	15.5 s
ECIES encryption	27.99 KB (0%)	560 ms (0%)	1.9 s (-23.48% 🔽)	2.5 s
Symmetric encryption	27.99 KB (0%)	560 ms (0%)	1.9 s (-33.61% 🔽)	2.4 s
DNS discovery	108.07 KB (0%)	2.2 s (0%)	4.6 s (+53.35% 🔺)	6.7 s
Privacy preserving protocols	116.36 KB (+0.13% 🔺)	2.4 s (+0.13% 🔺)	3.7 s (-0.03% 🔽)	6 s
Light protocols	118.17 KB (+0.16% 🔺)	2.4 s (+0.16% 🔺)	2.4 s (-16.78% 🔽)	4.8 s
History retrieval protocols	118.19 KB (+0.11% 🔺)	2.4 s (+0.11% 🔺)	2.8 s (-24.18% 🔽)	5.1 s

weboko · 2023-03-22T23:24:32Z

packages/interfaces/src/receiver.ts

+    decoders: IDecoder<T>[],
+    callback: Callback<T>,
+    opts?: ProtocolOptions
+  ) => Unsubscribe<void> | Promise<Unsubscribe<Promise<void>>>;


This is another thing I want to figure out - either make it sync or async for the interface.

Well, it is expected to have both:

Relay: all actions are local as the node relays all messages of the pubsub topic so listening (subscribe) or stopping to listen (unsubscribe) to given content topics are local actions hence it's sync

Filter: filtering of subscription is done by remote node, hence it's async

I don't see the dichotomy as an issue.

The problem here is that types are not reliable - we don't know if subscribe should be awaited or not, same goes for Unsubscribe.

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

fryorcraken · 2023-03-23T04:07:01Z

packages/interfaces/src/receiver.ts

+    decoders: IDecoder<T>[],
+    callback: Callback<T>,
+    opts?: ProtocolOptions
+  ) => Unsubscribe<void> | Promise<Unsubscribe<Promise<void>>>;


Well, it is expected to have both:

Relay: all actions are local as the node relays all messages of the pubsub topic so listening (subscribe) or stopping to listen (unsubscribe) to given content topics are local actions hence it's sync

Filter: filtering of subscription is done by remote node, hence it's async

I don't see the dichotomy as an issue.

packages/interfaces/src/receiver.ts

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

…iver

weboko · 2023-03-28T23:51:08Z

packages/create/src/index.ts

+  return new WakuNode(
+    options ?? {},
+    libp2p,
+    undefined,


Idea for following improvement: make builders be props of protocolOptions object.

…lready initialized gossipSub

…iver

weboko · 2023-03-29T01:08:13Z

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

+  return (libp2p: Libp2p) => new Relay(libp2p, init);
+}
+
+export function wakuGossipSub(


This is a consequence of gossipSub being a property and not an extension of the Relay.
My opinion - it makes sense because Relay is our abstraction and GossipSub is a tool that we use to implement what we need.

cc: @fryorcraken

I see how it is. I think it's fine for now. We can iterate on the API as we use it and get more feedback.

fryorcraken · 2023-03-29T04:11:01Z

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

+  return (libp2p: Libp2p) => new Relay(libp2p, init);
+}
+
+export function wakuGossipSub(


I see how it is. I think it's fine for now. We can iterate on the API as we use it and get more feedback.

…iver

packages/tests/src/nwaku.ts

weboko commented Mar 6, 2023

View reviewed changes

packages/core/src/lib/relay/index.ts Show resolved Hide resolved

weboko marked this pull request as ready for review March 17, 2023 21:01

fryorcraken approved these changes Mar 20, 2023

View reviewed changes

packages/core/src/lib/relay/index.ts Show resolved Hide resolved

add IReceiver

0a6b6e5

weboko force-pushed the weboko/receiver branch from 8d48ef0 to 0a6b6e5 Compare March 22, 2023 01:04

make it compatible with composition

221fbbc

weboko commented Mar 22, 2023

View reviewed changes

weboko added 2 commits March 23, 2023 00:14

update tests

46b71c8

update tests

9fcb5e3

weboko commented Mar 22, 2023

View reviewed changes

improve removing

cf306ea

fryorcraken approved these changes Mar 23, 2023

View reviewed changes

weboko added 4 commits March 24, 2023 23:07

update Unsubscribe type

538b2b1

remove unubscribe on Relay

d560787

add single decoder support

caccaea

Merge branch 'master' of github.com:waku-org/js-waku into weboko/rece…

8fb5c64

…iver

weboko force-pushed the weboko/receiver branch from 4d3743e to 8fb5c64 Compare March 24, 2023 22:35

weboko added 2 commits March 25, 2023 00:06

update tests to test new interface

f988b33

Merge branch 'master' of github.com:waku-org/js-waku into weboko/rece…

71df432

…iver

fryorcraken approved these changes Mar 27, 2023

View reviewed changes

weboko added 2 commits March 28, 2023 01:53

Merge branch 'master' of github.com:waku-org/js-waku into weboko/rece…

4633416

…iver

add wakuPubSub builder

9f394c2

weboko commented Mar 28, 2023

View reviewed changes

weboko added 2 commits March 29, 2023 02:56

detach gossipSub from Relay and add subscriptions if Relay received a…

40529e3

…lready initialized gossipSub

Merge branch 'master' of github.com:waku-org/js-waku into weboko/rece…

c4ad2db

…iver

rename to wakuGossipSub

a24d732

weboko commented Mar 29, 2023

View reviewed changes

weboko requested a review from fryorcraken March 29, 2023 01:08

fryorcraken approved these changes Mar 29, 2023

View reviewed changes

weboko added 3 commits March 29, 2023 23:57

try to fix test

7a74358

return prev test

a3129ed

Merge branch 'master' of github.com:waku-org/js-waku into weboko/rece…

6d6545d

…iver

fryorcraken reviewed Mar 30, 2023

View reviewed changes

packages/tests/src/nwaku.ts Outdated Show resolved Hide resolved

weboko and others added 2 commits March 31, 2023 00:07

use custom libp2p options

6c47c1c

Merge branch 'master' into weboko/receiver

b57881d

weboko merged commit e11e5b4 into master Mar 31, 2023

weboko deleted the weboko/receiver branch March 31, 2023 01:17

fryorcraken mentioned this pull request Mar 31, 2023

chore: release master #1289

Merged

weboko mentioned this pull request Apr 3, 2023

feat: simplify sending/receiving API #1220

Closed

4 tasks

fryorcraken mentioned this pull request Aug 2, 2023

chore: release master #1452

Closed

This was referenced Sep 11, 2023

chore: release master #1554

Closed

chore: release master #1555

Closed

chore: release master #1556

Closed

chore: release master #1558

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: add and implement IReceiver #1219

feat!: add and implement IReceiver #1219

weboko commented Mar 6, 2023

weboko commented Mar 7, 2023

fryorcraken commented Mar 7, 2023

weboko commented Mar 17, 2023

fryorcraken commented Mar 20, 2023

weboko Mar 22, 2023

github-actions bot commented Mar 22, 2023 •

edited

Loading

weboko Mar 22, 2023

fryorcraken Mar 23, 2023

weboko Mar 24, 2023

fryorcraken Mar 23, 2023

weboko Mar 28, 2023

weboko Mar 29, 2023

fryorcraken Mar 29, 2023

fryorcraken Mar 29, 2023

feat!: add and implement IReceiver #1219

feat!: add and implement IReceiver #1219

Conversation

weboko commented Mar 6, 2023

Problem

Solution

Notes

weboko commented Mar 7, 2023

fryorcraken commented Mar 7, 2023

weboko commented Mar 17, 2023

fryorcraken commented Mar 20, 2023

Choose a reason for hiding this comment

github-actions bot commented Mar 22, 2023 • edited Loading

size-limit report 📦

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

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

github-actions bot commented Mar 22, 2023 •

edited

Loading