Make proposer selection implementation more functional

[milosevic]

Proposer selection function is one of critical elements for correctness of Tendermint algorithm. Specification can be found here:
https://github.com/tendermint/tendermint/blob/master/docs/specification/new-spec/reactors/consensus/proposer-selection.md

Although according to the spec proposer selection is a pure function that depends on current validator set, height and round, current implementation is not very functional (it is done in the context of ValidatorSet data-structure (https://github.com/tendermint/tendermint/blob/master/types/validator_set.go#L50). This make it a bit hard to understand and reason about its correctness.

This is a proposal to make proposer selection more explicit and functional. Furthermore, the spec should be extended with the algorithm of the implementation currently used.

[ebuchman]

A critical bug was just found in the proposer selection: #2718 (comment)

This stems from the confounding between data structure and algorithm.

We likely need to remove the Accum from the core Validator struct and track it separately.

We still want a GetProposer function, but it should be a method on some entity whose sole purpose is to handle proposer selection. The trouble is the ValidatorSet is doing too much - it’s CRUD for the validators, but its also doing the algorithmic proposer selection, entangling data from the algorithm with data from the CRUD functionality (ie. Accum in the Validator struct) which led to the issue, where something that was pure CRUD (abci EndBlock) caused us to lose data when it got converted to the validator set data type.

If accum was not part of the core validator set structures, but was tracked separately, the data loss during conversion from abci type would never have occurred

[ebuchman]

Work is ongoing to improve/fix the proposer selection (eg. #3049) but a larger refactor of the code along these lines might be too disruptive at this time. So let's remove from the launch milestone and address a larger code cleanup here at some point in the future.

[ebuchman]

@milosevic do you think we can close this based on the Anca's recent refactors? Or do you think we can still really improve this code?

[milosevic]

I think we can close this.