From b1c83f2edb8a0f7acd9d064c816d18bc200934fc Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Fri, 6 Apr 2018 13:11:26 +0200 Subject: [PATCH 01/12] WIP: Track validator height (closes #582) --- x/stake/keeper.go | 6 +++--- x/stake/keeper_keys.go | 10 +++++++--- x/stake/types.go | 28 ++++++++++++++++------------ 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 751b84017b..2dcfd3cafa 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -101,9 +101,9 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { // update the list ordered by voting power if oldFound { - store.Delete(GetValidatorKey(address, oldCandidate.Assets, k.cdc)) + store.Delete(GetValidatorKey(address, oldCandidate.Assets, oldCandidate.ValidatorHeight, k.cdc)) } - store.Set(GetValidatorKey(address, validator.Power, k.cdc), bz) + store.Set(GetValidatorKey(address, validator.Power, validator.Height, k.cdc), bz) // add to the validators to update list if is already a validator // or is a new validator @@ -136,7 +136,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.ValidatorHeight, k.cdc)) // delete from recent and power weighted validator groups if the validator // exists and add validator with zero power to the validator updates diff --git a/x/stake/keeper_keys.go b/x/stake/keeper_keys.go index 45e0570bc1..ae8f9d7331 100644 --- 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" ) @@ -30,9 +32,11 @@ 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 uint64, cdc *wire.Codec) []byte { + powerBytes := []byte(power.ToLeftPadded(maxDigitsForAccount)) // power big-endian (more powerful validators first) + heightBytes := make([]byte, 8) + binary.LittleEndian.PutUint64(heightBytes, height) // height little-endian (older validators first) + return append(ValidatorsKey, append(powerBytes, append(heightBytes, addr.Bytes()...)...)...) } // get the key for the accumulated update validators diff --git a/x/stake/types.go b/x/stake/types.go index 9f7d97ae51..7757566bfa 100644 --- a/x/stake/types.go +++ b/x/stake/types.go @@ -59,12 +59,13 @@ 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 + ValidatorHeight uint64 `json:"validator_height"` // If considered a validator, height when first considered a validator, else 0 } // Candidates - list of Candidates @@ -73,12 +74,13 @@ 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, + ValidatorHeight: uint64(0), } } @@ -114,6 +116,7 @@ func (c Candidate) validator() Validator { Address: c.Address, PubKey: c.PubKey, Power: c.Assets, + Height: c.ValidatorHeight, } } @@ -127,6 +130,7 @@ type Validator struct { Address sdk.Address `json:"address"` PubKey crypto.PubKey `json:"pub_key"` Power sdk.Rat `json:"voting_power"` + Height uint64 `json:"height"` // If considered a validator, height when first considered a validator, else 0 } // abci validator from stake validator type From 905a9eefb9b6113a5675bc7e095ef51d9bb1b797 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Mon, 9 Apr 2018 12:17:45 +0200 Subject: [PATCH 02/12] Cleanup implementation --- x/stake/keeper.go | 7 ++++++- x/stake/keeper_keys.go | 4 ++-- x/stake/types.go | 6 +++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 2dcfd3cafa..a51877b3c8 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -80,6 +80,9 @@ 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 { @@ -87,7 +90,7 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { } store.Set(GetCandidateKey(candidate.Address), bz) - // mashal the new validator record + // marshal the new validator record validator := candidate.validator() bz, err = k.cdc.MarshalBinary(validator) if err != nil { @@ -121,7 +124,9 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { panic(err) } store.Set(GetAccUpdateValidatorKey(validator.Address), bz) + } + return } diff --git a/x/stake/keeper_keys.go b/x/stake/keeper_keys.go index ae8f9d7331..a8adc2a127 100644 --- a/x/stake/keeper_keys.go +++ b/x/stake/keeper_keys.go @@ -32,10 +32,10 @@ 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 uint64, cdc *wire.Codec) []byte { +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, height) // height little-endian (older validators first) + binary.LittleEndian.PutUint64(heightBytes, uint64(height)) // height little-endian (older validators first) return append(ValidatorsKey, append(powerBytes, append(heightBytes, addr.Bytes()...)...)...) } diff --git a/x/stake/types.go b/x/stake/types.go index 7757566bfa..052a50fa28 100644 --- a/x/stake/types.go +++ b/x/stake/types.go @@ -65,7 +65,7 @@ type Candidate struct { 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 - ValidatorHeight uint64 `json:"validator_height"` // If considered a validator, height when first considered a validator, else 0 + ValidatorHeight int64 `json:"validator_height"` // Earliest height at current voting power } // Candidates - list of Candidates @@ -80,7 +80,7 @@ func NewCandidate(address sdk.Address, pubKey crypto.PubKey, description Descrip Assets: sdk.ZeroRat, Liabilities: sdk.ZeroRat, Description: description, - ValidatorHeight: uint64(0), + ValidatorHeight: int64(0), } } @@ -130,7 +130,7 @@ type Validator struct { Address sdk.Address `json:"address"` PubKey crypto.PubKey `json:"pub_key"` Power sdk.Rat `json:"voting_power"` - Height uint64 `json:"height"` // If considered a validator, height when first considered a validator, else 0 + Height int64 `json:"height"` // Earliest height at current voting power } // abci validator from stake validator type From 36f579766049e8cf2dd0c0b4a85944e12d53b0d6 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Mon, 9 Apr 2018 13:12:08 +0200 Subject: [PATCH 03/12] 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 a51877b3c8..f6aa01c63c 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 a8adc2a127..e9db23c2c3 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 e01df1aaec..baaa2ec8c6 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]) From 91e850b568b244272b5503ecb9d74f02b0e81021 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Fri, 6 Apr 2018 13:11:26 +0200 Subject: [PATCH 04/12] WIP: Track validator height (closes #582) --- x/stake/keeper.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index f6aa01c63c..7f35da1a9d 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -96,24 +96,6 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { 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) - if err != nil { - panic(err) - } - store.Set(GetValidatorKey(address, validator.Power, validator.Height, k.cdc), bz) // add to the validators to update list if is already a validator From 7fa634e772952cde9b0de68c8eff5fdc2228752a Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Mon, 9 Apr 2018 12:17:45 +0200 Subject: [PATCH 05/12] Cleanup implementation --- x/stake/keeper.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 7f35da1a9d..a51877b3c8 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -80,6 +80,9 @@ 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 { @@ -87,6 +90,13 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { } 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 From 90d88114d9063bcec2dc2b0b459fe9b1f896116c Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Mon, 9 Apr 2018 13:12:08 +0200 Subject: [PATCH 06/12] Ordering fixes, testcases --- x/stake/keeper.go | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index a51877b3c8..f6aa01c63c 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 From 166711742e0694d7e8fa24e2fed89a9f8769b6a7 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 12 Apr 2018 11:52:54 +0200 Subject: [PATCH 07/12] Rebase, test fix --- x/stake/keeper.go | 8 +++++--- x/stake/keeper_test.go | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index f6aa01c63c..8cce331cff 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -94,12 +94,14 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { // update the list ordered by voting power if oldFound { + // retain the old validator height, even though stake has changed + candidate.ValidatorHeight = oldCandidate.ValidatorHeight store.Delete(GetValidatorKey(address, oldCandidate.Assets, oldCandidate.ValidatorHeight, k.cdc)) + } else { + // update the validator block height + candidate.ValidatorHeight = ctx.BlockHeight() } - // update the validator block height - candidate.ValidatorHeight = ctx.BlockHeight() - // update the candidate record bz, err = k.cdc.MarshalBinary(candidate) if err != nil { diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index baaa2ec8c6..85b2d72ba4 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -254,7 +254,7 @@ func TestGetValidators(t *testing.T) { 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) + assert.Equal(t, int64(0), validators[1].Height, "%v", validators) // no change in voting power - no change in sort ctx = ctx.WithBlockHeight(20) @@ -274,8 +274,8 @@ func TestGetValidators(t *testing.T) { 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) + assert.Equal(t, candidates[4].Address, validators[0].Address, "%v", validators) + assert.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators) // reset assets / heights candidates[3].Assets = sdk.NewRat(300) From 6424bb85ff1addd3ba13851840f2966e0f4d5914 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 12 Apr 2018 12:37:38 +0200 Subject: [PATCH 08/12] Update ordering logic --- x/stake/keeper.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 8cce331cff..88d82650a5 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -94,11 +94,16 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { // update the list ordered by voting power if oldFound { - // retain the old validator height, even though stake has changed - candidate.ValidatorHeight = oldCandidate.ValidatorHeight + if k.isNewValidator(ctx, store, candidate.Address) { + // already in the validator set - retain the old validator height + candidate.ValidatorHeight = oldCandidate.ValidatorHeight + } else { + // wasn't in the validator set, update the validator block height + candidate.ValidatorHeight = ctx.BlockHeight() + } store.Delete(GetValidatorKey(address, oldCandidate.Assets, oldCandidate.ValidatorHeight, k.cdc)) } else { - // update the validator block height + // wasn't a candidate, update the validator block height candidate.ValidatorHeight = ctx.BlockHeight() } From 4e72250dd1d936279403603ac6f8cd979d171b6d Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Fri, 13 Apr 2018 13:28:41 +0200 Subject: [PATCH 09/12] Add testcases (ref #582 discussion) --- x/stake/keeper_test.go | 62 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index 85b2d72ba4..a137047e5c 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -264,7 +264,7 @@ func TestGetValidators(t *testing.T) { 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 + // 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]) @@ -277,10 +277,68 @@ func TestGetValidators(t *testing.T) { assert.Equal(t, candidates[4].Address, validators[0].Address, "%v", validators) assert.Equal(t, candidates[3].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 4 was set before candidate 3 + require.Equal(t, candidates[4].Address, validators[1].Address, "%v", validators) + + // simulate scenario from https://github.com/cosmos/cosmos-sdk/issues/582#issuecomment-380757108 + 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.ValidatorHeight, int64(40)) + + // first transaction wins + 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[1].Address, validators[0].Address, "%v", validators) + require.Equal(t, candidates[2].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[2].Address, validators[0].Address, "%v", validators) + require.Equal(t, candidates[1].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]) @@ -295,7 +353,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) From 7f38f3470935a5e1aacd1335e5602e992b032621 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Mon, 16 Apr 2018 13:22:05 +0200 Subject: [PATCH 10/12] Stake module block-local tx index counter --- x/stake/keeper.go | 68 +++++++++++++++++++++++++++++++++++------- x/stake/keeper_keys.go | 8 +++-- x/stake/keeper_test.go | 32 +++++++++++--------- x/stake/tick.go | 3 ++ x/stake/types.go | 34 +++++++++++---------- 5 files changed, 104 insertions(+), 41 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 88d82650a5..935a0ab46a 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -33,6 +33,42 @@ func NewKeeper(cdc *wire.Codec, key sdk.StoreKey, ck bank.CoinKeeper, codespace return keeper } +// InitGenesis - store genesis parameters +func (k Keeper) InitGenesis(ctx sdk.Context, data json.RawMessage) error { + var state GenesisState + if err := json.Unmarshal(data, &state); err != nil { + return err + } + k.setPool(ctx, state.Pool) + k.setParams(ctx, state.Params) + return nil +} + +// get the current counter +func (k Keeper) getCounter(ctx sdk.Context) int64 { + store := ctx.KVStore(k.storeKey) + b := store.Get(CounterKey) + if b == nil { + return 0 + } + var counter int64 + err := k.cdc.UnmarshalBinary(b, &counter) + if err != nil { + panic(err) + } + return counter +} + +// set the current counter +func (k Keeper) setCounter(ctx sdk.Context, counter int64) { + 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 +116,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.ValidatorHeight = oldCandidate.ValidatorHeight + candidate.ValidatorCounter = oldCandidate.ValidatorCounter + } + // marshal the candidate record and add to the state bz, err := k.cdc.MarshalBinary(candidate) if err != nil { @@ -92,19 +134,25 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { return } + updateHeight := false + // update the list ordered by voting power if oldFound { - if k.isNewValidator(ctx, store, candidate.Address) { - // already in the validator set - retain the old validator height - candidate.ValidatorHeight = oldCandidate.ValidatorHeight - } else { - // wasn't in the validator set, update the validator block height - candidate.ValidatorHeight = ctx.BlockHeight() + if !k.isNewValidator(ctx, store, candidate.Address) { + updateHeight = true } - store.Delete(GetValidatorKey(address, oldCandidate.Assets, oldCandidate.ValidatorHeight, k.cdc)) + // else already in the validator set - retain the old validator height and counter + store.Delete(GetValidatorKey(address, oldCandidate.Assets, oldCandidate.ValidatorHeight, oldCandidate.ValidatorCounter, k.cdc)) } else { - // wasn't a candidate, update the validator block height + updateHeight = true + } + + if updateHeight { + // wasn't a candidate or wasn't in the validator set, update the validator block height and counter candidate.ValidatorHeight = ctx.BlockHeight() + counter := k.getCounter(ctx) + candidate.ValidatorCounter = counter + k.setCounter(ctx, counter+1) } // update the candidate record @@ -121,7 +169,7 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { panic(err) } - store.Set(GetValidatorKey(address, validator.Power, validator.Height, 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 @@ -156,7 +204,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, candidate.ValidatorHeight, k.cdc)) + store.Delete(GetValidatorKey(address, candidate.Assets, candidate.ValidatorHeight, candidate.ValidatorCounter, k.cdc)) // delete from recent and power weighted validator groups if the validator // exists and add validator with zero power to the validator updates diff --git a/x/stake/keeper_keys.go b/x/stake/keeper_keys.go index e9db23c2c3..c34d40480f 100644 --- a/x/stake/keeper_keys.go +++ b/x/stake/keeper_keys.go @@ -22,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 @@ -32,11 +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, height int64, cdc *wire.Codec) []byte { +func GetValidatorKey(addr sdk.Address, power sdk.Rat, height int64, counter int64, 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) - return append(ValidatorsKey, append(powerBytes, append(heightBytes, addr.Bytes()...)...)...) + counterBytes := make([]byte, binary.MaxVarintLen64) + binary.BigEndian.PutUint64(counterBytes, ^uint64(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 diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index a137047e5c..442bc2c5ea 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -251,8 +251,8 @@ func TestGetValidators(t *testing.T) { 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, 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) @@ -261,8 +261,8 @@ func TestGetValidators(t *testing.T) { 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) + 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) @@ -274,8 +274,8 @@ func TestGetValidators(t *testing.T) { keeper.setCandidate(ctx, candidates[4]) validators = keeper.GetValidators(ctx) require.Equal(t, len(validators), n, "%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, 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) @@ -286,8 +286,8 @@ func TestGetValidators(t *testing.T) { validators = keeper.GetValidators(ctx) require.Equal(t, uint16(len(validators)), params.MaxValidators) require.Equal(t, candidates[0].Address, validators[0].Address, "%v", validators) - // candidate 4 was set before candidate 3 - require.Equal(t, candidates[4].Address, validators[1].Address, "%v", validators) + // candidate 3 was set before candidate 4 + require.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators) // simulate scenario from https://github.com/cosmos/cosmos-sdk/issues/582#issuecomment-380757108 ctx = ctx.WithBlockHeight(40) @@ -309,23 +309,25 @@ func TestGetValidators(t *testing.T) { require.Equal(t, exists, true) require.Equal(t, candidate.ValidatorHeight, int64(40)) - // first transaction wins + // first transaction wins, 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[1].Address, validators[0].Address, "%v", validators) - require.Equal(t, candidates[2].Address, validators[1].Address, "%v", validators) + 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[2].Address, validators[0].Address, "%v", validators) - require.Equal(t, candidates[1].Address, validators[1].Address, "%v", validators) + 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 @@ -627,7 +629,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 = int64(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)) diff --git a/x/stake/tick.go b/x/stake/tick.go index 129be58f01..0b3dd1c83b 100644 --- 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 diff --git a/x/stake/types.go b/x/stake/types.go index 052a50fa28..64f1e3dc2b 100644 --- a/x/stake/types.go +++ b/x/stake/types.go @@ -59,13 +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 - ValidatorHeight int64 `json:"validator_height"` // Earliest height at current voting power + 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 + ValidatorHeight int64 `json:"validator_height"` // Earliest height at current voting power + ValidatorCounter int64 `json:"validator_counter"` // Block-local tx index of validator change } // Candidates - list of Candidates @@ -74,13 +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, - ValidatorHeight: int64(0), + Status: Unbonded, + Address: address, + PubKey: pubKey, + Assets: sdk.ZeroRat, + Liabilities: sdk.ZeroRat, + Description: description, + ValidatorHeight: int64(0), + ValidatorCounter: int64(0), } } @@ -117,6 +119,7 @@ func (c Candidate) validator() Validator { PubKey: c.PubKey, Power: c.Assets, Height: c.ValidatorHeight, + Counter: c.ValidatorCounter, } } @@ -130,7 +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 at current voting power + Height int64 `json:"height"` // Earliest height at current voting power + Counter int64 `json:"counter"` // Block-local tx index for resolving equal voting power & height } // abci validator from stake validator type From d2fa76aa3765e7fadcb379634ef249afe2aaa092 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 19 Apr 2018 13:52:57 +0200 Subject: [PATCH 11/12] Rebase onto develop --- x/stake/keeper.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 935a0ab46a..ffb8ad80e6 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -33,17 +33,6 @@ func NewKeeper(cdc *wire.Codec, key sdk.StoreKey, ck bank.CoinKeeper, codespace return keeper } -// InitGenesis - store genesis parameters -func (k Keeper) InitGenesis(ctx sdk.Context, data json.RawMessage) error { - var state GenesisState - if err := json.Unmarshal(data, &state); err != nil { - return err - } - k.setPool(ctx, state.Pool) - k.setParams(ctx, state.Params) - return nil -} - // get the current counter func (k Keeper) getCounter(ctx sdk.Context) int64 { store := ctx.KVStore(k.storeKey) From 12336f249c3b2c3948f759b31269308f0a2774e6 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 19 Apr 2018 14:09:12 +0200 Subject: [PATCH 12/12] Address PR comments Clarify field names, explain testing scenarios in more depth --- x/stake/keeper.go | 22 +++++++++++----------- x/stake/keeper_keys.go | 6 +++--- x/stake/keeper_test.go | 25 +++++++++++++++++++++---- x/stake/types.go | 40 ++++++++++++++++++++-------------------- 4 files changed, 55 insertions(+), 38 deletions(-) diff --git a/x/stake/keeper.go b/x/stake/keeper.go index ffb8ad80e6..a3502960ea 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -33,14 +33,14 @@ func NewKeeper(cdc *wire.Codec, key sdk.StoreKey, ck bank.CoinKeeper, codespace return keeper } -// get the current counter -func (k Keeper) getCounter(ctx sdk.Context) int64 { +// 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 int64 + var counter int16 err := k.cdc.UnmarshalBinary(b, &counter) if err != nil { panic(err) @@ -48,8 +48,8 @@ func (k Keeper) getCounter(ctx sdk.Context) int64 { return counter } -// set the current counter -func (k Keeper) setCounter(ctx sdk.Context, counter int64) { +// 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 { @@ -107,8 +107,8 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { // if found, copy the old block height and counter if oldFound { - candidate.ValidatorHeight = oldCandidate.ValidatorHeight - candidate.ValidatorCounter = oldCandidate.ValidatorCounter + candidate.ValidatorBondHeight = oldCandidate.ValidatorBondHeight + candidate.ValidatorBondCounter = oldCandidate.ValidatorBondCounter } // marshal the candidate record and add to the state @@ -131,16 +131,16 @@ func (k Keeper) setCandidate(ctx sdk.Context, candidate Candidate) { updateHeight = true } // else already in the validator set - retain the old validator height and counter - store.Delete(GetValidatorKey(address, oldCandidate.Assets, oldCandidate.ValidatorHeight, oldCandidate.ValidatorCounter, k.cdc)) + 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.ValidatorHeight = ctx.BlockHeight() + candidate.ValidatorBondHeight = ctx.BlockHeight() counter := k.getCounter(ctx) - candidate.ValidatorCounter = counter + candidate.ValidatorBondCounter = counter k.setCounter(ctx, counter+1) } @@ -193,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, candidate.ValidatorHeight, candidate.ValidatorCounter, 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 diff --git a/x/stake/keeper_keys.go b/x/stake/keeper_keys.go index c34d40480f..eb641b883b 100644 --- a/x/stake/keeper_keys.go +++ b/x/stake/keeper_keys.go @@ -34,12 +34,12 @@ 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, counter int64, cdc *wire.Codec) []byte { +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, binary.MaxVarintLen64) - binary.BigEndian.PutUint64(counterBytes, ^uint64(counter)) // invert counter (first txns have priority) + 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()...)...)...)...) } diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index 442bc2c5ea..c42f7182b6 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -289,7 +289,19 @@ func TestGetValidators(t *testing.T) { // candidate 3 was set before candidate 4 require.Equal(t, candidates[3].Address, validators[1].Address, "%v", validators) - // simulate scenario from https://github.com/cosmos/cosmos-sdk/issues/582#issuecomment-380757108 + /* + 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) @@ -307,9 +319,14 @@ func TestGetValidators(t *testing.T) { 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.ValidatorHeight, int64(40)) + require.Equal(t, candidate.ValidatorBondHeight, int64(40)) - // first transaction wins, https://github.com/cosmos/cosmos-sdk/issues/582#issuecomment-381250392 + /* + 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) @@ -630,7 +647,7 @@ func TestIsRecentValidator(t *testing.T) { require.Equal(t, 2, len(validators)) assert.Equal(t, candidatesIn[0].validator(), validators[0]) c1ValWithCounter := candidatesIn[1].validator() - c1ValWithCounter.Counter = int64(1) + c1ValWithCounter.Counter = int16(1) assert.Equal(t, c1ValWithCounter, validators[1]) // test a basic retrieve of something that should be a recent validator diff --git a/x/stake/types.go b/x/stake/types.go index 64f1e3dc2b..be48afe3e3 100644 --- a/x/stake/types.go +++ b/x/stake/types.go @@ -59,14 +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 - ValidatorHeight int64 `json:"validator_height"` // Earliest height at current voting power - ValidatorCounter int64 `json:"validator_counter"` // Block-local tx index of validator change + 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 @@ -75,14 +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, - ValidatorHeight: int64(0), - ValidatorCounter: int64(0), + Status: Unbonded, + Address: address, + PubKey: pubKey, + Assets: sdk.ZeroRat, + Liabilities: sdk.ZeroRat, + Description: description, + ValidatorBondHeight: int64(0), + ValidatorBondCounter: int16(0), } } @@ -118,8 +118,8 @@ func (c Candidate) validator() Validator { Address: c.Address, PubKey: c.PubKey, Power: c.Assets, - Height: c.ValidatorHeight, - Counter: c.ValidatorCounter, + Height: c.ValidatorBondHeight, + Counter: c.ValidatorBondCounter, } } @@ -133,8 +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 at current voting power - Counter int64 `json:"counter"` // Block-local tx index for resolving equal voting power & height + 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