Skip to content

Commit

Permalink
Merge pull request #328 from terra-money/fix/rounding-issue
Browse files Browse the repository at this point in the history
fix: increase rounding sensitivity
  • Loading branch information
javiersuweijie authored Mar 21, 2024
2 parents c0e35d6 + cbff7be commit 6a837a8
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 3 deletions.
2 changes: 1 addition & 1 deletion x/alliance/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions x/alliance/keeper/tests/delegation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
93 changes: 93 additions & 0 deletions x/alliance/keeper/tests/unbonding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -103,3 +104,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,
)
}
1 change: 1 addition & 0 deletions x/alliance/tests/e2e/delegate_undelegate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"time"

"cosmossdk.io/math"

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

"github.com/terra-money/alliance/x/alliance/keeper"
Expand Down
4 changes: 2 additions & 2 deletions x/alliance/types/asset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand All @@ -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())
}

Expand Down
4 changes: 4 additions & 0 deletions x/alliance/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 6a837a8

Please sign in to comment.