From 457b67f8295e48cb2496260b80e6ba5680681ff2 Mon Sep 17 00:00:00 2001 From: javiersuweijie Date: Wed, 20 Mar 2024 14:23:55 +0800 Subject: [PATCH 1/2] fix: increase rounding sensitivity --- x/alliance/keeper/delegation.go | 2 +- x/alliance/keeper/tests/delegation_test.go | 91 ++++++++++++++++++++++ x/alliance/types/asset.go | 4 +- x/alliance/types/params.go | 4 + 4 files changed, 98 insertions(+), 3 deletions(-) diff --git a/x/alliance/keeper/delegation.go b/x/alliance/keeper/delegation.go index a563ace7..d9d937f6 100644 --- a/x/alliance/keeper/delegation.go +++ b/x/alliance/keeper/delegation.go @@ -383,7 +383,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(sdk.NewDecWithPrec(1, 6)) { + if delegation.Shares.Sub(delegationSharesToUpdate).Abs().LT(types.Rounder) { return delegation.Shares, nil } diff --git a/x/alliance/keeper/tests/delegation_test.go b/x/alliance/keeper/tests/delegation_test.go index 472f7f01..26dd7f28 100644 --- a/x/alliance/keeper/tests/delegation_test.go +++ b/x/alliance/keeper/tests/delegation_test.go @@ -4,6 +4,8 @@ import ( "testing" "time" + "cosmossdk.io/math" + test_helpers "github.com/terra-money/alliance/app" "github.com/terra-money/alliance/x/alliance" "github.com/terra-money/alliance/x/alliance/keeper" @@ -949,3 +951,92 @@ func TestDelegationWithNativeStakingChanges(t *testing.T) { // Check total bonded tokens require.Equal(t, sdk.NewInt(26_000_000), app.StakingKeeper.TotalBondedTokens(ctx)) } + +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, + } + app.AllianceKeeper.SetAsset(ctx, allianceAsset) + + // Query staking module unbonding time to assert later on + unbondingTime := app.StakingKeeper.UnbondingTime(ctx) + + // Get the native delegations to have a validator address where to delegate + delegations := app.StakingKeeper.GetAllDelegations(ctx) + 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), + }}, + 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), + }}, + 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), + }}, + unbondings, + ) +} diff --git a/x/alliance/types/asset.go b/x/alliance/types/asset.go index a8fe8f3b..c2f405cc 100644 --- a/x/alliance/types/asset.go +++ b/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(sdk.NewDecWithPrec(1, 6)) + delTokens = delTokens.Add(Rounder) return sdk.NewCoin(asset.Denom, delTokens.TruncateInt()) } @@ -58,7 +58,7 @@ func GetDelegationTokensWithShares(delegatorShares sdk.Dec, val AllianceValidato // We add a small epsilon before rounding down to make sure cases like // 9.999999 get round to 10 - delTokens = delTokens.Add(sdk.NewDecWithPrec(1, 6)) + delTokens = delTokens.Add(Rounder) return sdk.NewCoin(asset.Denom, delTokens.TruncateInt()) } diff --git a/x/alliance/types/params.go b/x/alliance/types/params.go index 34faa563..843b69e3 100644 --- a/x/alliance/types/params.go +++ b/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") From 0efcbc1d8983af551b98874851c475c0950a6bac Mon Sep 17 00:00:00 2001 From: javiersuweijie Date: Wed, 20 Mar 2024 23:27:26 +0800 Subject: [PATCH 2/2] fix: lint --- x/alliance/keeper/tests/delegation_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/alliance/keeper/tests/delegation_test.go b/x/alliance/keeper/tests/delegation_test.go index f676c37e..0ac15b9f 100644 --- a/x/alliance/keeper/tests/delegation_test.go +++ b/x/alliance/keeper/tests/delegation_test.go @@ -1065,7 +1065,8 @@ func TestUndelegatingLargeNumbers(t *testing.T) { RewardWeightRange: types.RewardWeightRange{Min: math.LegacyNewDec(0), Max: math.LegacyNewDec(1)}, IsInitialized: true, } - app.AllianceKeeper.SetAsset(ctx, allianceAsset) + 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)