Merge pull request #804 from cosmos/cwgoes/staking-validator-updates

Validator age ordering
2018-04-19 13:40:00 -04:00 · 2018-04-19 13:40:00 -04:00 · e089f8ed67
commit e089f8ed67
parent 1b9afdab48 12336f249c
5 changed files with 225 additions and 29 deletions
--- a/x/stake/keeper.go
+++ b/x/stake/keeper.go
@ -33,6 +33,31 @@ func NewKeeper(cdc *wire.Codec, key sdk.StoreKey, ck bank.CoinKeeper, codespace
 	return keeper
 }

+// get the current in-block validator operation counter
+func (k Keeper) getCounter(ctx sdk.Context) int16 {
+	store := ctx.KVStore(k.storeKey)
+	b := store.Get(CounterKey)
+	if b == nil {
+		return 0
+	}
+	var counter int16
+	err := k.cdc.UnmarshalBinary(b, &counter)
+	if err != nil {
+		panic(err)
+	}
+	return counter
+}
+
+// set the current in-block validator operation counter
+func (k Keeper) setCounter(ctx sdk.Context, counter int16) {
+	store := ctx.KVStore(k.storeKey)
+	bz, err := k.cdc.MarshalBinary(counter)
+	if err != nil {
+		panic(err)
+	}
+	store.Set(CounterKey, bz)
+}
+
 //_________________________________________________________________________

 // get a single candidate
@ -80,6 +105,12 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) {
 	// retreive the old candidate record
 	oldCandidate, oldFound := k.GetCandidate(ctx, address)

+	// if found, copy the old block height and counter
+	if oldFound {
+		candidate.ValidatorBondHeight = oldCandidate.ValidatorBondHeight
+		candidate.ValidatorBondCounter = oldCandidate.ValidatorBondCounter
+	}
+
 	// marshal the candidate record and add to the state
 	bz, err := k.cdc.MarshalBinary(candidate)
 	if err != nil {
@ -87,23 +118,47 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) {
 	}
 	store.Set(GetCandidateKey(candidate.Address), bz)

-	// mashal the new validator record
+	// if the voting power is the same no need to update any of the other indexes
+	if oldFound && oldCandidate.Assets.Equal(candidate.Assets) {
+		return
+	}
+
+	updateHeight := false
+
+	// update the list ordered by voting power
+	if oldFound {
+		if !k.isNewValidator(ctx, store, candidate.Address) {
+			updateHeight = true
+		}
+		// else already in the validator set - retain the old validator height and counter
+		store.Delete(GetValidatorKey(address, oldCandidate.Assets, oldCandidate.ValidatorBondHeight, oldCandidate.ValidatorBondCounter, k.cdc))
+	} else {
+		updateHeight = true
+	}
+
+	if updateHeight {
+		// wasn't a candidate or wasn't in the validator set, update the validator block height and counter
+		candidate.ValidatorBondHeight = ctx.BlockHeight()
+		counter := k.getCounter(ctx)
+		candidate.ValidatorBondCounter = counter
+		k.setCounter(ctx, counter+1)
+	}
+
+	// update the candidate record
+	bz, err = k.cdc.MarshalBinary(candidate)
+	if err != nil {
+		panic(err)
+	}
+	store.Set(GetCandidateKey(candidate.Address), bz)
+
+	// marshal the new validator record
 	validator := candidate.validator()
 	bz, err = k.cdc.MarshalBinary(validator)
 	if err != nil {
 		panic(err)
 	}

-	// if the voting power is the same no need to update any of the other indexes
-	if oldFound && oldCandidate.Assets.Equal(candidate.Assets) {
-		return
-	}
-
-	// update the list ordered by voting power
-	if oldFound {
-		store.Delete(GetValidatorKey(address, oldCandidate.Assets, k.cdc))
-	}
-	store.Set(GetValidatorKey(address, validator.Power, k.cdc), bz)
+	store.Set(GetValidatorKey(address, validator.Power, validator.Height, validator.Counter, k.cdc), bz)

 	// add to the validators to update list if is already a validator
 	// or is a new validator
@ -121,7 +176,9 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) {
 			panic(err)
 		}
 		store.Set(GetAccUpdateValidatorKey(validator.Address), bz)
+
 	}
+
 	return
 }

