fix ProposerSelection by persisting proposer

types/validator_set.go

@@ -8,6 +8,7 @@ import (
 
 	cmn "github.com/tendermint/go-common"
 	"github.com/tendermint/go-merkle"
+	"github.com/tendermint/go-wire"
 )
 
 // ValidatorSet represent a set of *Validator at a given height.
@@ -22,10 +23,10 @@ import (
 // TODO: consider validator Accum overflow
 // TODO: move valset into an iavl tree where key is 'blockbonded|pubkey'
 type ValidatorSet struct {
-	Validators []*Validator // NOTE: persisted via reflect, must be exported.
+	Validators   []*Validator // NOTE: persisted via reflect, must be exported.
+	LastProposer *Validator
 
 	// cached (unexported)
-	proposer         *Validator
 	totalVotingPower int64
 }
 
@@ -42,26 +43,28 @@ func NewValidatorSet(vals []*Validator) *ValidatorSet {
 	if vals != nil {
 		vs.IncrementAccum(1)
 	}
+
 	return vs
 }
 
+// incrementAccum and update the proposer
 // TODO: mind the overflow when times and votingPower shares too large.
 func (valSet *ValidatorSet) IncrementAccum(times int) {
 	// Add VotingPower * times to each validator and order into heap.
 	validatorsHeap := cmn.NewHeap()
 	for _, val := range valSet.Validators {
 		val.Accum += int64(val.VotingPower) * int64(times) // TODO: mind overflow
-		validatorsHeap.Push(val, accumComparable(val.Accum))
+		validatorsHeap.Push(val, accumComparable{val})
 	}
 
-	// Decrement the validator with most accum, times times.
+	// Decrement the validator with most accum times times
 	for i := 0; i < times; i++ {
 		mostest := validatorsHeap.Peek().(*Validator)
 		if i == times-1 {
-			valSet.proposer = mostest
+			valSet.LastProposer = mostest
 		}
 		mostest.Accum -= int64(valSet.TotalVotingPower())
-		validatorsHeap.Update(mostest, accumComparable(mostest.Accum))
+		validatorsHeap.Update(mostest, accumComparable{mostest})
 	}
 }
 
@@ -73,7 +76,7 @@ func (valSet *ValidatorSet) Copy() *ValidatorSet {
 	}
 	return &ValidatorSet{
 		Validators:       validators,
-		proposer:         valSet.proposer,
+		LastProposer:     valSet.LastProposer,
 		totalVotingPower: valSet.totalVotingPower,
 	}
 }
