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.

Sign up for GitHub

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

chore: move protocols CreateOptions into interfaces and #1145

Merged
merged 3 commits into from
Feb 2, 2023
Merged

chore: move protocols CreateOptions into interfaces and #1145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a91b0ad
move protocols `CreateOptions` into interfaces and
danisharora099 Feb 1, 2023
460252f
remove: createOptions for PeerExchange
danisharora099 Feb 1, 2023
dd0bb84
update test with new API
danisharora099 Feb 1, 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
30 changes: 11 additions & 19 deletions packages/core/src/lib/filter/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
IDecoder,
IFilter,
IMessage,
ProtocolCreateOptions,
ProtocolOptions,
} from "@waku/interfaces";
import {
Expand Down Expand Up @@ -43,18 +44,6 @@ export interface FilterComponents {
connectionManager: ConnectionManager;
}

export interface CreateOptions {
/**
* The PubSub Topic to use. Defaults to {@link DefaultPubSubTopic}.
*
* The usage of the default pubsub topic is recommended.
* See [Waku v2 Topic Usage Recommendations](https://rfc.vac.dev/spec/23/) for details.
*
* @default {@link DefaultPubSubTopic}
*/
pubSubTopic?: string;
}

export type UnsubscribeFunction = () => Promise<void>;

/**
Expand All @@ -66,18 +55,21 @@ export type UnsubscribeFunction = () => Promise<void>;
*/
class Filter implements IFilter {
multicodec: string;
pubSubTopic: string;
options: ProtocolCreateOptions;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Changelog update needed to highlight breaking change in API.

private subscriptions: Map<string, Callback<any>>;
private decoders: Map<
string, // content topic
Set<IDecoder<any>>
>;

constructor(public components: FilterComponents, options?: CreateOptions) {
constructor(
public components: FilterComponents,
options?: ProtocolCreateOptions
) {
this.options = options ?? {};
this.multicodec = FilterCodec;
this.subscriptions = new Map();
this.decoders = new Map();
this.pubSubTopic = options?.pubSubTopic ?? DefaultPubSubTopic;
this.components.registrar
.handle(FilterCodec, this.onRequest.bind(this))
.catch((e) => log("Failed to register filter protocol", e));
Expand All @@ -94,7 +86,7 @@ class Filter implements IFilter {
callback: Callback<T>,
opts?: ProtocolOptions
): Promise<UnsubscribeFunction> {
const topic = opts?.pubSubTopic ?? this.pubSubTopic;
const { pubSubTopic = DefaultPubSubTopic } = this.options;

const groupedDecoders = groupByContentTopic(decoders);
const contentTopics = Array.from(groupedDecoders.keys());
Expand All @@ -103,7 +95,7 @@ class Filter implements IFilter {
contentTopic,
}));
const request = FilterRPC.createRequest(
topic,
pubSubTopic,
contentFilters,
undefined,
true
Expand Down Expand Up @@ -144,7 +136,7 @@ class Filter implements IFilter {
this.addCallback(requestId, callback);

return async () => {
await this.unsubscribe(topic, contentFilters, requestId, peer);
await this.unsubscribe(pubSubTopic, contentFilters, requestId, peer);
this.deleteDecoders(groupedDecoders);
this.deleteCallback(requestId);
};
Expand Down Expand Up @@ -309,7 +301,7 @@ class Filter implements IFilter {
}

export function wakuFilter(
init: Partial<CreateOptions> = {}
init: Partial<ProtocolCreateOptions> = {}
): (components: FilterComponents) => IFilter {
return (components: FilterComponents) => new Filter(components, init);
}
28 changes: 10 additions & 18 deletions packages/core/src/lib/light_push/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
IEncoder,
ILightPush,
IMessage,
ProtocolCreateOptions,
ProtocolOptions,
SendResult,
} from "@waku/interfaces";
Expand Down Expand Up @@ -36,40 +37,31 @@ export interface LightPushComponents {
connectionManager: ConnectionManager;
}

export interface CreateOptions {
/**
* The PubSub Topic to use. Defaults to {@link DefaultPubSubTopic}.
*
* The usage of the default pubsub topic is recommended.
* See [Waku v2 Topic Usage Recommendations](https://rfc.vac.dev/spec/23/) for details.
*
* @default {@link DefaultPubSubTopic}
*/
pubSubTopic?: string;
}

/**
* Implements the [Waku v2 Light Push protocol](https://rfc.vac.dev/spec/19/).
*/
class LightPush implements ILightPush {
multicodec: string;
pubSubTopic: string;
options: ProtocolCreateOptions;
Copy link
Collaborator

Choose a reason for hiding this comment

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

update changelog to highlight api breaking change


constructor(public components: LightPushComponents, options?: CreateOptions) {
constructor(
public components: LightPushComponents,
options?: ProtocolCreateOptions
) {
this.multicodec = LightPushCodec;
this.pubSubTopic = options?.pubSubTopic ?? DefaultPubSubTopic;
this.options = options || {};
}

async push(
encoder: IEncoder,
message: IMessage,
opts?: ProtocolOptions
): Promise<SendResult> {
const pubSubTopic = opts?.pubSubTopic ? opts.pubSubTopic : this.pubSubTopic;
const { pubSubTopic = DefaultPubSubTopic } = this.options;

const res = await selectPeerForProtocol(
this.components.peerStore,
[LightPushCodec],
[this.multicodec],
opts?.peerId
);

Expand Down Expand Up @@ -152,7 +144,7 @@ class LightPush implements ILightPush {
}

export function wakuLightPush(
init: Partial<CreateOptions> = {}
init: Partial<ProtocolCreateOptions> = {}
): (components: LightPushComponents) => ILightPush {
return (components: LightPushComponents) => new LightPush(components, init);
}
34 changes: 13 additions & 21 deletions packages/core/src/lib/relay/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
IEncoder,
IMessage,
IRelay,
ProtocolCreateOptions,
SendResult,
} from "@waku/interfaces";
import { IDecodedMessage } from "@waku/interfaces";
Expand All @@ -30,22 +31,7 @@ export type Observer<T extends IDecodedMessage> = {
callback: Callback<T>;
};

export interface RelayCreateOptions extends GossipsubOpts {
/**
* The PubSub Topic to use. Defaults to {@link DefaultPubSubTopic}.
*
* One and only one pubsub topic is used by Waku. This is used by:
* - WakuRelay to receive, route and send messages,
* - WakuLightPush to send messages,
* - WakuStore to retrieve messages.
*
* The usage of the default pubsub topic is recommended.
* See [Waku v2 Topic Usage Recommendations](https://rfc.vac.dev/spec/23/) for details.
*
* @default {@link DefaultPubSubTopic}
*/
pubSubTopic?: string;
}
export type RelayCreateOptions = ProtocolCreateOptions & GossipsubOpts;

/**
* Implements the [Waku v2 Relay protocol](https://rfc.vac.dev/spec/11/).
Expand All @@ -54,7 +40,7 @@ export interface RelayCreateOptions extends GossipsubOpts {
* @implements {require('libp2p-interfaces/src/pubsub')}
*/
class Relay extends GossipSub implements IRelay {
pubSubTopic: string;
options: Partial<RelayCreateOptions>;
defaultDecoder: IDecoder<IDecodedMessage>;
public static multicodec: string = constants.RelayCodecs[0];

Expand All @@ -73,12 +59,13 @@ class Relay extends GossipSub implements IRelay {
globalSignaturePolicy: SignaturePolicy.StrictNoSign,
fallbackToFloodsub: false,
});

super(components, options);
this.multicodecs = constants.RelayCodecs;

this.observers = new Map();

this.pubSubTopic = options?.pubSubTopic ?? DefaultPubSubTopic;
this.options = options ?? {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

you end up applyuing the default pubsub topic and every usage of this.option. This is cumbersome and prone to developer error.

Apply the default values in the constructor:

this.options = Object.assign({pubSubTopic: DefaultPubSubTopic}, options, {})

Or same logic in a getter get options(): ProtocolOptions

Or create a class that handles the ProtocolOptions and automatically apply default values

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TypeScript doesn't know if a particular value as been assigned to an object (``Object.assign) so would need to explicitly typecast it. Instead, I've replaced this.options` with a private `pubSubTopic` variable.


// TODO: User might want to decide what decoder should be used (e.g. for RLN)
this.defaultDecoder = new TopicOnlyDecoder();
Expand All @@ -92,20 +79,24 @@ class Relay extends GossipSub implements IRelay {
* @returns {void}
*/
public async start(): Promise<void> {
const { pubSubTopic = DefaultPubSubTopic } = this.options;
await super.start();
this.subscribe(this.pubSubTopic);
this.subscribe(pubSubTopic);
}

/**
* Send Waku message.
*/
public async send(encoder: IEncoder, message: IMessage): Promise<SendResult> {
const { pubSubTopic = DefaultPubSubTopic } = this.options;

const msg = await encoder.toWire(message);
if (!msg) {
log("Failed to encode message, aborting publish");
return { recipients: [] };
}
return this.publish(this.pubSubTopic, msg);

return this.publish(pubSubTopic, msg);
}

/**
Expand Down Expand Up @@ -181,7 +172,8 @@ class Relay extends GossipSub implements IRelay {
}

getMeshPeers(topic?: TopicStr): PeerIdStr[] {
return super.getMeshPeers(topic ?? this.pubSubTopic);
const { pubSubTopic = DefaultPubSubTopic } = this.options;
return super.getMeshPeers(topic ?? pubSubTopic);
}
}

Expand Down
33 changes: 11 additions & 22 deletions packages/core/src/lib/store/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
IDecoder,
Index,
IStore,
ProtocolCreateOptions,
} from "@waku/interfaces";
import {
getPeersForProtocol,
Expand Down Expand Up @@ -43,18 +44,6 @@ export interface StoreComponents {
connectionManager: ConnectionManager;
}

export interface CreateOptions {
/**
* The PubSub Topic to use. Defaults to {@link DefaultPubSubTopic}.
*
* The usage of the default pubsub topic is recommended.
* See [Waku v2 Topic Usage Recommendations](https://rfc.vac.dev/spec/23/) for details.
*
* @default {@link DefaultPubSubTopic}
*/
pubSubTopic?: string;
}

export interface TimeFilter {
startTime: Date;
endTime: Date;
Expand All @@ -65,11 +54,6 @@ export interface QueryOptions {
* The peer to query. If undefined, a pseudo-random peer is selected from the connected Waku Store peers.
*/
peerId?: PeerId;
/**
* The pubsub topic to pass to the query.
* See [Waku v2 Topic Usage Recommendations](https://rfc.vac.dev/spec/23/).
*/
pubSubTopic?: string;
/**
* The direction in which pages are retrieved:
* - { @link PageDirection.BACKWARD }: Most recent page first.
Expand Down Expand Up @@ -106,11 +90,14 @@ export interface QueryOptions {
*/
class Store implements IStore {
multicodec: string;
pubSubTopic: string;
options: ProtocolCreateOptions;

constructor(public components: StoreComponents, options?: CreateOptions) {
constructor(
public components: StoreComponents,
options?: ProtocolCreateOptions
) {
this.multicodec = StoreCodec;
this.pubSubTopic = options?.pubSubTopic ?? DefaultPubSubTopic;
this.options = options ?? {};
}

/**
Expand Down Expand Up @@ -221,6 +208,8 @@ class Store implements IStore {
decoders: IDecoder<T>[],
options?: QueryOptions
): AsyncGenerator<Promise<T | undefined>[]> {
const { pubSubTopic = DefaultPubSubTopic } = this.options;

let startTime, endTime;

if (options?.timeFilter) {
Expand All @@ -242,7 +231,7 @@ class Store implements IStore {

const queryOpts = Object.assign(
{
pubSubTopic: this.pubSubTopic,
pubSubTopic: pubSubTopic,
pageDirection: PageDirection.BACKWARD,
pageSize: DefaultPageSize,
},
Expand Down Expand Up @@ -433,7 +422,7 @@ export async function createCursor(
}

export function wakuStore(
init: Partial<CreateOptions> = {}
init: Partial<ProtocolCreateOptions> = {}
): (components: StoreComponents) => IStore {
return (components: StoreComponents) => new Store(components, init);
}
6 changes: 4 additions & 2 deletions packages/create/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ export async function createLightNode(
const store = wakuStore(options);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You have not removed the CreateOptions definition in this file. I expect it to be replaced with CreateProtocolOPtions from @waku/interfaces

const lightPush = wakuLightPush(options);
const filter = wakuFilter(options);
const peerExchange = wakuPeerExchange(options);

Copy link
Collaborator

Choose a reason for hiding this comment

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

odd

const peerExchange = wakuPeerExchange();

return new WakuNode(
options ?? {},
Expand Down Expand Up @@ -155,7 +156,8 @@ export async function createFullNode(
const store = wakuStore(options);
const lightPush = wakuLightPush(options);
const filter = wakuFilter(options);
const peerExchange = wakuPeerExchange(options);

const peerExchange = wakuPeerExchange();

return new WakuNode(
options ?? {},
Expand Down
20 changes: 19 additions & 1 deletion packages/interfaces/src/protocols.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,26 @@ export interface PointToPointProtocol {
peers: () => Promise<Peer[]>;
}

export type ProtocolOptions = {
export type ProtocolCreateOptions = {
/**
* The PubSub Topic to use. Defaults to {@link @waku/core/DefaultPubSubTopic }.
*
* One and only one pubsub topic is used by Waku. This is used by:
* - WakuRelay to receive, route and send messages,
* - WakuLightPush to send messages,
* - WakuStore to retrieve messages.
*
* The usage of the default pubsub topic is recommended.
* See [Waku v2 Topic Usage Recommendations](https://rfc.vac.dev/spec/23/) for details.
*
*/
pubSubTopic?: string;
};

//TODO
// we can probably move `peerId` into `ProtocolCreateOptions` and remove `ProtocolOptions` and pass it in the constructor
// however, filter protocol can use multiple peers, so we need to think about this
Comment on lines +37 to +38
Copy link
Collaborator

Choose a reason for hiding this comment

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

All protocol can use multiple peers. I don't think PeerId belongs to CreateOptions.

Do note that it makes sense to have PubSubTopic in ProtocolOptions (too). Let's see how Waku scaling goes:

For now I'd remove this todo and leave it as it is and we can decide later once we ahve more information on sharding and Waku usage.

export type ProtocolOptions = {
/**
* Optionally specify an PeerId for the protocol request. If not included, will use a random peer.
*/
Expand Down
Loading