Skip to content

Commit

Permalink
fix(x/staking): dust share (#18841)
Browse files Browse the repository at this point in the history
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Co-authored-by: atheeshp <[email protected]>
Co-authored-by: Facundo Medica <[email protected]>
  • Loading branch information
4 people authored Feb 20, 2024
1 parent 52106a6 commit adc0f75
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 6 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
* (crypto) [#19371](https://github.com/cosmos/cosmos-sdk/pull/19371) Avoid cli redundant log in stdout, log to stderr instead.
* (client) [#19393](https://github.com/cosmos/cosmos-sdk/pull/19393/) Add `ReadDefaultValuesFromDefaultClientConfig` to populate the default values from the default client config in client.Context without creating a app folder.

### State Machine Breaking

* (x/staking) [#18841](https://github.com/cosmos/cosmos-sdk/pull/18841) In a undelegation or redelegation if the shares being left delegated correspond to less than 1 token (in base denom) the entire delegation gets removed.

### API Breaking Changes

* (x/consensus) [#19488](https://github.com/cosmos/cosmos-sdk/pull/19488) Consensus module creation takes `appmodule.Environment` instead of individual services.
Expand Down Expand Up @@ -1234,7 +1238,7 @@ replace github.com/confio/ics23/go => github.com/cosmos/cosmos-sdk/ics23/go v0.8
* (x/group) [#13214](https://github.com/cosmos/cosmos-sdk/pull/13214) Add `withdraw-proposal` command to group module's CLI transaction commands.
* (x/auth) [#13048](https://github.com/cosmos/cosmos-sdk/pull/13048) Add handling of AccountNumberStoreKeyPrefix to the simulation decoder.
* (simapp) [#13107](https://github.com/cosmos/cosmos-sdk/pull/13107) Call `SetIAVLCacheSize` with the configured value in simapp.
* [#13301](https://github.com/cosmos/cosmos-sdk/pull/13301) Keep the balance query endpoint compatible with legacy blocks
* [#13301](https://github.com/cosmos/cosmos-sdk/pull/13301) Keep the balance query endpoint compatible with legacy blocks
* [#13321](https://github.com/cosmos/cosmos-sdk/pull/13321) Add flag to disable fast node migration and usage.

### Bug Fixes
Expand Down
14 changes: 9 additions & 5 deletions x/staking/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -1195,11 +1195,15 @@ func (k Keeper) ValidateUnbondAmount(
return shares, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "invalid shares amount")
}

// 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.
if shares.GT(delShares) {
// Depending on the share, amount can be smaller than unit amount(1stake).
// If the remain amount after unbonding is smaller than the minimum share,
// it's completely unbonded to avoid leaving dust shares.
tolerance, err := validator.SharesFromTokens(math.OneInt())
if err != nil {
return shares, err
}

if delShares.Sub(shares).LT(tolerance) {
shares = delShares
}

Expand Down
48 changes: 48 additions & 0 deletions x/staking/keeper/delegation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1176,3 +1176,51 @@ func (s *KeeperTestSuite) TestSetUnbondingDelegationEntry() {
// unbondingID comes from a global counter -> gaps in unbondingIDs are OK as long as every unbondingID is unique
require.Equal(uint64(2), resUnbonding.Entries[1].UnbondingId)
}

func (s *KeeperTestSuite) TestUndelegateWithDustShare() {
ctx, keeper := s.ctx, s.stakingKeeper
require := s.Require()

addrDels, valAddrs := createValAddrs(2)

s.accountKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes()

// construct the validators[0] & slash 1stake
amt := math.NewInt(100)
validator := testutil.NewValidator(s.T(), valAddrs[0], PKs[0])
validator, _ = validator.AddTokensFromDel(amt)
validator = validator.RemoveTokens(math.NewInt(1))
validator = stakingkeeper.TestingUpdateValidator(keeper, ctx, validator, true)

// first add a validators[0] to delegate too
bond1to1 := stakingtypes.NewDelegation(addrDels[0].String(), valAddrs[0].String(), math.LegacyNewDec(100))
require.NoError(keeper.SetDelegation(ctx, bond1to1))
resBond, err := keeper.Delegations.Get(ctx, collections.Join(addrDels[0], valAddrs[0]))
require.NoError(err)
require.Equal(bond1to1, resBond)

// second delegators[1] add a validators[0] to delegate
bond2to1 := stakingtypes.NewDelegation(addrDels[1].String(), valAddrs[0].String(), math.LegacyNewDec(1))
validator, delegatorShare := validator.AddTokensFromDel(math.NewInt(1))
bond2to1.Shares = delegatorShare
_ = stakingkeeper.TestingUpdateValidator(keeper, ctx, validator, true)
require.NoError(keeper.SetDelegation(ctx, bond2to1))
resBond, err = keeper.Delegations.Get(ctx, collections.Join(addrDels[1], valAddrs[0]))
require.NoError(err)
require.Equal(bond2to1, resBond)

// check delegation state
delegations, err := keeper.GetValidatorDelegations(ctx, valAddrs[0])
require.NoError(err)
require.Equal(2, len(delegations))

// undelegate all delegator[0]'s delegate
_, err = s.msgServer.Undelegate(ctx, stakingtypes.NewMsgUndelegate(addrDels[0].String(), valAddrs[0].String(), sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(99))))
require.NoError(err)

// remain only delegator[1]'s delegate
delegations, err = keeper.GetValidatorDelegations(ctx, valAddrs[0])
require.NoError(err)
require.Equal(1, len(delegations))
require.Equal(delegations[0].DelegatorAddress, addrDels[1].String())
}

0 comments on commit adc0f75

Please sign in to comment.