Skip to content
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.

Sign up for GitHub

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

Update devp2p API names and access specifiers #2889

Merged
merged 25 commits into from
Jul 17, 2023
Merged

Update devp2p API names and access specifiers #2889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3437b24
Change access specifiers for RLPx _privateKey, _id, _debug, _timeout
scorbajio Jul 13, 2023
d6e994e
Renaming accessed id property in client tests
scorbajio Jul 13, 2023
edd692a
Change access specifiers for RLPx _maxPeers, _clientId, _remoteClient…
scorbajio Jul 13, 2023
640f52b
Ignore error message from reassigning readonly property in tests
scorbajio Jul 13, 2023
1b9f734
Change access specifiers for _common, _listenPort, and _dpt
scorbajio Jul 13, 2023
be2d41d
Change access specifiers for _peersLRU, _peersQueue, _server, _peers,…
scorbajio Jul 13, 2023
abcd71b
Ignore accessibility errors in examples
scorbajio Jul 16, 2023
666e077
Update names and access specifiers of Peer fields
scorbajio Jul 16, 2023
29849e9
Ignore access errors for _eciesSession
scorbajio Jul 16, 2023
991397c
Make common field public for Peer class
scorbajio Jul 16, 2023
b623ef0
Update names and access specifiers of Peer fields
scorbajio Jul 16, 2023
7fcc607
Make id public readonly
scorbajio Jul 16, 2023
584f5ef
Update names and access specifiers of Mac class fields
scorbajio Jul 16, 2023
8c73139
Update names and access specifiers of ECIES class fields
scorbajio Jul 16, 2023
12d266d
Update names and access specifiers of Peer class fields
scorbajio Jul 16, 2023
00c2cac
Ignore accessibility errors in examples
scorbajio Jul 16, 2023
aa10a2e
Update example
scorbajio Jul 16, 2023
40b15a1
Update names and access specifiers of class fields in protocol subpac…
scorbajio Jul 16, 2023
6c84f17
Update names and access specifiers of class fields in ext subpackage
scorbajio Jul 16, 2023
e8e7f22
Update names and access specifiers of class fields in dpt subpackage
scorbajio Jul 16, 2023
2250869
Update names and access specifiers of class fields in dns subpackage
scorbajio Jul 16, 2023
cea1c28
Merge branch 'master' into devp2p-fields-access-and-naming-cleanup
scorbajio Jul 16, 2023
34a1ab9
Fix name of accessed field
scorbajio Jul 16, 2023
cc708e2
Update tests
scorbajio Jul 16, 2023
9cfb5b8
Merge branch 'devp2p-fields-access-and-naming-cleanup' of github.com:…
scorbajio Jul 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/client/src/net/server/rlpxserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class RlpxServer extends Server {
ports: { discovery: this.config.port, listener: this.config.port },
}
}
const id = bytesToUnprefixedHex(this.rlpx._id)
const id = bytesToUnprefixedHex(this.rlpx.id)
return {
enode: `enode://${id}@${listenAddr}`,
id,
Expand Down Expand Up @@ -254,7 +254,9 @@ export class RlpxServer extends Server {
let peer: RlpxPeer | null = new RlpxPeer({
config: this.config,
id: bytesToUnprefixedHex(rlpxPeer.getId()!),
// @ts-ignore
host: rlpxPeer._socket.remoteAddress!,
// @ts-ignore
port: rlpxPeer._socket.remotePort!,
protocols: Array.from(this.protocols),
// @ts-ignore: Property 'server' does not exist on type 'Socket'.
Expand Down
12 changes: 9 additions & 3 deletions packages/client/test/net/server/rlpxserver.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ tape('[RlpxServer]', async (t) => {
;(server as any).rlpx = td.object({
destroy: td.func(),
})
server.rlpx!._id = hexToBytes('0x' + mockId)

// @ts-ignore
server.rlpx!.id = hexToBytes('0x' + mockId)
td.when(
server.dpt!.bootstrap({ address: '10.0.0.1', udpPort: 1234, tcpPort: 1234 })
).thenResolve(undefined)
Expand Down Expand Up @@ -172,7 +174,9 @@ tape('[RlpxServer]', async (t) => {
;(server as any).rlpx = td.object({
destroy: td.func(),
})
server.rlpx!._id = hexToBytes('0x' + mockId)

// @ts-ignore
server.rlpx!.id = hexToBytes('0x' + mockId)
td.when(
server.dpt!.bootstrap({ address: '10.0.0.1', udpPort: 1234, tcpPort: 1234 })
).thenResolve(undefined)
Expand Down Expand Up @@ -261,7 +265,9 @@ tape('[RlpxServer]', async (t) => {
;(server as any).peers.set('01', { id: '01' } as any)
server.rlpx!.emit('peer:removed', rlpxPeer)
server.rlpx!.emit('peer:error', rlpxPeer, new Error('err0'))
server.rlpx!._id = hexToBytes('0xff')

// @ts-ignore
server.rlpx!.id = hexToBytes('0xff')
server.rlpx!.emit('listening')
})

Expand Down
5 changes: 5 additions & 0 deletions packages/devp2p/examples/peer-communication-les.cts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const REMOTE_CLIENTID_FILTER = [
'prichain',
]

// @ts-ignore
const getPeerAddr = (peer: Peer) => `${peer._socket.remoteAddress}:${peer._socket.remotePort}`

// DPT
Expand Down Expand Up @@ -220,7 +221,11 @@ async function isValidBlock(block: Block) {
setInterval(() => {
const peersCount = dpt.getPeers().length
const openSlots = rlpx._getOpenSlots()

// @ts-ignore
const queueLength = rlpx._peersQueue.length

// @ts-ignore
const queueLength2 = rlpx._peersQueue.filter((o) => o.ts <= Date.now()).length

console.log(
Expand Down
5 changes: 5 additions & 0 deletions packages/devp2p/examples/peer-communication.cts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const CHECK_BLOCK_HEADER = RLP.decode(
'0xf90219a0d44a4d33e28d7ea9edd12b69bd32b394587eee498b0e2543ce2bad1877ffbeaca01dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347941ad91ee08f21be3de0ba2ba6918e714da6b45836a0fdec060ee45e55da9e36060fc95dddd0bdc47e447224666a895d9f0dc9adaa0ca0092d9fcc02ca9b372daec726704ce720d3aa366739868f4820ecaabadb9ac309a0974fee017515a46303f467b6fd50872994db1b0ea64d3455bad93ff9678aced9b90100356050004c5c89691add79838a01d4c302419252a4d3c96e9273908b7ee84660886c070607b4928c416a1800746a0d1dbb442d0baf06eea321422263726748600cc200e82aec08336863514d12d665718016989189c116bc0947046cc6718110586c11464a189000a11a41cc96991970153d88840768170244197e164c6204249b9091a0052ac85088c8108a4418dd2903690a036722623888ea14e90458a390a305a2342cb02766094f68c4100036330719848b48411614686717ab6068a46318204232429dc42020608802ceecd66c3c33a3a1fc6e82522049470328a4a81ba07c6604228ba94f008476005087a6804463696b41002650c0fdf548448a90408717ca31b6d618e883bad42083be153b83bdfbb1846078104798307834383639373636353666366532303530366636663663a0ae1de0acd35a98e211c7e276ad7524bb84a5e1b8d33dd7d1c052b095b564e8b888cca66773148b6e12'
)

// @ts-ignore
const getPeerAddr = (peer: Peer) => `${peer._socket.remoteAddress}:${peer._socket.remotePort}`

// DPT
Expand Down Expand Up @@ -371,7 +372,11 @@ async function isValidBlock(block: Block) {
setInterval(() => {
const peersCount = dpt.getPeers().length
const openSlots = rlpx._getOpenSlots()

// @ts-ignore
const queueLength = rlpx._peersQueue.length

// @ts-ignore
const queueLength2 = rlpx._peersQueue.filter((o) => o.ts <= Date.now()).length

console.log(
Expand Down
4 changes: 2 additions & 2 deletions packages/devp2p/src/dns/dns.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ type SearchContext = {
}

export class DNS {
private _DNSTreeCache: { [key: string]: string }
private readonly _errorTolerance: number = 10
protected _DNSTreeCache: { [key: string]: string }
protected readonly _errorTolerance: number = 10
scorbajio marked this conversation as resolved.
Show resolved Hide resolved

constructor(options: DNSOptions = {}) {
this._DNSTreeCache = {}
Expand Down
10 changes: 5 additions & 5 deletions packages/devp2p/src/dpt/ban-list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,19 @@ const debug = createDebugLogger('devp2p:dpt:ban-list')
const verbose = createDebugLogger('verbose').enabled

export class BanList {
private lru: LRUCache<string, boolean>
private _lru: LRUCache<string, boolean>
scorbajio marked this conversation as resolved.
Show resolved Hide resolved
constructor() {
this.lru = new LRU({ max: 10000 })
this._lru = new LRU({ max: 10000 })
}

add(obj: string | Uint8Array | PeerInfo, maxAge?: number) {
for (const key of KBucket.getKeys(obj)) {
this.lru.set(key, true, { ttl: maxAge })
debug(`Added peer ${formatLogId(key, verbose)}, size: ${this.lru.size}`)
this._lru.set(key, true, { ttl: maxAge })
debug(`Added peer ${formatLogId(key, verbose)}, size: ${this._lru.size}`)
}
}

has(obj: string | Uint8Array | PeerInfo): boolean {
return KBucket.getKeys(obj).some((key: string) => Boolean(this.lru.get(key)))
return KBucket.getKeys(obj).some((key: string) => Boolean(this._lru.get(key)))
}
}
60 changes: 30 additions & 30 deletions packages/devp2p/src/dpt/dpt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,43 +15,43 @@ import type { Debugger } from 'debug'
const DEBUG_BASE_NAME = 'dpt'

export class DPT extends EventEmitter {
privateKey: Uint8Array
banlist: BanList
dns: DNS
_debug: Debugger

private _id: Uint8Array | undefined
private _kbucket: KBucket
private _server: DPTServer
private _refreshIntervalId: NodeJS.Timeout
private _refreshIntervalSelectionCounter: number = 0
private _shouldFindNeighbours: boolean
private _shouldGetDnsPeers: boolean
private _dnsRefreshQuantity: number
private _dnsNetworks: string[]
private _dnsAddr: string
protected _privateKey: Uint8Array
protected _banlist: BanList
protected _dns: DNS
private _debug: Debugger

public readonly id: Uint8Array | undefined
protected _kbucket: KBucket
protected _server: DPTServer
protected _refreshIntervalId: NodeJS.Timeout
protected _refreshIntervalSelectionCounter: number = 0
protected _shouldFindNeighbours: boolean
protected _shouldGetDnsPeers: boolean
protected _dnsRefreshQuantity: number
protected _dnsNetworks: string[]
protected _dnsAddr: string

constructor(privateKey: Uint8Array, options: DPTOptions) {
super()

this.privateKey = privateKey
this._id = pk2id(secp256k1.getPublicKey(this.privateKey, false))
this._privateKey = privateKey
this.id = pk2id(secp256k1.getPublicKey(this._privateKey, false))
this._shouldFindNeighbours = options.shouldFindNeighbours ?? true
this._shouldGetDnsPeers = options.shouldGetDnsPeers ?? false
// By default, tries to connect to 12 new peers every 3s
this._dnsRefreshQuantity = Math.floor((options.dnsRefreshQuantity ?? 25) / 2)
this._dnsNetworks = options.dnsNetworks ?? []
this._dnsAddr = options.dnsAddr ?? '8.8.8.8'

this.dns = new DNS({ dnsServerAddress: this._dnsAddr })
this.banlist = new BanList()
this._dns = new DNS({ dnsServerAddress: this._dnsAddr })
this._banlist = new BanList()

this._kbucket = new KBucket(this._id)
this._kbucket = new KBucket(this.id)
this._kbucket.on('added', (peer: PeerInfo) => this.emit('peer:added', peer))
this._kbucket.on('removed', (peer: PeerInfo) => this.emit('peer:removed', peer))
this._kbucket.on('ping', this._onKBucketPing.bind(this))

this._server = new DPTServer(this, this.privateKey, {
this._server = new DPTServer(this, this._privateKey, {
timeout: options.timeout,
endpoint: options.endpoint,
createSocket: options.createSocket,
Expand Down Expand Up @@ -82,21 +82,21 @@ export class DPT extends EventEmitter {
}

_onKBucketPing(oldPeers: PeerInfo[], newPeer: PeerInfo): void {
if (this.banlist.has(newPeer)) return
if (this._banlist.has(newPeer)) return

let count = 0
let err: Error | null = null
for (const peer of oldPeers) {
this._server
.ping(peer)
.catch((_err: Error) => {
this.banlist.add(peer, 300000) // 5 min * 60 * 1000
this._banlist.add(peer, 300000) // 5 min * 60 * 1000
this._kbucket.remove(peer)
err = err ?? _err
})
.then(() => {
if (++count < oldPeers.length) return
if (err === null) this.banlist.add(newPeer, 300000) // 5 min * 60 * 1000
if (err === null) this._banlist.add(newPeer, 300000) // 5 min * 60 * 1000
else this._kbucket.add(newPeer)
})
}
Expand All @@ -122,14 +122,14 @@ export class DPT extends EventEmitter {
this.emit('error', error)
return
}
if (!this._id) return
if (!this.id) return
if (this._shouldFindNeighbours) {
this._server.findneighbours(peer, this._id)
this._server.findneighbours(peer, this.id)
}
}

async addPeer(obj: PeerInfo): Promise<PeerInfo> {
if (this.banlist.has(obj)) throw new Error('Peer is banned')
if (this._banlist.has(obj)) throw new Error('Peer is banned')
this._debug(`attempt adding peer ${obj.address}:${obj.udpPort}`)

// check k-bucket first
Expand All @@ -143,7 +143,7 @@ export class DPT extends EventEmitter {
this._kbucket.add(peer)
return peer
} catch (err: any) {
this.banlist.add(obj, 300000) // 5 min * 60 * 1000
this._banlist.add(obj, 300000) // 5 min * 60 * 1000
throw err
}
}
Expand All @@ -165,12 +165,12 @@ export class DPT extends EventEmitter {
}

banPeer(obj: string | PeerInfo | Uint8Array, maxAge?: number) {
this.banlist.add(obj, maxAge)
this._banlist.add(obj, maxAge)
this._kbucket.remove(obj)
}

async getDnsPeers(): Promise<PeerInfo[]> {
return this.dns.getPeers(this._dnsRefreshQuantity, this._dnsNetworks)
return this._dns.getPeers(this._dnsRefreshQuantity, this._dnsNetworks)
}

async refresh(): Promise<void> {
Expand Down
4 changes: 2 additions & 2 deletions packages/devp2p/src/dpt/kbucket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ const KBUCKET_SIZE = 16
const KBUCKET_CONCURRENCY = 3

export class KBucket extends EventEmitter {
_peers: Map<string, PeerInfo> = new Map()
_kbucket: _KBucket
protected _peers: Map<string, PeerInfo> = new Map()
protected _kbucket: _KBucket
constructor(localNodeId: Uint8Array) {
super()

Expand Down
16 changes: 8 additions & 8 deletions packages/devp2p/src/dpt/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ const verbose = createDebugLogger('verbose').enabled
const VERSION = 0x04

export class Server extends EventEmitter {
_dpt: DPT
_privateKey: Uint8Array
_timeout: number
_endpoint: PeerInfo
_requests: Map<string, any>
_requestsCache: LRUCache<string, Promise<any>>
_socket: DgramSocket | null
_debug: Debugger
protected _dpt: DPT
protected _privateKey: Uint8Array
protected _timeout: number
protected _endpoint: PeerInfo
protected _requests: Map<string, any>
protected _requestsCache: LRUCache<string, Promise<any>>
protected _socket: DgramSocket | null
private _debug: Debugger
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely not practically relevant and only a small nit, but I would think that it would make sense to just not distinguish between "even more internal" or so variables and just also make _debug protected, maybe someone wants to customize the devp2p package and add own debug messages, whatever.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was on the fence about this one, but since debuggers are usually initialized specific to a class, I opted for private.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The protected version is also remaining in the class scope, so this is just a bit broader so that people can e.g. subclass the Server class, e.g. for a custom binance devp2p implementation (super random example) or whatever. And then on the subclass one might want to adjust some properties and it is just a bit unlucky if these are then not accessible. So at some point in time [tm] we generally decided to favor protected over private, since it (more or less) achieves the same thing and has fewer side effects.


constructor(dpt: DPT, privateKey: Uint8Array, options: DPTServerOptions) {
super()
Expand Down
Loading