-
Notifications
You must be signed in to change notification settings - Fork 773
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
Rework rlpx-simulator.spec.ts tests / Fix devp2p/genesis/wallet Coverage Reporting #3760
Changes from 10 commits
58523f9
29fa66c
e23680b
055f677
7ba4cae
c4a04b5
414be15
3add80e
79ea020
83422fb
e21afb5
8cffe14
c105b39
774bd6d
492c062
ed6a457
6980be5
98201ce
74da3dc
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 |
---|---|---|
|
@@ -6,76 +6,63 @@ import { DISCONNECT_REASON } from '../../src/types.js' | |
import * as util from './util.js' | ||
|
||
describe('RLPx simulator tests', () => { | ||
it('RLPX: add working node', () => { | ||
it('RLPX: add working node', async () => { | ||
const basePort = 40404 | ||
const rlpxs = util.initTwoPeerRLPXSetup(undefined, undefined, undefined, basePort) | ||
|
||
rlpxs[0].events.on('peer:added', async (peer) => { | ||
assert.equal( | ||
peer['_port'], | ||
basePort + 1, | ||
'should have added peer on peer:added after successful handshake', | ||
) | ||
assert.equal(rlpxs[0].getPeers().length, 1, 'peer list length should be 1') | ||
assert.equal(rlpxs[0]._getOpenSlots(), 9, 'should have maxPeers - 1 open slots left') | ||
await util.delay(500) | ||
util.destroyRLPXs(rlpxs) | ||
const { rlpxs, peer } = util.initTwoPeerRLPXSetup(undefined, undefined, undefined, basePort + 1) | ||
rlpxs[0]['_dpt']!.addPeer(peer).catch(() => { | ||
throw new Error('Peering failed') | ||
}) | ||
}) | ||
|
||
it('RLPX: ban node with missing tcp port', () => { | ||
const rlpxs = util.initTwoPeerRLPXSetup(undefined, undefined, undefined, 40444) | ||
|
||
rlpxs[0].events.on('peer:added', async () => { | ||
const peer = { | ||
id: hexToBytes('0xabcd'), | ||
address: '127.0.0.1', | ||
udpPort: 30308, | ||
tcpPort: null, | ||
} | ||
assert.notOk( | ||
rlpxs[0]['_dpt']!['_banlist'].has(peer), | ||
'should not be in ban list before bad peer discovered', | ||
) | ||
rlpxs[0]['_dpt']!.events.emit('peer:new', peer) | ||
assert.ok( | ||
rlpxs[0]['_dpt']!['_banlist'].has(peer), | ||
'should be in ban list after bad peer discovered', | ||
) | ||
await util.delay(500) | ||
util.destroyRLPXs(rlpxs) | ||
await new Promise((resolve) => { | ||
rlpxs[0].events.on('peer:added', async (peer) => { | ||
assert.equal( | ||
peer['_port'], | ||
basePort + 1, | ||
'should have added peer on peer:added after successful handshake', | ||
) | ||
assert.equal(rlpxs[0].getPeers().length, 1, 'peer list length should be 1') | ||
assert.equal(rlpxs[0]._getOpenSlots(), 9, 'should have maxPeers - 1 open slots left') | ||
await util.delay(500) | ||
util.destroyRLPXs(rlpxs) | ||
resolve(undefined) | ||
}) | ||
}) | ||
}) | ||
|
||
it('RLPX: remove node', () => { | ||
const rlpxs = util.initTwoPeerRLPXSetup(undefined, undefined, undefined, 40504) | ||
|
||
try { | ||
rlpxs[0].events.once('peer:added', (peer) => { | ||
rlpxs[0].disconnect(peer['_remoteId']) | ||
}) | ||
rlpxs[0].events.once('peer:removed', async (_, reason: any) => { | ||
assert.equal( | ||
reason, | ||
DISCONNECT_REASON.CLIENT_QUITTING, | ||
'should close with CLIENT_QUITTING disconnect reason', | ||
it('RLPX: ban node with missing tcp port', async () => { | ||
const { rlpxs, peer } = util.initTwoPeerRLPXSetup(undefined, undefined, undefined, 40444) | ||
rlpxs[0]['_dpt']!.addPeer(peer).catch(() => { | ||
throw new Error('Peering failed') | ||
}) | ||
await new Promise((resolve) => { | ||
rlpxs[0].events.on('peer:added', async () => { | ||
const peer = { | ||
id: hexToBytes('0xabcd'), | ||
address: '127.0.0.1', | ||
udpPort: 30308, | ||
tcpPort: null, | ||
} | ||
assert.notOk( | ||
rlpxs[0]['_dpt']!['_banlist'].has(peer), | ||
'should not be in ban list before bad peer discovered', | ||
) | ||
rlpxs[0]['_dpt']!.events.emit('peer:new', peer) | ||
assert.ok( | ||
rlpxs[0]['_dpt']!['_banlist'].has(peer), | ||
'should be in ban list after bad peer discovered', | ||
) | ||
assert.equal(rlpxs[0]._getOpenSlots(), 10, 'should have maxPeers open slots left') | ||
await util.delay(500) | ||
util.destroyRLPXs(rlpxs) | ||
resolve(undefined) | ||
}) | ||
} catch (err) { | ||
assert.fail(`An unexpected error occurred: ${err}`) | ||
} | ||
}) | ||
}) | ||
|
||
it('RLPX: test peer queue / refill connections', () => { | ||
it('RLPX: test peer queue / refill connections', async () => { | ||
const basePort = 60661 | ||
const rlpxs = util.getTestRLPXs(3, 1, basePort) | ||
const peer = { address: util.localhost, udpPort: basePort + 1, tcpPort: basePort + 1 } | ||
rlpxs[0]['_dpt']!.addPeer(peer) | ||
try { | ||
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. I reworked this test case to be a lot more similar to how the test was originally written to preserve the test coverage it gives for queuing/refilling connections while also updating it to run asynchronously. |
||
await new Promise((resolve) => { | ||
rlpxs[0].events.on('peer:added', async (peer) => { | ||
//@ts-ignore | ||
if ((peer['_socket'] as any)._peername.port === basePort + 1) { | ||
assert.equal(rlpxs[0]['_peersQueue'].length, 0, 'peers queue should contain no peers') | ||
const peer2 = { | ||
|
@@ -90,10 +77,33 @@ describe('RLPx simulator tests', () => { | |
if ((peer['_socket'] as any)._peername.port === basePort + 2) { | ||
assert.equal(rlpxs[0]['_peersQueue'].length, 0, 'peers queue should contain no peers') | ||
util.destroyRLPXs(rlpxs) | ||
resolve(undefined) | ||
} | ||
}) | ||
} catch (err) { | ||
assert.fail(`An unexpected error occurred: ${err}`) | ||
} | ||
}) | ||
}, 30000) | ||
it('RLPX: remove node', async () => { | ||
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. I both swapped around the final two test cases and increased the timeout of one of them since they have some interference happening with one another: They both pass in isolation and when ran in this new relative order, but fail with the old order they were in. Not sure if the cleanup is experiencing issues. |
||
const { rlpxs, peer } = util.initTwoPeerRLPXSetup(undefined, undefined, undefined, 40504) | ||
rlpxs[0] | ||
['_dpt']!.addPeer(peer) | ||
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 reason I moved the |
||
.then((peer1) => { | ||
rlpxs[0].disconnect(peer1['id']!) | ||
}) | ||
.catch((e) => { | ||
throw new Error(`Peering failed: ${e}: ${e.stack}`) | ||
}) | ||
await new Promise((resolve) => { | ||
rlpxs[0].events.once('peer:removed', async (_, reason: any) => { | ||
assert.equal( | ||
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. This assert is failing, expecting a |
||
reason, | ||
DISCONNECT_REASON.CLIENT_QUITTING, | ||
'should close with CLIENT_QUITTING disconnect reason', | ||
) | ||
assert.equal(rlpxs[0]._getOpenSlots(), 10, 'should have maxPeers open slots left') | ||
await util.delay(500) | ||
util.destroyRLPXs(rlpxs) | ||
}) | ||
resolve(undefined) | ||
}) | ||
}) | ||
}) |
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 believe
basePort + 1
is correct here since the following assert is checking if the peer's port is connecting as expected.