Set accum of freshly added validator -(total voting power)

lite/doc.go

@@ -121,7 +121,7 @@ If we cannot update directly from H -> H' because there was too much change to
 the validator set, then we can look for some Hm (H < Hm < H') with a validator
 set Vm.  Then we try to update H -> Hm and then Hm -> H' in two steps.  If one
 of these steps doesn't work, then we continue bisecting, until we eventually
-have to externally validate the valdiator set changes at every block.
+have to externally validate the validator set changes at every block.
 
 Since we never trust any server in this protocol, only the signatures
 themselves, it doesn't matter if the seed comes from a (possibly malicious)

state/execution.go

@@ -315,29 +315,55 @@ func updateValidators(currentSet *types.ValidatorSet, abciUpdates []abci.Validat
 	if err != nil {
 		return nil, err
 	}
-
-	// these are tendermint types now
 	for _, valUpdate := range updates {
 		if valUpdate.VotingPower < 0 {
 			return nil, fmt.Errorf("Voting power can't be negative %v", valUpdate)
 		}
 
 		address := valUpdate.Address
 		_, val := currentSet.GetByAddress(address)
-		if valUpdate.VotingPower == 0 {
-			// remove val
+		if valUpdate.VotingPower == 0 { // remove val
 			_, removed := currentSet.Remove(address)
 			if !removed {
 				return nil, fmt.Errorf("Failed to remove validator %X", address)
 			}
-		} else if val == nil {
-			// add val
+		} else if val == nil { // add val
+			// make sure we do not exceed MaxTotalVotingPower by adding this validator:
+			totalVotingPower := currentSet.TotalVotingPower()
+			overflow := (valUpdate.VotingPower+totalVotingPower) > types.MaxTotalVotingPower ||
+				(valUpdate.VotingPower+totalVotingPower) < 0
+			if overflow {
+				return nil, fmt.Errorf(
+					"Failed to add new validator %v. Adding it would exceed max allowed total voting power %v",
+					valUpdate,
+					types.MaxTotalVotingPower)
+			}
+			// TODO: issue #1558 update spec according to the following:
+			// Set Accum to -C*totalVotingPower (with C ~= 1.125) to make sure validators can't
+			// unbond/rebond to reset their (potentially previously negative) Accum to zero.
+			//
+			// Contract: totalVotingPower < MaxTotalVotingPower to ensure Accum does
+			// not exceed the bounds of int64.
+			valUpdate.Accum = -(totalVotingPower + (totalVotingPower >> 3))
 			added := currentSet.Add(valUpdate)
 			if !added {
 				return nil, fmt.Errorf("Failed to add new validator %v", valUpdate)
 			}
-		} else {
-			// update val
+		} else { // update val
+			// make sure we do not exceed MaxTotalVotingPower by updating this validator:
+			totalVotingPower := currentSet.TotalVotingPower()
+			_, curVal := currentSet.GetByAddress(valUpdate.Address)
+			if curVal != nil {
+				curVotingPower := curVal.VotingPower
+				updatedVotingPower := totalVotingPower - curVotingPower + valUpdate.VotingPower
+				overflow := updatedVotingPower > types.MaxTotalVotingPower || updatedVotingPower < 0
+				if overflow {
+					return nil, fmt.Errorf(
+						"Failed to updated new validator %v. Updating it would exceed max allowed total voting power %v",
+						valUpdate,
+						types.MaxTotalVotingPower)
+				}
+			}
 			updated := currentSet.Update(valUpdate)
 			if !updated {
 				return nil, fmt.Errorf("Failed to update validator %X to %v", address, valUpdate)

types/validator.go

@@ -81,13 +81,13 @@ func (v *Validator) Hash() []byte {
 // as its redundant with the pubkey. This also excludes accum which changes
 // every round.
 func (v *Validator) Bytes() []byte {
-	return cdcEncode((struct {
+	return cdcEncode(struct {
 		PubKey      crypto.PubKey
 		VotingPower int64
 	}{
 		v.PubKey,
 		v.VotingPower,
-	}))
+	})
 }
 
 //----------------------------------------

types/validator_set.go

@@ -4,13 +4,19 @@ import (
 	"bytes"
 	"fmt"
 	"math"
+	"math/big"
 	"sort"
 	"strings"
 
 	"github.com/tendermint/tendermint/crypto/merkle"
 	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
+// The maximum allowed total voting power.
+// This is the largest int64 `x` with the property that `x + (x >> 3)` is
+// still in the bounds of int64.
+const MaxTotalVotingPower = 8198552921648689607
+
 // ValidatorSet represent a set of *Validator at a given height.
 // The validators can be fetched by address or index.
 // The index is in order of .Address, so the indices are fixed
@@ -68,25 +74,64 @@ func (vals *ValidatorSet) IncrementAccum(times int) {
 		panic("Cannot call IncrementAccum with non-positive times")
 	}
 
-	// Add VotingPower * times to each validator and order into heap.
+	const shiftEveryNthIter = 10
+	var proposer *Validator
+	// call IncrementAccum(1) times times:
+	for i := 0; i < times; i++ {
+		shiftByAvgAccum := i%shiftEveryNthIter == 0
+		proposer = vals.incrementAccum(shiftByAvgAccum)
+	}
+	isShiftedAvgOnLastIter := (times-1)%shiftEveryNthIter == 0
+	if !isShiftedAvgOnLastIter {
+		validatorsHeap := cmn.NewHeap()
+		vals.shiftByAvgAccum(validatorsHeap)
+	}
+	vals.Proposer = proposer
+}
+
+func (vals *ValidatorSet) incrementAccum(subAvg bool) *Validator {
+	for _, val := range vals.Validators {
+		// Check for overflow for sum.
+		val.Accum = safeAddClip(val.Accum, val.VotingPower)
+	}
+
 	validatorsHeap := cmn.NewHeap()
+	if subAvg { // shift by avg accum
+		vals.shiftByAvgAccum(validatorsHeap)
+	} else { // just update the heap
+		for _, val := range vals.Validators {
+			validatorsHeap.PushComparable(val, accumComparable{val})
+		}
+	}
+
+	// Decrement the validator with most accum:
+	mostest := validatorsHeap.Peek().(*Validator)
+	// mind underflow
+	mostest.Accum = safeSubClip(mostest.Accum, vals.TotalVotingPower())
+
+	return mostest
+}
+
+func (vals *ValidatorSet) computeAvgAccum() int64 {
+	n := int64(len(vals.Validators))
+	sum := big.NewInt(0)
 	for _, val := range vals.Validators {
-		// Check for overflow both multiplication and sum.
-		val.Accum = safeAddClip(val.Accum, safeMulClip(val.VotingPower, int64(times)))
-		validatorsHeap.PushComparable(val, accumComparable{val})
+		sum.Add(sum, big.NewInt(val.Accum))
+	}
+	avg := sum.Div(sum, big.NewInt(n))
+	if avg.IsInt64() {
+		return avg.Int64()
 	}
 
-	// Decrement the validator with most accum times times.
-	for i := 0; i < times; i++ {
-		mostest := validatorsHeap.Peek().(*Validator)
-		// mind underflow
-		mostest.Accum = safeSubClip(mostest.Accum, vals.TotalVotingPower())
+	// this should never happen: each val.Accum is in bounds of int64
+	panic(fmt.Sprintf("Cannot represent avg accum as an int64 %v", avg))
+}
 
-		if i == times-1 {
-			vals.Proposer = mostest
-		} else {
-			validatorsHeap.Update(mostest, accumComparable{mostest})
-		}
+func (vals *ValidatorSet) shiftByAvgAccum(validatorsHeap *cmn.Heap) {
+	avgAccum := vals.computeAvgAccum()
+	for _, val := range vals.Validators {
+		val.Accum = safeSubClip(val.Accum, avgAccum)
+		validatorsHeap.PushComparable(val, accumComparable{val})
 	}
 }
 
@@ -144,10 +189,15 @@ func (vals *ValidatorSet) Size() int {
 // TotalVotingPower returns the sum of the voting powers of all validators.
 func (vals *ValidatorSet) TotalVotingPower() int64 {
 	if vals.totalVotingPower == 0 {
+		sum := int64(0)
 		for _, val := range vals.Validators {
 			// mind overflow
-			vals.totalVotingPower = safeAddClip(vals.totalVotingPower, val.VotingPower)
+			sum = safeAddClip(sum, val.VotingPower)
+		}
+		if sum > MaxTotalVotingPower {
+			sum = MaxTotalVotingPower
 		}
+		vals.totalVotingPower = sum
 	}
 	return vals.totalVotingPower
 }
@@ -476,24 +526,7 @@ func RandValidatorSet(numValidators int, votingPower int64) (*ValidatorSet, []Pr
 }
 
 ///////////////////////////////////////////////////////////////////////////////
-// Safe multiplication and addition/subtraction
-
-func safeMul(a, b int64) (int64, bool) {
-	if a == 0 || b == 0 {
-		return 0, false
-	}
-	if a == 1 {
-		return b, false
-	}
-	if b == 1 {
-		return a, false
-	}
-	if a == math.MinInt64 || b == math.MinInt64 {
-		return -1, true
-	}
-	c := a * b
-	return c, c/b != a
-}
+// Safe addition/subtraction
 
 func safeAdd(a, b int64) (int64, bool) {
 	if b > 0 && a > math.MaxInt64-b {
@@ -513,17 +546,6 @@ func safeSub(a, b int64) (int64, bool) {
 	return a - b, false
 }
 
-func safeMulClip(a, b int64) int64 {
-	c, overflow := safeMul(a, b)
-	if overflow {
-		if (a < 0 || b < 0) && !(a < 0 && b < 0) {
-			return math.MinInt64
-		}
-		return math.MaxInt64
-	}
-	return c
-}
-
 func safeAddClip(a, b int64) int64 {
 	c, overflow := safeAdd(a, b)
 	if overflow {

types/validator_set_test.go

@@ -309,46 +309,26 @@ func TestValidatorSetTotalVotingPowerOverflows(t *testing.T) {
 		{Address: []byte("c"), VotingPower: math.MaxInt64, Accum: 0},
 	})
 
-	assert.EqualValues(t, math.MaxInt64, vset.TotalVotingPower())
+	assert.EqualValues(t, MaxTotalVotingPower, vset.TotalVotingPower())
 }
 
-func TestValidatorSetIncrementAccumOverflows(t *testing.T) {
-	// NewValidatorSet calls IncrementAccum(1)
-	vset := NewValidatorSet([]*Validator{
-		// too much voting power
-		0: {Address: []byte("a"), VotingPower: math.MaxInt64, Accum: 0},
-		// too big accum
-		1: {Address: []byte("b"), VotingPower: 10, Accum: math.MaxInt64},
-		// almost too big accum
-		2: {Address: []byte("c"), VotingPower: 10, Accum: math.MaxInt64 - 5},
-	})
-
-	assert.Equal(t, int64(0), vset.Validators[0].Accum, "0") // because we decrement val with most voting power
-	assert.EqualValues(t, math.MaxInt64, vset.Validators[1].Accum, "1")
-	assert.EqualValues(t, math.MaxInt64, vset.Validators[2].Accum, "2")
-}
-
-func TestValidatorSetIncrementAccumUnderflows(t *testing.T) {
-	// NewValidatorSet calls IncrementAccum(1)
-	vset := NewValidatorSet([]*Validator{
-		0: {Address: []byte("a"), VotingPower: math.MaxInt64, Accum: math.MinInt64},
-		1: {Address: []byte("b"), VotingPower: 1, Accum: math.MinInt64},
-	})
-
-	vset.IncrementAccum(5)
-
-	assert.EqualValues(t, math.MinInt64, vset.Validators[0].Accum, "0")
-	assert.EqualValues(t, math.MinInt64, vset.Validators[1].Accum, "1")
-}
-
-func TestSafeMul(t *testing.T) {
-	f := func(a, b int64) bool {
-		c, overflow := safeMul(a, b)
-		return overflow || (!overflow && c == a*b)
+func TestAvgAccum(t *testing.T) {
+	// Create Validator set without calling IncrementAccum:
+	tcs := []struct {
+		vs   ValidatorSet
+		want int64
+	}{
+		0: {ValidatorSet{Validators: []*Validator{{Accum: 0}, {Accum: 0}, {Accum: 0}}}, 0},
+		1: {ValidatorSet{Validators: []*Validator{{Accum: math.MaxInt64}, {Accum: 0}, {Accum: 0}}}, math.MaxInt64 / 3},
+		2: {ValidatorSet{Validators: []*Validator{{Accum: math.MaxInt64}, {Accum: 0}}}, math.MaxInt64 / 2},
+		3: {ValidatorSet{Validators: []*Validator{{Accum: math.MaxInt64}, {Accum: math.MaxInt64}}}, math.MaxInt64},
+		4: {ValidatorSet{Validators: []*Validator{{Accum: math.MinInt64}, {Accum: math.MinInt64}}}, math.MinInt64},
 	}
-	if err := quick.Check(f, nil); err != nil {
-		t.Error(err)
+	for i, tc := range tcs {
+		got := tc.vs.computeAvgAccum()
+		assert.Equal(t, tc.want, got, "test case: %v", i)
 	}
+
 }
 
 func TestSafeAdd(t *testing.T) {
@@ -361,13 +341,6 @@ func TestSafeAdd(t *testing.T) {
 	}
 }
 
-func TestSafeMulClip(t *testing.T) {
-	assert.EqualValues(t, math.MaxInt64, safeMulClip(math.MinInt64, math.MinInt64))
-	assert.EqualValues(t, math.MinInt64, safeMulClip(math.MaxInt64, math.MinInt64))
-	assert.EqualValues(t, math.MinInt64, safeMulClip(math.MinInt64, math.MaxInt64))
-	assert.EqualValues(t, math.MaxInt64, safeMulClip(math.MaxInt64, 2))
-}
-
 func TestSafeAddClip(t *testing.T) {
 	assert.EqualValues(t, math.MaxInt64, safeAddClip(math.MaxInt64, 10))
 	assert.EqualValues(t, math.MaxInt64, safeAddClip(math.MaxInt64, math.MaxInt64))