Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R4R: Don't store denominations in staking #3561

Merged
merged 5 commits into from
Feb 8, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ BREAKING CHANGES
* [\#3490](https://github.com/cosmos/cosmos-sdk/issues/3490) ReadRESTReq() returns bool to avoid callers to write error responses twice.
* [\#3502](https://github.com/cosmos/cosmos-sdk/pull/3502) Fixes issue when comparing genesis states
* [\#3514](https://github.com/cosmos/cosmos-sdk/pull/3514) Various clean ups:
- Replace all GetKeyBase* functions family in favor of NewKeyBaseFromDir and NewKeyBaseFromHomeFlag.
- Replace all GetKeyBase\* functions family in favor of NewKeyBaseFromDir and NewKeyBaseFromHomeFlag.
- Remove Get prefix from all TxBuilder's getters.
* [\#3522](https://github.com/cosmos/cosmos-sdk/pull/3522) Get rid of double negatives: Coins.IsNotNegative() -> Coins.IsAnyNegative().
* \#3561 Don't unnecessarily store denominations in staking

* Tendermint

Expand Down
6 changes: 3 additions & 3 deletions client/lcd/lcd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ func TestBonding(t *testing.T) {

ubd := getUnbondingDelegation(t, port, addr, operAddrs[0])
require.Len(t, ubd.Entries, 1)
require.Equal(t, delTokens.DivRaw(2), ubd.Entries[0].Balance.Amount)
require.Equal(t, delTokens.DivRaw(2), ubd.Entries[0].Balance)

// test redelegation
rdTokens := staking.TokensFromTendermintPower(30)
Expand Down Expand Up @@ -650,7 +650,7 @@ func TestBonding(t *testing.T) {
delegatorUbds := getDelegatorUnbondingDelegations(t, port, addr)
require.Len(t, delegatorUbds, 1)
require.Len(t, delegatorUbds[0].Entries, 1)
require.Equal(t, rdTokens, delegatorUbds[0].Entries[0].Balance.Amount)
require.Equal(t, rdTokens, delegatorUbds[0].Entries[0].Balance)

delegatorReds := getRedelegations(t, port, addr, nil, nil)
require.Len(t, delegatorReds, 1)
Expand All @@ -659,7 +659,7 @@ func TestBonding(t *testing.T) {
validatorUbds := getValidatorUnbondingDelegations(t, port, operAddrs[0])
require.Len(t, validatorUbds, 1)
require.Len(t, validatorUbds[0].Entries, 1)
require.Equal(t, rdTokens, validatorUbds[0].Entries[0].Balance.Amount)
require.Equal(t, rdTokens, validatorUbds[0].Entries[0].Balance)

validatorReds := getRedelegations(t, port, nil, operAddrs[0], nil)
require.Len(t, validatorReds, 1)
Expand Down
2 changes: 1 addition & 1 deletion cmd/gaia/cli_test/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ func TestGaiaCLICreateValidator(t *testing.T) {
validatorUbds := f.QueryStakingUnbondingDelegationsFrom(barVal)
require.Len(t, validatorUbds, 1)
require.Len(t, validatorUbds[0].Entries, 1)
require.Equal(t, remainingTokens.String(), validatorUbds[0].Entries[0].Balance.Amount.String())
require.Equal(t, remainingTokens.String(), validatorUbds[0].Entries[0].Balance.String())

f.Cleanup()
}
Expand Down
4 changes: 2 additions & 2 deletions x/staking/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func handleMsgCreateValidator(ctx sdk.Context, msg types.MsgCreateValidator, k k

// move coins from the msg.Address account to a (self-delegation) delegator account
// the validator account and global shares are updated within here
_, err = k.Delegate(ctx, msg.DelegatorAddr, msg.Value, validator, true)
_, err = k.Delegate(ctx, msg.DelegatorAddr, msg.Value.Amount, validator, true)
if err != nil {
return err.Result()
}
Expand Down Expand Up @@ -201,7 +201,7 @@ func handleMsgDelegate(ctx sdk.Context, msg types.MsgDelegate, k keeper.Keeper)
return ErrBadDenom(k.Codespace()).Result()
}

_, err := k.Delegate(ctx, msg.DelegatorAddr, msg.Value, validator, true)
_, err := k.Delegate(ctx, msg.DelegatorAddr, msg.Value.Amount, validator, true)
if err != nil {
return err.Result()
}
Expand Down
4 changes: 2 additions & 2 deletions x/staking/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,7 @@ func TestBondUnbondRedelegateSlashTwice(t *testing.T) {
ubd, found := keeper.GetUnbondingDelegation(ctx, del, valA)
require.True(t, found)
require.Len(t, ubd.Entries, 1)
require.Equal(t, ubdTokens.DivRaw(2), ubd.Entries[0].Balance.Amount)
require.Equal(t, ubdTokens.DivRaw(2), ubd.Entries[0].Balance)

// redelegation should have been slashed by half
redelegation, found := keeper.GetRedelegation(ctx, del, valA, valB)
Expand All @@ -1200,7 +1200,7 @@ func TestBondUnbondRedelegateSlashTwice(t *testing.T) {
ubd, found = keeper.GetUnbondingDelegation(ctx, del, valA)
require.True(t, found)
require.Len(t, ubd.Entries, 1)
require.Equal(t, ubdTokens.DivRaw(2), ubd.Entries[0].Balance.Amount)
require.Equal(t, ubdTokens.DivRaw(2), ubd.Entries[0].Balance)

// redelegation should be unchanged
redelegation, found = keeper.GetRedelegation(ctx, del, valA, valB)
Expand Down
20 changes: 9 additions & 11 deletions x/staking/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func (k Keeper) RemoveUnbondingDelegation(ctx sdk.Context, ubd types.UnbondingDe
// the given addresses. It creates the unbonding delegation if it does not exist
func (k Keeper) SetUnbondingDelegationEntry(ctx sdk.Context,
delegatorAddr sdk.AccAddress, validatorAddr sdk.ValAddress,
creationHeight int64, minTime time.Time, balance sdk.Coin) types.UnbondingDelegation {
creationHeight int64, minTime time.Time, balance sdk.Int) types.UnbondingDelegation {

ubd, found := k.GetUnbondingDelegation(ctx, delegatorAddr, validatorAddr)
if found {
Expand Down Expand Up @@ -347,7 +347,7 @@ func (k Keeper) SetRedelegation(ctx sdk.Context, red types.Redelegation) {
func (k Keeper) SetRedelegationEntry(ctx sdk.Context,
delegatorAddr sdk.AccAddress, validatorSrcAddr,
validatorDstAddr sdk.ValAddress, creationHeight int64,
minTime time.Time, balance sdk.Coin,
minTime time.Time, balance sdk.Int,
sharesSrc, sharesDst sdk.Dec) types.Redelegation {

red, found := k.GetRedelegation(ctx, delegatorAddr, validatorSrcAddr, validatorDstAddr)
Expand Down Expand Up @@ -443,7 +443,7 @@ func (k Keeper) DequeueAllMatureRedelegationQueue(ctx sdk.Context, currTime time
}

// Perform a delegation, set/update everything necessary within the store.
func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, bondAmt sdk.Coin,
func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, bondAmt sdk.Int,
validator types.Validator, subtractAccount bool) (newShares sdk.Dec, err sdk.Error) {

// In some situations, the exchange rate becomes invalid, e.g. if
Expand Down Expand Up @@ -471,13 +471,13 @@ func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, bondAmt sdk.Co
}

if subtractAccount {
_, err := k.bankKeeper.DelegateCoins(ctx, delegation.DelegatorAddr, sdk.Coins{bondAmt})
_, err := k.bankKeeper.DelegateCoins(ctx, delegation.DelegatorAddr, sdk.Coins{sdk.NewCoin(k.GetParams(ctx).BondDenom, bondAmt)})
if err != nil {
return sdk.Dec{}, err
}
}

validator, newShares = k.AddValidatorTokensAndShares(ctx, validator, bondAmt.Amount)
validator, newShares = k.AddValidatorTokensAndShares(ctx, validator, bondAmt)

// Update delegation
delegation.Shares = delegation.Shares.Add(newShares)
Expand Down Expand Up @@ -599,7 +599,7 @@ func (k Keeper) Undelegate(ctx sdk.Context, delAddr sdk.AccAddress,
}

ubd := k.SetUnbondingDelegationEntry(ctx, delAddr,
valAddr, height, completionTime, balance)
valAddr, height, completionTime, returnAmount)

k.InsertUBDQueue(ctx, ubd, completionTime)
return completionTime, nil
Expand All @@ -626,7 +626,7 @@ func (k Keeper) CompleteUnbonding(ctx sdk.Context, delAddr sdk.AccAddress,

// track undelegation only when remaining or truncated shares are non-zero
if !entry.Balance.IsZero() {
_, err := k.bankKeeper.UndelegateCoins(ctx, ubd.DelegatorAddr, sdk.Coins{entry.Balance})
_, err := k.bankKeeper.UndelegateCoins(ctx, ubd.DelegatorAddr, sdk.Coins{sdk.NewCoin(k.GetParams(ctx).BondDenom, entry.Balance)})
if err != nil {
return err
}
Expand Down Expand Up @@ -670,14 +670,12 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delAddr sdk.AccAddress,
if returnAmount.IsZero() {
return time.Time{}, types.ErrVerySmallRedelegation(k.Codespace())
}
returnCoin := sdk.NewCoin(k.BondDenom(ctx), returnAmount)

dstValidator, found := k.GetValidator(ctx, valDstAddr)
if !found {
return time.Time{}, types.ErrBadRedelegationDst(k.Codespace())
}

sharesCreated, err := k.Delegate(ctx, delAddr, returnCoin, dstValidator, false)
sharesCreated, err := k.Delegate(ctx, delAddr, returnAmount, dstValidator, false)
if err != nil {
return time.Time{}, err
}
Expand All @@ -690,7 +688,7 @@ func (k Keeper) BeginRedelegation(ctx sdk.Context, delAddr sdk.AccAddress,
}

red := k.SetRedelegationEntry(ctx, delAddr, valSrcAddr, valDstAddr,
height, completionTime, returnCoin, sharesAmount, sharesCreated)
height, completionTime, returnAmount, sharesAmount, sharesCreated)
k.InsertRedelegationQueue(ctx, red, completionTime)
return completionTime, nil
}
Expand Down
10 changes: 5 additions & 5 deletions x/staking/keeper/delegation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func TestUnbondingDelegation(t *testing.T) {
ctx, _, keeper := CreateTestInput(t, false, 0)

ubd := types.NewUnbondingDelegation(addrDels[0], addrVals[0], 0,
time.Unix(0, 0), sdk.NewInt64Coin(types.DefaultBondDenom, 5))
time.Unix(0, 0), sdk.NewInt(5))

// set and retrieve a record
keeper.SetUnbondingDelegation(ctx, ubd)
Expand All @@ -142,7 +142,7 @@ func TestUnbondingDelegation(t *testing.T) {
require.True(t, ubd.Equal(resUnbond))

// modify a records, save, and retrieve
ubd.Entries[0].Balance = sdk.NewInt64Coin(types.DefaultBondDenom, 21)
ubd.Entries[0].Balance = sdk.NewInt(21)
keeper.SetUnbondingDelegation(ctx, ubd)

resUnbonds := keeper.GetUnbondingDelegations(ctx, addrDels[0], 5)
Expand Down Expand Up @@ -361,7 +361,7 @@ func TestUndelegateFromUnbondingValidator(t *testing.T) {
ubd, found := keeper.GetUnbondingDelegation(ctx, addrDels[0], addrVals[0])
require.True(t, found)
require.Len(t, ubd.Entries, 1)
require.True(t, ubd.Entries[0].Balance.IsEqual(sdk.NewInt64Coin(params.BondDenom, 6)))
require.True(t, ubd.Entries[0].Balance.Equal(sdk.NewInt(6)))
assert.Equal(t, blockHeight, ubd.Entries[0].CreationHeight)
assert.True(t, blockTime.Add(params.UnbondingTime).Equal(ubd.Entries[0].CompletionTime))
}
Expand Down Expand Up @@ -505,7 +505,7 @@ func TestGetRedelegationsFromValidator(t *testing.T) {
ctx, _, keeper := CreateTestInput(t, false, 0)

rd := types.NewRedelegation(addrDels[0], addrVals[0], addrVals[1], 0,
time.Unix(0, 0), sdk.NewInt64Coin(types.DefaultBondDenom, 5),
time.Unix(0, 0), sdk.NewInt(5),
sdk.NewDec(5))

// set and retrieve a record
Expand All @@ -529,7 +529,7 @@ func TestRedelegation(t *testing.T) {
ctx, _, keeper := CreateTestInput(t, false, 0)

rd := types.NewRedelegation(addrDels[0], addrVals[0], addrVals[1], 0,
time.Unix(0, 0), sdk.NewInt64Coin(types.DefaultBondDenom, 5),
time.Unix(0, 0), sdk.NewInt(5),
sdk.NewDec(5))

// test shouldn't have and redelegations
Expand Down
8 changes: 4 additions & 4 deletions x/staking/keeper/slash.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,21 +178,21 @@ func (k Keeper) slashUnbondingDelegation(ctx sdk.Context, unbondingDelegation ty
}

// Calculate slash amount proportional to stake contributing to infraction
slashAmountDec := slashFactor.MulInt(entry.InitialBalance.Amount)
slashAmountDec := slashFactor.MulInt(entry.InitialBalance)
slashAmount := slashAmountDec.TruncateInt()
totalSlashAmount = totalSlashAmount.Add(slashAmount)

// Don't slash more tokens than held
// Possible since the unbonding delegation may already
// have been slashed, and slash amounts are calculated
// according to stake held at time of infraction
unbondingSlashAmount := sdk.MinInt(slashAmount, entry.Balance.Amount)
unbondingSlashAmount := sdk.MinInt(slashAmount, entry.Balance)

// Update unbonding delegation if necessary
if unbondingSlashAmount.IsZero() {
continue
}
entry.Balance.Amount = entry.Balance.Amount.Sub(unbondingSlashAmount)
entry.Balance = entry.Balance.Sub(unbondingSlashAmount)
unbondingDelegation.Entries[i] = entry
k.SetUnbondingDelegation(ctx, unbondingDelegation)
pool := k.GetPool(ctx)
Expand Down Expand Up @@ -232,7 +232,7 @@ func (k Keeper) slashRedelegation(ctx sdk.Context, validator types.Validator, re
}

// Calculate slash amount proportional to stake contributing to infraction
slashAmountDec := slashFactor.MulInt(entry.InitialBalance.Amount)
slashAmountDec := slashFactor.MulInt(entry.InitialBalance)
slashAmount := slashAmountDec.TruncateInt()
totalSlashAmount = totalSlashAmount.Add(slashAmount)

Expand Down
36 changes: 18 additions & 18 deletions x/staking/keeper/slash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ func TestRevocation(t *testing.T) {

// tests slashUnbondingDelegation
func TestSlashUnbondingDelegation(t *testing.T) {
ctx, keeper, params := setupHelper(t, 10)
ctx, keeper, _ := setupHelper(t, 10)
fraction := sdk.NewDecWithPrec(5, 1)

// set an unbonding delegation with expiration timestamp (beyond which the
// unbonding delegation shouldn't be slashed)
ubd := types.NewUnbondingDelegation(addrDels[0], addrVals[0], 0,
time.Unix(5, 0), sdk.NewInt64Coin(params.BondDenom, 10))
time.Unix(5, 0), sdk.NewInt(10))

keeper.SetUnbondingDelegation(ctx, ubd)

Expand All @@ -99,23 +99,23 @@ func TestSlashUnbondingDelegation(t *testing.T) {
require.Len(t, ubd.Entries, 1)

// initial balance unchanged
require.Equal(t, sdk.NewInt64Coin(params.BondDenom, 10), ubd.Entries[0].InitialBalance)
require.Equal(t, sdk.NewInt(10), ubd.Entries[0].InitialBalance)

// balance decreased
require.Equal(t, sdk.NewInt64Coin(params.BondDenom, 5), ubd.Entries[0].Balance)
require.Equal(t, sdk.NewInt(5), ubd.Entries[0].Balance)
newPool := keeper.GetPool(ctx)
require.Equal(t, int64(5), oldPool.NotBondedTokens.Sub(newPool.NotBondedTokens).Int64())
}

// tests slashRedelegation
func TestSlashRedelegation(t *testing.T) {
ctx, keeper, params := setupHelper(t, 10)
ctx, keeper, _ := setupHelper(t, 10)
fraction := sdk.NewDecWithPrec(5, 1)

// set a redelegation with an expiration timestamp beyond which the
// redelegation shouldn't be slashed
rd := types.NewRedelegation(addrDels[0], addrVals[0], addrVals[1], 0,
time.Unix(5, 0), sdk.NewInt64Coin(params.BondDenom, 10), sdk.NewDec(10))
time.Unix(5, 0), sdk.NewInt(10), sdk.NewDec(10))

keeper.SetRedelegation(ctx, rd)

Expand Down Expand Up @@ -154,7 +154,7 @@ func TestSlashRedelegation(t *testing.T) {
require.Equal(t, 1, len(updates))

// initialbalance unchanged
require.Equal(t, sdk.NewInt64Coin(params.BondDenom, 10), rd.Entries[0].InitialBalance)
require.Equal(t, sdk.NewInt(10), rd.Entries[0].InitialBalance)

// shares decreased
del, found = keeper.GetDelegation(ctx, addrDels[0], addrVals[1])
Expand Down Expand Up @@ -231,15 +231,15 @@ func TestSlashValidatorAtCurrentHeight(t *testing.T) {

// tests Slash at a previous height with an unbonding delegation
func TestSlashWithUnbondingDelegation(t *testing.T) {
ctx, keeper, params := setupHelper(t, 10)
ctx, keeper, _ := setupHelper(t, 10)
consAddr := sdk.ConsAddress(PKs[0].Address())
fraction := sdk.NewDecWithPrec(5, 1)

// set an unbonding delegation with expiration timestamp beyond which the
// unbonding delegation shouldn't be slashed
ubdTokens := types.TokensFromTendermintPower(4)
ubd := types.NewUnbondingDelegation(addrDels[0], addrVals[0], 11,
time.Unix(0, 0), sdk.NewCoin(params.BondDenom, ubdTokens))
time.Unix(0, 0), ubdTokens)
keeper.SetUnbondingDelegation(ctx, ubd)

// slash validator for the first time
Expand All @@ -258,7 +258,7 @@ func TestSlashWithUnbondingDelegation(t *testing.T) {
require.True(t, found)
require.Len(t, ubd.Entries, 1)
// balance decreased
require.Equal(t, types.TokensFromTendermintPower(2), ubd.Entries[0].Balance.Amount)
require.Equal(t, types.TokensFromTendermintPower(2), ubd.Entries[0].Balance)
// read updated pool
newPool := keeper.GetPool(ctx)
// bonded tokens burned
Expand All @@ -279,7 +279,7 @@ func TestSlashWithUnbondingDelegation(t *testing.T) {
require.True(t, found)
require.Len(t, ubd.Entries, 1)
// balance decreased again
require.Equal(t, sdk.NewInt(0), ubd.Entries[0].Balance.Amount)
require.Equal(t, sdk.NewInt(0), ubd.Entries[0].Balance)
// read updated pool
newPool = keeper.GetPool(ctx)
// bonded tokens burned again
Expand All @@ -300,7 +300,7 @@ func TestSlashWithUnbondingDelegation(t *testing.T) {
require.True(t, found)
require.Len(t, ubd.Entries, 1)
// balance unchanged
require.Equal(t, sdk.NewInt(0), ubd.Entries[0].Balance.Amount)
require.Equal(t, sdk.NewInt(0), ubd.Entries[0].Balance)
// read updated pool
newPool = keeper.GetPool(ctx)
// bonded tokens burned again
Expand All @@ -321,7 +321,7 @@ func TestSlashWithUnbondingDelegation(t *testing.T) {
require.True(t, found)
require.Len(t, ubd.Entries, 1)
// balance unchanged
require.Equal(t, sdk.NewInt(0), ubd.Entries[0].Balance.Amount)
require.Equal(t, sdk.NewInt(0), ubd.Entries[0].Balance)
// read updated pool
newPool = keeper.GetPool(ctx)
// just 1 bonded token burned again since that's all the validator now has
Expand All @@ -337,14 +337,14 @@ func TestSlashWithUnbondingDelegation(t *testing.T) {

// tests Slash at a previous height with a redelegation
func TestSlashWithRedelegation(t *testing.T) {
ctx, keeper, params := setupHelper(t, 10)
ctx, keeper, _ := setupHelper(t, 10)
consAddr := sdk.ConsAddress(PKs[0].Address())
fraction := sdk.NewDecWithPrec(5, 1)

// set a redelegation
rdTokens := types.TokensFromTendermintPower(6)
rd := types.NewRedelegation(addrDels[0], addrVals[0], addrVals[1], 11,
time.Unix(0, 0), sdk.NewCoin(params.BondDenom, rdTokens), sdk.NewDecFromInt(rdTokens))
time.Unix(0, 0), rdTokens, sdk.NewDecFromInt(rdTokens))
keeper.SetRedelegation(ctx, rd)

// set the associated delegation
Expand Down Expand Up @@ -445,14 +445,14 @@ func TestSlashWithRedelegation(t *testing.T) {

// tests Slash at a previous height with both an unbonding delegation and a redelegation
func TestSlashBoth(t *testing.T) {
ctx, keeper, params := setupHelper(t, 10)
ctx, keeper, _ := setupHelper(t, 10)
fraction := sdk.NewDecWithPrec(5, 1)

// set a redelegation with expiration timestamp beyond which the
// redelegation shouldn't be slashed
rdATokens := types.TokensFromTendermintPower(6)
rdA := types.NewRedelegation(addrDels[0], addrVals[0], addrVals[1], 11,
time.Unix(0, 0), sdk.NewCoin(params.BondDenom, rdATokens),
time.Unix(0, 0), rdATokens,
sdk.NewDecFromInt(rdATokens))
keeper.SetRedelegation(ctx, rdA)

Expand All @@ -464,7 +464,7 @@ func TestSlashBoth(t *testing.T) {
// unbonding delegation shouldn't be slashed)
ubdATokens := types.TokensFromTendermintPower(4)
ubdA := types.NewUnbondingDelegation(addrDels[0], addrVals[0], 11,
time.Unix(0, 0), sdk.NewCoin(params.BondDenom, ubdATokens))
time.Unix(0, 0), ubdATokens)
keeper.SetUnbondingDelegation(ctx, ubdA)

// slash validator
Expand Down
Loading