Skip to content

Commit

Permalink
fix: interpret circuit relay expiry as seconds (#1636)
Browse files Browse the repository at this point in the history
Spec update: libp2p/specs#531

Fixes #1635 which causes the circuit relay to repeatedly connect
to a discovered relay. If the relay returns connection expiration in
seconds, we get a negative ttl when calculating 
`expiration - new Date().getTime()`.

This caused the `addRelay` function to set the timeout at 0ms which
triggers instantly.

---------

Co-authored-by: achingbrain <alex@achingbrain.net>
  • Loading branch information
ckousik and achingbrain committed Mar 21, 2023
1 parent 4d47333 commit 5de0f07
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 8 deletions.
14 changes: 10 additions & 4 deletions src/circuit/transport/reservation-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { ConnectionManager } from '@libp2p/interface-connection-manager'
import type { Connection } from '@libp2p/interface-connection'
import type { Reservation } from '../pb/index.js'
import { HopMessage, Status } from '../pb/index.js'
import { getExpiration } from '../utils.js'
import { getExpirationMilliseconds } from '../utils.js'
import type { TransportManager } from '@libp2p/interface-transport'
import type { Startable } from '@libp2p/interfaces/dist/src/startable.js'
import { CustomEvent, EventEmitter } from '@libp2p/interfaces/events'
Expand All @@ -23,6 +23,9 @@ const REFRESH_WINDOW = (60 * 1000) * 10
// try to refresh relay reservations 5 minutes before expiry
const REFRESH_TIMEOUT = (60 * 1000) * 5

// minimum duration before which a reservation must not be refreshed
const REFRESH_TIMEOUT_MIN = 30 * 1000

export interface RelayStoreComponents {
peerId: PeerId
connectionManager: ConnectionManager
Expand Down Expand Up @@ -131,7 +134,7 @@ export class ReservationStore extends EventEmitter<ReservationStoreEvents> imple
const existingReservation = this.reservations.get(peerId)

if (existingReservation != null) {
if (getExpiration(existingReservation.reservation.expire) > REFRESH_WINDOW) {
if (getExpirationMilliseconds(existingReservation.reservation.expire) > REFRESH_WINDOW) {
log('already have reservation on relay peer %p and it expires in more than 10 minutes', peerId)
return
}
Expand Down Expand Up @@ -162,13 +165,16 @@ export class ReservationStore extends EventEmitter<ReservationStoreEvents> imple

log('created reservation on relay peer %p', peerId)

const expiration = getExpiration(reservation.expire)
const expiration = getExpirationMilliseconds(reservation.expire)

// sets a lower bound on the timeout
const timeoutDuration = Math.max(expiration - REFRESH_TIMEOUT, REFRESH_TIMEOUT_MIN)

const timeout = setTimeout(() => {
this.addRelay(peerId, type).catch(err => {
log.error('could not refresh reservation to relay %p', peerId, err)
})
}, Math.max(expiration - REFRESH_TIMEOUT, 0))
}, timeoutDuration)

this.reservations.set(peerId, {
timeout,
Expand Down
8 changes: 6 additions & 2 deletions src/circuit/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ export async function namespaceToCid (namespace: string): Promise<CID> {
/**
* returns number of ms between now and expiration time
*/
export function getExpiration (expireTime: bigint): number {
return Number(expireTime) - new Date().getTime()
export function getExpirationMilliseconds (expireTimeSeconds: bigint): number {
const expireTimeMillis = expireTimeSeconds * BigInt(1000)
const currentTime = new Date().getTime()

// downcast to number to use with setTimeout
return Number(expireTimeMillis - BigInt(currentTime))
}
4 changes: 2 additions & 2 deletions test/circuit/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { mockStream } from '@libp2p/interface-mocks'
import { expect } from 'aegir/chai'
import { createLimitedRelay, getExpiration, namespaceToCid } from '../../src/circuit/utils.js'
import { createLimitedRelay, getExpirationMilliseconds, namespaceToCid } from '../../src/circuit/utils.js'
import { fromString as uint8arrayFromString } from 'uint8arrays/from-string'
import delay from 'delay'
import drain from 'it-drain'
Expand Down Expand Up @@ -209,7 +209,7 @@ describe('circuit-relay utils', () => {
it('should get expiration time', () => {
const delta = 10
const time = BigInt(Date.now() + delta)
const expiration = getExpiration(time)
const expiration = getExpirationMilliseconds(time)

expect(expiration).to.be.above(delta / 2)
})
Expand Down

0 comments on commit 5de0f07

Please sign in to comment.