@ -136,7 +193,7 @@ func (k Keeper) removeCandidate(ctx sdk.Context, address sdk.Address) {
 	// delete the old candidate record
 	store := ctx.KVStore(k.storeKey)
 	store.Delete(GetCandidateKey(address))
-	store.Delete(GetValidatorKey(address, candidate.Assets, k.cdc))
+	store.Delete(GetValidatorKey(address, candidate.Assets, candidate.ValidatorBondHeight, candidate.ValidatorBondCounter, k.cdc))

 	// delete from recent and power weighted validator groups if the validator
 	// exists and add validator with zero power to the validator updates
--- a/x/stake/keeper_keys.go
+++ b/x/stake/keeper_keys.go
@ -1,6 +1,8 @@
 package stake

 import (
+	"encoding/binary"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/wire"
 )
@ -20,6 +22,8 @@ var (
 	ToKickOutValidatorsKey = []byte{0x06} // prefix for each key to the last updated validator group

 	DelegatorBondKeyPrefix = []byte{0x07} // prefix for each key to a delegator's bond
+
+	CounterKey = []byte{0x08} // key for block-local tx index
 )

 const maxDigitsForAccount = 12 // ~220,000,000 atoms created at launch
@ -30,9 +34,13 @@ func GetCandidateKey(addr sdk.Address) []byte {
 }

 // get the key for the validator used in the power-store
-func GetValidatorKey(addr sdk.Address, power sdk.Rat, cdc *wire.Codec) []byte {
-	powerBytes := []byte(power.ToLeftPadded(maxDigitsForAccount))
-	return append(ValidatorsKey, append(powerBytes, addr.Bytes()...)...)
+func GetValidatorKey(addr sdk.Address, power sdk.Rat, height int64, counter int16, cdc *wire.Codec) []byte {
+	powerBytes := []byte(power.ToLeftPadded(maxDigitsForAccount)) // power big-endian (more powerful validators first)
+	heightBytes := make([]byte, binary.MaxVarintLen64)
+	binary.BigEndian.PutUint64(heightBytes, ^uint64(height)) // invert height (older validators first)
+	counterBytes := make([]byte, 2)
+	binary.BigEndian.PutUint16(counterBytes, ^uint16(counter)) // invert counter (first txns have priority)
+	return append(ValidatorsKey, append(powerBytes, append(heightBytes, append(counterBytes, addr.Bytes()...)...)...)...)
 }

 // get the key for the accumulated update validators
--- a/x/stake/keeper_test.go
+++ b/x/stake/keeper_test.go
@ -243,6 +243,124 @@ func TestGetValidators(t *testing.T) {
 	assert.Equal(t, sdk.NewRat(300), validators[0].Power, "%v", validators)
 	assert.Equal(t, candidates[3].Address, validators[0].Address, "%v", validators)

+	// test equal voting power, different age
+	candidates[3].Assets = sdk.NewRat(200)
+	ctx = ctx.WithBlockHeight(10)
+	keeper.setCandidate(ctx, candidates[3])
+	validators = keeper.GetValidators(ctx)
+	require.Equal(t, len(validators), n)
+	assert.Equal(t, sdk.NewRat(200), validators[0].Power, "%v", validators)
+	assert.Equal(t, sdk.NewRat(200), validators[1].Power, "%v", validators)
+	assert.Equal(t, candidates[3].Address, validators[0].Address, "%v", validators)
+	assert.Equal(t, candidates[4].Address, validators[1].Address, "%v", validators)
+	assert.Equal(t, int64(0), validators[0].Height, "%v", validators)
+	assert.Equal(t, int64(0), validators[1].Height, "%v", validators)
+
+	// no change in voting power - no change in sort
+	ctx = ctx.WithBlockHeight(20)
+	keeper.setCandidate(ctx, candidates[4])
+	validators = keeper.GetValidators(ctx)
+	require.Equal(t, len(validators), n)
+	assert.Equal(t, candidates[3].Address, validators[0].Address, "%v", validators)
+	assert.Equal(t, candidates[4].Address, validators[1].Address, "%v", validators)
+
+	// change in voting power of both candidates, both still in v-set, no age change
+	candidates[3].Assets = sdk.NewRat(300)
+	candidates[4].Assets = sdk.NewRat(300)
+	keeper.setCandidate(ctx, candidates[3])
+	validators = keeper.GetValidators(ctx)
+	require.Equal(t, len(validators), n)
+	ctx = ctx.WithBlockHeight(30)
+	keeper.setCandidate(ctx, candidates[4])
+	validators = keeper.GetValidators(ctx)
+	require.Equal(t, len(validators), n, "%v", validators)
+	assert.Equal(t, candidates[3].Address, validators[0].Address, "%v", validators)
+	assert.Equal(t, candidates[4].Address, validators[1].Address, "%v", validators)
+
+	// now 2 max validators
+	params := keeper.GetParams(ctx)
+	params.MaxValidators = 2
+	keeper.setParams(ctx, params)
+	candidates[0].Assets = sdk.NewRat(500)
+	keeper.setCandidate(ctx, candidates[0])
+	validators = keeper.GetValidators(ctx)
+	require.Equal(t, uint16(len(validators)), params.MaxValidators)
+	require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators)
+	// candidate 3 was set before candidate 4
+	require.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators)
+
+	/*
+	   A candidate which leaves the validator set due to a decrease in voting power,
+	   then increases to the original voting power, does not get its spot back in the
+	   case of a tie.
+
+	   ref https://github.com/cosmos/cosmos-sdk/issues/582#issuecomment-380757108
+	*/
+	candidates[4].Assets = sdk.NewRat(301)
+	keeper.setCandidate(ctx, candidates[4])
+	validators = keeper.GetValidators(ctx)
+	require.Equal(t, uint16(len(validators)), params.MaxValidators)
+	require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators)
+	require.Equal(t, candidates[4].Address, validators[1].Address, "%v", validators)
+	ctx = ctx.WithBlockHeight(40)
+	// candidate 4 kicked out temporarily
+	candidates[4].Assets = sdk.NewRat(200)
+	keeper.setCandidate(ctx, candidates[4])
+	validators = keeper.GetValidators(ctx)
+	require.Equal(t, uint16(len(validators)), params.MaxValidators)
+	require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators)
+	require.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators)
+	// candidate 4 does not get spot back
+	candidates[4].Assets = sdk.NewRat(300)
+	keeper.setCandidate(ctx, candidates[4])
+	validators = keeper.GetValidators(ctx)
+	require.Equal(t, uint16(len(validators)), params.MaxValidators)
+	require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators)
+	require.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators)
+	candidate, exists := keeper.GetCandidate(ctx, candidates[4].Address)
+	require.Equal(t, exists, true)
+	require.Equal(t, candidate.ValidatorBondHeight, int64(40))
+
+	/*
+	   If two candidates both increase to the same voting power in the same block,
+	   the one with the first transaction should take precedence (become a validator).
+
+	   ref https://github.com/cosmos/cosmos-sdk/issues/582#issuecomment-381250392
+	*/
+	candidates[0].Assets = sdk.NewRat(2000)
+	keeper.setCandidate(ctx, candidates[0])
+	candidates[1].Assets = sdk.NewRat(1000)
+	candidates[2].Assets = sdk.NewRat(1000)
+	keeper.setCandidate(ctx, candidates[1])
+	keeper.setCandidate(ctx, candidates[2])
+	validators = keeper.GetValidators(ctx)
+	require.Equal(t, uint16(len(validators)), params.MaxValidators)
+	require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators)
+	require.Equal(t, candidates[1].Address, validators[1].Address, "%v", validators)
+	candidates[1].Assets = sdk.NewRat(1100)
+	candidates[2].Assets = sdk.NewRat(1100)
+	keeper.setCandidate(ctx, candidates[2])
+	keeper.setCandidate(ctx, candidates[1])
+	validators = keeper.GetValidators(ctx)
+	require.Equal(t, uint16(len(validators)), params.MaxValidators)
+	require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators)
+	require.Equal(t, candidates[2].Address, validators[1].Address, "%v", validators)
+
+	// reset assets / heights
+	params.MaxValidators = 100
+	keeper.setParams(ctx, params)
+	candidates[0].Assets = sdk.NewRat(0)
+	candidates[1].Assets = sdk.NewRat(100)
+	candidates[2].Assets = sdk.NewRat(1)
+	candidates[3].Assets = sdk.NewRat(300)
+	candidates[4].Assets = sdk.NewRat(200)
+	ctx = ctx.WithBlockHeight(0)
+	keeper.setCandidate(ctx, candidates[0])
+	keeper.setCandidate(ctx, candidates[1])
+	keeper.setCandidate(ctx, candidates[2])
+	keeper.setCandidate(ctx, candidates[3])
+	keeper.setCandidate(ctx, candidates[4])
+
 	// test a swap in voting power
 	candidates[0].Assets = sdk.NewRat(600)
 	keeper.setCandidate(ctx, candidates[0])
