From e70249b631c797e3ea6a215fff0c821de290039e Mon Sep 17 00:00:00 2001 From: rigelrozanski Date: Sun, 20 May 2018 17:39:04 -0400 Subject: [PATCH] more bucky comments, single status validator, only one store now for core validator object --- cmd/gaia/app/app_test.go | 2 +- types/stake.go | 1 - x/auth/ante.go | 3 +- x/stake/client/cli/query.go | 4 -- x/stake/handler.go | 12 ++-- x/stake/handler_test.go | 6 +- x/stake/keeper.go | 61 ++++++++-------- x/stake/keeper_test.go | 34 ++++----- x/stake/shares.go | 57 +++++++-------- x/stake/tick_test.go | 6 +- x/stake/validator.go | 113 +++++++++++++++--------------- x/stake/validator_test.go | 40 +++++------ x/stake/view_slash_keeper_test.go | 4 +- 13 files changed, 166 insertions(+), 177 deletions(-) diff --git a/cmd/gaia/app/app_test.go b/cmd/gaia/app/app_test.go index de15e0127f..f88db59a6b 100644 --- a/cmd/gaia/app/app_test.go +++ b/cmd/gaia/app/app_test.go @@ -407,7 +407,7 @@ func TestStakeMsgs(t *testing.T) { require.Equal(t, genCoins.Minus(sdk.Coins{bondCoin}), res1.GetCoins()) validator, found := gapp.stakeKeeper.GetValidator(ctxDeliver, addr1) require.True(t, found) - require.Equal(t, addr1, validator.Address) + require.Equal(t, addr1, validator.Owner) require.Equal(t, sdk.Bonded, validator.Status) require.True(sdk.RatEq(t, sdk.NewRat(10), validator.PoolShares.Bonded())) diff --git a/types/stake.go b/types/stake.go index 68a592352a..df74a705b9 100644 --- a/types/stake.go +++ b/types/stake.go @@ -13,7 +13,6 @@ const ( Unbonded BondStatus = 0x00 Unbonding BondStatus = 0x01 Bonded BondStatus = 0x02 - Revoked BondStatus = 0x03 ) // validator for a delegated proof of stake system diff --git a/x/auth/ante.go b/x/auth/ante.go index c9c30b6405..21d17fb9be 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -166,5 +166,4 @@ func deductFees(acc sdk.Account, fee sdk.StdFee) (sdk.Account, sdk.Result) { } // BurnFeeHandler burns all fees (decreasing total supply) -func BurnFeeHandler(ctx sdk.Context, fee sdk.Coins) { -} +func BurnFeeHandler(_ sdk.Context, _ sdk.Tx, _ sdk.Coins) {} diff --git a/x/stake/client/cli/query.go b/x/stake/client/cli/query.go index cb4a00300a..079dd003d8 100644 --- a/x/stake/client/cli/query.go +++ b/x/stake/client/cli/query.go @@ -37,10 +37,6 @@ func GetCmdQueryValidator(storeName string, cdc *wire.Codec) *cobra.Command { // parse out the validator validator := new(stake.Validator) cdc.MustUnmarshalBinary(res, validator) - err = cdc.UnmarshalBinary(res, validator) - if err != nil { - return err - } output, err := wire.MarshalJSONIndent(cdc, validator) fmt.Println(string(output)) diff --git a/x/stake/handler.go b/x/stake/handler.go index 4ce73a6d48..a9eafa7c4c 100644 --- a/x/stake/handler.go +++ b/x/stake/handler.go @@ -113,7 +113,7 @@ func handleMsgDelegate(ctx sdk.Context, msg MsgDelegate, k Keeper) sdk.Result { if msg.Bond.Denom != k.GetParams(ctx).BondDenom { return ErrBadBondingDenom(k.codespace).Result() } - if validator.Status == sdk.Revoked { + if validator.Revoked == true { return ErrValidatorRevoked(k.codespace).Result() } if ctx.IsCheckTx() { @@ -212,7 +212,7 @@ func handleMsgUnbond(ctx sdk.Context, msg MsgUnbond, k Keeper) sdk.Result { // if the bond is the owner of the validator then // trigger a revoke candidacy if bytes.Equal(bond.DelegatorAddr, validator.Owner) && - validator.Status != sdk.Revoked { + validator.Revoked == false { revokeCandidacy = true } @@ -235,13 +235,11 @@ func handleMsgUnbond(ctx sdk.Context, msg MsgUnbond, k Keeper) sdk.Result { if revokeCandidacy { // change the share types to unbonded if they were not already - if validator.Status == sdk.Bonded { - validator.Status = sdk.Unbonded - validator, pool = validator.UpdateSharesLocation(pool) + if validator.Status() == sdk.Bonded { + validator, pool = validator.UpdateStatus(pool, sdk.Unbonded) } - // lastly update the status - validator.Status = sdk.Revoked + validator.Revoked = true } // deduct shares from the validator diff --git a/x/stake/handler_test.go b/x/stake/handler_test.go index 4150c395d1..8e9fdabe81 100644 --- a/x/stake/handler_test.go +++ b/x/stake/handler_test.go @@ -44,7 +44,7 @@ func TestDuplicatesMsgDeclareCandidacy(t *testing.T) { validator, found := keeper.GetValidator(ctx, validatorAddr) require.True(t, found) assert.Equal(t, sdk.Bonded, validator.Status) - assert.Equal(t, validatorAddr, validator.Address) + assert.Equal(t, validatorAddr, validator.Owner) assert.Equal(t, pk, validator.PubKey) assert.Equal(t, sdk.NewRat(10), validator.PoolShares.Bonded()) assert.Equal(t, sdk.NewRat(10), validator.DelegatorShares) @@ -233,7 +233,7 @@ func TestMultipleMsgDeclareCandidacy(t *testing.T) { require.Equal(t, (i + 1), len(validators)) val := validators[i] balanceExpd := initBond - 10 - balanceGot := accMapper.GetAccount(ctx, val.Address).GetCoins().AmountOf(params.BondDenom) + balanceGot := accMapper.GetAccount(ctx, val.Owner).GetCoins().AmountOf(params.BondDenom) require.Equal(t, i+1, len(validators), "expected %d validators got %d, validators: %v", i+1, len(validators), validators) require.Equal(t, 10, int(val.DelegatorShares.Evaluate()), "expected %d shares, got %d", 10, val.DelegatorShares) require.Equal(t, balanceExpd, balanceGot, "expected account to have %d, got %d", balanceExpd, balanceGot) @@ -256,7 +256,7 @@ func TestMultipleMsgDeclareCandidacy(t *testing.T) { require.False(t, found) expBalance := initBond - gotBalance := accMapper.GetAccount(ctx, validatorPre.Address).GetCoins().AmountOf(params.BondDenom) + gotBalance := accMapper.GetAccount(ctx, validatorPre.Owner).GetCoins().AmountOf(params.BondDenom) require.Equal(t, expBalance, gotBalance, "expected account to have %d, got %d", expBalance, gotBalance) } } diff --git a/x/stake/keeper.go b/x/stake/keeper.go index 19649f22e8..6adf0acf85 100644 --- a/x/stake/keeper.go +++ b/x/stake/keeper.go @@ -35,6 +35,11 @@ func NewKeeper(cdc *wire.Codec, key sdk.StoreKey, ck bank.Keeper, codespace sdk. // get a single validator func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.Address) (validator Validator, found bool) { store := ctx.KVStore(k.storeKey) + return getValidator(store, addr) +} + +// get a single validator +func (k Keeper) getValidator(store sdk.KVStore, addr sdk.Address) (validator Validator, found bool) { b := store.Get(GetValidatorKey(addr)) if b == nil { return validator, false @@ -103,7 +108,7 @@ func (k Keeper) setValidator(ctx sdk.Context, validator Validator) Validator { // if the voting power/status is the same no need to update any of the other indexes // TODO will need to implement this to have regard for "unrevoke" transaction however // it shouldn't return here under that transaction - if oldValidator.Status == validator.Status && + if oldValidator.Status() == validator.Status() && oldValidator.PoolShares.Equal(validator.PoolShares) { return validator } else if oldValidator.PoolShares.Bonded().LT(validator.PoolShares.Bonded()) { @@ -114,7 +119,7 @@ func (k Keeper) setValidator(ctx sdk.Context, validator Validator) Validator { } // if already a validator, copy the old block height and counter, else set them - if oldFound && oldValidator.Status == sdk.Bonded { + if oldFound && oldValidator.Status() == sdk.Bonded { validator.BondHeight = oldValidator.BondHeight validator.BondIntraTxCounter = oldValidator.BondIntraTxCounter } else { @@ -126,14 +131,14 @@ func (k Keeper) setValidator(ctx sdk.Context, validator Validator) Validator { // update the list ordered by voting power bz := k.cdc.MustMarshalBinary(validator) - store.Set(GetValidatorsBondedByPowerKey(validator, pool), bz) + store.Set(GetValidatorsBondedByPowerKey(validator, pool), validator.Address) // efficiency case: // add to the validators and return to update list if is already a validator and power is increasing - if powerIncreasing && oldFound && oldValidator.Status == sdk.Bonded { + if powerIncreasing && oldFound && oldValidator.Status() == sdk.Bonded { // update the store for bonded validators - store.Set(GetValidatorsBondedKey(validator.PubKey), bz) + store.Set(GetValidatorsBondedKey(validator.PubKey), validator.Address) // and the Tendermint updates bz := k.cdc.MustMarshalBinary(validator.abciValidator(k.cdc)) @@ -163,6 +168,7 @@ func (k Keeper) removeValidator(ctx sdk.Context, address sdk.Address) { pool := k.getPool(store) store.Delete(GetValidatorKey(address)) store.Delete(GetValidatorsBondedByPowerKey(validator, pool)) + store.Delete(GetValidatorsBondedKey(validator.PubKey)) // delete from the current and power weighted validator groups if the validator // is bonded - and add validator with zero power to the validator updates @@ -171,7 +177,6 @@ func (k Keeper) removeValidator(ctx sdk.Context, address sdk.Address) { } bz := k.cdc.MustMarshalBinary(validator.abciValidatorZero(k.cdc)) store.Set(GetTendermintUpdatesKey(address), bz) - store.Delete(GetValidatorsBondedKey(validator.PubKey)) } //___________________________________________________________________________ @@ -192,9 +197,8 @@ func (k Keeper) GetValidatorsBonded(ctx sdk.Context) (validators []Validator) { if i > int(maxValidators-1) { panic("maxValidators is less than the number of records in ValidatorsBonded Store, store should have been updated") } - bz := iterator.Value() - var validator Validator - k.cdc.MustUnmarshalBinary(bz, &validator) + address := iterator.Value() + validator := getValidator(store, address) validators[i] = validator i++ } @@ -214,10 +218,9 @@ func (k Keeper) GetValidatorsBondedByPower(ctx sdk.Context) []Validator { iterator.Close() break } - bz := iterator.Value() - var validator Validator - k.cdc.MustUnmarshalBinary(bz, &validator) - if validator.Status == sdk.Bonded { + address := iterator.Value() + validator := getValidator(store, address) + if validator.Status() == sdk.Bonded { validators[i] = validator i++ } @@ -257,7 +260,7 @@ func (k Keeper) updateBondedValidators(ctx sdk.Context, store sdk.KVStore, pool // iterator.Value is the validator object toKickOut[string(addr)] = iterator.Value() - store.Delete(iterator.Key()) + // XXX store.Delete(iterator.Key()) } iterator.Close() @@ -280,8 +283,8 @@ func (k Keeper) updateBondedValidators(ctx sdk.Context, store sdk.KVStore, pool // remove from ToKickOut group delete(toKickOut, string(validator.Owner)) - // also add to the current validators group - store.Set(GetValidatorsBondedKey(validator.PubKey), bz) + // XXX also add to the current validators group + //store.Set(GetValidatorsBondedKey(validator.PubKey), validator.Address) } else { // if it wasn't in the toKickOut group it means @@ -314,22 +317,21 @@ func (k Keeper) unbondValidator(ctx sdk.Context, store sdk.KVStore, validator Va pool := k.GetPool(ctx) // sanity check - if validator.Status == sdk.Unbonded { + if validator.Status() == sdk.Unbonded { panic(fmt.Sprintf("should not already be be unbonded, validator: %v\n", validator)) } - // first delete the old record in the pool - store.Delete(GetValidatorsBondedByPowerKey(validator, pool)) + // XXX first delete the old record in the pool + //store.Delete(GetValidatorsBondedByPowerKey(validator, pool)) // set the status - validator.Status = sdk.Unbonded - validator, pool = validator.UpdateSharesLocation(pool) + validator, pool = validator.UpdateStatus(pool, sdk.Unbonded) k.setPool(ctx, pool) // save the now unbonded validator record bzVal := k.cdc.MustMarshalBinary(validator) store.Set(GetValidatorKey(validator.Owner), bzVal) - store.Set(GetValidatorsBondedByPowerKey(validator, pool), bzVal) + // XXX store.Set(GetValidatorsBondedByPowerKey(validator, pool), validator.Address) // add to accumulated changes for tendermint bzABCI := k.cdc.MustMarshalBinary(validator.abciValidatorZero(k.cdc)) @@ -344,23 +346,22 @@ func (k Keeper) bondValidator(ctx sdk.Context, store sdk.KVStore, validator Vali pool := k.GetPool(ctx) // sanity check - if validator.Status == sdk.Bonded { + if validator.Status() == sdk.Bonded { panic(fmt.Sprintf("should not already be be bonded, validator: %v\n", validator)) } - // first delete the old record in the pool - store.Delete(GetValidatorsBondedByPowerKey(validator, pool)) + // XXX first delete the old record in the pool + //store.Delete(GetValidatorsBondedByPowerKey(validator, pool)) // set the status - validator.Status = sdk.Bonded - validator, pool = validator.UpdateSharesLocation(pool) + validator, pool = validator.UpdateStatus(pool, sdk.Bonded) k.setPool(ctx, pool) - // save the now bonded validator record to the three referened stores + // save the now bonded validator record to the three referenced stores bzVal := k.cdc.MustMarshalBinary(validator) store.Set(GetValidatorKey(validator.Owner), bzVal) - store.Set(GetValidatorsBondedByPowerKey(validator, pool), bzVal) - store.Set(GetValidatorsBondedKey(validator.PubKey), bzVal) + // XXX store.Set(GetValidatorsBondedByPowerKey(validator, pool), validator.Address) + store.Set(GetValidatorsBondedKey(validator.PubKey), validator.Address) // add to accumulated changes for tendermint bzABCI := k.cdc.MustMarshalBinary(validator.abciValidator(k.cdc)) diff --git a/x/stake/keeper_test.go b/x/stake/keeper_test.go index 548916460a..528bd054f4 100644 --- a/x/stake/keeper_test.go +++ b/x/stake/keeper_test.go @@ -121,7 +121,7 @@ func TestValidatorBasics(t *testing.T) { assert.True(ValEq(t, validators[2], resVals[1])) // remove a record - keeper.removeValidator(ctx, validators[1].Address) + keeper.removeValidator(ctx, validators[1].Owner) _, found = keeper.GetValidator(ctx, addrVals[1]) assert.False(t, found) } @@ -149,11 +149,11 @@ func GetValidatorSortingUnmixed(t *testing.T) { assert.Equal(t, sdk.NewRat(100), resValidators[2].PoolShares.Bonded(), "%v", resValidators) assert.Equal(t, sdk.NewRat(1), resValidators[3].PoolShares.Bonded(), "%v", resValidators) assert.Equal(t, sdk.NewRat(0), resValidators[4].PoolShares.Bonded(), "%v", resValidators) - assert.Equal(t, validators[3].Address, resValidators[0].Address, "%v", resValidators) - assert.Equal(t, validators[4].Address, resValidators[1].Address, "%v", resValidators) - assert.Equal(t, validators[1].Address, resValidators[2].Address, "%v", resValidators) - assert.Equal(t, validators[2].Address, resValidators[3].Address, "%v", resValidators) - assert.Equal(t, validators[0].Address, resValidators[4].Address, "%v", resValidators) + assert.Equal(t, validators[3].Owner, resValidators[0].Owner, "%v", resValidators) + assert.Equal(t, validators[4].Owner, resValidators[1].Owner, "%v", resValidators) + assert.Equal(t, validators[1].Owner, resValidators[2].Owner, "%v", resValidators) + assert.Equal(t, validators[2].Owner, resValidators[3].Owner, "%v", resValidators) + assert.Equal(t, validators[0].Owner, resValidators[4].Owner, "%v", resValidators) // test a basic increase in voting power validators[3].PoolShares = NewBondedShares(sdk.NewRat(500)) @@ -252,11 +252,11 @@ func GetValidatorSortingMixed(t *testing.T) { assert.Equal(t, sdk.NewRat(100), resValidators[2].PoolShares.Bonded(), "%v", resValidators) assert.Equal(t, sdk.NewRat(1), resValidators[3].PoolShares.Bonded(), "%v", resValidators) assert.Equal(t, sdk.NewRat(0), resValidators[4].PoolShares.Bonded(), "%v", resValidators) - assert.Equal(t, validators[3].Address, resValidators[0].Address, "%v", resValidators) - assert.Equal(t, validators[4].Address, resValidators[1].Address, "%v", resValidators) - assert.Equal(t, validators[1].Address, resValidators[2].Address, "%v", resValidators) - assert.Equal(t, validators[2].Address, resValidators[3].Address, "%v", resValidators) - assert.Equal(t, validators[0].Address, resValidators[4].Address, "%v", resValidators) + assert.Equal(t, validators[3].Owner, resValidators[0].Owner, "%v", resValidators) + assert.Equal(t, validators[4].Owner, resValidators[1].Owner, "%v", resValidators) + assert.Equal(t, validators[1].Owner, resValidators[2].Owner, "%v", resValidators) + assert.Equal(t, validators[2].Owner, resValidators[3].Owner, "%v", resValidators) + assert.Equal(t, validators[0].Owner, resValidators[4].Owner, "%v", resValidators) } // TODO seperate out into multiple tests @@ -280,7 +280,7 @@ func TestGetValidatorsEdgeCases(t *testing.T) { validators[i] = keeper.setValidator(ctx, validators[i]) } for i := range amts { - validators[i], found = keeper.GetValidator(ctx, validators[i].Address) + validators[i], found = keeper.GetValidator(ctx, validators[i].Owner) require.True(t, found) } resValidators := keeper.GetValidatorsBondedByPower(ctx) @@ -301,7 +301,7 @@ func TestGetValidatorsEdgeCases(t *testing.T) { // validator 3 enters bonded validator set ctx = ctx.WithBlockHeight(40) - validators[3], found = keeper.GetValidator(ctx, validators[3].Address) + validators[3], found = keeper.GetValidator(ctx, validators[3].Owner) require.True(t, found) validators[3].PoolShares = NewUnbondedShares(sdk.NewRat(401)) validators[3] = keeper.setValidator(ctx, validators[3]) @@ -325,7 +325,7 @@ func TestGetValidatorsEdgeCases(t *testing.T) { require.Equal(t, nMax, uint16(len(resValidators))) assert.True(ValEq(t, validators[0], resValidators[0])) assert.True(ValEq(t, validators[2], resValidators[1])) - validator, exists := keeper.GetValidator(ctx, validators[3].Address) + validator, exists := keeper.GetValidator(ctx, validators[3].Owner) require.Equal(t, exists, true) require.Equal(t, int64(40), validator.BondHeight) } @@ -389,7 +389,7 @@ func TestFullValidatorSetPowerChange(t *testing.T) { } for i := range amts { var found bool - validators[i], found = keeper.GetValidator(ctx, validators[i].Address) + validators[i], found = keeper.GetValidator(ctx, validators[i].Owner) require.True(t, found) } assert.Equal(t, sdk.Unbonded, validators[0].Status) @@ -458,8 +458,8 @@ func TestGetTendermintUpdatesAllNone(t *testing.T) { keeper.clearTendermintUpdates(ctx) assert.Equal(t, 0, len(keeper.getTendermintUpdates(ctx))) - keeper.removeValidator(ctx, validators[0].Address) - keeper.removeValidator(ctx, validators[1].Address) + keeper.removeValidator(ctx, validators[0].Owner) + keeper.removeValidator(ctx, validators[1].Owner) updates = keeper.getTendermintUpdates(ctx) require.Equal(t, 2, len(updates)) diff --git a/x/stake/shares.go b/x/stake/shares.go index 425596197f..d5fe93844d 100644 --- a/x/stake/shares.go +++ b/x/stake/shares.go @@ -7,42 +7,35 @@ import ( // kind of shares type PoolShareKind byte -// nolint -const ( - ShareUnbonded PoolShareKind = 0x00 - ShareUnbonding PoolShareKind = 0x01 - ShareBonded PoolShareKind = 0x02 -) - // pool shares held by a validator type PoolShares struct { - Kind PoolShareKind `json:"kind"` - Amount sdk.Rat `json:"shares"` // total shares of type ShareKind + Status sdk.BondStatus `json:"status"` + Amount sdk.Rat `json:"amount"` // total shares of type ShareKind } // only the vitals - does not check bond height of IntraTxCounter func (s PoolShares) Equal(s2 PoolShares) bool { - return s.Kind == s2.Kind && + return s.Status == s2.Status && s.Amount.Equal(s2.Amount) } func NewUnbondedShares(amount sdk.Rat) PoolShares { return PoolShares{ - Kind: ShareUnbonded, + Status: sdk.Unbonded, Amount: amount, } } func NewUnbondingShares(amount sdk.Rat) PoolShares { return PoolShares{ - Kind: ShareUnbonding, + Status: sdk.Unbonding, Amount: amount, } } func NewBondedShares(amount sdk.Rat) PoolShares { return PoolShares{ - Kind: ShareBonded, + Status: sdk.Bonded, Amount: amount, } } @@ -51,7 +44,7 @@ func NewBondedShares(amount sdk.Rat) PoolShares { // amount of unbonded shares func (s PoolShares) Unbonded() sdk.Rat { - if s.Kind == ShareUnbonded { + if s.Status == sdk.Unbonded { return s.Amount } return sdk.ZeroRat() @@ -59,7 +52,7 @@ func (s PoolShares) Unbonded() sdk.Rat { // amount of unbonding shares func (s PoolShares) Unbonding() sdk.Rat { - if s.Kind == ShareUnbonding { + if s.Status == sdk.Unbonding { return s.Amount } return sdk.ZeroRat() @@ -67,7 +60,7 @@ func (s PoolShares) Unbonding() sdk.Rat { // amount of bonded shares func (s PoolShares) Bonded() sdk.Rat { - if s.Kind == ShareBonded { + if s.Status == sdk.Bonded { return s.Amount } return sdk.ZeroRat() @@ -78,14 +71,14 @@ func (s PoolShares) Bonded() sdk.Rat { // equivalent amount of shares if the shares were unbonded func (s PoolShares) ToUnbonded(p Pool) PoolShares { var amount sdk.Rat - switch s.Kind { - case ShareBonded: + switch s.Status { + case sdk.Bonded: exRate := p.bondedShareExRate().Quo(p.unbondedShareExRate()) // (tok/bondedshr)/(tok/unbondedshr) = unbondedshr/bondedshr amount = s.Amount.Mul(exRate) // bondedshr*unbondedshr/bondedshr = unbondedshr - case ShareUnbonding: + case sdk.Unbonding: exRate := p.unbondingShareExRate().Quo(p.unbondedShareExRate()) // (tok/unbondingshr)/(tok/unbondedshr) = unbondedshr/unbondingshr amount = s.Amount.Mul(exRate) // unbondingshr*unbondedshr/unbondingshr = unbondedshr - case ShareUnbonded: + case sdk.Unbonded: amount = s.Amount } return NewUnbondedShares(amount) @@ -94,13 +87,13 @@ func (s PoolShares) ToUnbonded(p Pool) PoolShares { // equivalent amount of shares if the shares were unbonding func (s PoolShares) ToUnbonding(p Pool) PoolShares { var amount sdk.Rat - switch s.Kind { - case ShareBonded: + switch s.Status { + case sdk.Bonded: exRate := p.bondedShareExRate().Quo(p.unbondingShareExRate()) // (tok/bondedshr)/(tok/unbondingshr) = unbondingshr/bondedshr amount = s.Amount.Mul(exRate) // bondedshr*unbondingshr/bondedshr = unbondingshr - case ShareUnbonding: + case sdk.Unbonding: amount = s.Amount - case ShareUnbonded: + case sdk.Unbonded: exRate := p.unbondedShareExRate().Quo(p.unbondingShareExRate()) // (tok/unbondedshr)/(tok/unbondingshr) = unbondingshr/unbondedshr amount = s.Amount.Mul(exRate) // unbondedshr*unbondingshr/unbondedshr = unbondingshr } @@ -110,13 +103,13 @@ func (s PoolShares) ToUnbonding(p Pool) PoolShares { // equivalent amount of shares if the shares were bonded func (s PoolShares) ToBonded(p Pool) PoolShares { var amount sdk.Rat - switch s.Kind { - case ShareBonded: + switch s.Status { + case sdk.Bonded: amount = s.Amount - case ShareUnbonding: + case sdk.Unbonding: exRate := p.unbondingShareExRate().Quo(p.bondedShareExRate()) // (tok/ubshr)/(tok/bshr) = bshr/ubshr amount = s.Amount.Mul(exRate) // ubshr*bshr/ubshr = bshr - case ShareUnbonded: + case sdk.Unbonded: exRate := p.unbondedShareExRate().Quo(p.bondedShareExRate()) // (tok/ubshr)/(tok/bshr) = bshr/ubshr amount = s.Amount.Mul(exRate) // ubshr*bshr/ubshr = bshr } @@ -127,12 +120,12 @@ func (s PoolShares) ToBonded(p Pool) PoolShares { // get the equivalent amount of tokens contained by the shares func (s PoolShares) Tokens(p Pool) sdk.Rat { - switch s.Kind { - case ShareBonded: + switch s.Status { + case sdk.Bonded: return p.unbondedShareExRate().Mul(s.Amount) // (tokens/shares) * shares - case ShareUnbonding: + case sdk.Unbonding: return p.unbondedShareExRate().Mul(s.Amount) - case ShareUnbonded: + case sdk.Unbonded: return p.unbondedShareExRate().Mul(s.Amount) default: panic("unknown share kind") diff --git a/x/stake/tick_test.go b/x/stake/tick_test.go index 8631ec039d..ee73f7b325 100644 --- a/x/stake/tick_test.go +++ b/x/stake/tick_test.go @@ -75,10 +75,10 @@ func TestProcessProvisions(t *testing.T) { var validators [5]Validator validators[0] = NewValidator(addrs[0], pks[0], Description{}) validators[0], pool, _ = validators[0].addTokensFromDel(pool, 150000000) - keeper.setPool(ctx, pool) - assert.Equal(t, bondedShares, pool.BondedTokens) - fmt.Printf("debug pool: %v\n", pool) validators[0] = keeper.setValidator(ctx, validators[0]) + keeper.setPool(ctx, pool) + assert.Equal(t, bondedShares, pool.BondedTokens, "%v", pool) + fmt.Printf("debug pool: %v validator: %v\n", pool, validators[0]) validators[1] = NewValidator(addrs[1], pks[1], Description{}) validators[1], pool, _ = validators[1].addTokensFromDel(pool, 100000000) keeper.setPool(ctx, pool) diff --git a/x/stake/validator.go b/x/stake/validator.go index 1b4220cb57..9aa8d3768b 100644 --- a/x/stake/validator.go +++ b/x/stake/validator.go @@ -17,9 +17,9 @@ import ( // exchange rate. Voting power can be calculated as total bonds multiplied by // exchange rate. type Validator struct { - Status sdk.BondStatus `json:"status"` // bonded status - Owner sdk.Address `json:"owner"` // sender of BondTx - UnbondTx returns here - PubKey crypto.PubKey `json:"pub_key"` // pubkey of validator + Owner sdk.Address `json:"owner"` // sender of BondTx - UnbondTx returns here + PubKey crypto.PubKey `json:"pub_key"` // pubkey of validator + Revoked bool `json:"pub_key"` // has the validator been revoked from bonded status? PoolShares PoolShares `json:"pool_shares"` // total shares for tokens held in the pool DelegatorShares sdk.Rat `json:"delegator_shares"` // total shares issued to a validator's delegators @@ -44,7 +44,6 @@ type Validators []Validator // NewValidator - initialize a new validator func NewValidator(owner sdk.Address, pubKey crypto.PubKey, description Description) Validator { return Validator{ - Status: sdk.Unbonded, Owner: owner, PubKey: pubKey, PoolShares: NewUnbondedShares(sdk.ZeroRat()), @@ -63,8 +62,7 @@ func NewValidator(owner sdk.Address, pubKey crypto.PubKey, description Descripti // only the vitals - does not check bond height of IntraTxCounter func (v Validator) equal(c2 Validator) bool { - return v.Status == c2.Status && - v.PubKey.Equals(c2.PubKey) && + return v.PubKey.Equals(c2.PubKey) && bytes.Equal(v.Owner, c2.Owner) && v.PoolShares.Equal(c2.PoolShares) && v.DelegatorShares.Equal(c2.DelegatorShares) && @@ -115,39 +113,44 @@ func (v Validator) abciValidatorZero(cdc *wire.Codec) abci.Validator { } } +// abci validator from stake validator type +func (v Validator) Status() sdk.BondStatus { + return v.PoolShares.Status +} + // update the location of the shares within a validator if its bond status has changed -func (v Validator) UpdateSharesLocation(p Pool) (Validator, Pool) { +func (v Validator) UpdateStatus(pool Pool, NewStatus sdk.BondStatus) (Validator, Pool) { var tokens int64 - switch v.PoolShares.Kind { - case ShareUnbonded: - if v.Status == sdk.Unbonded { - return v, p + switch v.Status() { + case sdk.Unbonded: + if NewStatus == sdk.Unbonded { + return v, pool } - p, tokens = p.removeSharesUnbonded(v.PoolShares.Amount) + pool, tokens = pool.removeSharesUnbonded(v.PoolShares.Amount) - case ShareUnbonding: - if v.Status == sdk.Unbonding { - return v, p - } - p, tokens = p.removeSharesUnbonding(v.PoolShares.Amount) - - case ShareBonded: - if v.Status == sdk.Bonded { // return if nothing needs switching - return v, p - } - p, tokens = p.removeSharesBonded(v.PoolShares.Amount) - } - - switch v.Status { - case sdk.Unbonded, sdk.Revoked: - p, v.PoolShares = p.addTokensUnbonded(tokens) case sdk.Unbonding: - p, v.PoolShares = p.addTokensUnbonding(tokens) + if NewStatus == sdk.Unbonding { + return v, pool + } + pool, tokens = pool.removeSharesUnbonding(v.PoolShares.Amount) + case sdk.Bonded: - p, v.PoolShares = p.addTokensBonded(tokens) + if NewStatus == sdk.Bonded { // return if nothing needs switching + return v, pool + } + pool, tokens = pool.removeSharesBonded(v.PoolShares.Amount) } - return v, p + + switch NewStatus { + case sdk.Unbonded: + pool, v.PoolShares = pool.addTokensUnbonded(tokens) + case sdk.Unbonding: + pool, v.PoolShares = pool.addTokensUnbonding(tokens) + case sdk.Bonded: + pool, v.PoolShares = pool.addTokensBonded(tokens) + } + return v, pool } // XXX TEST @@ -155,70 +158,70 @@ func (v Validator) UpdateSharesLocation(p Pool) (Validator, Pool) { // if bonded, the power is the BondedShares // if not bonded, the power is the amount of bonded shares which the // the validator would have it was bonded -func (v Validator) EquivalentBondedShares(p Pool) (eqBondedShares sdk.Rat) { - return v.PoolShares.ToBonded(p).Amount +func (v Validator) EquivalentBondedShares(pool Pool) (eqBondedShares sdk.Rat) { + return v.PoolShares.ToBonded(pool).Amount } //_________________________________________________________________________________________________________ // XXX Audit this function further to make sure it's correct // add tokens to a validator -func (v Validator) addTokensFromDel(p Pool, +func (v Validator) addTokensFromDel(pool Pool, amount int64) (validator2 Validator, p2 Pool, issuedDelegatorShares sdk.Rat) { - exRate := v.DelegatorShareExRate(p) // bshr/delshr + exRate := v.DelegatorShareExRate(pool) // bshr/delshr var poolShares PoolShares var equivalentBondedShares sdk.Rat - switch v.Status { - case sdk.Unbonded, sdk.Revoked: - p, poolShares = p.addTokensUnbonded(amount) + switch v.Status() { + case sdk.Unbonded: + pool, poolShares = pool.addTokensUnbonded(amount) case sdk.Unbonding: - p, poolShares = p.addTokensUnbonding(amount) + pool, poolShares = pool.addTokensUnbonding(amount) case sdk.Bonded: - p, poolShares = p.addTokensBonded(amount) + pool, poolShares = pool.addTokensBonded(amount) } v.PoolShares.Amount = v.PoolShares.Amount.Add(poolShares.Amount) - equivalentBondedShares = poolShares.ToBonded(p).Amount + equivalentBondedShares = poolShares.ToBonded(pool).Amount issuedDelegatorShares = equivalentBondedShares.Quo(exRate) // bshr/(bshr/delshr) = delshr v.DelegatorShares = v.DelegatorShares.Add(issuedDelegatorShares) - return v, p, issuedDelegatorShares + return v, pool, issuedDelegatorShares } // remove delegator shares from a validator // NOTE this function assumes the shares have already been updated for the validator status -func (v Validator) removeDelShares(p Pool, +func (v Validator) removeDelShares(pool Pool, delShares sdk.Rat) (validator2 Validator, p2 Pool, createdCoins int64) { - amount := v.DelegatorShareExRate(p).Mul(delShares) + amount := v.DelegatorShareExRate(pool).Mul(delShares) eqBondedSharesToRemove := NewBondedShares(amount) v.DelegatorShares = v.DelegatorShares.Sub(delShares) - switch v.Status { - case sdk.Unbonded, sdk.Revoked: - unbondedShares := eqBondedSharesToRemove.ToUnbonded(p).Amount - p, createdCoins = p.removeSharesUnbonded(unbondedShares) + switch v.Status() { + case sdk.Unbonded: + unbondedShares := eqBondedSharesToRemove.ToUnbonded(pool).Amount + pool, createdCoins = pool.removeSharesUnbonded(unbondedShares) v.PoolShares.Amount = v.PoolShares.Amount.Sub(unbondedShares) case sdk.Unbonding: - unbondingShares := eqBondedSharesToRemove.ToUnbonding(p).Amount - p, createdCoins = p.removeSharesUnbonding(unbondingShares) + unbondingShares := eqBondedSharesToRemove.ToUnbonding(pool).Amount + pool, createdCoins = pool.removeSharesUnbonding(unbondingShares) v.PoolShares.Amount = v.PoolShares.Amount.Sub(unbondingShares) case sdk.Bonded: - p, createdCoins = p.removeSharesBonded(eqBondedSharesToRemove.Amount) + pool, createdCoins = pool.removeSharesBonded(eqBondedSharesToRemove.Amount) v.PoolShares.Amount = v.PoolShares.Amount.Sub(eqBondedSharesToRemove.Amount) } - return v, p, createdCoins + return v, pool, createdCoins } // get the exchange rate of tokens over delegator shares // UNITS: eq-val-bonded-shares/delegator-shares -func (v Validator) DelegatorShareExRate(p Pool) sdk.Rat { +func (v Validator) DelegatorShareExRate(pool Pool) sdk.Rat { if v.DelegatorShares.IsZero() { return sdk.OneRat() } - eqBondedShares := v.PoolShares.ToBonded(p).Amount + eqBondedShares := v.PoolShares.ToBonded(pool).Amount return eqBondedShares.Quo(v.DelegatorShares) } @@ -228,7 +231,7 @@ func (v Validator) DelegatorShareExRate(p Pool) sdk.Rat { var _ sdk.Validator = Validator{} // nolint - for sdk.Validator -func (v Validator) GetStatus() sdk.BondStatus { return v.Status } +func (v Validator) GetStatus() sdk.BondStatus { return v.Status() } func (v Validator) GetOwner() sdk.Address { return v.Owner } func (v Validator) GetPubKey() crypto.PubKey { return v.PubKey } func (v Validator) GetPower() sdk.Rat { return v.PoolShares.Bonded() } diff --git a/x/stake/validator_test.go b/x/stake/validator_test.go index 9b86ce6d13..5014141ac7 100644 --- a/x/stake/validator_test.go +++ b/x/stake/validator_test.go @@ -71,9 +71,9 @@ func TestRemoveShares(t *testing.T) { poolA := keeper.GetPool(ctx) valA := Validator{ Status: sdk.Bonded, - Address: addrs[0], + Owner: addrs[0], PubKey: pks[0], - PoolShares: NewBondedShares(sdk.NewRat(9)), + PoolShares: NewBondedShares(sdk.NewRat(9)), DelegatorShares: sdk.NewRat(9), } poolA.BondedTokens = valA.PoolShares.Bonded().Evaluate() @@ -95,9 +95,9 @@ func TestRemoveShares(t *testing.T) { delShares := sdk.NewRat(115) val := Validator{ Status: sdk.Bonded, - Address: addrs[0], + Owner: addrs[0], PubKey: pks[0], - PoolShares: NewBondedShares(poolShares), + PoolShares: NewBondedShares(poolShares), DelegatorShares: delShares, } pool := Pool{ @@ -111,7 +111,7 @@ func TestRemoveShares(t *testing.T) { } shares := sdk.NewRat(29) msg := fmt.Sprintf("validator %s (status: %d, poolShares: %v, delShares: %v, DelegatorShareExRate: %v)", - val.Address, val.Status, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(pool)) + val.Owner, val.Status, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(pool)) msg = fmt.Sprintf("Removed %v shares from %s", shares, msg) _, newPool, tokens := val.removeDelShares(pool, shares) require.Equal(t, @@ -174,9 +174,9 @@ func randomValidator(r *rand.Rand) Validator { } return Validator{ Status: status, - Address: addrs[0], + Owner: addrs[0], PubKey: pks[0], - PoolShares: pShares, + PoolShares: pShares, DelegatorShares: delShares, } } @@ -210,11 +210,11 @@ func OpBondOrUnbond(r *rand.Rand, p Pool, val Validator) (Pool, Validator, int64 var msg string if val.Status == sdk.Bonded { msg = fmt.Sprintf("sdk.Unbonded previously bonded validator %s (poolShares: %v, delShares: %v, DelegatorShareExRate: %v)", - val.Address, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(p)) + val.Owner, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(p)) val.Status = sdk.Unbonded } else if val.Status == sdk.Unbonded { msg = fmt.Sprintf("sdk.Bonded previously unbonded validator %s (poolShares: %v, delShares: %v, DelegatorShareExRate: %v)", - val.Address, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(p)) + val.Owner, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(p)) val.Status = sdk.Bonded } val, p = val.UpdateSharesLocation(p) @@ -225,7 +225,7 @@ func OpBondOrUnbond(r *rand.Rand, p Pool, val Validator) (Pool, Validator, int64 func OpAddTokens(r *rand.Rand, p Pool, val Validator) (Pool, Validator, int64, string) { tokens := int64(r.Int31n(1000)) msg := fmt.Sprintf("validator %s (status: %d, poolShares: %v, delShares: %v, DelegatorShareExRate: %v)", - val.Address, val.Status, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(p)) + val.Owner, val.Status, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(p)) val, p, _ = val.addTokensFromDel(p, tokens) msg = fmt.Sprintf("Added %d tokens to %s", tokens, msg) return p, val, -1 * tokens, msg // tokens are removed so for accounting must be negative @@ -242,7 +242,7 @@ func OpRemoveShares(r *rand.Rand, p Pool, val Validator) (Pool, Validator, int64 } msg := fmt.Sprintf("Removed %v shares from validator %s (status: %d, poolShares: %v, delShares: %v, DelegatorShareExRate: %v)", - shares, val.Address, val.Status, val.PoolShares, val.DelegatorShares, val.DelegatorShareExRate(p)) + shares, val.Owner, val.Status, val.PoolShares, val.DelegatorShares, val.DelegatorShareExRate(p)) val, p, tokens := val.removeDelShares(p, shares) return p, val, tokens, msg @@ -300,30 +300,30 @@ func assertInvariants(t *testing.T, msg string, // nonnegative ex rate require.False(t, vMod.DelegatorShareExRate(pMod).LT(sdk.ZeroRat()), - "Applying operation \"%s\" resulted in negative validator.DelegatorShareExRate(): %v (validator.Address: %s)", + "Applying operation \"%s\" resulted in negative validator.DelegatorShareExRate(): %v (validator.Owner: %s)", msg, vMod.DelegatorShareExRate(pMod), - vMod.Address, + vMod.Owner, ) // nonnegative poolShares require.False(t, vMod.PoolShares.Bonded().LT(sdk.ZeroRat()), - "Applying operation \"%s\" resulted in negative validator.PoolShares.Bonded(): %v (validator.DelegatorShares: %v, validator.DelegatorShareExRate: %v, validator.Address: %s)", + "Applying operation \"%s\" resulted in negative validator.PoolShares.Bonded(): %v (validator.DelegatorShares: %v, validator.DelegatorShareExRate: %v, validator.Owner: %s)", msg, vMod.PoolShares.Bonded(), vMod.DelegatorShares, vMod.DelegatorShareExRate(pMod), - vMod.Address, + vMod.Owner, ) // nonnegative delShares require.False(t, vMod.DelegatorShares.LT(sdk.ZeroRat()), - "Applying operation \"%s\" resulted in negative validator.DelegatorShares: %v (validator.PoolShares.Bonded(): %v, validator.DelegatorShareExRate: %v, validator.Address: %s)", + "Applying operation \"%s\" resulted in negative validator.DelegatorShares: %v (validator.PoolShares.Bonded(): %v, validator.DelegatorShareExRate: %v, validator.Owner: %s)", msg, vMod.DelegatorShares, vMod.PoolShares.Bonded(), vMod.DelegatorShareExRate(pMod), - vMod.Address, + vMod.Owner, ) } @@ -335,9 +335,9 @@ func TestPossibleOverflow(t *testing.T) { delShares := sdk.NewRat(391432570689183511).Quo(sdk.NewRat(40113011844664)) val := Validator{ Status: sdk.Bonded, - Address: addrs[0], + Owner: addrs[0], PubKey: pks[0], - PoolShares: NewBondedShares(poolShares), + PoolShares: NewBondedShares(poolShares), DelegatorShares: delShares, } pool := Pool{ @@ -351,7 +351,7 @@ func TestPossibleOverflow(t *testing.T) { } tokens := int64(71) msg := fmt.Sprintf("validator %s (status: %d, poolShares: %v, delShares: %v, DelegatorShareExRate: %v)", - val.Address, val.Status, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(pool)) + val.Owner, val.Status, val.PoolShares.Bonded(), val.DelegatorShares, val.DelegatorShareExRate(pool)) newValidator, _, _ := val.addTokensFromDel(pool, tokens) msg = fmt.Sprintf("Added %d tokens to %s", tokens, msg) diff --git a/x/stake/view_slash_keeper_test.go b/x/stake/view_slash_keeper_test.go index 53e83eae07..ee83024bbf 100644 --- a/x/stake/view_slash_keeper_test.go +++ b/x/stake/view_slash_keeper_test.go @@ -18,9 +18,9 @@ func TestViewSlashBond(t *testing.T) { var validators [3]Validator for i, amt := range amts { validators[i] = Validator{ - Address: addrVals[i], + Owner: addrVals[i], PubKey: pks[i], - PoolShares: NewBondedShares(sdk.NewRat(amt)), + PoolShares: NewBondedShares(sdk.NewRat(amt)), DelegatorShares: sdk.NewRat(amt), } }