Skip to content

Commit

Permalink
fix: allow DHT self-query to time out (#2169)
Browse files Browse the repository at this point in the history
When we run our first self-query we first check the routing table to
see if it has peers.

If not we wait for some peers to be discovered before continuing.

The change here is to use the general query AbortSignal to abort
waiting for peers to be discovered.  If they are not discovered allow
other queries to continue and fail with eaiser to understand error
messages.

Also fixes some logging typos and makes error messages more understandable.
  • Loading branch information
achingbrain authored Oct 25, 2023
1 parent 6850493 commit ce0e38d
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 35 deletions.
12 changes: 1 addition & 11 deletions packages/kad-dht/src/dual-kad-dht.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ function multiaddrIsPublic (multiaddr: Multiaddr): boolean {

// dns4 or dns6 or dnsaddr
if (tuples[0][0] === DNS4_CODE || tuples[0][0] === DNS6_CODE || tuples[0][0] === DNSADDR_CODE) {
log('%m is public %s', multiaddr, true)

return true
}

Expand All @@ -113,8 +111,6 @@ function multiaddrIsPublic (multiaddr: Multiaddr): boolean {
const result = isPrivate(`${tuples[0][1]}`)
const isPublic = result == null || !result

log('%m is public %s', multiaddr, isPublic)

return isPublic
}

Expand Down Expand Up @@ -170,13 +166,7 @@ export class DefaultDualKadDHT extends EventEmitter<PeerDiscoveryEvents> impleme
components.events.addEventListener('self:peer:update', (evt) => {
log('received update of self-peer info')
const hasPublicAddress = evt.detail.peer.addresses
.some(({ multiaddr }) => {
const isPublic = multiaddrIsPublic(multiaddr)

log('%m is public %s', multiaddr, isPublic)

return isPublic
})
.some(({ multiaddr }) => multiaddrIsPublic(multiaddr))

this.getMode()
.then(async mode => {
Expand Down
24 changes: 14 additions & 10 deletions packages/kad-dht/src/query-self.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,6 @@ export class QuerySelf implements Startable {

this.querySelfPromise = pDefer()

if (this.routingTable.size === 0) {
// wait to discover at least one DHT peer
await pEvent(this.routingTable, 'peer:add')
}

if (this.started) {
this.controller = new AbortController()
const signal = anySignal([this.controller.signal, AbortSignal.timeout(this.queryTimeout)])
Expand All @@ -122,7 +117,16 @@ export class QuerySelf implements Startable {
} catch {} // fails on node < 15.4

try {
if (this.routingTable.size === 0) {
this.log('routing table was empty, waiting for some peers before running query')
// wait to discover at least one DHT peer
await pEvent(this.routingTable, 'peer:add', {
signal
})
}

this.log('run self-query, look for %d peers timing out after %dms', this.count, this.queryTimeout)
const start = Date.now()

const found = await pipe(
this.peerRouting.getClosestPeers(this.components.peerId.toBytes(), {
Expand All @@ -133,16 +137,16 @@ export class QuerySelf implements Startable {
async (source) => length(source)
)

this.log('self-query ran successfully - found %d peers', found)
this.log('self-query found %d peers in %dms', found, Date.now() - start)
} catch (err: any) {
this.log.error('self-query error', err)
} finally {
signal.clear()

if (this.initialQuerySelfHasRun != null) {
this.initialQuerySelfHasRun.resolve()
this.initialQuerySelfHasRun = undefined
}
} catch (err: any) {
this.log.error('self-query error', err)
} finally {
signal.clear()
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/kad-dht/src/record/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function bestRecord (selectors: Selectors, k: Uint8Array, records: Uint8A
const selector = selectors[parts[1].toString()]

if (selector == null) {
const errMsg = `Unrecognized key prefix: ${parts[1]}`
const errMsg = `No selector function configured for key type "${parts[1]}"`

throw new CodeError(errMsg, 'ERR_UNRECOGNIZED_KEY_PREFIX')
}
Expand Down
2 changes: 1 addition & 1 deletion packages/kad-dht/src/record/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export async function verifyRecord (validators: Validators, record: Libp2pRecord
const validator = validators[parts[1].toString()]

if (validator == null) {
const errMsg = 'Invalid record keytype'
const errMsg = `No validator available for key type "${parts[1]}"`

throw new CodeError(errMsg, 'ERR_INVALID_RECORD_KEY_TYPE')
}
Expand Down
12 changes: 3 additions & 9 deletions packages/kad-dht/test/record/selection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,20 @@ describe('selection', () => {
it('throws no records given when no records received', () => {
expect(
() => selection.bestRecord({}, uint8ArrayFromString('/'), [])
).to.throw(
/No records given/
)
).to.throw().with.property('code', 'ERR_NO_RECORDS_RECEIVED')
})

it('throws on missing selector in the record key', () => {
expect(
() => selection.bestRecord({}, uint8ArrayFromString('/'), records)
).to.throw(
/Record key does not have a selector function/
)
).to.throw().with.property('code', 'ERR_NO_SELECTOR_FUNCTION_FOR_RECORD_KEY')
})

it('throws on unknown key prefix', () => {
expect(
// @ts-expect-error invalid input
() => selection.bestRecord({ world () {} }, uint8ArrayFromString('/hello/'), records)
).to.throw(
/Unrecognized key prefix: hello/
)
).to.throw().with.property('code', 'ERR_UNRECOGNIZED_KEY_PREFIX')
})

it('returns the index from the matching selector', () => {
Expand Down
4 changes: 1 addition & 3 deletions packages/kad-dht/test/record/validator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ describe('validator', () => {
}
}
await expect(validator.verifyRecord(validators, rec))
.to.eventually.rejectedWith(
/Invalid record keytype/
)
.to.eventually.rejected.with.property('code', 'ERR_INVALID_RECORD_KEY_TYPE')
})
})

Expand Down

0 comments on commit ce0e38d

Please sign in to comment.