Skip to content

Commit

Permalink
refactor: blacklist to denylist (libp2p#341)
Browse files Browse the repository at this point in the history
"denylist" more adequately describes what this list is for.

BREAKING CHANGE: Constructor options `blacklistTTL` and `blackListAttempts` have been renamed to `denyTTL` and `denyAttempts`. The error code from errors thrown when dial is currently denied has changed from `ERR_BLACKLISTED` to `ERR_DENIED`.

License: MIT
Signed-off-by: Alan Shaw <[email protected]>
  • Loading branch information
Alan Shaw authored and jacobheun committed Aug 1, 2019
1 parent a386bb7 commit f64c73b
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 74 deletions.
7 changes: 2 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

### Bug Fixes

* clear blacklist for peer when connection is established ([#340](https://github.com/libp2p/js-libp2p-switch/issues/340)) ([f306cba](https://github.com/libp2p/js-libp2p-switch/commit/f306cba))
* clear denylist for peer when connection is established ([#340](https://github.com/libp2p/js-libp2p-switch/issues/340)) ([f306cba](https://github.com/libp2p/js-libp2p-switch/commit/f306cba))
* dont blindly add observed addresses to our list ([#337](https://github.com/libp2p/js-libp2p-switch/issues/337)) ([f879cfc](https://github.com/libp2p/js-libp2p-switch/commit/f879cfc))


Expand Down Expand Up @@ -72,7 +72,7 @@

### Bug Fixes

* dont blacklist good peers ([#319](https://github.com/libp2p/js-libp2p-switch/issues/319)) ([f31663f](https://github.com/libp2p/js-libp2p-switch/commit/f31663f))
* dont denylist good peers ([#319](https://github.com/libp2p/js-libp2p-switch/issues/319)) ([f31663f](https://github.com/libp2p/js-libp2p-switch/commit/f31663f))
* revert to try each ([#320](https://github.com/libp2p/js-libp2p-switch/issues/320)) ([805d1ad](https://github.com/libp2p/js-libp2p-switch/commit/805d1ad))


Expand Down Expand Up @@ -1067,6 +1067,3 @@ using the connection returned via the callback.

<a name="0.1.0"></a>
# 0.1.0 (2015-07-19)



5 changes: 2 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,8 @@ const sw = new switch(peerInfo , peerBook [, options])

If defined, `options` should be an object with the following keys and respective values:

- `blacklistTTL`: - number of ms a peer should not be dialable to after it errors. Each successive blacklisting will increase the ttl from the base value. Defaults to 5 minutes
- `blackListAttempts`: - number of blacklists before a peer
is permanently blacklisted. Defaults to 5.
- `denyTTL`: - number of ms a peer should not be dialable to after it errors. Each successive deny will increase the TTL from the base value. Defaults to 5 minutes
- `denyAttempts`: - number of times a peer can be denied before they are permanently denied. Defaults to 5.
- `maxParallelDials`: - number of concurrent dials the switch should allow. Defaults to `100`
- `maxColdCalls`: - number of queued cold calls that are allowed. Defaults to `50`
- `dialTimeout`: - number of ms a dial to a peer should be allowed to run. Defaults to `30000` (30 seconds)
Expand Down
8 changes: 4 additions & 4 deletions src/connection/manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ class ConnectionManager {
this.switch.emit('connection:start', connection.theirPeerInfo)
if (connection.getState() === 'MUXED') {
this.switch.emit('peer-mux-established', connection.theirPeerInfo)
// Clear the blacklist of the peer
this.switch.dialer.clearBlacklist(connection.theirPeerInfo)
// Clear the denylist of the peer
this.switch.dialer.clearDenylist(connection.theirPeerInfo)
} else {
connection.once('muxed', () => {
this.switch.emit('peer-mux-established', connection.theirPeerInfo)
// Clear the blacklist of the peer
this.switch.dialer.clearBlacklist(connection.theirPeerInfo)
// Clear the denylist of the peer
this.switch.dialer.clearDenylist(connection.theirPeerInfo)
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/constants.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
'use strict'

module.exports = {
BLACK_LIST_TTL: 5 * 60 * 1e3, // How long before an errored peer can be dialed again
BLACK_LIST_ATTEMPTS: 5, // Num of unsuccessful dials before a peer is permanently blacklisted
DENY_TTL: 5 * 60 * 1e3, // How long before an errored peer can be dialed again
DENY_ATTEMPTS: 5, // Num of unsuccessful dials before a peer is permanently denied
DIAL_TIMEOUT: 30e3, // How long in ms a dial attempt is allowed to take
MAX_COLD_CALLS: 50, // How many dials w/o protocols that can be queued
MAX_PARALLEL_DIALS: 100, // Maximum allowed concurrent dials
Expand Down
16 changes: 8 additions & 8 deletions src/dialer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
const DialQueueManager = require('./queueManager')
const getPeerInfo = require('../get-peer-info')
const {
BLACK_LIST_ATTEMPTS,
BLACK_LIST_TTL,
DENY_ATTEMPTS,
DENY_TTL,
MAX_COLD_CALLS,
MAX_PARALLEL_DIALS,
PRIORITY_HIGH,
Expand Down Expand Up @@ -59,11 +59,11 @@ module.exports = function (_switch) {
}

/**
* Clears the blacklist for a given peer
* Clears the denylist for a given peer
* @param {PeerInfo} peerInfo
*/
function clearBlacklist (peerInfo) {
dialQueueManager.clearBlacklist(peerInfo)
function clearDenylist (peerInfo) {
dialQueueManager.clearDenylist(peerInfo)
}

/**
Expand Down Expand Up @@ -110,9 +110,9 @@ module.exports = function (_switch) {
connect,
dial,
dialFSM,
clearBlacklist,
BLACK_LIST_ATTEMPTS: isNaN(_switch._options.blackListAttempts) ? BLACK_LIST_ATTEMPTS : _switch._options.blackListAttempts,
BLACK_LIST_TTL: isNaN(_switch._options.blacklistTTL) ? BLACK_LIST_TTL : _switch._options.blacklistTTL,
clearDenylist,
DENY_ATTEMPTS: isNaN(_switch._options.denyAttempts) ? DENY_ATTEMPTS : _switch._options.denyAttempts,
DENY_TTL: isNaN(_switch._options.denyTTL) ? DENY_TTL : _switch._options.denyTTL,
MAX_COLD_CALLS: isNaN(_switch._options.maxColdCalls) ? MAX_COLD_CALLS : _switch._options.maxColdCalls,
MAX_PARALLEL_DIALS: isNaN(_switch._options.maxParallelDials) ? MAX_PARALLEL_DIALS : _switch._options.maxParallelDials
}
Expand Down
38 changes: 19 additions & 19 deletions src/dialer/queue.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict'

const ConnectionFSM = require('../connection')
const { DIAL_ABORTED, ERR_BLACKLISTED } = require('../errors')
const { DIAL_ABORTED, ERR_DENIED } = require('../errors')
const nextTick = require('async/nextTick')
const once = require('once')
const debug = require('debug')
Expand Down Expand Up @@ -68,8 +68,8 @@ class Queue {
this.id = peerId
this.switch = _switch
this._queue = []
this.blackListed = null
this.blackListCount = 0
this.denylisted = null
this.denylistCount = 0
this.isRunning = false
this.onStopped = onStopped
}
Expand All @@ -86,7 +86,7 @@ class Queue {
*/
add (protocol, useFSM, callback) {
if (!this.isDialAllowed()) {
return nextTick(callback, ERR_BLACKLISTED())
return nextTick(callback, ERR_DENIED())
}
this._queue.push({ protocol, useFSM, callback })
}
Expand All @@ -96,10 +96,10 @@ class Queue {
* @returns {boolean}
*/
isDialAllowed () {
if (this.blackListed) {
// If the blacklist ttl has passed, reset it
if (Date.now() > this.blackListed) {
this.blackListed = null
if (this.denylisted) {
// If the deny ttl has passed, reset it
if (Date.now() > this.denylisted) {
this.denylisted = null
return true
}
// Dial is not allowed
Expand Down Expand Up @@ -146,25 +146,25 @@ class Queue {
}

/**
* Marks the queue as blacklisted. The queue will be immediately aborted.
* Marks the queue as denylisted. The queue will be immediately aborted.
* @returns {void}
*/
blacklist () {
this.blackListCount++
denylist () {
this.denylistCount++

if (this.blackListCount >= this.switch.dialer.BLACK_LIST_ATTEMPTS) {
this.blackListed = Infinity
if (this.denylistCount >= this.switch.dialer.DENY_ATTEMPTS) {
this.denylisted = Infinity
return
}

let ttl = this.switch.dialer.BLACK_LIST_TTL * Math.pow(this.blackListCount, 3)
let ttl = this.switch.dialer.DENY_TTL * Math.pow(this.denylistCount, 3)
const minTTL = ttl * 0.9
const maxTTL = ttl * 1.1

// Add a random jitter of 20% to the ttl
ttl = Math.floor(Math.random() * (maxTTL - minTTL) + minTTL)

this.blackListed = Date.now() + ttl
this.denylisted = Date.now() + ttl
this.abort()
}

Expand Down Expand Up @@ -244,11 +244,11 @@ class Queue {
// depending on the error.
connectionFSM.once('error', (err) => {
queuedDial.callback(err)
// Dont blacklist peers we have identified and that we are connected to
// Dont denylist peers we have identified and that we are connected to
if (peerInfo.protocols.size > 0 && peerInfo.isConnected()) {
return
}
this.blacklist()
this.denylist()
})

connectionFSM.once('close', () => {
Expand All @@ -257,14 +257,14 @@ class Queue {

// If we're not muxed yet, add listeners
connectionFSM.once('muxed', () => {
this.blackListCount = 0 // reset blacklisting on good connections
this.denylistCount = 0 // reset denylisting on good connections
queuedDial.connection = connectionFSM
createConnectionWithProtocol(queuedDial)
next()
})

connectionFSM.once('unmuxed', () => {
this.blackListCount = 0
this.denylistCount = 0
queuedDial.connection = connectionFSM
createConnectionWithProtocol(queuedDial)
next()
Expand Down
20 changes: 10 additions & 10 deletions src/dialer/queueManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,22 @@ class DialQueueManager {

/**
* Runs through all queues, aborts and removes them if they
* are no longer valid. A queue that is blacklisted indefinitely,
* are no longer valid. A queue that is denylisted indefinitely,
* is considered no longer valid.
* @private
*/
_clean () {
const queues = Object.values(this._queues)
queues.forEach(dialQueue => {
// Clear if the queue has reached max blacklist
if (dialQueue.blackListed === Infinity) {
// Clear if the queue has reached max denylist
if (dialQueue.denylisted === Infinity) {
dialQueue.abort()
delete this._queues[dialQueue.id]
return
}

// Keep track of blacklisted queues
if (dialQueue.blackListed) return
// Keep track of denylisted queues
if (dialQueue.denylisted) return

// Clear if peer is no longer active
// To avoid reallocating memory, dont delete queues of
Expand Down Expand Up @@ -125,7 +125,7 @@ class DialQueueManager {

// If we're already connected to the peer, start the queue now
// While it might cause queues to go over the max parallel amount,
// it avoids blocking peers we're already connected to
// it avoids denying peers we're already connected to
if (peerInfo.isConnected()) {
targetQueue.start()
return
Expand Down Expand Up @@ -184,13 +184,13 @@ class DialQueueManager {
}

/**
* Will remove the `peerInfo` from the dial blacklist
* Will remove the `peerInfo` from the dial denylist
* @param {PeerInfo} peerInfo
*/
clearBlacklist (peerInfo) {
clearDenylist (peerInfo) {
const queue = this.getQueue(peerInfo)
queue.blackListed = null
queue.blackListCount = 0
queue.denylisted = null
queue.denylistCount = 0
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const errCode = require('err-code')
module.exports = {
CONNECTION_FAILED: (err) => errCode(err, 'CONNECTION_FAILED'),
DIAL_ABORTED: () => errCode('Dial was aborted', 'DIAL_ABORTED'),
ERR_BLACKLISTED: () => errCode('Dial is currently blacklisted for this peer', 'ERR_BLACKLISTED'),
ERR_DENIED: () => errCode('Dial is currently denied for this peer', 'ERR_DENIED'),
DIAL_SELF: () => errCode('A node cannot dial itself', 'DIAL_SELF'),
INVALID_STATE_TRANSITION: (err) => errCode(err, 'INVALID_STATE_TRANSITION'),
NO_TRANSPORTS_REGISTERED: () => errCode('No transports registered, dial not possible', 'NO_TRANSPORTS_REGISTERED'),
Expand Down
2 changes: 1 addition & 1 deletion test/circuit-relay.node.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const utils = require('./utils')
const createInfos = utils.createInfos
const Swarm = require('../src')
const switchOptions = {
blacklistTTL: 0 // nullifies blacklisting
denyTTL: 0 // nullifies denylisting
}

describe(`circuit`, function () {
Expand Down
20 changes: 10 additions & 10 deletions test/dial-fsm.node.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ describe('dialFSM', () => {
protocol = '/error/1.0.0'
switchC.handle(protocol, () => { })

switchA.dialer.clearBlacklist(switchC._peerInfo)
switchA.dialer.clearDenylist(switchC._peerInfo)
switchA.dialFSM(switchC._peerInfo, protocol, (err, connFSM) => {
expect(err).to.not.exist()
connFSM.once('error', (err) => {
Expand All @@ -124,34 +124,34 @@ describe('dialFSM', () => {
})
})

it('should error when the peer is blacklisted', (done) => {
it('should error when the peer is denylisted', (done) => {
protocol = '/error/1.0.0'
switchC.handle(protocol, () => { })

switchA.dialer.clearBlacklist(switchC._peerInfo)
switchA.dialer.clearDenylist(switchC._peerInfo)
switchA.dialFSM(switchC._peerInfo, protocol, (err, connFSM) => {
expect(err).to.not.exist()
connFSM.once('error', () => {
// dial with the blacklist
// dial with the denylist
switchA.dialFSM(switchC._peerInfo, protocol, (err) => {
expect(err).to.exist()
expect(err.code).to.eql('ERR_BLACKLISTED')
expect(err.code).to.eql('ERR_DENIED')
done()
})
})
})
})

it('should not blacklist a peer that was successfully connected', (done) => {
protocol = '/noblacklist/1.0.0'
it('should not denylist a peer that was successfully connected', (done) => {
protocol = '/nodenylist/1.0.0'
switchB.handle(protocol, () => { })

switchA.dialer.clearBlacklist(switchB._peerInfo)
switchA.dialer.clearDenylist(switchB._peerInfo)
switchA.dialFSM(switchB._peerInfo, protocol, (err, connFSM) => {
expect(err).to.not.exist()
connFSM.once('connection', () => {
connFSM.once('close', () => {
// peer should not be blacklisted
// peer should not be denylisted
switchA.dialFSM(switchB._peerInfo, protocol, (err, conn) => {
expect(err).to.not.exist()
conn.once('close', done)
Expand All @@ -163,7 +163,7 @@ describe('dialFSM', () => {
})
})

it('should clear the blacklist for a peer that connected to us', (done) => {
it('should clear the denylist for a peer that connected to us', (done) => {
series([
// Attempt to dial the peer that's not listening
(cb) => switchC.dial(switchDialOnly._peerInfo, (err) => {
Expand Down
Loading

0 comments on commit f64c73b

Please sign in to comment.