From 06b9c0db9029dbb85a2cd078e2900fb35d23cf7e 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/unbonding_test.go | 92 +++++++++++++++++++++++ x/alliance/types/asset.go | 4 +- x/alliance/types/params.go | 3 + 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/unbonding_test.go b/x/alliance/keeper/tests/unbonding_test.go index 32904c41..e848289c 100644 --- a/x/alliance/keeper/tests/unbonding_test.go +++ b/x/alliance/keeper/tests/unbonding_test.go @@ -103,3 +103,95 @@ func TestUnbondingMethods(t *testing.T) { unbondings, ) } + +func TestUnbondingMethodsLargeNumbers(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.GetUnbondingsByDelegator(ctx, 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, + ) +} 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 c9f74de6..44d90d8f 100644 --- a/x/alliance/types/params.go +++ b/x/alliance/types/params.go @@ -1,6 +1,7 @@ package types import ( + "cosmossdk.io/math" "fmt" "time" @@ -10,6 +11,8 @@ import ( ) 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 cbff7be0ad3792747bc4302d77582a2014c57d36 Mon Sep 17 00:00:00 2001 From: javiersuweijie Date: Wed, 20 Mar 2024 22:47:23 +0800 Subject: [PATCH 2/2] fix: lint --- x/alliance/keeper/tests/delegation_test.go | 1 + x/alliance/keeper/tests/unbonding_test.go | 1 + x/alliance/tests/e2e/delegate_undelegate_test.go | 1 + x/alliance/types/params.go | 3 ++- 4 files changed, 5 insertions(+), 1 deletion(-) diff --git a/x/alliance/keeper/tests/delegation_test.go b/x/alliance/keeper/tests/delegation_test.go index 6bfa38e6..1117bbb8 100644 --- a/x/alliance/keeper/tests/delegation_test.go +++ b/x/alliance/keeper/tests/delegation_test.go @@ -5,6 +5,7 @@ import ( "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" diff --git a/x/alliance/keeper/tests/unbonding_test.go b/x/alliance/keeper/tests/unbonding_test.go index e848289c..e5585fda 100644 --- a/x/alliance/keeper/tests/unbonding_test.go +++ b/x/alliance/keeper/tests/unbonding_test.go @@ -8,6 +8,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + test_helpers "github.com/terra-money/alliance/app" "github.com/terra-money/alliance/x/alliance/types" ) diff --git a/x/alliance/tests/e2e/delegate_undelegate_test.go b/x/alliance/tests/e2e/delegate_undelegate_test.go index 6eca657a..e6a1fae6 100644 --- a/x/alliance/tests/e2e/delegate_undelegate_test.go +++ b/x/alliance/tests/e2e/delegate_undelegate_test.go @@ -5,6 +5,7 @@ import ( "time" "cosmossdk.io/math" + "github.com/terra-money/alliance/x/alliance" "github.com/terra-money/alliance/x/alliance/keeper" diff --git a/x/alliance/types/params.go b/x/alliance/types/params.go index 44d90d8f..51c77588 100644 --- a/x/alliance/types/params.go +++ b/x/alliance/types/params.go @@ -1,10 +1,11 @@ package types import ( - "cosmossdk.io/math" "fmt" "time" + "cosmossdk.io/math" + "golang.org/x/exp/slices" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"