-
Notifications
You must be signed in to change notification settings - Fork 447
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: configuration validation #1778
Changes from all commits
6ba3dd8
68f6e83
5874c4e
71490db
3aeb754
36f66ef
722dc7a
27a9789
73f9fc7
893e068
6c5f7fd
1e9b0e3
c70f7c1
dd17fcd
df1ba0e
f86432a
c3ed878
be8bd3a
6bd7bec
d4ec37a
8e23b59
083414f
0820b6f
83a5f0e
dbbcbbd
0d4c8e4
a89818a
264ec7c
b4fb523
6c7935a
6638573
e0080fb
d45851d
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 |
---|---|---|
|
@@ -24,7 +24,9 @@ export default { | |
const peerId = await createEd25519PeerId() | ||
const libp2p = await createLibp2p({ | ||
connectionManager: { | ||
inboundConnectionThreshold: Infinity, | ||
inboundConnectionThreshold: 1000, | ||
maxIncomingPendingConnections: 1000, | ||
maxConnections: 1000, | ||
Comment on lines
+28
to
+29
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. The default value for these was fine before, why do these now need to be set? 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. See my comment #1778 |
||
minConnections: 0 | ||
}, | ||
addresses: { | ||
|
@@ -51,7 +53,7 @@ export default { | |
fetch: fetchService(), | ||
relay: circuitRelayServer({ | ||
reservations: { | ||
maxReservations: Infinity | ||
maxReservations: 100000 | ||
} | ||
}) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,7 @@ import map from 'it-map' | |
import parallel from 'it-parallel' | ||
import { pipe } from 'it-pipe' | ||
import isPrivateIp from 'private-ip' | ||
import { number, object, string } from 'yup' | ||
import { codes } from '../errors.js' | ||
import { | ||
MAX_INBOUND_STREAMS, | ||
|
@@ -108,14 +109,23 @@ class DefaultAutoNATService implements Startable { | |
private started: boolean | ||
|
||
constructor (components: AutoNATComponents, init: AutoNATServiceInit) { | ||
const validatedConfig = object({ | ||
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. Please have config definitions outside of the class so we don't create new validators on every object instantiation: const configValidator = object({
protocolPrefix: string().default(PROTOCOL_PREFIX),
// ...etc
})
class DefaultAutoNATService implements Startable {
// ...etc
constructor (components: AutoNATComponents, init: AutoNATServiceInit) {
const config = configValidator.validateSync(init)
// ...etc
}
} |
||
protocolPrefix: string().default(PROTOCOL_PREFIX), | ||
timeout: number().integer().default(TIMEOUT), | ||
startupDelay: number().integer().default(STARTUP_DELAY), | ||
refreshInterval: number().integer().default(REFRESH_INTERVAL), | ||
maxInboundStreams: number().integer().default(MAX_INBOUND_STREAMS), | ||
maxOutboundStreams: number().integer().default(MAX_OUTBOUND_STREAMS) | ||
}).validateSync(init) | ||
|
||
this.components = components | ||
this.started = false | ||
this.protocol = `/${init.protocolPrefix ?? PROTOCOL_PREFIX}/${PROTOCOL_NAME}/${PROTOCOL_VERSION}` | ||
this.timeout = init.timeout ?? TIMEOUT | ||
this.maxInboundStreams = init.maxInboundStreams ?? MAX_INBOUND_STREAMS | ||
this.maxOutboundStreams = init.maxOutboundStreams ?? MAX_OUTBOUND_STREAMS | ||
this.startupDelay = init.startupDelay ?? STARTUP_DELAY | ||
this.refreshInterval = init.refreshInterval ?? REFRESH_INTERVAL | ||
this.protocol = `/${validatedConfig.protocolPrefix}/${PROTOCOL_NAME}/${PROTOCOL_VERSION}` | ||
this.timeout = validatedConfig.timeout | ||
this.maxInboundStreams = validatedConfig.maxInboundStreams | ||
this.maxOutboundStreams = validatedConfig.maxOutboundStreams | ||
this.startupDelay = validatedConfig.startupDelay | ||
this.refreshInterval = validatedConfig.refreshInterval | ||
this._verifyExternalAddresses = this._verifyExternalAddresses.bind(this) | ||
} | ||
|
||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import { FaultTolerance } from '@libp2p/interface/transport' | ||
import { publicAddressesFirst } from '@libp2p/utils/address-sort' | ||
import { dnsaddrResolver } from '@multiformats/multiaddr/resolvers' | ||
import mergeOptions from 'merge-options' | ||
import { object } from 'yup' | ||
import { validateAddressManagerConfig } from '../address-manager/utils.js' | ||
import { validateConnectionManagerConfig } from '../connection-manager/utils.js' | ||
import type { AddressManagerInit } from '../address-manager' | ||
import type { ConnectionManagerInit } from '../connection-manager/index.js' | ||
import type { Libp2pInit } from '../index.js' | ||
import type { ServiceMap, RecursivePartial } from '@libp2p/interface' | ||
|
||
const DefaultConfig: Partial<Libp2pInit> = { | ||
connectionManager: { | ||
resolvers: { | ||
dnsaddr: dnsaddrResolver | ||
}, | ||
addressSorter: publicAddressesFirst | ||
}, | ||
transportManager: { | ||
faultTolerance: FaultTolerance.FATAL_ALL | ||
} | ||
} | ||
|
||
export function validateConfig <T extends ServiceMap = Record<string, unknown>> (opts: RecursivePartial<Libp2pInit<T>>): Libp2pInit<T> { | ||
const libp2pConfig = object({ | ||
addresses: validateAddressManagerConfig(opts?.addresses as AddressManagerInit), | ||
connectionManager: validateConnectionManagerConfig(opts?.connectionManager as ConnectionManagerInit) | ||
}) | ||
|
||
if ((opts?.services) != null) { | ||
// @ts-expect-error until we resolve https://github.com/libp2p/js-libp2p/pull/1762 and have a better way of discovering type dependencies | ||
// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions | ||
if ((opts.services?.kadDHT || opts.services?.relay || opts.services?.ping) && !opts.services.identify) { | ||
throw new Error('identify service is required when using kadDHT, relay, or ping') | ||
} | ||
} | ||
|
||
const parsedOpts = libp2pConfig.validateSync(opts) | ||
|
||
const resultingOptions: Libp2pInit<T> = mergeOptions(DefaultConfig, parsedOpts) | ||
|
||
return resultingOptions | ||
} | ||
maschad marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import { multiaddr } from '@multiformats/multiaddr' | ||
|
||
export const validateMultiaddr = (value: Array<string | undefined> | undefined): boolean => { | ||
value?.forEach((addr) => { | ||
try { | ||
multiaddr(addr) | ||
} catch (err) { | ||
throw new Error(`invalid multiaddr: ${addr}`) | ||
} | ||
}) | ||
return true | ||
} |
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.
I take it yup doesn't accept
Infinity
as a number?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.
Correct