Make current integration tests pass - Closes #971 and addresses #900

[jondubois]

What was the problem?

Core integration tests related to launching a node, connecting to seed peers and doing the first round of discovery were failing. We hadn't yet brought the node into a verifiable working state.

How did I fix it?

• Fixed bugs in several places; two related to removing event listeners incorrectly.
• Allowed using real timers in integration tests. Added an environment variable which can be used to optionally enable them (though they're still disabled by default).
• Renamed PeerInfo to P2PPeerInfo since it is exposed publicly by the API (e.g. as part of getNetworkStatus).
• Modified P2PPeerInfo and P2PNodeInfo to be more generic; added an options field which can be used to specify custom fields/values on P2PPeerInfo/P2PNodeInfo objects. This allows the P2P module to support more different kinds of applications.
• More consistent handling of P2PNodeInfo and P2PPeerInfo throughout P2P -> PeerPool -> Peer.
• Added some TODO comments.
• Made selectForConnection work with P2PPeerInfo objects instead of Peer objects; it doesn't make sense to filter Peer objects if we are yet to connect to them; it forced us to instantiate Peer objects before we needed them. We should think about other selection functions as well if we want to use P2PPeerInfo instead of Peer; the P2PPeerInfo is safer to expose to users but may offer less control/flexibility in certain scenarios.
• selectPeers function is now used by the list endpoint because we can't send back our full peer list and the selectPeers function seemed like a good way to do that type of filtering. As a result of this, selectPeers was changed to only return tried peers for which we have complete info.
• ^ selectPeers relies on peer height which is not relevant for sending back our peer list. Instead, the node now sends back a subset of their randomly shuffled peer list; uses the same algorithm as current Lisk core node.
• Made integration tests work with basic discovery scenarios; we still need to add assertions to existing test cases and also add more new test cases; right now the node only does a single round of discovery and this is a problem when all nodes launch at the same time (which is the current situation in integration tests).

How to test it?

npm run test:integration

Review checklist

• The PR resolves Make current integration tests pass #971 and addresses Add discovery process #900
• All new code is covered with unit tests
• All new code was formatted with Prettier
• Linting passes
• Tests pass
• Commit messages follow the commit guidelines
• Documentation has been added/updated

[shuse2]

Could you create issue for each todo you added?
Also, there is lint problem

ERROR: (ordered-imports) /home/lisk/workspace/lisk-elements_PR-1048/packages/lisk-p2p/src/peer_discovery.ts[16, 1]: Import sources within a group must be alphabetized.

ERROR: (ordered-imports) /home/lisk/workspace/lisk-elements_PR-1048/packages/lisk-p2p/src/peer_selection.ts[17, 1]: Import sources within a group must be alphabetized.

ERROR: (ordered-imports) /home/lisk/workspace/lisk-elements_PR-1048/packages/lisk-p2p/src/sanitization.ts[18, 10]: Named imports must be alphabetized.

[shuse2]

why is this undefinable?
shouldn't information always supplied in constructor?

[jondubois]

It's possible to provide nodeInfo to the constructor but then I think we should be clear about how it's provided to the constructor; maybe by adding an explicit nodeInfo field. I think this makes sense actually. nodeInfo will be information which may be used internally by the node and which should be exposed publicly to other nodes.

[shuse2]

did this file came back? i think name changed to validation

[shuse2]

maybe below is cleaner?

global.sandbox = sinon.createSandbox({
	useFakeTimers: process.env.USE_REAL_TIMERS !== 'true',
});

[ishantiw]

Maybe its not a good idea to expose isTriedPeer field to the user as its more low level concept of managing peer bucket. Maybe its better if we could have a separate P2PPeerInfo without isTriedPeer and we can extend it to have a internal PeerInfo definition that will hold more fields related to peer bucket management and selection algo like isBannedPeer from LIP

[jondubois]

@ishantiw About isTriedPeer; that sounds good. We should do this as a separate PR though because otherwise this PR will get huge and it's already addressing too many problems. At least once we've merged this PR we can change stuff and then run the integration tests to make sure that we don't break things.

[ishantiw]

Looks good