From 0e268a804a7e36ba712d57b94cc2029bab3f812e Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Tue, 15 Feb 2022 13:33:57 -0800 Subject: [PATCH] Add a method to osmoutils for having a function write to state if no err @antstalepresh made a neat method for doing this in a few places in the superfluid epoch logic. This just refactors it to a standalone method in osmoutils. Furthermore, this PR also renames osmotestutils -> osmoutils, which is a change I've wanted to make for a while. --- .../cmd/balances_from_state_export.go | 4 +-- osmoutils/cache_ctx.go | 21 ++++++++++++++ {osmotestutils => osmoutils}/cli_helpers.go | 2 +- x/gamm/client/cli/cli_test.go | 16 +++++------ x/lockup/client/cli/cli_test.go | 6 ++-- x/pool-incentives/client/cli/tx.go | 10 +++---- x/superfluid/keeper/distribution.go | 28 +++++++------------ 7 files changed, 50 insertions(+), 37 deletions(-) create mode 100644 osmoutils/cache_ctx.go rename {osmotestutils => osmoutils}/cli_helpers.go (98%) diff --git a/cmd/osmosisd/cmd/balances_from_state_export.go b/cmd/osmosisd/cmd/balances_from_state_export.go index a6d4f6e8367..b9b4e9c4019 100644 --- a/cmd/osmosisd/cmd/balances_from_state_export.go +++ b/cmd/osmosisd/cmd/balances_from_state_export.go @@ -13,7 +13,7 @@ import ( banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" appparams "github.com/osmosis-labs/osmosis/app/params" - "github.com/osmosis-labs/osmosis/osmotestutils" + "github.com/osmosis-labs/osmosis/osmoutils" claimtypes "github.com/osmosis-labs/osmosis/x/claim/types" gammtypes "github.com/osmosis-labs/osmosis/x/gamm/types" lockuptypes "github.com/osmosis-labs/osmosis/x/lockup/types" @@ -170,7 +170,7 @@ Example: } selectBondedPoolIDs := []uint64{} if selectPoolIdsStr != "" { - selectBondedPoolIDs, err = osmotestutils.ParseUint64SliceFromString(selectPoolIdsStr, ",") + selectBondedPoolIDs, err = osmoutils.ParseUint64SliceFromString(selectPoolIdsStr, ",") if err != nil { return err } diff --git a/osmoutils/cache_ctx.go b/osmoutils/cache_ctx.go new file mode 100644 index 00000000000..a89c8125611 --- /dev/null +++ b/osmoutils/cache_ctx.go @@ -0,0 +1,21 @@ +package osmoutils + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// This function lets you run the function f, but if theres an error +// drop the state machine change and log the error. +// If there is no error, proceeds as normal (but with some slowdown due to SDK store weirdness) +// Try to avoid usage of iterators in f. +func ApplyFuncIfNoError(ctx sdk.Context, f func(ctx sdk.Context) error) { + // makes a new cache context, which all state changes get wrapped inside of. + cacheCtx, write := ctx.CacheContext() + err := f(cacheCtx) + if err != nil { + ctx.Logger().Error(err.Error()) + } else { + // no error, write the output of f + write() + } +} diff --git a/osmotestutils/cli_helpers.go b/osmoutils/cli_helpers.go similarity index 98% rename from osmotestutils/cli_helpers.go rename to osmoutils/cli_helpers.go index 265eea6788d..1c87c2a2ef4 100644 --- a/osmotestutils/cli_helpers.go +++ b/osmoutils/cli_helpers.go @@ -1,4 +1,4 @@ -package osmotestutils +package osmoutils import ( "fmt" diff --git a/x/gamm/client/cli/cli_test.go b/x/gamm/client/cli/cli_test.go index f91668acc9c..5e62a1cf1b0 100644 --- a/x/gamm/client/cli/cli_test.go +++ b/x/gamm/client/cli/cli_test.go @@ -16,7 +16,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" banktestutil "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" "github.com/osmosis-labs/osmosis/app" - "github.com/osmosis-labs/osmosis/osmotestutils" + "github.com/osmosis-labs/osmosis/osmoutils" "github.com/osmosis-labs/osmosis/x/gamm/client/cli" gammtestutil "github.com/osmosis-labs/osmosis/x/gamm/client/testutil" "github.com/osmosis-labs/osmosis/x/gamm/types" @@ -79,7 +79,7 @@ func (s *IntegrationTestSuite) TestNewCreatePoolCmd() { newAddr, sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 200000000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - osmotestutils.DefaultFeeString(s.cfg), + osmoutils.DefaultFeeString(s.cfg), ) s.Require().NoError(err) @@ -325,7 +325,7 @@ func (s *IntegrationTestSuite) TestNewCreatePoolCmd() { // common args fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - osmotestutils.DefaultFeeString(s.cfg), + osmoutils.DefaultFeeString(s.cfg), fmt.Sprintf("--%s=%s", flags.FlagGas, fmt.Sprint(300000)), } @@ -358,7 +358,7 @@ func (s IntegrationTestSuite) TestNewJoinPoolCmd() { newAddr, sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 20000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - osmotestutils.DefaultFeeString(s.cfg), + osmoutils.DefaultFeeString(s.cfg), ) s.Require().NoError(err) @@ -496,7 +496,7 @@ func (s IntegrationTestSuite) TestNewSwapExactAmountOutCmd() { newAddr, sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 20000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - osmotestutils.DefaultFeeString(s.cfg), + osmoutils.DefaultFeeString(s.cfg), ) s.Require().NoError(err) @@ -560,7 +560,7 @@ func (s IntegrationTestSuite) TestNewJoinSwapExternAmountInCmd() { newAddr, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(20000))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - osmotestutils.DefaultFeeString(s.cfg), + osmoutils.DefaultFeeString(s.cfg), ) s.Require().NoError(err) @@ -668,7 +668,7 @@ func (s IntegrationTestSuite) TestNewJoinSwapShareAmountOutCmd() { newAddr, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(20000))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - osmotestutils.DefaultFeeString(s.cfg), + osmoutils.DefaultFeeString(s.cfg), ) s.Require().NoError(err) @@ -1119,7 +1119,7 @@ func (s IntegrationTestSuite) TestNewSwapExactAmountInCmd() { newAddr, sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 20000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - osmotestutils.DefaultFeeString(s.cfg), + osmoutils.DefaultFeeString(s.cfg), ) s.Require().NoError(err) diff --git a/x/lockup/client/cli/cli_test.go b/x/lockup/client/cli/cli_test.go index 409496d6dd3..949f424f30c 100644 --- a/x/lockup/client/cli/cli_test.go +++ b/x/lockup/client/cli/cli_test.go @@ -17,7 +17,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" banktestutil "github.com/cosmos/cosmos-sdk/x/bank/client/testutil" "github.com/osmosis-labs/osmosis/app" - "github.com/osmosis-labs/osmosis/osmotestutils" + "github.com/osmosis-labs/osmosis/osmoutils" "github.com/osmosis-labs/osmosis/x/lockup/client/cli" lockuptestutil "github.com/osmosis-labs/osmosis/x/lockup/client/testutil" "github.com/osmosis-labs/osmosis/x/lockup/types" @@ -153,7 +153,7 @@ func (s *IntegrationTestSuite) TestBeginUnlockingCmd() { newAddr, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(20000))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - osmotestutils.DefaultFeeString(s.cfg), + osmoutils.DefaultFeeString(s.cfg), ) s.Require().NoError(err) @@ -221,7 +221,7 @@ func (s *IntegrationTestSuite) TestNewBeginUnlockPeriodLockCmd() { newAddr, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(20000))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), - osmotestutils.DefaultFeeString(s.cfg), + osmoutils.DefaultFeeString(s.cfg), ) s.Require().NoError(err) diff --git a/x/pool-incentives/client/cli/tx.go b/x/pool-incentives/client/cli/tx.go index 857ec77c720..4631e7b79f3 100644 --- a/x/pool-incentives/client/cli/tx.go +++ b/x/pool-incentives/client/cli/tx.go @@ -12,7 +12,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/gov/client/cli" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" - "github.com/osmosis-labs/osmosis/osmotestutils" + "github.com/osmosis-labs/osmosis/osmoutils" "github.com/osmosis-labs/osmosis/x/pool-incentives/types" ) @@ -45,12 +45,12 @@ func NewCmdSubmitUpdatePoolIncentivesProposal() *cobra.Command { } // TODO: Make a parse uint64 slice function - gaugeIds, err := osmotestutils.ParseUint64SliceFromString(args[0], ",") + gaugeIds, err := osmoutils.ParseUint64SliceFromString(args[0], ",") if err != nil { return err } - weights, err := osmotestutils.ParseSdkIntFromString(args[1], ",") + weights, err := osmoutils.ParseSdkIntFromString(args[1], ",") if err != nil { return err } @@ -127,12 +127,12 @@ func NewCmdSubmitReplacePoolIncentivesProposal() *cobra.Command { return err } - gaugeIds, err := osmotestutils.ParseUint64SliceFromString(args[0], ",") + gaugeIds, err := osmoutils.ParseUint64SliceFromString(args[0], ",") if err != nil { return err } - weights, err := osmotestutils.ParseSdkIntFromString(args[1], ",") + weights, err := osmoutils.ParseSdkIntFromString(args[1], ",") if err != nil { return err } diff --git a/x/superfluid/keeper/distribution.go b/x/superfluid/keeper/distribution.go index 5ff6322db64..af5c5dbbda1 100644 --- a/x/superfluid/keeper/distribution.go +++ b/x/superfluid/keeper/distribution.go @@ -2,6 +2,7 @@ package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/osmosis-labs/osmosis/osmoutils" ) func (k Keeper) MoveSuperfluidDelegationRewardToGauges(ctx sdk.Context) { @@ -15,25 +16,16 @@ func (k Keeper) MoveSuperfluidDelegationRewardToGauges(ctx sdk.Context) { // To avoid unexpected issues on WithdrawDelegationRewards and AddToGaugeRewards // we use cacheCtx and apply the changes later - cacheCtx, write := ctx.CacheContext() - - // Withdraw delegation rewards into intermediary accounts - _, err = k.dk.WithdrawDelegationRewards(cacheCtx, addr, valAddr) - if err != nil { - k.Logger(ctx).Error(err.Error()) - } else { - write() - } + osmoutils.ApplyFuncIfNoError(ctx, func(cacheCtx sdk.Context) error { + _, err := k.dk.WithdrawDelegationRewards(cacheCtx, addr, valAddr) + return err + }) // Send delegation rewards to gauges - cacheCtx, write = ctx.CacheContext() - bondDenom := k.sk.BondDenom(cacheCtx) - balance := k.bk.GetBalance(cacheCtx, addr, bondDenom) - err = k.ik.AddToGaugeRewards(cacheCtx, addr, sdk.Coins{balance}, acc.GaugeId) - if err != nil { - k.Logger(ctx).Error(err.Error()) - } else { - write() - } + osmoutils.ApplyFuncIfNoError(ctx, func(cacheCtx sdk.Context) error { + bondDenom := k.sk.BondDenom(cacheCtx) + balance := k.bk.GetBalance(cacheCtx, addr, bondDenom) + return k.ik.AddToGaugeRewards(cacheCtx, addr, sdk.Coins{balance}, acc.GaugeId) + }) } }