From 36f579766049e8cf2dd0c0b4a85944e12d53b0d6 Mon Sep 17 00:00:00 2001
From: Christopher Goes <cwgoes@protonmail.ch>
Date: Mon, 9 Apr 2018 13:12:08 +0200
Subject: [PATCH] Ordering fixes, testcases

---
 x/stake/keeper.go      | 32 ++++++++++++++++++++------------
 x/stake/keeper_keys.go |  4 ++--
 x/stake/keeper_test.go | 41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/x/stake/keeper.go b/x/stake/keeper.go
index a51877b3c..f6aa01c63 100644
--- a/x/stake/keeper.go
+++ b/x/stake/keeper.go
@@ -80,9 +80,6 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) {
 	// retreive the old candidate record
 	oldCandidate, oldFound := k.GetCandidate(ctx, address)
 
-	// update the validator block height (will only get written if stake has changed)
-	candidate.ValidatorHeight = ctx.BlockHeight()
-
 	// marshal the candidate record and add to the state
 	bz, err := k.cdc.MarshalBinary(candidate)
 	if err != nil {
@@ -90,6 +87,26 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) {
 	}
 	store.Set(GetCandidateKey(candidate.Address), bz)
 
+	// 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, oldCandidate.ValidatorHeight, k.cdc))
+	}
+
+	// update the validator block height
+	candidate.ValidatorHeight = ctx.BlockHeight()
+
+	// 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)
@@ -97,15 +114,6 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) {
 		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, oldCandidate.ValidatorHeight, k.cdc))
-	}
 	store.Set(GetValidatorKey(address, validator.Power, validator.Height, k.cdc), bz)
 
 	// add to the validators to update list if is already a validator
diff --git a/x/stake/keeper_keys.go b/x/stake/keeper_keys.go
index a8adc2a12..e9db23c2c 100644
--- a/x/stake/keeper_keys.go
+++ b/x/stake/keeper_keys.go
@@ -34,8 +34,8 @@ 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, height int64, cdc *wire.Codec) []byte {
 	powerBytes := []byte(power.ToLeftPadded(maxDigitsForAccount)) // power big-endian (more powerful validators first)
-	heightBytes := make([]byte, 8)
-	binary.LittleEndian.PutUint64(heightBytes, uint64(height)) // height little-endian (older validators first)
+	heightBytes := make([]byte, binary.MaxVarintLen64)
+	binary.BigEndian.PutUint64(heightBytes, ^uint64(height)) // invert height (older validators first)
 	return append(ValidatorsKey, append(powerBytes, append(heightBytes, addr.Bytes()...)...)...)
 }
 
diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go
index e01df1aae..baaa2ec8c 100644
--- a/x/stake/keeper_test.go
+++ b/x/stake/keeper_test.go
@@ -243,6 +243,47 @@ 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[4].Address, validators[0].Address, "%v", validators)
+	assert.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators)
+	assert.Equal(t, int64(0), validators[0].Height, "%v", validators)
+	assert.Equal(t, int64(10), 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[4].Address, validators[0].Address, "%v", validators)
+	assert.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators)
+
+	// change in voting power of both candidates, ages swapped
+	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)
+
+	// reset assets / heights
+	candidates[3].Assets = sdk.NewRat(300)
+	candidates[4].Assets = sdk.NewRat(200)
+	ctx = ctx.WithBlockHeight(0)
+	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])