Create shuffled round robin proposer selector and use it by default (e…

…thereum#536)

* create new shuffled round robin order policy and set as default

* add Random function to contract_comm/random and utilize it to seed proposer selection

* fixed typo

* add todo note

* change comments to be more vauge

* use explicit state in call to Random

* move randomness retrieval into Validators and ParentValidators functions

* fix typo

* log proposer policy

* move round shuffled round robin up

* remove overly verbose trace

* add command line flag for proposer policy

* fix type errors

cmd/geth/main.go

@@ -139,6 +139,7 @@ var (
 		configFileFlag,
 		utils.IstanbulRequestTimeoutFlag,
 		utils.IstanbulBlockPeriodFlag,
+		utils.IstanbulProposerPolicyFlag,
 		utils.PingIPFromPacketFlag,
 		utils.UseInMemoryDiscoverTable,
 		utils.VersionCheckFlag,

cmd/geth/usage.go

@@ -253,6 +253,7 @@ var AppHelpFlagGroups = []flagGroup{
 		Flags: []cli.Flag{
 			utils.IstanbulRequestTimeoutFlag,
 			utils.IstanbulBlockPeriodFlag,
+			utils.IstanbulProposerPolicyFlag,
 		},
 	},
 }

cmd/utils/flags.go

@@ -35,6 +35,7 @@ import (
 	"github.com/ethereum/go-ethereum/consensus"
 	"github.com/ethereum/go-ethereum/consensus/clique"
 	"github.com/ethereum/go-ethereum/consensus/ethash"
+	"github.com/ethereum/go-ethereum/consensus/istanbul"
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/state"
 	"github.com/ethereum/go-ethereum/core/vm"
@@ -660,6 +661,11 @@ var (
 		Usage: "Default minimum difference between two consecutive block's timestamps in seconds",
 		Value: eth.DefaultConfig.Istanbul.BlockPeriod,
 	}
+	IstanbulProposerPolicyFlag = cli.Uint64Flag{
+		Name:  "istanbul.proposerpolicy",
+		Usage: "Default minimum difference between two consecutive block's timestamps in seconds",
+		Value: uint64(eth.DefaultConfig.Istanbul.ProposerPolicy),
+	}
 )
 
 // MakeDataDir retrieves the currently requested data directory, terminating
@@ -1166,6 +1172,9 @@ func setIstanbul(ctx *cli.Context, cfg *eth.Config) {
 	if ctx.GlobalIsSet(IstanbulBlockPeriodFlag.Name) {
 		cfg.Istanbul.BlockPeriod = ctx.GlobalUint64(IstanbulBlockPeriodFlag.Name)
 	}
+	if ctx.GlobalIsSet(IstanbulProposerPolicyFlag.Name) {
+		cfg.Istanbul.ProposerPolicy = istanbul.ProposerPolicy(ctx.GlobalUint64(IstanbulProposerPolicyFlag.Name))
+	}
 }
 
 // checkExclusive verifies that only a single isntance of the provided flags was

consensus/istanbul/backend/backend.go

@@ -30,6 +30,7 @@ import (
 	istanbulCore "github.com/ethereum/go-ethereum/consensus/istanbul/core"
 	"github.com/ethereum/go-ethereum/consensus/istanbul/validator"
 	"github.com/ethereum/go-ethereum/contract_comm/election"
+	"github.com/ethereum/go-ethereum/contract_comm/random"
 	"github.com/ethereum/go-ethereum/contract_comm/validators"
 	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/state"
@@ -50,6 +51,9 @@ const (
 var (
 	// errInvalidSigningFn is returned when the consensus signing function is invalid.
 	errInvalidSigningFn = errors.New("invalid signing function for istanbul messages")
+
+	// errNoBlockHeader is returned when the requested block header could not be found.
+	errNoBlockHeader = errors.New("failed to retrieve block header")
 )
 
 // Entries for the recent announce messages
@@ -156,7 +160,15 @@ func (sb *Backend) Close() error {
 
 // Validators implements istanbul.Backend.Validators
 func (sb *Backend) Validators(proposal istanbul.Proposal) istanbul.ValidatorSet {
-	return sb.getValidators(proposal.Number().Uint64(), proposal.Hash())
+	valSet := sb.getValidators(proposal.Number().Uint64(), proposal.Hash())
+
+	seed, err := sb.validatorRandomnessAtBlockNumber(proposal.Number().Uint64(), proposal.Hash())
+	if err != nil {
+		sb.logger.Error("Failed to set randomness for proposer selection", "number", proposal.Number().Uint64(), "hash", proposal.Hash(), "error", err)
+	}
+	valSet.SetRandomness(seed)
+
+	return valSet
 }
 
 func (sb *Backend) GetValidators(blockNumber *big.Int, headerHash common.Hash) []istanbul.Validator {
@@ -375,6 +387,18 @@ func (sb *Backend) getNewValidatorSet(header *types.Header, state *state.StateDB
 	return newValSet, err
 }
 
+func (sb *Backend) validatorRandomnessAtBlockNumber(number uint64, hash common.Hash) (common.Hash, error) {
+	header := sb.chain.GetHeader(hash, number)
+	if header == nil {
+		return common.Hash{}, errNoBlockHeader
+	}
+	state, err := sb.stateAt(header.Hash())
+	if err != nil {
+		return common.Hash{}, err
+	}
+	return random.Random(header, state)
+}
+
 func (sb *Backend) verifyValSetDiff(proposal istanbul.Proposal, block *types.Block, state *state.StateDB) error {
 	header := block.Header()
 
@@ -471,7 +495,15 @@ func (sb *Backend) GetProposer(number uint64) common.Address {
 // ParentValidators implements istanbul.Backend.GetParentValidators
 func (sb *Backend) ParentValidators(proposal istanbul.Proposal) istanbul.ValidatorSet {
 	if block, ok := proposal.(*types.Block); ok {
-		return sb.getValidators(block.Number().Uint64()-1, block.ParentHash())
+		valSet := sb.getValidators(block.Number().Uint64()-1, block.ParentHash())
+
+		seed, err := sb.validatorRandomnessAtBlockNumber(proposal.Number().Uint64()-1, block.ParentHash())
+		if err != nil {
+			sb.logger.Error("Failed to set randomness for proposer selection", "number", proposal.Number().Uint64()-1, "hash", block.ParentHash(), "error", err)
+		}
+		valSet.SetRandomness(seed)
+
+		return valSet
 	}
 	return validator.NewSet(nil, sb.config.ProposerPolicy)
 }

consensus/istanbul/backend/handler.go

@@ -117,6 +117,7 @@ func (sb *Backend) NewChainHead() error {
 		} else {
 			sb.logger.Info("Validators Election Results: Node IN ValidatorSet")
 		}
+		// Establish connections to new peers and tear down connections to old ones.
 		go sb.RefreshValPeers(valset)
 	}
 

consensus/istanbul/config.go

@@ -21,6 +21,7 @@ type ProposerPolicy uint64
 const (
 	RoundRobin ProposerPolicy = iota
 	Sticky
+	ShuffledRoundRobin
 )
 
 type Config struct {
@@ -33,6 +34,6 @@ type Config struct {
 var DefaultConfig = &Config{
 	RequestTimeout: 3000,
 	BlockPeriod:    1,
-	ProposerPolicy: RoundRobin,
+	ProposerPolicy: ShuffledRoundRobin,
 	Epoch:          30000,
 }

consensus/istanbul/validator.go

@@ -77,9 +77,23 @@ type Validators []Validator
 type ValidatorSet interface {
 	// Calculate the proposer
 	CalcProposer(lastProposer common.Address, round uint64)
+	// Get current proposer
+	GetProposer() Validator
+	// Check whether the validator with given address is the current proposer
+	IsProposer(address common.Address) bool
+	// Policy by which this selector chooses proposers
+	Policy() ProposerPolicy
+	// Sets the randomness for use in the proposer policy
+	SetRandomness(seed common.Hash)
+
 	// Return the validator size
 	PaddedSize() int
 	Size() int
+	// Get the maximum number of faulty nodes
+	F() int
+	// Get the minimum quorum size
+	MinQuorumSize() int
+
 	// Return the validator array
 	List() []Validator
 	// Return the validator array without holes
@@ -90,24 +104,16 @@ type ValidatorSet interface {
 	GetByIndex(i uint64) Validator
 	// Get validator by given address
 	GetByAddress(addr common.Address) (int, Validator)
-	// Get current proposer
-	GetProposer() Validator
-	// Check whether the validator with given address is a proposer
-	IsProposer(address common.Address) bool
+
 	// Add validators
 	AddValidators(validators []ValidatorData) bool
 	// Remove validators
 	RemoveValidators(removedValidators *big.Int) bool
 	// Copy validator set
 	Copy() ValidatorSet
-	// Get the maximum number of faulty nodes
-	F() int
-	// Get proposer policy
-	Policy() ProposerPolicy
-	// Get the minimum quorum size
-	MinQuorumSize() int
 }
 
 // ----------------------------------------------------------------------------
 
-type ProposalSelector func(ValidatorSet, common.Address, uint64) Validator
+// Returns the block proposer for a round given the last proposer, round number, and randomness.
+type ProposerSelector func(ValidatorSet, common.Address, uint64, common.Hash) Validator

consensus/istanbul/validator/default.go

@@ -17,6 +17,7 @@
 package validator
 
 import (
+	"fmt"
 	"math"
 	"math/big"
 	"reflect"
@@ -51,7 +52,8 @@ type defaultSet struct {
 
 	proposer    istanbul.Validator
 	validatorMu sync.RWMutex
-	selector    istanbul.ProposalSelector
+	selector    istanbul.ProposerSelector
+	randomness  common.Hash
 }
 
 func newDefaultSet(validators []istanbul.ValidatorData, policy istanbul.ProposerPolicy) *defaultSet {
@@ -67,9 +69,17 @@ func newDefaultSet(validators []istanbul.ValidatorData, policy istanbul.Proposer
 	if valSet.Size() > 0 {
 		valSet.proposer = valSet.GetByIndex(0)
 	}
-	valSet.selector = roundRobinProposer
-	if policy == istanbul.Sticky {
-		valSet.selector = stickyProposer
+
+	switch policy {
+	case istanbul.Sticky:
+		valSet.selector = StickyProposer
+	case istanbul.RoundRobin:
+		valSet.selector = RoundRobinProposer
+	case istanbul.ShuffledRoundRobin:
+		valSet.selector = ShuffledRoundRobinProposer
+	default:
+		// Programming error.
+		panic(fmt.Sprintf("unknown proposer selection policy: %v", policy))
 	}
 
 	return valSet
@@ -154,51 +164,7 @@ func (valSet *defaultSet) IsProposer(address common.Address) bool {
 func (valSet *defaultSet) CalcProposer(lastProposer common.Address, round uint64) {
 	valSet.validatorMu.RLock()
 	defer valSet.validatorMu.RUnlock()
-	valSet.proposer = valSet.selector(valSet, lastProposer, round)
-}
-
-func calcSeed(valSet istanbul.ValidatorSet, proposer common.Address, round uint64) uint64 {
-	offset := 0
-	if idx := valSet.GetFilteredIndex(proposer); idx >= 0 {
-		offset = idx
-	}
-	return uint64(offset) + round
-}
-
-func emptyAddress(addr common.Address) bool {
-	return addr == common.Address{}
-}
-
-func roundRobinProposer(valSet istanbul.ValidatorSet, proposer common.Address, round uint64) istanbul.Validator {
-	if valSet.Size() == 0 {
-		return nil
-	}
-	seed := uint64(0)
-	if emptyAddress(proposer) {
-		seed = round
-	} else {
-		seed = calcSeed(valSet, proposer, round) + 1
-	}
-
-	filteredList := valSet.FilteredList()
-	pick := seed % uint64(valSet.Size())
-	return filteredList[pick]
-}
-
-func stickyProposer(valSet istanbul.ValidatorSet, proposer common.Address, round uint64) istanbul.Validator {
-	if valSet.Size() == 0 {
-		return nil
-	}
-	seed := uint64(0)
-	if emptyAddress(proposer) {
-		seed = round
-	} else {
-		seed = calcSeed(valSet, proposer, round)
-	}
-
-	filteredList := valSet.FilteredList()
-	pick := seed % uint64(valSet.Size())
-	return filteredList[pick]
+	valSet.proposer = valSet.selector(valSet, lastProposer, round, valSet.randomness)
 }
 
 func (valSet *defaultSet) AddValidators(validators []istanbul.ValidatorData) bool {
@@ -278,8 +244,10 @@ func (valSet *defaultSet) Copy() istanbul.ValidatorSet {
 
 func (valSet *defaultSet) F() int { return int(math.Ceil(float64(valSet.Size())/3)) - 1 }
 
-func (valSet *defaultSet) Policy() istanbul.ProposerPolicy { return valSet.policy }
-
 func (valSet *defaultSet) MinQuorumSize() int {
 	return int(math.Ceil(float64(2*valSet.Size()) / 3))
 }
+
+func (valSet *defaultSet) Policy() istanbul.ProposerPolicy { return valSet.policy }
+
+func (valSet *defaultSet) SetRandomness(seed common.Hash) { valSet.randomness = seed }

consensus/istanbul/validator/default_test.go

@@ -36,7 +36,6 @@ func TestValidatorSet(t *testing.T) {
 	testNewValidatorSet(t)
 	testNormalValSet(t)
 	testEmptyValSet(t)
-	testStickyProposer(t)
 	testAddAndRemoveValidator(t)
 	testQuorumSizes(t)
 }
@@ -102,26 +101,6 @@ func testNormalValSet(t *testing.T) {
 	if _, val := valSet.GetByAddress(invalidAddr); val != nil {
 		t.Errorf("validator mismatch: have %v, want nil", val)
 	}
-	// test get proposer
-	if val := valSet.GetProposer(); !reflect.DeepEqual(val, val1) {
-		t.Errorf("proposer mismatch: have %v, want %v", val, val1)
-	}
-	// test calculate proposer
-	lastProposer := addr1
-	valSet.CalcProposer(lastProposer, uint64(0))
-	if val := valSet.GetProposer(); !reflect.DeepEqual(val, val2) {
-		t.Errorf("proposer mismatch: have %v, want %v", val, val2)
-	}
-	valSet.CalcProposer(lastProposer, uint64(3))
-	if val := valSet.GetProposer(); !reflect.DeepEqual(val, val1) {
-		t.Errorf("proposer mismatch: have %v, want %v", val, val1)
-	}
-	// test empty last proposer
-	lastProposer = common.Address{}
-	valSet.CalcProposer(lastProposer, uint64(3))
-	if val := valSet.GetProposer(); !reflect.DeepEqual(val, val2) {
-		t.Errorf("proposer mismatch: have %v, want %v", val, val2)
-	}
 }
 
 func testEmptyValSet(t *testing.T) {
@@ -196,40 +175,6 @@ func testAddAndRemoveValidator(t *testing.T) {
 	}
 }
 
-func testStickyProposer(t *testing.T) {
-	b1 := common.Hex2Bytes(testAddress)
-	b2 := common.Hex2Bytes(testAddress2)
-	addr1 := common.BytesToAddress(b1)
-	addr2 := common.BytesToAddress(b2)
-	val1 := New(addr1, []byte{})
-	val2 := New(addr2, []byte{})
-
-	validators, _ := istanbul.CombineIstanbulExtraToValidatorData([]common.Address{addr1, addr2}, [][]byte{{}, {}})
-	valSet := newDefaultSet(validators, istanbul.Sticky)
-
-	// test get proposer
-	if val := valSet.GetProposer(); !reflect.DeepEqual(val, val1) {
-		t.Errorf("proposer mismatch: have %v, want %v", val, val1)
-	}
-	// test calculate proposer
-	lastProposer := addr1
-	valSet.CalcProposer(lastProposer, uint64(0))
-	if val := valSet.GetProposer(); !reflect.DeepEqual(val, val1) {
-		t.Errorf("proposer mismatch: have %v, want %v", val, val1)
-	}
-
-	valSet.CalcProposer(lastProposer, uint64(1))
-	if val := valSet.GetProposer(); !reflect.DeepEqual(val, val2) {
-		t.Errorf("proposer mismatch: have %v, want %v", val, val2)
-	}
-	// test empty last proposer
-	lastProposer = common.Address{}
-	valSet.CalcProposer(lastProposer, uint64(3))
-	if val := valSet.GetProposer(); !reflect.DeepEqual(val, val2) {
-		t.Errorf("proposer mismatch: have %v, want %v", val, val2)
-	}
-}
-
 func generateValidators(n int) ([]istanbul.ValidatorData, [][]byte) {
 	vals := make([]istanbul.ValidatorData, 0)
 	keys := make([][]byte, 0)