@@ -118,12 +121,20 @@ func (valSet *ValidatorSet) Proposer() (proposer *Validator) {
 	if len(valSet.Validators) == 0 {
 		return nil
 	}
-	if valSet.proposer == nil {
-		for _, val := range valSet.Validators {
-			valSet.proposer = valSet.proposer.CompareAccum(val)
+	if valSet.LastProposer == nil {
+		valSet.LastProposer = valSet.findProposer()
+	}
+	return valSet.LastProposer.Copy()
+}
+
+func (valSet *ValidatorSet) findProposer() *Validator {
+	var proposer *Validator
+	for _, val := range valSet.Validators {
+		if proposer == nil || !bytes.Equal(val.Address, proposer.Address) {
+			proposer = proposer.CompareAccum(val)
 		}
 	}
-	return valSet.proposer.Copy()
+	return proposer
 }
 
 func (valSet *ValidatorSet) Hash() []byte {
@@ -145,7 +156,7 @@ func (valSet *ValidatorSet) Add(val *Validator) (added bool) {
 	if idx == len(valSet.Validators) {
 		valSet.Validators = append(valSet.Validators, val)
 		// Invalidate cache
-		valSet.proposer = nil
+		valSet.LastProposer = nil
 		valSet.totalVotingPower = 0
 		return true
 	} else if bytes.Compare(valSet.Validators[idx].Address, val.Address) == 0 {
@@ -157,7 +168,7 @@ func (valSet *ValidatorSet) Add(val *Validator) (added bool) {
 		copy(newValidators[idx+1:], valSet.Validators[idx:])
 		valSet.Validators = newValidators
 		// Invalidate cache
-		valSet.proposer = nil
+		valSet.LastProposer = nil
 		valSet.totalVotingPower = 0
 		return true
 	}
@@ -170,7 +181,7 @@ func (valSet *ValidatorSet) Update(val *Validator) (updated bool) {
 	} else {
 		valSet.Validators[index] = val.Copy()
 		// Invalidate cache
-		valSet.proposer = nil
+		valSet.LastProposer = nil
 		valSet.totalVotingPower = 0
 		return true
 	}
@@ -190,7 +201,7 @@ func (valSet *ValidatorSet) Remove(address []byte) (val *Validator, removed bool
 		}
 		valSet.Validators = newValidators
 		// Invalidate cache
-		valSet.proposer = nil
+		valSet.LastProposer = nil
 		valSet.totalVotingPower = 0
 		return removedVal, true
 	}
@@ -278,6 +289,24 @@ func (valSet *ValidatorSet) VerifyCommitAny(chainID string, blockID BlockID, hei
 	*/
 }
 
+func (valSet *ValidatorSet) ToBytes() []byte {
+	buf, n, err := new(bytes.Buffer), new(int), new(error)
+	wire.WriteBinary(valSet, buf, n, err)
+	if *err != nil {
+		cmn.PanicCrisis(*err)
+	}
+	return buf.Bytes()
+}
+
+func (valSet *ValidatorSet) FromBytes(b []byte) {
+	r, n, err := bytes.NewReader(b), new(int), new(error)
+	wire.ReadBinary(valSet, r, 0, n, err)
+	if *err != nil {
+		// DATA HAS BEEN CORRUPTED OR THE SPEC HAS CHANGED
+		cmn.PanicCrisis(*err)
+	}
+}
+
 func (valSet *ValidatorSet) String() string {
 	return valSet.StringIndented("")
 }
@@ -325,11 +354,15 @@ func (vs ValidatorsByAddress) Swap(i, j int) {
 //-------------------------------------
 // Use with Heap for sorting validators by accum
 
-type accumComparable int64
+type accumComparable struct {
+	*Validator
+}
 
 // We want to find the validator with the greatest accum.
 func (ac accumComparable) Less(o interface{}) bool {
-	return int64(ac) > int64(o.(accumComparable))
+	other := o.(accumComparable).Validator
+	larger := ac.CompareAccum(other)
+	return bytes.Equal(larger.Address, ac.Address)
 }
 
 //----------------------------------------

types/validator_set_test.go

@@ -44,7 +44,7 @@ func TestCopy(t *testing.T) {
 	}
 }
 
-func TestProposerSelection(t *testing.T) {
+func TestProposerSelection1(t *testing.T) {
 	vset := NewValidatorSet([]*Validator{
 		newValidator([]byte("foo"), 1000),
 		newValidator([]byte("bar"), 300),
@@ -67,74 +67,76 @@ func newValidator(address []byte, power int64) *Validator {
 }
 
 func TestProposerSelection2(t *testing.T) {
-	addr1 := []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
-	addr2 := []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}
-	addr3 := []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}
+	addr0 := []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
+	addr1 := []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}
+	addr2 := []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}
 
 	// when all voting power is same, we go in order of addresses
-	val1, val2, val3 := newValidator(addr1, 100), newValidator(addr2, 100), newValidator(addr3, 100)
-	valList := []*Validator{val1, val2, val3}
+	val0, val1, val2 := newValidator(addr0, 100), newValidator(addr1, 100), newValidator(addr2, 100)
+	valList := []*Validator{val0, val1, val2}
 	vals := NewValidatorSet(valList)
 	for i := 0; i < len(valList)*5; i++ {
-		ii := i % len(valList)
+		ii := (i) % len(valList)
 		prop := vals.Proposer()
 		if !bytes.Equal(prop.Address, valList[ii].Address) {
-			t.Fatalf("Expected %X. Got %X", valList[ii].Address, prop.Address)
+			t.Fatalf("(%d): Expected %X. Got %X", i, valList[ii].Address, prop.Address)
 		}
 		vals.IncrementAccum(1)
 	}
 
 	// One validator has more than the others, but not enough to propose twice in a row
-	*val3 = *newValidator(addr3, 400)
+	*val2 = *newValidator(addr2, 400)
 	vals = NewValidatorSet(valList)
+	// vals.IncrementAccum(1)
 	prop := vals.Proposer()
-	if !bytes.Equal(prop.Address, addr3) {
+	if !bytes.Equal(prop.Address, addr2) {
 		t.Fatalf("Expected address with highest voting power to be first proposer. Got %X", prop.Address)
 	}
 	vals.IncrementAccum(1)
 	prop = vals.Proposer()
-	if !bytes.Equal(prop.Address, addr1) {
+	if !bytes.Equal(prop.Address, addr0) {
 		t.Fatalf("Expected smallest address to be validator. Got %X", prop.Address)
 	}
 
 	// One validator has more than the others, and enough to be proposer twice in a row
-	*val3 = *newValidator(addr3, 401)
+	*val2 = *newValidator(addr2, 401)
 	vals = NewValidatorSet(valList)
 	prop = vals.Proposer()
-	if !bytes.Equal(prop.Address, addr3) {
+	if !bytes.Equal(prop.Address, addr2) {
 		t.Fatalf("Expected address with highest voting power to be first proposer. Got %X", prop.Address)
 	}
 	vals.IncrementAccum(1)
 	prop = vals.Proposer()
-	if !bytes.Equal(prop.Address, addr3) {
+	if !bytes.Equal(prop.Address, addr2) {
 		t.Fatalf("Expected address with highest voting power to be second proposer. Got %X", prop.Address)
 	}
 	vals.IncrementAccum(1)
 	prop = vals.Proposer()
-	if !bytes.Equal(prop.Address, addr1) {
+	if !bytes.Equal(prop.Address, addr0) {
 		t.Fatalf("Expected smallest address to be validator. Got %X", prop.Address)
 	}
 
 	// each validator should be the proposer a proportional number of times
-	val1, val2, val3 = newValidator(addr1, 4), newValidator(addr2, 5), newValidator(addr3, 3)
-	valList = []*Validator{val1, val2, val3}
+	val0, val1, val2 = newValidator(addr0, 4), newValidator(addr1, 5), newValidator(addr2, 3)
+	valList = []*Validator{val0, val1, val2}
 	propCount := make([]int, 3)
 	vals = NewValidatorSet(valList)
-	for i := 0; i < 120; i++ {
+	N := 1
+	for i := 0; i < 120*N; i++ {
 		prop := vals.Proposer()
 		ii := prop.Address[19]
 		propCount[ii] += 1
 		vals.IncrementAccum(1)
 	}
 
-	if propCount[0] != 40 {
-		t.Fatalf("Expected prop count for validator with 4/12 of voting power to be 40/120. Got %d/120", propCount[0])
+	if propCount[0] != 40*N {
+		t.Fatalf("Expected prop count for validator with 4/12 of voting power to be %d/%d. Got %d/%d", 40*N, 120*N, propCount[0], 120*N)
 	}
-	if propCount[1] != 50 {
-		t.Fatalf("Expected prop count for validator with 5/12 of voting power to be 50/120. Got %d/120", propCount[1])
+	if propCount[1] != 50*N {
+		t.Fatalf("Expected prop count for validator with 5/12 of voting power to be %d/%d. Got %d/%d", 50*N, 120*N, propCount[1], 120*N)
 	}
-	if propCount[2] != 30 {
-		t.Fatalf("Expected prop count for validator with 3/12 of voting power to be 30/120. Got %d/120", propCount[2])
+	if propCount[2] != 30*N {
+		t.Fatalf("Expected prop count for validator with 3/12 of voting power to be %d/%d. Got %d/%d", 30*N, 120*N, propCount[2], 120*N)
 	}
 }
 
@@ -154,18 +156,30 @@ func TestProposerSelection3(t *testing.T) {
 
 	// i for the loop
 	// j for the times
-	// we should go in order for ever, despite occasional IncrementAccums with times > 1
+	// we should go in order for ever, despite some IncrementAccums with times > 1
 	var i, j int
-	for ; i < 1000; i++ {
+	for ; i < 10000; i++ {
 		got := vset.Proposer().Address
 		expected := proposerOrder[j%4].Address
 		if !bytes.Equal(got, expected) {
 			t.Fatalf(cmn.Fmt("vset.Proposer (%X) does not match expected proposer (%X) for (%d, %d)", got, expected, i, j))
 		}
 
+		// serialize, deserialize, check proposer
+		b := vset.ToBytes()
+		vset.FromBytes(b)
+
+		computed := vset.Proposer() // findProposer()
+		if i != 0 {
+			if !bytes.Equal(got, computed.Address) {
+				t.Fatalf(cmn.Fmt("vset.Proposer (%X) does not match computed proposer (%X) for (%d, %d)", got, computed.Address, i, j))
+			}
+		}
+
 		// times is usually 1
 		times := 1
-		if cmn.RandInt()%2 > 0 {
+		mod := (cmn.RandInt() % 5) + 1
+		if cmn.RandInt()%mod > 0 {
 			// sometimes its up to 5
 			times = cmn.RandInt() % 5
 		}