Skip to content

Commit

Permalink
Merge pull request #87 from terra-money/feat/handle-dust-delegations
Browse files Browse the repository at this point in the history
feat: delete dust delegations
  • Loading branch information
javiersuweijie authored Feb 2, 2023
2 parents 137d586 + 5a9de21 commit f7b444f
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 7 deletions.
10 changes: 8 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,16 @@ install: go.sum
### Test ###
###############################################################################

test: test-unit
test: test-unit test-e2e

test-unit:
@VERSION=$(VERSION) go test -v -mod=readonly -tags='ledger test_ledger_mock' github.com/terra-money/alliance/x/alliance/keeper/tests github.com/terra-money/alliance/x/alliance/types/tests
@VERSION=$(VERSION) go test -mod=readonly -tags='ledger test_ledger_mock' github.com/terra-money/alliance/x/alliance/keeper/tests github.com/terra-money/alliance/x/alliance/types/tests

test-e2e:
@VERSION=$(VERSION) go test -mod=readonly -tags='ledger test_ledger_mock' github.com/terra-money/alliance/x/alliance/tests/e2e

test-benchmark:
@VERSION=$(VERSION) go test -v -mod=readonly -tags='ledger test_ledger_mock' github.com/terra-money/alliance/x/alliance/tests/benchmark

###############################################################################
### Linting ###
Expand Down
52 changes: 47 additions & 5 deletions x/alliance/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ func (k Keeper) Redelegate(ctx sdk.Context, delAddr sdk.AccAddress, srcVal types
false,
)

k.ClearDustDelegation(ctx, delAddr, srcVal, asset)

// Add tokens and shares to dst validator
_, newDelegationShares := k.upsertDelegationWithNewTokens(ctx, delAddr, dstVal, coin, asset)
k.updateValidatorShares(
Expand Down Expand Up @@ -206,11 +208,7 @@ func (k Keeper) Undelegate(ctx sdk.Context, delAddr sdk.AccAddress, validator ty
false,
)

// When there are no more tokens recorded in the asset, clear all share records that might remain
// from rounding errors to prevent dust amounts from staying in the stores
if asset.TotalTokens.IsZero() {
k.ResetAssetAndValidators(ctx, asset)
}
k.ClearDustDelegation(ctx, delAddr, validator, asset)

// Queue undelegation messages to distribute tokens after undelegation completes in the future
completionTime := k.queueUndelegation(ctx, delAddr, validator.GetOperator(), coin)
Expand Down Expand Up @@ -427,6 +425,14 @@ func (k Keeper) ValidateDelegatedAmount(delegation types.Delegation, coin sdk.Co
if delegation.Shares.LT(delegationSharesToUpdate.TruncateDec()) {
return sdk.Dec{}, stakingtypes.ErrInsufficientShares
}
// Cap the shares at the delegation's shares. Shares being greater could occur
// due to rounding, however we don't want to truncate the shares or take the
// minimum because we want to allow for the full withdraw of shares from a
// delegation.
// This logic is similar to that found in x/staking
if delegationSharesToUpdate.GT(delegation.Shares) {
delegationSharesToUpdate = delegation.Shares
}

return delegationSharesToUpdate, nil
}
Expand Down Expand Up @@ -605,3 +611,39 @@ func (k Keeper) ResetAssetAndValidators(ctx sdk.Context, asset types.AllianceAss
asset.TotalValidatorShares = sdk.ZeroDec()
k.SetAsset(ctx, asset)
}

func (k Keeper) ClearDustDelegation(ctx sdk.Context, delAddr sdk.AccAddress, validator types.AllianceValidator, asset types.AllianceAsset) {
delegation, found := k.GetDelegation(ctx, delAddr, validator, asset.Denom)
// If not found then the delegation has already been deleted, do nothing else
if !found {
return
}
delegatorSharesToRemove := sdk.NewDecCoinFromDec(asset.Denom, sdk.ZeroDec())
validatorSharesToRemove := sdk.NewDecCoinFromDec(asset.Denom, sdk.ZeroDec())

tokensLeft := types.GetDelegationTokensWithShares(delegation.Shares, validator, asset)
// If there are no tokens that can be claimed by the delegation, delete the delegation
if tokensLeft.IsZero() {
store := ctx.KVStore(k.storeKey)
delAddr := sdk.MustAccAddressFromBech32(delegation.DelegatorAddress) // acc address should always be valid here
key := types.GetDelegationKey(delAddr, validator.GetOperator(), asset.Denom)
store.Delete(key)

delegatorSharesToRemove = sdk.NewDecCoinFromDec(asset.Denom, delegation.Shares)
}

validatorTokensLeft := validator.TotalTokensWithAsset(asset)
if validatorTokensLeft.IsZero() {
validatorSharesToRemove = sdk.NewDecCoinFromDec(asset.Denom, validator.ValidatorSharesWithDenom(asset.Denom))
}

// Reduce the dust shares from validator to make sure everything adds up
validator.ReduceShares(sdk.NewDecCoins(delegatorSharesToRemove), sdk.NewDecCoins(validatorSharesToRemove))
k.SetValidator(ctx, validator)

// When there are no more tokens recorded in the asset, clear all share records that might remain
// from rounding errors to prevent dust amounts from staying in the stores
if asset.TotalTokens.IsZero() {
k.ResetAssetAndValidators(ctx, asset)
}
}
62 changes: 62 additions & 0 deletions x/alliance/tests/e2e/delegate_undelegate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"testing"
"time"

"github.com/terra-money/alliance/x/alliance"

"github.com/terra-money/alliance/x/alliance/keeper"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -67,6 +69,66 @@ func TestDelegateThenTakeRateThenUndelegate(t *testing.T) {

_, err = app.AllianceKeeper.Delegate(ctx, dels[0], val0, sdk.NewCoin("test", sdk.NewInt(33333)))
require.NoError(t, err)

_, stop := alliance.RunAllInvariants(ctx, app.AllianceKeeper)
require.False(t, stop)
}

// TestDelegateThenTakeRateThenRedelegate
// This test makes sure that full redelegation after some take rate has been
// applied will not cause a division by zero error. Also ensure that dust delegations are not kept around
func TestDelegateThenTakeRateThenRedelegate(t *testing.T) {
app, ctx, vals, dels := setupApp(t, 5, 2, sdk.NewCoins(sdk.NewCoin("test", sdk.NewInt(1000000000000000000))))
err := app.AllianceKeeper.CreateAlliance(ctx, &types.MsgCreateAllianceProposal{
Title: "",
Description: "",
Denom: "test",
RewardWeight: sdk.MustNewDecFromStr("0.03"),
TakeRate: sdk.MustNewDecFromStr("0.02"),
RewardChangeRate: sdk.MustNewDecFromStr("0.01"),
RewardChangeInterval: time.Second * 60,
})
require.NoError(t, err)

val0, err := app.AllianceKeeper.GetAllianceValidator(ctx, vals[0])
require.NoError(t, err)
val1, err := app.AllianceKeeper.GetAllianceValidator(ctx, vals[1])
require.NoError(t, err)

_, err = app.AllianceKeeper.Delegate(ctx, dels[0], val0, sdk.NewCoin("test", sdk.NewInt(100033333333333333)))
require.NoError(t, err)

val0, err = app.AllianceKeeper.GetAllianceValidator(ctx, vals[0])
require.NoError(t, err)
require.Equal(t, sdk.NewDec(100033333333333333), sdk.DecCoins(val0.TotalDelegatorShares).AmountOf("test"))

lastClaim := ctx.BlockTime()
ctx = ctx.WithBlockTime(ctx.BlockTime().Add(time.Hour))
assets := app.AllianceKeeper.GetAllAssets(ctx)
_, err = app.AllianceKeeper.DeductAssetsWithTakeRate(ctx, lastClaim, assets)
require.NoError(t, err)

asset, _ := app.AllianceKeeper.GetAssetByDenom(ctx, "test")

del0, found := app.AllianceKeeper.GetDelegation(ctx, dels[0], val0, "test")
require.True(t, found)
tokens := types.GetDelegationTokens(del0, val0, asset)
_, err = app.AllianceKeeper.Redelegate(ctx, dels[0], val0, val1, tokens)
require.NoError(t, err)

_, found = app.AllianceKeeper.GetDelegation(ctx, dels[0], val0, "test")
require.False(t, found)

val0, err = app.AllianceKeeper.GetAllianceValidator(ctx, vals[0])
require.NoError(t, err)

require.Equal(t, sdk.ZeroDec(), sdk.DecCoins(val0.ValidatorShares).AmountOf("test"))

_, err = app.AllianceKeeper.Delegate(ctx, dels[0], val0, sdk.NewCoin("test", sdk.NewInt(33333)))
require.NoError(t, err)

_, stop := alliance.RunAllInvariants(ctx, app.AllianceKeeper)
require.False(t, stop)
}

// Tests delegating a small amount that triggers a re-balancing event that adds < 1 utoken to a validator.
Expand Down

0 comments on commit f7b444f

Please sign in to comment.