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