-
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?
Conversation
size-limit report 📦
|
export async function createLightNode( | ||
options: CreateWakuNodeOptions = {} | ||
options: ProtocolCreateOptions = {} |
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 a createNode
argument -- perhaps change naming/functionality here?
@@ -31,6 +26,9 @@ type MetadataService = { | |||
|
|||
const log = new Logger("sdk:create"); | |||
|
|||
const DefaultUserAgent = "js-waku"; |
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.
nit: we're using mostly SNAKE_CASE for constants
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); |
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.
nit: we can move this to a private function
type Timestamp = number; | ||
|
||
export class MessageCache { | ||
private intervalID: number | undefined = undefined; |
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.
nit:
private intervalID: number | undefined = undefined; | |
private intervalId: number | undefined = undefined; |
log.info( | ||
`requestRenew: Renewed peer ${peerId.toString()} to ${newPeer.id.toString()}` | ||
); |
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.
can we provide a potential reason of why this could've failed?
Possibly no peers available to renew with?
Problem
During debugging sessions our team had we determined points for improvement for
Filter/Subscription
entities.It was clear that they can be improved with better handling of configuration and peer management.
Solution
This PR improves configuration handling and
PeerManager
as well as adds timedMessageCache
.Notes
This PR is second to #2191