Skip to content

Commit

Permalink
refactor!: remove isStarted method from Startable (#2145)
Browse files Browse the repository at this point in the history
We have an `isStarted` method on the `Startable` interface but we only
really use it in tests. Our implementations tend to guard on being
started twice so it just adds noise to every implementation.

BREAKING CHANGE: the `isStarted` method has been removed from the `Startable` interface
  • Loading branch information
achingbrain committed Nov 3, 2023
1 parent 9f2ed20 commit 88b3f2e
Show file tree
Hide file tree
Showing 9 changed files with 169 additions and 245 deletions.
2 changes: 0 additions & 2 deletions packages/interface-compliance-tests/src/pubsub/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export default (common: TestSetup<PubSub, PubSubArgs>): void => {

await start(...Object.values(components))

expect(pubsub.isStarted()).to.equal(true)
expect(components.registrar.register).to.have.property('callCount', 1)
})

Expand All @@ -62,7 +61,6 @@ export default (common: TestSetup<PubSub, PubSubArgs>): void => {
await start(...Object.values(components))
await stop(...Object.values(components))

expect(pubsub.isStarted()).to.equal(false)
expect(components.registrar.unregister).to.have.property('callCount', 1)
})

Expand Down
2 changes: 0 additions & 2 deletions packages/interface/src/startable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
* Implemented by components that have a lifecycle
*/
export interface Startable {
isStarted(): boolean

/**
* If implemented, this method will be invoked before the start method.
*
Expand Down
47 changes: 0 additions & 47 deletions packages/libp2p/test/core/start.spec.ts

This file was deleted.

2 changes: 0 additions & 2 deletions packages/libp2p/test/identify/service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ describe('identify', () => {

// Wait for identify to finish
await identityServiceIdentifySpy.firstCall.returnValue
sinon.stub(libp2p, 'isStarted').returns(true)

// Cause supported protocols to change
await libp2p.handle('/echo/2.0.0', () => {})
Expand Down Expand Up @@ -251,7 +250,6 @@ describe('identify', () => {

// Wait for identify to finish
await identityServiceIdentifySpy.firstCall.returnValue
sinon.stub(libp2p, 'isStarted').returns(true)

await libp2p.peerStore.merge(libp2p.peerId, {
multiaddrs: [multiaddr('/ip4/180.0.0.1/tcp/15001/ws')]
Expand Down
3 changes: 0 additions & 3 deletions packages/libp2p/test/transports/transport-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ describe('libp2p.transportManager (dial only)', () => {
start: false
})

expect(libp2p.isStarted()).to.be.false()
await expect(libp2p.start()).to.eventually.be.rejected
.with.property('code', ErrorCodes.ERR_NO_VALID_ADDRESSES)
})
Expand All @@ -147,7 +146,6 @@ describe('libp2p.transportManager (dial only)', () => {
start: false
})

expect(libp2p.isStarted()).to.be.false()
await expect(libp2p.start()).to.eventually.be.undefined()
})

Expand All @@ -169,7 +167,6 @@ describe('libp2p.transportManager (dial only)', () => {
start: false
})

expect(libp2p.isStarted()).to.be.false()
await expect(libp2p.start()).to.eventually.be.undefined()
})
})
183 changes: 3 additions & 180 deletions packages/peer-discovery-mdns/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,186 +76,9 @@
* ```
*/

import { CustomEvent, TypedEventEmitter } from '@libp2p/interface/events'
import { peerDiscovery } from '@libp2p/interface/peer-discovery'
import { logger } from '@libp2p/logger'
import multicastDNS from 'multicast-dns'
import * as query from './query.js'
import { stringGen } from './utils.js'
import type { PeerDiscovery, PeerDiscoveryEvents } from '@libp2p/interface/peer-discovery'
import type { PeerInfo } from '@libp2p/interface/peer-info'
import type { Startable } from '@libp2p/interface/src/startable.js'
import type { AddressManager } from '@libp2p/interface-internal/address-manager'

const log = logger('libp2p:mdns')

export interface MulticastDNSInit {
/**
* (true/false) announce our presence through mDNS, default `true`
*/
broadcast?: boolean

/**
* query interval, default 10 \* 1000 (10 seconds)
*/
interval?: number

/**
* name of the service announce , default '_p2p._udp.local\`
*/
serviceTag?: string
/**
* Peer name to announce (should not be peeer id), default random string
*/
peerName?: string

/**
* UDP port to broadcast to
*/
port?: number

/**
* UDP IP to broadcast to
*/
ip?: string
}

export interface MulticastDNSComponents {
addressManager: AddressManager
}

class MulticastDNS extends TypedEventEmitter<PeerDiscoveryEvents> implements PeerDiscovery, Startable {
public mdns?: multicastDNS.MulticastDNS

private readonly broadcast: boolean
private readonly interval: number
private readonly serviceTag: string
private readonly peerName: string
private readonly port: number
private readonly ip: string
private _queryInterval: ReturnType<typeof setInterval> | null
private readonly components: MulticastDNSComponents

constructor (components: MulticastDNSComponents, init: MulticastDNSInit = {}) {
super()

this.broadcast = init.broadcast !== false
this.interval = init.interval ?? (1e3 * 10)
this.serviceTag = init.serviceTag ?? '_p2p._udp.local'
this.ip = init.ip ?? '224.0.0.251'
this.peerName = init.peerName ?? stringGen(63)
// 63 is dns label limit
if (this.peerName.length >= 64) {
throw new Error('Peer name should be less than 64 chars long')
}
this.port = init.port ?? 5353
this.components = components
this._queryInterval = null
this._onMdnsQuery = this._onMdnsQuery.bind(this)
this._onMdnsResponse = this._onMdnsResponse.bind(this)
this._onMdnsWarning = this._onMdnsWarning.bind(this)
this._onMdnsError = this._onMdnsError.bind(this)
}

readonly [peerDiscovery] = this

readonly [Symbol.toStringTag] = '@libp2p/mdns'

isStarted (): boolean {
return Boolean(this.mdns)
}

/**
* Start sending queries to the LAN.
*
* @returns {void}
*/
async start (): Promise<void> {
if (this.mdns != null) {
return
}

this.mdns = multicastDNS({ port: this.port, ip: this.ip })
this.mdns.on('query', this._onMdnsQuery)
this.mdns.on('response', this._onMdnsResponse)
this.mdns.on('warning', this._onMdnsWarning)
this.mdns.on('error', this._onMdnsError)

this._queryInterval = query.queryLAN(this.mdns, this.serviceTag, this.interval)
}

_onMdnsQuery (event: multicastDNS.QueryPacket): void {
if (this.mdns == null) {
return
}

log.trace('received incoming mDNS query')
query.gotQuery(
event,
this.mdns,
this.peerName,
this.components.addressManager.getAddresses(),
this.serviceTag,
this.broadcast)
}

_onMdnsResponse (event: multicastDNS.ResponsePacket): void {
log.trace('received mDNS query response')

try {
const foundPeer = query.gotResponse(event, this.peerName, this.serviceTag)

if (foundPeer != null) {
log('discovered peer in mDNS query response %p', foundPeer.id)

this.dispatchEvent(new CustomEvent<PeerInfo>('peer', {
detail: foundPeer
}))
}
} catch (err) {
log.error('Error processing peer response', err)
}
}

_onMdnsWarning (err: Error): void {
log.error('mdns warning', err)
}

_onMdnsError (err: Error): void {
log.error('mdns error', err)
}

/**
* Stop sending queries to the LAN.
*
* @returns {Promise}
*/
async stop (): Promise<void> {
if (this.mdns == null) {
return
}

this.mdns.removeListener('query', this._onMdnsQuery)
this.mdns.removeListener('response', this._onMdnsResponse)
this.mdns.removeListener('warning', this._onMdnsWarning)
this.mdns.removeListener('error', this._onMdnsError)

if (this._queryInterval != null) {
clearInterval(this._queryInterval)
this._queryInterval = null
}

await new Promise<void>((resolve) => {
if (this.mdns != null) {
this.mdns.destroy(resolve)
} else {
resolve()
}
})

this.mdns = undefined
}
}
import { MulticastDNS } from './mdns.js'
import type { MulticastDNSInit, MulticastDNSComponents } from './mdns.js'
import type { PeerDiscovery } from '@libp2p/interface/peer-discovery'

export function mdns (init: MulticastDNSInit = {}): (components: MulticastDNSComponents) => PeerDiscovery {
return (components: MulticastDNSComponents) => new MulticastDNS(components, init)
Expand Down
Loading

0 comments on commit 88b3f2e

Please sign in to comment.