stellar-deprecated / stellard

INACTIVE. Server in the Stellar network. Maintains the distributed ledger. Introduces and validates transactions. This repo is not in active development, it is being replaced by stellar-core.
Other
271 stars 60 forks source link

Consensus timing changes #176

Closed MonsieurNicolas closed 9 years ago

MonsieurNicolas commented 9 years ago

see individual commit comments for details.

Changes are attempting to do two things:

the combination of the two seems to be causing "mini forks" in the network that should be avoided

MonsieurNicolas commented 9 years ago

@JoelKatz do you think this makes sense? I suspect Ripple has the same issue (I didn't see changes in consensus).

The change of using future validation vs current should force consensus to honor the LEDGER_MIN_CLOSE pause during preClose

MonsieurNicolas commented 9 years ago

I added documentation for the consensus process to help understand what I did

MonsieurNicolas commented 9 years ago

FYI: for review purpose, I am keeping separate commits for now, but will rebase before merging

JoelKatz commented 9 years ago

The trade-off is that to jump to something else, we have to abandon where we are. That risks creating new attack models (if we're in the right place and give up our influence). The initial design intentionally makes us rather "sticky', staying put unless evidence to the contrary is overwhelming. This may mean we occasionally are forked from the main ledger chain, but that's perfectly fine. Every server can be out of sync 1% of the time, and the network will still have more than 90% of its consensus power more than 95% of the time, which is fine.

Don't try to save individual servers, there's really no benefit. What's important from consensus is that the network as a whole makes rapid forward progress. That's the priority.

I'm not saying the code is perfect or that it necessarily makes the trade-offs the best possible way. But it is very easy to make apparently slight changes and create new attack models.

MonsieurNicolas commented 9 years ago

We are seeing with the existing code partitioning happening much more often than 1% of the time. So we already have a situation where we a set of "super nodes" is running the show (they happen to be ours, but still), leaving behind other nodes that decide to create 1 or 2 nodes forks. simplified sample output of what I am seeing (output of NetworkOPs when not running on consensus ledger): t=0, n=0 t=4, n=5 t=1, n=1 t=1, n=0 t=1, n=0 t=1, n=0

From what I can tell, there are times where some peers don't see any (or most) positions, which causes them to enter consensus with a number of positions that is unacceptable because it reaches a timeout. So even if previous count was let's say 10 peers, peers will jump into consensus with 1 peer (or even just themselves) if the right conditions are met (I did not want to mess with the various timeouts that trigger the system to "move on"), which resets the expected number of peers for the next round, which in turn causes the next consensus round to potentially be bogus (as the instance will move on faster). I suspect this is due to some timing differences for when nodes enter consensus (they don't join at the same time but in staggered fashion due to server load, etc).

The changes that I am proposing are trying to cause peers to properly move together in the various stages of consensus. The trick (like you pointed out) is to find metrics that are not going to open new ways to attack the network.

As far as abandoning consensus to jump to something else, I imagine that this is the right thing to do if we see that we are on a fork with too little participants (this branch will eventually be abandoned anyways because the network's position is different). The problem with waiting on such bad branch is that it keeps the instance from joining the rest of the peers.

MonsieurNicolas commented 9 years ago

for the change related to not waiting while in ESTABLISH state: I am thinking of doing the more conservative change for this and use something like ( (currentValidations *100 ) / (mPreviousProposers+1) ) <= 60 for the condition on waiting. This basically means that in the 99% case where mPreviousProposers is the desired target for number of proposers, and consider that there is no point in keeping the local instance build better consensus with the minority pool (40%).

MonsieurNicolas commented 9 years ago

I am closing this as we are doing the one validator work instead