diff --git a/packages/client/bin/cli.ts b/packages/client/bin/cli.ts index 5ef315cd4d..29f4a3df68 100755 --- a/packages/client/bin/cli.ts +++ b/packages/client/bin/cli.ts @@ -102,7 +102,8 @@ const args: ClientOpts = yargs(hideBin(process.argv)) default: true, }) .option('bootnodes', { - describe: 'Comma-separated list of network bootnodes', + describe: + 'Comma-separated list of network bootnodes (format: "enode://@,enode://..." ("[?discport=]" not supported) or path to a bootnode.txt file', array: true, }) .option('port', { @@ -811,7 +812,31 @@ async function run() { } logger = getLogger(args) - const bootnodes = args.bootnodes !== undefined ? parseMultiaddrs(args.bootnodes) : undefined + let bootnodes + if (args.bootnodes !== undefined) { + // File path passed, read bootnodes from disk + if ( + Array.isArray(args.bootnodes) && + args.bootnodes.length === 1 && + args.bootnodes[0].includes('.txt') + ) { + const file = readFileSync(args.bootnodes[0], 'utf-8') + let nodeURLs = file.split(/\r?\n/).filter((url) => (url !== '' ? true : false)) + nodeURLs = nodeURLs.map((url) => { + const discportIndex = url.indexOf('?discport') + if (discportIndex > 0) { + return url.substring(0, discportIndex) + } else { + return url + } + }) + bootnodes = parseMultiaddrs(nodeURLs) + logger.info(`Reading bootnodes file=${args.bootnodes[0]} num=${nodeURLs.length}`) + } else { + bootnodes = parseMultiaddrs(args.bootnodes) + } + } + const multiaddrs = args.multiaddrs !== undefined ? parseMultiaddrs(args.multiaddrs) : undefined const mine = args.mine !== undefined ? args.mine : args.dev !== undefined const isSingleNode = args.isSingleNode !== undefined ? args.isSingleNode : args.dev !== undefined diff --git a/packages/client/src/config.ts b/packages/client/src/config.ts index 52da3f3869..28b97568cb 100644 --- a/packages/client/src/config.ts +++ b/packages/client/src/config.ts @@ -518,7 +518,7 @@ export class Config { this.execCommon = common.copy() this.discDns = this.getDnsDiscovery(options.discDns) - this.discV4 = this.getV4Discovery(options.discV4) + this.discV4 = options.discV4 ?? true this.logger = options.logger ?? getLogger({ loglevel: 'error' }) @@ -672,16 +672,7 @@ export class Config { */ getDnsDiscovery(option: boolean | undefined): boolean { if (option !== undefined) return option - const dnsNets = ['goerli', 'sepolia'] + const dnsNets = ['holesky', 'sepolia'] return dnsNets.includes(this.chainCommon.chainName()) } - - /** - * Returns specified option or the default setting for whether v4 peer discovery - * is enabled based on chainName. `true` for `mainnet` - */ - getV4Discovery(option: boolean | undefined): boolean { - if (option !== undefined) return option - return this.chainCommon.chainName() === 'mainnet' - } } diff --git a/packages/client/src/net/server/rlpxserver.ts b/packages/client/src/net/server/rlpxserver.ts index 5f70bf761c..5382151500 100644 --- a/packages/client/src/net/server/rlpxserver.ts +++ b/packages/client/src/net/server/rlpxserver.ts @@ -216,6 +216,7 @@ export class RlpxServer extends Server { udpPort: null, tcpPort: null, }, + onlyConfirmed: this.config.chainCommon.chainName() === 'mainnet' ? false : true, shouldFindNeighbours: this.config.discV4, shouldGetDnsPeers: this.config.discDns, dnsRefreshQuantity: this.config.maxPeers, @@ -233,9 +234,16 @@ export class RlpxServer extends Server { resolve() }) + this.config.events.on(Event.PEER_CONNECTED, (peer) => { + this.dpt?.confirmPeer(peer.id) + }) + if (typeof this.config.port === 'number') { this.dpt.bind(this.config.port, '0.0.0.0') } + this.config.logger.info( + `Started discovery service discV4=${this.config.discV4} dns=${this.config.discDns} refreshInterval=${this.refreshInterval}` + ) }) } diff --git a/packages/client/src/net/server/server.ts b/packages/client/src/net/server/server.ts index b6ce4b942e..022572f19b 100644 --- a/packages/client/src/net/server/server.ts +++ b/packages/client/src/net/server/server.ts @@ -74,7 +74,7 @@ export class Server { const protocols: Protocol[] = Array.from(this.protocols) await Promise.all(protocols.map((p) => p.open())) this.started = true - this.config.logger.info(`Started ${this.name} server.`) + this.config.logger.info(`Started ${this.name} server maxPeers=${this.config.maxPeers}`) return true } diff --git a/packages/client/src/service/service.ts b/packages/client/src/service/service.ts index c3b1011875..44eac3a46d 100644 --- a/packages/client/src/service/service.ts +++ b/packages/client/src/service/service.ts @@ -198,6 +198,7 @@ export class Service { const heapUsed = Math.round(used_heap_size / 1000 / 1000) // MB const percentage = Math.round((100 * used_heap_size) / heap_size_limit) const msg = `Memory stats usage=${heapUsed} MB percentage=${percentage}%` + if (this._statsCounter % 4 === 0) { this.config.logger.info(msg) this._statsCounter = 0 diff --git a/packages/client/test/cli/cli.spec.ts b/packages/client/test/cli/cli.spec.ts index c92ad52012..1b88b06e4b 100644 --- a/packages/client/test/cli/cli.spec.ts +++ b/packages/client/test/cli/cli.spec.ts @@ -460,6 +460,21 @@ describe('[CLI]', () => { } await clientRunHelper(cliArgs, onData) }, 30000) + it('should start client with file path for bootnodes option', async () => { + const cliArgs = ['--bootnodes=./test/testdata/bootnode.txt'] + const onData = async ( + message: string, + child: ChildProcessWithoutNullStreams, + resolve: Function + ) => { + if (message.includes('Reading bootnodes')) { + assert.ok(message.includes('num=2'), 'passing bootnode.txt URL for bootnodes option works') + child.kill(9) + resolve(undefined) + } + } + await clientRunHelper(cliArgs, onData) + }, 30000) // test experimental feature options it('should start client when passed options for experimental features', async () => { const cliArgs = ['--mine=true', '--forceSnapSync=true', '--dev=poa', '--port=30393'] diff --git a/packages/client/test/config.spec.ts b/packages/client/test/config.spec.ts index 60dddddc9a..f894cf9064 100644 --- a/packages/client/test/config.spec.ts +++ b/packages/client/test/config.spec.ts @@ -36,17 +36,6 @@ describe('[Config]', () => { assert.equal(config.discV4, true, 'enables DNS peer discovery for mainnet') }) - it('peer discovery default testnet settings', () => { - let config - - for (const chain of [Chain.Goerli, Chain.Sepolia]) { - const common = new Common({ chain }) - config = new Config({ common }) - assert.equal(config.discDns, true, `enables DNS peer discovery for ${chain}`) - assert.equal(config.discV4, false, `disables V4 peer discovery for ${chain}`) - } - }) - it('--discDns=true/false', () => { let common, config, chain @@ -67,11 +56,11 @@ describe('[Config]', () => { chain = Chain.Mainnet common = new Common({ chain }) config = new Config({ common, discV4: false }) - assert.equal(config.discDns, false, `default discV4 setting can be overridden to false`) + assert.equal(config.discV4, false, `default discV4 setting can be overridden to false`) chain = Chain.Goerli common = new Common({ chain }) config = new Config({ common, discV4: true }) - assert.equal(config.discDns, true, `default discV4 setting can be overridden to true`) + assert.equal(config.discV4, true, `default discV4 setting can be overridden to true`) }) }) diff --git a/packages/client/test/testdata/bootnode.txt b/packages/client/test/testdata/bootnode.txt new file mode 100644 index 0000000000..8a8cc59af9 --- /dev/null +++ b/packages/client/test/testdata/bootnode.txt @@ -0,0 +1,2 @@ +enode://abc@127.0.0.1:30303 +enode://abc@127.0.0.1:30304 \ No newline at end of file diff --git a/packages/devp2p/src/dpt/dpt.ts b/packages/devp2p/src/dpt/dpt.ts index 4ec296d233..ae4e84ee17 100644 --- a/packages/devp2p/src/dpt/dpt.ts +++ b/packages/devp2p/src/dpt/dpt.ts @@ -1,4 +1,4 @@ -import { bytesToInt, randomBytes } from '@ethereumjs/util' +import { bytesToInt, bytesToUnprefixedHex, randomBytes } from '@ethereumjs/util' import { secp256k1 } from 'ethereum-cryptography/secp256k1.js' import { EventEmitter } from 'events' @@ -32,6 +32,9 @@ export class DPT { protected _dnsNetworks: string[] protected _dnsAddr: string + protected _onlyConfirmed: boolean + protected _confirmedPeers: Set + constructor(privateKey: Uint8Array, options: DPTOptions) { this.events = new EventEmitter() this._privateKey = privateKey @@ -46,6 +49,9 @@ export class DPT { this._dns = new DNS({ dnsServerAddress: this._dnsAddr }) this._banlist = new BanList() + this._onlyConfirmed = options.onlyConfirmed ?? false + this._confirmedPeers = new Set() + this._kbucket = new KBucket(this.id) this._kbucket.events.on('added', (peer: PeerInfo) => this.events.emit('peer:added', peer)) this._kbucket.events.on('removed', (peer: PeerInfo) => this.events.emit('peer:removed', peer)) @@ -118,6 +124,9 @@ export class DPT { async bootstrap(peer: PeerInfo): Promise { try { peer = await this.addPeer(peer) + if (peer.id !== undefined) { + this._confirmedPeers.add(bytesToUnprefixedHex(peer.id)) + } } catch (error: any) { this.events.emit('error', error) return @@ -148,6 +157,20 @@ export class DPT { } } + /** + * Add peer to a confirmed list of peers (peers meeting some + * level of quality, e.g. being on the same network) to allow + * for a more selective findNeighbours request and sending + * (with activated `onlyConfirmed` setting) + * + * @param id Unprefixed hex id + */ + confirmPeer(id: string) { + if (this._confirmedPeers.size < 5000) { + this._confirmedPeers.add(id) + } + } + getPeer(obj: string | Uint8Array | PeerInfo) { return this._kbucket.get(obj) } @@ -156,11 +179,25 @@ export class DPT { return this._kbucket.getAll() } + numPeers() { + return this._kbucket.getAll().length + } + getClosestPeers(id: Uint8Array) { - return this._kbucket.closest(id) + let peers = this._kbucket.closest(id) + if (this._onlyConfirmed && this._confirmedPeers.size > 0) { + peers = peers.filter((peer) => + this._confirmedPeers.has(bytesToUnprefixedHex(peer.id as Uint8Array)) ? true : false + ) + } + return peers } removePeer(obj: string | PeerInfo | Uint8Array) { + const peer = this._kbucket.get(obj) + if (peer?.id !== undefined) { + this._confirmedPeers.delete(bytesToUnprefixedHex(peer.id as Uint8Array)) + } this._kbucket.remove(obj) } @@ -187,7 +224,14 @@ export class DPT { // Randomly distributed selector based on peer ID // to decide on subdivided execution const selector = bytesToInt((peer.id as Uint8Array).subarray(0, 1)) % 10 - if (selector === this._refreshIntervalSelectionCounter) { + let confirmed = true + if (this._onlyConfirmed && this._confirmedPeers.size > 0) { + const id = bytesToUnprefixedHex(peer.id as Uint8Array) + if (!this._confirmedPeers.has(id)) { + confirmed = false + } + } + if (confirmed && selector === this._refreshIntervalSelectionCounter) { this._server.findneighbours(peer, randomBytes(64)) } } diff --git a/packages/devp2p/src/types.ts b/packages/devp2p/src/types.ts index 605c9af821..1e99e623ca 100644 --- a/packages/devp2p/src/types.ts +++ b/packages/devp2p/src/types.ts @@ -76,6 +76,18 @@ export interface DPTOptions { */ shouldFindNeighbours?: boolean + /** + * Send findNeighbour requests to and only answer with respective peers + * being confirmed by calling the `confirmPeer()` method + * + * (allows for a more selective and noise reduced discovery) + * + * Note: Bootstrap nodes are confirmed by default. + * + * Default: false + */ + onlyConfirmed?: boolean + /** * Toggles whether or not peers should be discovered by querying EIP-1459 DNS lists * diff --git a/packages/devp2p/test/dpt.spec.ts b/packages/devp2p/test/dpt.spec.ts new file mode 100644 index 0000000000..4f37791f5e --- /dev/null +++ b/packages/devp2p/test/dpt.spec.ts @@ -0,0 +1,108 @@ +import { hexToBytes } from '@ethereumjs/util' +import { afterEach, assert, describe, expect, it, vi } from 'vitest' + +import { DPT } from '../src/dpt/index.js' + +import type { PeerInfo } from '../src/types.js' + +describe('DPT', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + const privateKey1 = hexToBytes( + '0x012e930448c53e0b73edbbbc433e8a741e978cda79be2be039905f538d6247c2' + ) + + const peers: PeerInfo[] = [] + + for (let i = 1; i <= 5; i++) { + const id = new Uint8Array([i]) + const address = '127.0.0.1' + const udpPort = 5000 + i + const peer: PeerInfo = { + id, + address, + udpPort, + } + peers.push(peer) + } + + class Server { + ping() {} + findneighbours() {} + destroy() {} + } + + it('should initialize and add peers', async () => { + const dpt = new DPT(privateKey1, {}) + dpt['_server'] = new Server() as any + assert.equal(dpt['_dnsAddr'], '8.8.8.8', 'should initialize with default values') + + dpt['_server'].ping = vi.fn().mockResolvedValue(peers[0]) + await dpt.bootstrap(peers[0]) + assert.equal(dpt.numPeers(), 1, 'should add peer on bootstrap()') + + // Attention! Not all peers are called by default in refresh() + // (take into account on test design) + const spy = vi.spyOn(dpt['_server'], 'findneighbours') + await dpt.refresh() + expect(spy).toHaveBeenCalledTimes(1) + + dpt['_server'].ping = vi.fn().mockResolvedValue(peers[1]) + await dpt.addPeer(peers[1]) + assert.equal(dpt.numPeers(), 2, 'should add another peer on addPeer()') + + assert.equal( + dpt.getClosestPeers(peers[0].id!).length, + 2, + 'should return all peers on getClosestPeers()' + ) + + dpt.destroy() + }) + + it('should only call to confirmed peers on activated flag', async () => { + const dpt = new DPT(privateKey1, { onlyConfirmed: true }) + dpt['_server'] = new Server() as any + + dpt['_server'].ping = vi.fn().mockResolvedValue(peers[0]) + await dpt.addPeer(peers[0]) + + const spy = vi.spyOn(dpt['_server'], 'findneighbours') + await dpt.refresh() + expect( + spy, + 'call findneighbours on unconfirmed if no confirmed peers yet' + ).toHaveBeenCalledTimes(1) + + dpt['_refreshIntervalSelectionCounter'] = 0 + dpt.confirmPeer('01') + await dpt.refresh() + expect(spy, 'call findneighbours on confirmed').toHaveBeenCalledTimes(2) + + dpt['_server'].ping = vi.fn().mockResolvedValue(peers[1]) + await dpt.addPeer(peers[1]) + assert.equal( + dpt.getClosestPeers(peers[0].id!).length, + 1, + 'should not return unconfirmed on getClosestPeers()' + ) + + dpt.confirmPeer('02') + assert.equal( + dpt.getClosestPeers(peers[0].id!).length, + 2, + 'should return confirmed on getClosestPeers()' + ) + + dpt.removePeer(peers[1]) + assert.equal( + dpt.getClosestPeers(peers[0].id!).length, + 1, + 'should work after peers being removed' + ) + + dpt.destroy() + }) +})