Revert "Create shuffled round robin proposer selector and use it by d…

…efault (ethereum#536)" (ethereum#548)

This reverts commit bdcd75c.

cmd/geth/main.go

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

cmd/geth/usage.go

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

cmd/utils/flags.go

@@ -35,7 +35,6 @@ 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"
@@ -661,11 +660,6 @@ 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
@@ -1172,9 +1166,6 @@ 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,7 +30,6 @@ 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"
@@ -51,9 +50,6 @@ 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
@@ -160,15 +156,7 @@ func (sb *Backend) Close() error {
 
 // Validators implements istanbul.Backend.Validators
 func (sb *Backend) Validators(proposal istanbul.Proposal) istanbul.ValidatorSet {
-	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
+	return sb.getValidators(proposal.Number().Uint64(), proposal.Hash())
 }
 
 func (sb *Backend) GetValidators(blockNumber *big.Int, headerHash common.Hash) []istanbul.Validator {
@@ -387,18 +375,6 @@ 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()
 
@@ -495,15 +471,7 @@ 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 {
-		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 sb.getValidators(block.Number().Uint64()-1, block.ParentHash())
 	}
 	return validator.NewSet(nil, sb.config.ProposerPolicy)
 }

consensus/istanbul/backend/handler.go

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

consensus/istanbul/validator.go

@@ -77,23 +77,9 @@ 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
@@ -104,16 +90,24 @@ 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
 }
 
 // ----------------------------------------------------------------------------
 
-// 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
+type ProposalSelector func(ValidatorSet, common.Address, uint64) Validator

consensus/istanbul/validator/default.go

@@ -17,7 +17,6 @@
 package validator
 
 import (
-	"fmt"
 	"math"
 	"math/big"
 	"reflect"
@@ -52,8 +51,7 @@ type defaultSet struct {
 
 	proposer    istanbul.Validator
 	validatorMu sync.RWMutex
-	selector    istanbul.ProposerSelector
-	randomness  common.Hash
+	selector    istanbul.ProposalSelector
 }
 
 func newDefaultSet(validators []istanbul.ValidatorData, policy istanbul.ProposerPolicy) *defaultSet {
@@ -69,17 +67,9 @@ func newDefaultSet(validators []istanbul.ValidatorData, policy istanbul.Proposer
 	if valSet.Size() > 0 {
 		valSet.proposer = valSet.GetByIndex(0)
 	}
-
-	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))
+	valSet.selector = roundRobinProposer
+	if policy == istanbul.Sticky {
+		valSet.selector = stickyProposer
 	}
 
 	return valSet
@@ -164,7 +154,51 @@ 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, valSet.randomness)
+	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]
 }
 
 func (valSet *defaultSet) AddValidators(validators []istanbul.ValidatorData) bool {
@@ -244,10 +278,8 @@ 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,6 +36,7 @@ func TestValidatorSet(t *testing.T) {
 	testNewValidatorSet(t)
 	testNormalValSet(t)
 	testEmptyValSet(t)
+	testStickyProposer(t)
 	testAddAndRemoveValidator(t)
 	testQuorumSizes(t)
 }
@@ -101,6 +102,26 @@ 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) {
@@ -175,6 +196,40 @@ 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)