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

fix: check status and version to find peers #30

Merged
merged 6 commits into from
Jan 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## Unreleased

## 0.1.20 - 2019-01-28

### Fixed
- Check status and version to find peers
- Clone peer list as to not overwrite default network peers

## 0.1.19 - 2019-01-25

### Changed
Expand Down
3 changes: 2 additions & 1 deletion __tests__/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ describe('API - Client', () => {
const apiPeers = peers.map(peer => {
return {
...peer,
status: 200
status: 200,
version: '2.0.0'
}
})

Expand Down
14 changes: 8 additions & 6 deletions lib/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const shuffle = require('lodash.shuffle')
const cloneDeep = require('lodash.clonedeep')
const HttpClient = require('./http')
const resources = require('./resources')
const initialPeers = require('./peers')
Expand All @@ -19,6 +20,10 @@ const mergePeerArrays = (oldArray, newArray) => {
return peers
}

const getPeerVersion = (peer) => {
return !peer.version || /^1\./.test(peer.version) ? 1 : 2
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use something like:

return (peer.version && peer.version.startsWith('1.')) ? 1 : 2

}

module.exports = class ApiClient {
/**
* Finds all the available peers, sorted by block height and delay
Expand All @@ -38,10 +43,7 @@ module.exports = class ApiClient {
shuffle(networkPeers)

const selfIps = ['127.0.0.1', '::ffff:127.0.0.1', '::1']
const versionStatus = {
1: 'OK',
2: 200
}
const versionStatus = ['OK', 200]

// Connect to each peer to get an updated list of peers until a success response
const client = new ApiClient('http://', version)
Expand Down Expand Up @@ -69,7 +71,7 @@ module.exports = class ApiClient {
if (responsePeers.length) {
// Ignore local and unavailable peers
Copy link
Contributor

Choose a reason for hiding this comment

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

// Ignore local and unavailable peers, and those with wrong version

responsePeers = responsePeers.filter(peer => {
return selfIps.indexOf(peer.ip) === -1 && peer.status === versionStatus[version]
return selfIps.indexOf(peer.ip) === -1 && versionStatus.includes(peer.status) && getPeerVersion(peer) === version
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use includes on both checks:

return !selfIps.includes(peer.ip) && versionStatus.includes(peer.status) && getPeerVersion(peer) === version

})

peers = mergePeerArrays(peers, responsePeers)
Expand All @@ -86,7 +88,7 @@ module.exports = class ApiClient {
}

// Return at least the initial (hardcoded) peers
return sortPeers(peers && peers.length ? peers : networkPeers)
return cloneDeep(sortPeers(peers && peers.length ? peers : networkPeers))
}

/**
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@arkecosystem/client",
"description": "A JavaScript library to interact with the Ark Blockchain",
"version": "0.1.19",
"version": "0.1.20",
"contributors": [
"Brian Faust <brian@ark.io>",
"Alex Barnsley <alex@ark.io>",
Expand Down Expand Up @@ -30,6 +30,7 @@
},
"dependencies": {
"axios": "^0.18.0",
"lodash.clonedeep": "^4.5.0",
"lodash.orderby": "^4.6.0",
"lodash.shuffle": "^4.2.0"
},
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4208,6 +4208,11 @@ locate-path@^3.0.0:
p-locate "^3.0.0"
path-exists "^3.0.0"

lodash.clonedeep@^4.5.0:
version "4.5.0"
resolved "https://registry.yarnpkg.com/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz#e23f3f9c4f8fbdde872529c1071857a086e5ccef"
integrity sha1-4j8/nE+Pvd6HJSnBBxhXoIblzO8=

lodash.debounce@^4.0.8:
version "4.0.8"
resolved "https://registry.yarnpkg.com/lodash.debounce/-/lodash.debounce-4.0.8.tgz#82d79bff30a67c4005ffd5e2515300ad9ca4d7af"
Expand Down