Merge pull request #330 from terra-money/fix/v0.4/rounding-issue

fix: increase rounding sensitivity

x/alliance/keeper/delegation.go

@@ -475,7 +475,7 @@ func (k Keeper) ValidateDelegatedAmount(delegation types.Delegation, coin sdk.Co
 	// Account for rounding in which shares for a full withdraw is slightly more or less than the number of shares recorded
 	// Withdraw all in that case
 	// 1e6 of margin should be enough to handle realistic rounding issues caused by using the fix-point math.
-	if delegation.Shares.Sub(delegationSharesToUpdate).Abs().LT(math.LegacyNewDecWithPrec(1, 6)) {
+	if delegation.Shares.Sub(delegationSharesToUpdate).Abs().LT(types.Rounder) {
 		return delegation.Shares, nil
 	}
 

x/alliance/keeper/tests/delegation_test.go

@@ -1046,3 +1046,98 @@ func TestDelegationWithNativeStakingChanges(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, math.NewInt(26_000_000), totalBonded)
 }
+
+func TestUndelegatingLargeNumbers(t *testing.T) {
+	// Setup the context with an alliance asset
+	app, ctx := createTestContext(t)
+	startTime := time.Now()
+	ctx = ctx.WithBlockTime(startTime).WithBlockHeight(1)
+	allianceAsset := types.AllianceAsset{
+		Denom:                AllianceDenom,
+		RewardWeight:         math.LegacyMustNewDecFromStr("0.025"),
+		TakeRate:             math.LegacyNewDec(0),
+		TotalTokens:          math.ZeroInt(),
+		TotalValidatorShares: math.LegacyZeroDec(),
+		RewardStartTime:      startTime,
+		RewardChangeRate:     math.LegacyNewDec(0),
+		RewardChangeInterval: time.Duration(0),
+		LastRewardChangeTime: startTime,
+		RewardWeightRange:    types.RewardWeightRange{Min: math.LegacyNewDec(0), Max: math.LegacyNewDec(1)},
+		IsInitialized:        true,
+	}
+	err := app.AllianceKeeper.SetAsset(ctx, allianceAsset)
+	require.NoError(t, err)
+
+	// Query staking module unbonding time to assert later on
+	unbondingTime, err := app.StakingKeeper.UnbondingTime(ctx)
+	require.NoError(t, err)
+
+	// Get the native delegations to have a validator address where to delegate
+	delegations, err := app.StakingKeeper.GetAllDelegations(ctx)
+	require.NoError(t, err)
+	valAddr, err := sdk.ValAddressFromBech32(delegations[0].ValidatorAddress)
+	require.NoError(t, err)
+
+	// Get a delegator address with funds
+	delAddrs := test_helpers.AddTestAddrsIncremental(app, ctx, 2, sdk.NewCoins(sdk.NewCoin(AllianceDenom, math.NewInt(1000_000_000_000_000))))
+	delAddr := delAddrs[0]
+	delAddr1 := delAddrs[1]
+
+	// Get an alliance validator
+	val, err := app.AllianceKeeper.GetAllianceValidator(ctx, valAddr)
+	require.NoError(t, err)
+
+	// Delegate the alliance asset with both accounts
+	res, err := app.AllianceKeeper.Delegate(ctx, delAddr, val, sdk.NewCoin(AllianceDenom, math.NewInt(830697941465481)))
+	require.Nil(t, err)
+	require.Equal(t, math.LegacyNewDec(830697941465481), *res)
+	res2, err := app.AllianceKeeper.Delegate(ctx, delAddr1, val, sdk.NewCoin(AllianceDenom, math.NewInt(975933204219431)))
+	require.Nil(t, err)
+	require.Equal(t, math.LegacyNewDec(975933204219431), *res2)
+
+	// Undelegate the alliance assets with both accounts
+	undelRes, err := app.AllianceKeeper.Undelegate(ctx, delAddr, val, sdk.NewCoin(AllianceDenom, math.NewInt(564360383558874)))
+	require.Nil(t, err)
+	require.Equal(t, ctx.BlockHeader().Time.Add(unbondingTime), *undelRes)
+	undelRes2, err := app.AllianceKeeper.Undelegate(ctx, delAddr1, val, sdk.NewCoin(AllianceDenom, math.NewInt(384108763572096)))
+	require.Nil(t, err)
+	require.Equal(t, ctx.BlockHeader().Time.Add(unbondingTime), *undelRes2)
+
+	// Validate that both user delegations executed the unbonding process
+	unbondings, err := app.AllianceKeeper.GetUnbondingsByDenomAndDelegator(ctx, AllianceDenom, delAddr)
+	require.NoError(t, err)
+	require.Equal(t,
+		[]types.UnbondingDelegation{{
+			ValidatorAddress: valAddr.String(),
+			Amount:           math.NewInt(564360383558874),
+			CompletionTime:   ctx.BlockHeader().Time.Add(unbondingTime),
+			Denom:            AllianceDenom,
+		}},
+		unbondings,
+	)
+
+	// Validate that both user delegations executed the unbonding process
+	unbondings, err = app.AllianceKeeper.GetUnbondingsByDenomAndDelegator(ctx, AllianceDenom, delAddr1)
+	require.NoError(t, err)
+	require.Equal(t,
+		[]types.UnbondingDelegation{{
+			ValidatorAddress: valAddr.String(),
+			Amount:           math.NewInt(384108763572096),
+			CompletionTime:   ctx.BlockHeader().Time.Add(unbondingTime),
+			Denom:            AllianceDenom,
+		}},
+		unbondings,
+	)
+
+	unbondings, err = app.AllianceKeeper.GetUnbondings(ctx, AllianceDenom, delAddr1, valAddr)
+	require.NoError(t, err)
+	require.Equal(t,
+		[]types.UnbondingDelegation{{
+			ValidatorAddress: valAddr.String(),
+			Amount:           math.NewInt(384108763572096),
+			CompletionTime:   ctx.BlockHeader().Time.Add(unbondingTime),
+			Denom:            AllianceDenom,
+		}},
+		unbondings,
+	)
+}

x/alliance/types/asset.go

@@ -47,7 +47,7 @@ func GetDelegationTokens(del Delegation, val AllianceValidator, asset AllianceAs
 
 	// We add a small epsilon before rounding down to make sure cases like
 	// 9.999999 get round to 10
-	delTokens = delTokens.Add(math.LegacyNewDecWithPrec(1, 6))
+	delTokens = delTokens.Add(Rounder)
 	return sdk.NewCoin(asset.Denom, delTokens.TruncateInt())
 }
 
@@ -58,7 +58,7 @@ func GetDelegationTokensWithShares(delegatorShares math.LegacyDec, val AllianceV
 
 	// We add a small epsilon before rounding down to make sure cases like
 	// 9.999999 get round to 10
-	delTokens = delTokens.Add(math.LegacyNewDecWithPrec(1, 6))
+	delTokens = delTokens.Add(Rounder)
 	return sdk.NewCoin(asset.Denom, delTokens.TruncateInt())
 }
 

x/alliance/types/params.go

@@ -4,12 +4,16 @@ import (
 	"fmt"
 	"time"
 
+	"cosmossdk.io/math"
+
 	"golang.org/x/exp/slices"
 
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 )
 
 var (
+	// Rounder is used to round up small errors due to fix point math
+	Rounder               = math.LegacyNewDecWithPrec(1, 2)
 	RewardDelayTime       = []byte("RewardDelayTime")
 	TakeRateClaimInterval = []byte("TakeRateClaimInterval")
 	LastTakeRateClaimTime = []byte("LastTakeRateClaimTime")