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

Wrong ordering in the result of the API call /api/peers?state=2&orderBy=height:desc #356

Closed
simonmorgenthaler opened this issue Dec 15, 2016 · 2 comments · Fixed by #378
Closed
Assignees

Comments

@simonmorgenthaler
Copy link

I would expect that this call returns all connected peers, beginning with "good" peers and the highest available height at the top. But the few peers at the beginning of this list look like this:
{"ip":"xx.xx.xx.xx","port":7000,"state":2,"os":"linux3.4.39","version":"0.5.0d","broadhash":null,"height":null}
meaning, height: null is rated higher than the real block height, which is wrong IMHO.

@Isabello
Copy link
Contributor

I think part of this is that null is considered below 0 or NaN, so it comes up at the very top of the list when sorting by descending. The real issue at hand is why some nodes are coming up with blank broadhash and height. Some evidence points to this being a symptom of verifying the blockchain from 0 using current tables.

Syncing from 0 without blocks in the database will also do this. However in that case the broadhash will be the genesis block broadhash until the node is reloaded or finishes the sync.

We probably need to simulate some of these scenarios to get a better understanding of why it happens. As a stop gap I propose we remove peers who have a broadhash or height of null, since we have no idea if they are on a fork or at current height. The safest bet is to not communicate with them at all until their system is generating proper system headers.

@4miners
Copy link
Contributor

4miners commented Dec 15, 2016

We can add NULLS LAST option to SQL query - that will make nulls appears after non-null values in the sort ordering.

Reference: https://www.postgresql.org/docs/9.6/static/queries-order.html

@MaciejBaj MaciejBaj self-assigned this Jan 5, 2017
MaciejBaj added a commit to MaciejBaj/lisk that referenced this issue Jan 5, 2017
Add NULLS LAST option to sql queries with ORDER_BY

closes LiskArchive#356
MaciejBaj added a commit to MaciejBaj/lisk that referenced this issue Jan 5, 2017
Add NULLS LAST option to sql queries with ORDER_BY

closes LiskArchive#356
MaciejBaj added a commit to MaciejBaj/lisk that referenced this issue Jan 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants