diff --git a/lite/doc.go b/lite/doc.go index 00dcce68c..f68798dc5 100644 --- a/lite/doc.go +++ b/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) diff --git a/state/execution.go b/state/execution.go index f1fc5e4ad..06f246f80 100644 --- a/state/execution.go +++ b/state/execution.go @@ -356,20 +356,48 @@ func updateValidators(currentSet *types.ValidatorSet, updates []*types.Validator address := valUpdate.Address _, val := currentSet.GetByAddress(address) // valUpdate.VotingPower is ensured to be non-negative in validation method - if valUpdate.VotingPower == 0 { - // remove val + if valUpdate.VotingPower == 0 { // remove val _, removed := currentSet.Remove(address) if !removed { return 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() + updatedVotingPower := valUpdate.VotingPower + totalVotingPower + overflow := updatedVotingPower > types.MaxTotalVotingPower || updatedVotingPower < 0 + if overflow { + return 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. + // + // Compute Accum = -1.125*totalVotingPower == -(totalVotingPower + (totalVotingPower >> 3)). + valUpdate.Accum = -(totalVotingPower + (totalVotingPower >> 3)) added := currentSet.Add(valUpdate) if !added { return 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() + curVotingPower := val.VotingPower + updatedVotingPower := totalVotingPower - curVotingPower + valUpdate.VotingPower + overflow := updatedVotingPower > types.MaxTotalVotingPower || updatedVotingPower < 0 + if overflow { + return fmt.Errorf( + "Failed to update existing validator %v. Updating it would exceed max allowed total voting power %v", + valUpdate, + types.MaxTotalVotingPower) + } + updated := currentSet.Update(valUpdate) if !updated { return fmt.Errorf("Failed to update validator %X to %v", address, valUpdate) diff --git a/types/signed_msg_type.go b/types/signed_msg_type.go index 301feec91..6bd5f057e 100644 --- a/types/signed_msg_type.go +++ b/types/signed_msg_type.go @@ -10,7 +10,6 @@ const ( // Proposals ProposalType SignedMsgType = 0x20 - ) // IsVoteTypeValid returns true if t is a valid vote type. diff --git a/types/validator.go b/types/validator.go index 4bfd78a6d..cffc28540 100644 --- a/types/validator.go +++ b/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, - })) + }) } //---------------------------------------- diff --git a/types/validator_set.go b/types/validator_set.go index d1bce28c3..366608854 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "math" + "math/big" "sort" "strings" @@ -11,6 +12,15 @@ import ( cmn "github.com/tendermint/tendermint/libs/common" ) +// The maximum allowed total voting power. +// We set the accum of freshly added validators to -1.125*totalVotingPower. +// To compute 1.125*totalVotingPower efficiently, we do: +// totalVotingPower + (totalVotingPower >> 3) because +// x + (x >> 3) = x + x/8 = x * (1 + 0.125). +// MaxTotalVotingPower 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 +78,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 +193,18 @@ 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 { + panic(fmt.Sprintf( + "Total voting power should be guarded to not exceed %v; got: %v", + MaxTotalVotingPower, + sum)) } + vals.totalVotingPower = sum } return vals.totalVotingPower } @@ -308,7 +365,7 @@ func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID, height i return nil } return fmt.Errorf("Invalid commit -- insufficient voting power: got %v, needed %v", - talliedVotingPower, (vals.TotalVotingPower()*2/3 + 1)) + talliedVotingPower, vals.TotalVotingPower()*2/3+1) } // VerifyFutureCommit will check to see if the set would be valid with a different @@ -391,7 +448,7 @@ func (vals *ValidatorSet) VerifyFutureCommit(newSet *ValidatorSet, chainID strin if oldVotingPower <= oldVals.TotalVotingPower()*2/3 { return cmn.NewError("Invalid commit -- insufficient old voting power: got %v, needed %v", - oldVotingPower, (oldVals.TotalVotingPower()*2/3 + 1)) + oldVotingPower, oldVals.TotalVotingPower()*2/3+1) } return nil } @@ -405,7 +462,7 @@ func (vals *ValidatorSet) StringIndented(indent string) string { if vals == nil { return "nil-ValidatorSet" } - valStrings := []string{} + var valStrings []string vals.Iterate(func(index int, val *Validator) bool { valStrings = append(valStrings, val.String()) return false @@ -476,24 +533,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 +553,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 { diff --git a/types/validator_set_test.go b/types/validator_set_test.go index d7a6a5d2b..094b2b7f8 100644 --- a/types/validator_set_test.go +++ b/types/validator_set_test.go @@ -128,7 +128,7 @@ func TestProposerSelection1(t *testing.T) { newValidator([]byte("bar"), 300), newValidator([]byte("baz"), 330), }) - proposers := []string{} + var proposers []string for i := 0; i < 99; i++ { val := vset.GetProposer() proposers = append(proposers, string(val.Address)) @@ -305,53 +305,37 @@ func (valSet *ValidatorSet) fromBytes(b []byte) { //------------------------------------------------------------------- -func TestValidatorSetTotalVotingPowerOverflows(t *testing.T) { - vset := NewValidatorSet([]*Validator{ - {Address: []byte("a"), VotingPower: math.MaxInt64, Accum: 0}, - {Address: []byte("b"), VotingPower: math.MaxInt64, Accum: 0}, - {Address: []byte("c"), VotingPower: math.MaxInt64, Accum: 0}, - }) - - assert.EqualValues(t, math.MaxInt64, 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) +func TestValidatorSetTotalVotingPowerPanicsOnOverflow(t *testing.T) { + // NewValidatorSet calls IncrementAccum which calls TotalVotingPower() + // which should panic on overflows: + shouldPanic := func() { + NewValidatorSet([]*Validator{ + {Address: []byte("a"), VotingPower: math.MaxInt64, Accum: 0}, + {Address: []byte("b"), VotingPower: math.MaxInt64, Accum: 0}, + {Address: []byte("c"), VotingPower: math.MaxInt64, Accum: 0}, + }) + } - assert.EqualValues(t, math.MinInt64, vset.Validators[0].Accum, "0") - assert.EqualValues(t, math.MinInt64, vset.Validators[1].Accum, "1") + assert.Panics(t, shouldPanic) } -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) { @@ -364,13 +348,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))