Neo: P2P sync has a certain percentage to make nodes sync to a forked height

[superboyiii]

We tried to sync a new node to the latest height via pure P2P without offline package importing, but it wasn't successful until we tried three times. That's because of some nodes in NEO network, always stay in the forked height which are able to lure new nodes to sync with them since the logic of syncing is only choosing a higher peers randomly in each block. Maybe we should make nodes choose a highest peer in each block to ensure it won't be taken to forked height. As I know, some exchanges like Huobi, which greatly focus on security, don't trust offline package importing. They tried to sync via P2P but these forked nodes make it not easy to be done.

[lock9]

Do you think this is related to #542 ?
Node selection policy is very important. How hard it is to implement this "fix"? If this is easy to fix, I think it may be fixed in neo 2. If this is going to take too much time, unfortunately, we have to leave this for neo 3 only.

@vncoelho @igormcoelho @shargon what is your perception of the importance of this issue? Should we fix in neo 2? Seems a very important issue, it is not critical because there are other ways of syncing the network, but if it seems it is a bug "in the protocol" that is quite impacting...

[shargon]

I think that is a important, but we should try to remove this nodes from the network, because the fork problem is fixed.
We can ban nodes if his height is not increased in certain time, and is lower than the expected?

[shargon]

Also, we can have the list of the fork hashes, and deny this blocks in p2p?

[igormcoelho]

I think that nodes could require future values and stay limited to height -1 for some time, to avoid locks

[erikzhang]

Maybe we can add some hard-coded checkpoints?

[lock9]

I think some hardcoded checkpoints would be perfect

[vncoelho]

I do not like these checkpoints, @erikzhang.
We are not going to completely know when a duplicated hash can come. Better to have a simple mechanism for checking the next header during syncing.
@jsolman used to have a branch with this here on the Neo project, but it was deleted after the dbft 2.0 merge.

[lock9]

@neo-project/core should we consider this a bug? Should we give this a higher priority?

[roman-khimov]

BTW, we've also encountered this issue in nspcc-dev/neo-go#465 and it's really annoying.

As these nodes are stuck at the known height with the known bad block one possible solution is to add a post-handshake quirk that if the peer node has this height than we will ask it (getdata) for this known bad block and if it answers (cmdblock) with it, then just drop the connection. Eventually it should lead to these nodes losing all P2P connections.