stratisproject / StratisBitcoinFullNode

Bitcoin full node in C#
https://stratisplatform.com
MIT License
787 stars 312 forks source link

[Peer File] Max Peers Limit #841

Closed fassadlr closed 6 years ago

fassadlr commented 6 years ago

Hi Guys,

Currently there's no limit on the amount of peers that can be in the peers file. What this means is that when we run the node in MainNet the peer selection logic (which I want to revisit in #838 ) takes too long to find a suitable (preferred) peer to connect to. Myself and @noescape00 have experienced scenarios where it can take up to 5 minutes to connect the 8 peers (there's over 10 000 peers in the file).

I suggest that we limit the peer file by peerAdded date (I'll submit PR to add this field), i.e. purge all peers older than X days and the amount of peers in the file (see #739). In addition to this, I suggest that we lock it down to the most recent 1000 peers (based on #838's agreed upon logic). Incidently, we also stop discovering peers after 1000 has been found.

This issue can be resolved in 2-3steps but I suggest that we do in this order:

  1. Add PeerAdded date to peers.json (then at least we know how long a peer has been in the file.
  2. Cap peers file to most recenty added peers (i.e. purge the rest)
  3. Update and implement better peer selection logic as defined in (#838).
noescape00 commented 6 years ago

I'm not sure if deleting them completely from the file is the right way - just don't use ones that are too old or over the limit unless you can't connect to any node from the main list of fresh and suitable ones.

In a hypothetical scenario when 80-90% of network peers disappear from the network at once (global cataclysm, let's say) if we have that old unused peers we will recover the network faster and connect that last 10% of nodes that are left online. And with limitation of 1000 peers there is a solid chance that some nodes will never reconnect to the network

fassadlr commented 6 years ago

@noescape00 thanks for your input!

My issue here is that, aren't we supposed to think "decentralized"? I mean, if we keep on connecting to good peers then that kind of voids the whole decentralized idea of what crypto stands for? Yes, we do want to connect to good peers, prefer them, but also randomly connect to new or unsuccessful ones.

My motivation for deleting old or non-preferred peers is that if they have been offline or fire walled or their node version is out of date, and we delete them, then when any of those scenarios become applicable again, i.e. they come back online for example, they'll just get picked up again by peer discovery.

I.e. my philosophy is this. If a peer was a good peer for a while, and then it hasn't been a good peer for awhile, for whatever reason, and it becomes a good peer again, our peer discovery will pick it up and it'll go on top of the pile again.

I want to go keep the peer pool fresh (to 30 days), so that it maximizes the decentralized notion of what p2p stands for.

Thoughts?

noescape00 commented 6 years ago

If you put it like that then yes, I agree. Keeping the pool fresh and actually removing old peers sounds like a good idea.

My issue here is that, aren't we supposed to think "decentralized"? I mean, if we keep on connecting to good peers then that kind of voids the whole decentralized idea of what crypto stands for? Yes, we do want to connect to good peers, prefer them, but also randomly connect to new or unsuccessful ones.

Agreed. We never should choose only the good ones. It will lead to various possible attack scenarios. For instance attacker can launch 20-30 peers and keep them working 24\7 so they become the most reliable ones. And as soon as target node is connected only to controlled ones it is now possible to send false blocks or dont propagate certain transactions to the target's mempool.

dangershony commented 6 years ago

Incidently, we also stop discovering peers after 1000 has been found.

I am not sure we needed stop discovering, I seem to remember after discovering 1k peers we would still use the GetAddress payload on each connected node.

I.e. my philosophy is this. If a peer was a good peer for a while, and then it hasn't been a good peer for awhile, for whatever reason, and it becomes a good peer again, our peer discovery will pick it up and it'll go on top of the pile again.

That's an interesting idea I have to say there is merit, but if we look deeper in to the scenario. If a peer drops off form nodes across the network when it comes back after 7 days it will just advertise its self and get back in to the active list. What happens with the node that came back online? will it use its list of peers that are older then 7 days to try to connect to or purge them and connect to the seed nodes? I think generally in bitcoin the seed nodes are only a fall back and peers will first attempt to connect to their internal ip list, this is more decentralized then needing to go via the seed nodes after being offline for 7 days don't you think?

dangershony commented 6 years ago

it is now possible to send false blocks

The whole idea is that its not so easy to falsify blocks 😉 how would a peer falsify blocks? Perhaps an attacker could prevent from relaying blocks not created by itself however that is an expensive attack springing up many nodes just to collect stake reward.

also in the current alog there is some randomness right? so the best nodes wont be picked all the time.

noescape00 commented 6 years ago

Perhaps an attacker could prevent from relaying blocks not created by itself however that is an expensive attack springing up many nodes just to collect stake reward.

I'm not entirely sure but as far as I know if no blocks produced for a long time staking difficulty would drop. So take a look from the target's perspective: Target won't receive any blocks from the attacking peers because they wont propagate them from the outer network. Target will assume that difficulty is dropping and total staking weight of the network is lower. Effectively attacking nodes and the target form an isolated part of the network where attacker has ~99% of nodes. Now one of the attacking nodes creates a block and sends it to target. Attacker don't need to accumulate a lot of funds to stake it. Even having a fraction of what target has (if he is staking at all) will give a good chance to create a block that is valid from the target's perspective.

dangershony commented 6 years ago

By expensive I meant maintaining a large amount of nodes, but this is also an attack that will be hard to keep for a long period if peers also attempt to randomly connect to other peers, once an isolated node connects to a valid longest chain it will reorg the attackers chain (I would think)

noescape00 commented 6 years ago

By expensive I meant maintaining a large amount of nodes, but this is also an attack that will be hard to keep for a long period if peers also attempt to randomly connect to other peers, once an isolated node connects to a valid longest chain it will reorg the attackers chain (I would think)

Exactly. If the algo for selecting peers is random that attack is not possible.

That's why I mentioned above that we should never peek all of the peers we are connecting to basing on a not-random selection algorithm.

fassadlr commented 6 years ago

Thank you @dangershony and @noescape00 for the valuable input on this!

Yes, the algorithm is currently (and was with NBitcoin) selecting a random peer from a set of preferred peers. I'm going to continue the selection algorithm improvements in #838 and close this issue as we have decided not to limit the peer file size at this time.