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.

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

Merged
merged 19 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
128 changes: 69 additions & 59 deletions packages/devp2p/test/integration/rlpx-simulator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
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 believe basePort + 1 is correct here since the following assert is checking if the peer's port is connecting as expected.

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 {
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 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 = {
Expand All @@ -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 () => {
Copy link
Contributor Author

@scorbajio scorbajio Oct 29, 2024

Choose a reason for hiding this comment

The 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)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I moved the addPeer call out of initTwoPeerRLPXSetup is to be able to chain the disconnect here before checking for peer:removed events in the awaited promise that follows.

.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(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This assert is failing, expecting a CLIENT_QUITTING but getting a TIMEOUT.

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)
})
})
})
20 changes: 14 additions & 6 deletions packages/devp2p/test/integration/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,23 @@ export function getTestRLPXs(
return rlpxs
}

type TestRLPXs = {
rlpxs: RLPx[]
peer: {
address: string
udpPort: number
tcpPort: number
}
}
export function initTwoPeerRLPXSetup(
maxPeers?: any,
capabilities?: any,
common?: Object | Common,
basePort = 30306,
): RLPx[] {
): TestRLPXs {
const rlpxs = getTestRLPXs(2, maxPeers, basePort, capabilities, common)
const peer = { address: localhost, udpPort: basePort + 1, tcpPort: basePort + 1 }
rlpxs[0]['_dpt']!.addPeer(peer).catch(() => {
/* Silently catch rejections here since not an actual test error */
})
return rlpxs
return { rlpxs, peer }
}

export function destroyRLPXs(rlpxs: any) {
Expand All @@ -135,7 +140,10 @@ export function twoPeerMsgExchange3(
common?: Object | Common,
basePort = 30306,
) {
const rlpxs = initTwoPeerRLPXSetup(null, capabilities, common, basePort)
const { rlpxs, peer } = initTwoPeerRLPXSetup(null, capabilities, common, basePort)
rlpxs[0]['_dpt']!.addPeer(peer).catch(() => {
throw new Error('Peering failed')
})
rlpxs[0].events.on('peer:added', function (peer: any) {
const protocol = peer.getProtocols()[0]
opts.sendMessage(rlpxs, protocol)
Expand Down
Loading