Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Peers communication protocol change to WS - Closes #446 #660

Merged
merged 140 commits into from
Jul 26, 2017

Conversation

MaciejBaj
Copy link
Contributor

Change peer communication protocol from HTTP to web socket.

Implementation is close connected to newly created library - wamp-socket-cluster. Please review also changes made there.

Previous PR: #580

Closes #446

- use prototype in peersManager
- don't use acceptable in returning peers list
- use self.findGoodPeers in loader
- introduce updatePeerStatus function
- remove setIntervals logs from peers module
- fixed syntax errors
- calculate consensus using random order of peers
- discover new peers every 60s instead of 10s
- remove peers ping function
@MaciejBaj MaciejBaj force-pushed the 446-ws-peers-rebased branch 3 times, most recently from 37657d4 to 10ddcfd Compare July 25, 2017 09:25
SargeKhan
SargeKhan previously approved these changes Jul 25, 2017
Copy link
Contributor

@SargeKhan SargeKhan left a comment

Choose a reason for hiding this comment

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

Great job! It looks great!

@Isabello
Copy link
Contributor

- fix remaining issues with other unit tests failing
- correct ports switching in config.non-forge.json
- recover blocks cache functional tests
- fix tests after object stub removal
- use ws post in accounts logic unit test
also:
- remove remaining console.logs
- switch http and ws ports in objectStubs
appName: 'lisk',
workerController: workersController.path,
perMessageDeflate: false,
secretKey: 'liskSecretKey',
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, thanks.

@@ -150,7 +147,7 @@ Process.prototype.loadBlocksOffset = function (limit, offset, verify, cb) {
var params = { limit: newLimit, offset: offset || 0 };

library.logger.debug('Loading blocks offset', {limit: limit, offset: offset, verify: verify});
// Execute in sequence via dbSequence
// Execute in sequence via dbSequence]
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks

@@ -209,29 +206,22 @@ Process.prototype.loadBlocksOffset = function (limit, offset, verify, cb) {
* @return {Object} cb.lastValidBlock Normalized new last block
*/
Process.prototype.loadBlocksFromPeer = function (peer, cb) {
// Set current last block as last valid block
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove that and other comments below, they are no longer valid?

@@ -107,7 +125,7 @@ __private.getByFilter = function (filter, cb) {
};

// Apply filters (by AND)
var peers = library.logic.peers.list(true);
var peers = library.logic.peers.list();
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, but from that list API response is generated, isn't it? So API response will contain that RPC property and not contains properties that library.logic.peers.list(true); is normalizing. Please check that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants