-
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: improve filter subscriptions #2193
base: weboko/peer-manager
Are you sure you want to change the base?
Changes from all commits
5147ee3
7eb67ff
09127df
2a5df25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,17 +12,12 @@ | |
type IMetadata, | ||
type Libp2p, | ||
type Libp2pComponents, | ||
type ProtocolCreateOptions, | ||
PubsubTopic | ||
} from "@waku/interfaces"; | ||
import { derivePubsubTopicsFromNetworkConfig, Logger } from "@waku/utils"; | ||
import { createLibp2p } from "libp2p"; | ||
|
||
import { | ||
CreateWakuNodeOptions, | ||
DefaultPingMaxInboundStreams, | ||
DefaultUserAgent | ||
} from "../waku/index.js"; | ||
|
||
import { defaultPeerDiscoveries } from "./discovery.js"; | ||
|
||
type MetadataService = { | ||
|
@@ -31,6 +26,9 @@ | |
|
||
const log = new Logger("sdk:create"); | ||
|
||
const DefaultUserAgent = "js-waku"; | ||
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. nit: we're using mostly SNAKE_CASE for constants |
||
const DefaultPingMaxInboundStreams = 10; | ||
|
||
export async function defaultLibp2p( | ||
pubsubTopics: PubsubTopic[], | ||
options?: Partial<CreateLibp2pOptions>, | ||
|
@@ -74,11 +72,11 @@ | |
...metadataService, | ||
...options?.services | ||
} | ||
}) as any as Libp2p; // TODO: make libp2p include it; | ||
Check warning on line 75 in packages/sdk/src/create/libp2p.ts GitHub Actions / check
|
||
} | ||
|
||
export async function createLibp2pAndUpdateOptions( | ||
options: CreateWakuNodeOptions | ||
options: ProtocolCreateOptions | ||
): Promise<{ libp2p: Libp2p; pubsubTopics: PubsubTopic[] }> { | ||
const { networkConfig } = options; | ||
const pubsubTopics = derivePubsubTopicsFromNetworkConfig( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,4 @@ | ||
export const DEFAULT_KEEP_ALIVE = 60_000; | ||
export const DEFAULT_MAX_PINGS = 3; | ||
export const DEFAULT_LIGHT_PUSH_FILTER_CHECK = false; | ||
export const DEFAULT_LIGHT_PUSH_FILTER_CHECK_INTERVAL = 10_000; | ||
|
||
export const DEFAULT_SUBSCRIBE_OPTIONS = { | ||
keepAlive: DEFAULT_KEEP_ALIVE, | ||
enableLightPushFilterCheck: DEFAULT_LIGHT_PUSH_FILTER_CHECK | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,20 @@ | ||
import { ConnectionManager, FilterCore } from "@waku/core"; | ||
import { | ||
type Callback, | ||
type CreateSubscriptionResult, | ||
type IAsyncIterator, | ||
type IDecodedMessage, | ||
type IDecoder, | ||
type IFilter, | ||
type ILightPush, | ||
type Libp2p, | ||
NetworkConfig, | ||
ProtocolError, | ||
type PubsubTopic, | ||
type SubscribeOptions, | ||
import type { | ||
Callback, | ||
CreateSubscriptionResult, | ||
FilterProtocolOptions, | ||
IAsyncIterator, | ||
IDecodedMessage, | ||
IDecoder, | ||
IFilter, | ||
ILightPush, | ||
IProtoMessage, | ||
Libp2p, | ||
PubsubTopic, | ||
SubscribeResult, | ||
type Unsubscribe | ||
Unsubscribe | ||
} from "@waku/interfaces"; | ||
import { NetworkConfig, ProtocolError } from "@waku/interfaces"; | ||
import { | ||
ensurePubsubTopicIsConfigured, | ||
groupByContentTopic, | ||
|
@@ -25,22 +25,29 @@ import { | |
|
||
import { PeerManager } from "../peer_manager.js"; | ||
|
||
import { DEFAULT_SUBSCRIBE_OPTIONS } from "./constants.js"; | ||
import { SubscriptionManager } from "./subscription_manager.js"; | ||
import { MessageCache } from "./message_cache.js"; | ||
import { Subscription } from "./subscription.js"; | ||
import { buildConfig } from "./utils.js"; | ||
|
||
const log = new Logger("sdk:filter"); | ||
|
||
class Filter implements IFilter { | ||
public readonly protocol: FilterCore; | ||
|
||
private activeSubscriptions = new Map<string, SubscriptionManager>(); | ||
private readonly config: FilterProtocolOptions; | ||
private readonly messageCache: MessageCache; | ||
private activeSubscriptions = new Map<string, Subscription>(); | ||
|
||
public constructor( | ||
private connectionManager: ConnectionManager, | ||
private libp2p: Libp2p, | ||
private peerManager: PeerManager, | ||
private lightPush?: ILightPush | ||
private lightPush?: ILightPush, | ||
config?: Partial<FilterProtocolOptions> | ||
) { | ||
this.config = buildConfig(config); | ||
this.messageCache = new MessageCache(libp2p); | ||
|
||
this.protocol = new FilterCore( | ||
async (pubsubTopic, wakuMessage, peerIdStr) => { | ||
const subscription = this.getActiveSubscription(pubsubTopic); | ||
|
@@ -50,6 +57,15 @@ class Filter implements IFilter { | |
); | ||
return; | ||
} | ||
|
||
if (this.messageCache.has(pubsubTopic, wakuMessage as IProtoMessage)) { | ||
log.info( | ||
`Skipping duplicate message for pubsubTopic:${pubsubTopic} peerId:${peerIdStr}` | ||
); | ||
return; | ||
} | ||
|
||
this.messageCache.set(pubsubTopic, wakuMessage as IProtoMessage); | ||
Comment on lines
+61
to
+68
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. nit: we can move this to a private function |
||
await subscription.processIncomingMessage(wakuMessage, peerIdStr); | ||
}, | ||
|
||
|
@@ -66,7 +82,6 @@ class Filter implements IFilter { | |
* | ||
* @param {IDecoder<T> | IDecoder<T>[]} decoders - A single decoder or an array of decoders to use for decoding messages. | ||
* @param {Callback<T>} callback - The callback function to be invoked with decoded messages. | ||
* @param {SubscribeOptions} [subscribeOptions=DEFAULT_SUBSCRIBE_OPTIONS] - Options for the subscription. | ||
* | ||
* @returns {Promise<SubscribeResult>} A promise that resolves to an object containing: | ||
* - subscription: The created subscription object if successful, or null if failed. | ||
|
@@ -100,8 +115,7 @@ class Filter implements IFilter { | |
*/ | ||
public async subscribe<T extends IDecodedMessage>( | ||
decoders: IDecoder<T> | IDecoder<T>[], | ||
callback: Callback<T>, | ||
subscribeOptions: SubscribeOptions = DEFAULT_SUBSCRIBE_OPTIONS | ||
callback: Callback<T> | ||
): Promise<SubscribeResult> { | ||
const uniquePubsubTopics = this.getUniquePubsubTopics(decoders); | ||
|
||
|
@@ -127,8 +141,7 @@ class Filter implements IFilter { | |
|
||
const { failures, successes } = await subscription.subscribe( | ||
decoders, | ||
callback, | ||
subscribeOptions | ||
callback | ||
); | ||
return { | ||
subscription, | ||
|
@@ -173,12 +186,13 @@ class Filter implements IFilter { | |
this.getActiveSubscription(pubsubTopic) ?? | ||
this.setActiveSubscription( | ||
pubsubTopic, | ||
new SubscriptionManager( | ||
new Subscription( | ||
pubsubTopic, | ||
this.protocol, | ||
this.connectionManager, | ||
this.peerManager, | ||
this.libp2p, | ||
this.config, | ||
this.lightPush | ||
) | ||
); | ||
|
@@ -206,8 +220,7 @@ class Filter implements IFilter { | |
*/ | ||
public async subscribeWithUnsubscribe<T extends IDecodedMessage>( | ||
decoders: IDecoder<T> | IDecoder<T>[], | ||
callback: Callback<T>, | ||
options: SubscribeOptions = DEFAULT_SUBSCRIBE_OPTIONS | ||
callback: Callback<T> | ||
): Promise<Unsubscribe> { | ||
const uniquePubsubTopics = this.getUniquePubsubTopics<T>(decoders); | ||
|
||
|
@@ -231,7 +244,7 @@ class Filter implements IFilter { | |
throw Error(`Failed to create subscription: ${error}`); | ||
} | ||
|
||
await subscription.subscribe(decoders, callback, options); | ||
await subscription.subscribe(decoders, callback); | ||
|
||
const contentTopics = Array.from( | ||
groupByContentTopic( | ||
|
@@ -250,17 +263,16 @@ class Filter implements IFilter { | |
return toAsyncIterator(this, decoders); | ||
} | ||
|
||
//TODO: move to SubscriptionManager | ||
private getActiveSubscription( | ||
pubsubTopic: PubsubTopic | ||
): SubscriptionManager | undefined { | ||
): Subscription | undefined { | ||
return this.activeSubscriptions.get(pubsubTopic); | ||
} | ||
|
||
private setActiveSubscription( | ||
pubsubTopic: PubsubTopic, | ||
subscription: SubscriptionManager | ||
): SubscriptionManager { | ||
subscription: Subscription | ||
): Subscription { | ||
this.activeSubscriptions.set(pubsubTopic, subscription); | ||
return subscription; | ||
} | ||
|
@@ -285,8 +297,9 @@ class Filter implements IFilter { | |
export function wakuFilter( | ||
connectionManager: ConnectionManager, | ||
peerManager: PeerManager, | ||
lightPush?: ILightPush | ||
lightPush?: ILightPush, | ||
config?: Partial<FilterProtocolOptions> | ||
): (libp2p: Libp2p) => IFilter { | ||
return (libp2p: Libp2p) => | ||
new Filter(connectionManager, libp2p, peerManager, lightPush); | ||
new Filter(connectionManager, libp2p, peerManager, lightPush, config); | ||
} |
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.
ProtocolCreateOptions
used for acreateNode
argument -- perhaps change naming/functionality here?