@ -254,7 +372,7 @@ func TestGetValidators(t *testing.T) {
 	assert.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators)

 	// test the max validators term
-	params := keeper.GetParams(ctx)
+	params = keeper.GetParams(ctx)
 	n = 2
 	params.MaxValidators = uint16(n)
 	keeper.setParams(ctx, params)
@ -528,7 +646,9 @@ func TestIsRecentValidator(t *testing.T) {
 	validators = keeper.GetValidators(ctx)
 	require.Equal(t, 2, len(validators))
 	assert.Equal(t, candidatesIn[0].validator(), validators[0])
-	assert.Equal(t, candidatesIn[1].validator(), validators[1])
+	c1ValWithCounter := candidatesIn[1].validator()
+	c1ValWithCounter.Counter = int16(1)
+	assert.Equal(t, c1ValWithCounter, validators[1])

 	// test a basic retrieve of something that should be a recent validator
 	assert.True(t, keeper.IsRecentValidator(ctx, candidatesIn[0].Address))
--- a/x/stake/tick.go
+++ b/x/stake/tick.go
@ -26,6 +26,9 @@ func (k Keeper) Tick(ctx sdk.Context) (change []abci.Validator) {
 	// save the params
 	k.setPool(ctx, p)

+	// reset the counter
+	k.setCounter(ctx, 0)
+
 	change = k.getAccUpdateValidators(ctx)

 	return
--- a/x/stake/types.go
+++ b/x/stake/types.go
@ -59,12 +59,14 @@ const (
 // exchange rate. Voting power can be calculated as total bonds multiplied by
 // exchange rate.
 type Candidate struct {
-	Status      CandidateStatus `json:"status"`      // Bonded status
-	Address     sdk.Address     `json:"owner"`       // Sender of BondTx - UnbondTx returns here
-	PubKey      crypto.PubKey   `json:"pub_key"`     // Pubkey of candidate
-	Assets      sdk.Rat         `json:"assets"`      // total shares of a global hold pools
-	Liabilities sdk.Rat         `json:"liabilities"` // total shares issued to a candidate's delegators
-	Description Description     `json:"description"` // Description terms for the candidate
+	Status               CandidateStatus `json:"status"`                 // Bonded status
+	Address              sdk.Address     `json:"owner"`                  // Sender of BondTx - UnbondTx returns here
+	PubKey               crypto.PubKey   `json:"pub_key"`                // Pubkey of candidate
+	Assets               sdk.Rat         `json:"assets"`                 // total shares of a global hold pools
+	Liabilities          sdk.Rat         `json:"liabilities"`            // total shares issued to a candidate's delegators
+	Description          Description     `json:"description"`            // Description terms for the candidate
+	ValidatorBondHeight  int64           `json:"validator_bond_height"`  // Earliest height as a bonded validator
+	ValidatorBondCounter int16           `json:"validator_bond_counter"` // Block-local tx index of validator change
 }

 // Candidates - list of Candidates
@ -73,12 +75,14 @@ type Candidates []Candidate
 // NewCandidate - initialize a new candidate
 func NewCandidate(address sdk.Address, pubKey crypto.PubKey, description Description) Candidate {
 	return Candidate{
-		Status:      Unbonded,
-		Address:     address,
-		PubKey:      pubKey,
-		Assets:      sdk.ZeroRat,
-		Liabilities: sdk.ZeroRat,
-		Description: description,
+		Status:               Unbonded,
+		Address:              address,
+		PubKey:               pubKey,
+		Assets:               sdk.ZeroRat,
+		Liabilities:          sdk.ZeroRat,
+		Description:          description,
+		ValidatorBondHeight:  int64(0),
+		ValidatorBondCounter: int16(0),
 	}
 }

@ -114,6 +118,8 @@ func (c Candidate) validator() Validator {
 		Address: c.Address,
 		PubKey:  c.PubKey,
 		Power:   c.Assets,
+		Height:  c.ValidatorBondHeight,
+		Counter: c.ValidatorBondCounter,
 	}
 }

@ -127,6 +133,8 @@ type Validator struct {
 	Address sdk.Address   `json:"address"`
 	PubKey  crypto.PubKey `json:"pub_key"`
 	Power   sdk.Rat       `json:"voting_power"`
+	Height  int64         `json:"height"`  // Earliest height as a validator
+	Counter int16         `json:"counter"` // Block-local tx index for resolving equal voting power & height
 }

 // abci validator from stake validator type