Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor gamm hooks to handle error instead of panics #2561

Closed
wants to merge 3 commits into from
Closed

Refactor gamm hooks to handle error instead of panics #2561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a2218e
Refactor gamm hooks to handle error instead of panics
Anmol1696 Sep 1, 2022
210f7d0
Add entry to changelog
Anmol1696 Sep 1, 2022
b7ffa41
Add empty error handler calling hooks in the gamm keeper
Anmol1696 Sep 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#2214](https://github.com/osmosis-labs/osmosis/pull/2214) Speedup epoch distribution, superfluid component
* [#2515](https://github.com/osmosis-labs/osmosis/pull/2515) Emit events from functions implementing epoch hooks' `panicCatchingEpochHook` cacheCtx
* [#2526](https://github.com/osmosis-labs/osmosis/pull/2526) EpochHooks interface methods (and hence modules implementing the hooks) return error instead of panic
* [#2561](https://github.com/osmosis-labs/osmosis/pull/2561) GammHooks interface methods (and hence modules implementing the hooks) return error instead of panic


### SDK Upgrades
* [#2245](https://github.com/osmosis-labs/osmosis/pull/2244) Upgrade SDK with the following major changes:
Expand Down
3 changes: 2 additions & 1 deletion x/gamm/keeper/pool_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ func (k Keeper) CreatePool(ctx sdk.Context, msg types.CreatePoolMsg) (uint64, er
return 0, err
}

k.hooks.AfterPoolCreated(ctx, sender, pool.GetId())
// Error is not handled as Hooks use osmoutils.ApplyFuncIfNoError()
_ = k.hooks.AfterPoolCreated(ctx, sender, pool.GetId())
k.RecordTotalLiquidityIncrease(ctx, initialPoolLiquidity)

return pool.GetId(), nil
Expand Down
8 changes: 6 additions & 2 deletions x/gamm/keeper/share.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ func (k Keeper) applyJoinPoolStateChange(ctx sdk.Context, pool types.PoolI, join
}

events.EmitAddLiquidityEvent(ctx, joiner, pool.GetId(), joinCoins)
k.hooks.AfterJoinPool(ctx, joiner, pool.GetId(), joinCoins, numShares)

// Error is not handled as Hooks use osmoutils.ApplyFuncIfNoError()
_ = k.hooks.AfterJoinPool(ctx, joiner, pool.GetId(), joinCoins, numShares)
k.RecordTotalLiquidityIncrease(ctx, joinCoins)
return nil
}
Expand All @@ -46,7 +48,9 @@ func (k Keeper) applyExitPoolStateChange(ctx sdk.Context, pool types.PoolI, exit
}

events.EmitRemoveLiquidityEvent(ctx, exiter, pool.GetId(), exitCoins)
k.hooks.AfterExitPool(ctx, exiter, pool.GetId(), numShares, exitCoins)

// Error is not handled as Hooks use osmoutils.ApplyFuncIfNoError()
_ = k.hooks.AfterExitPool(ctx, exiter, pool.GetId(), numShares, exitCoins)
k.RecordTotalLiquidityDecrease(ctx, exitCoins)
return nil
}
Expand Down
4 changes: 3 additions & 1 deletion x/gamm/keeper/swap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,9 @@ func (k Keeper) updatePoolForSwap(
}

events.EmitSwapEvent(ctx, sender, pool.GetId(), tokensIn, tokensOut)
k.hooks.AfterSwap(ctx, sender, pool.GetId(), tokensIn, tokensOut)

// Error is not handled as Hooks use osmoutils.ApplyFuncIfNoError()
_ = k.hooks.AfterSwap(ctx, sender, pool.GetId(), tokensIn, tokensOut)
k.RecordTotalLiquidityIncrease(ctx, tokensIn)
k.RecordTotalLiquidityDecrease(ctx, tokensOut)

Expand Down
60 changes: 47 additions & 13 deletions x/gamm/types/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
package types

import sdk "github.com/cosmos/cosmos-sdk/types"
import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/osmosis-labs/osmosis/v11/osmoutils"
)

type GammHooks interface {
// AfterPoolCreated is called after CreatePool
AfterPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64)
AfterPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) error

// AfterJoinPool is called after JoinPool, JoinSwapExternAmountIn, and JoinSwapShareAmountOut
AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, enterCoins sdk.Coins, shareOutAmount sdk.Int)
AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, enterCoins sdk.Coins, shareOutAmount sdk.Int) error

// AfterExitPool is called after ExitPool, ExitSwapShareAmountIn, and ExitSwapExternAmountOut
AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, shareInAmount sdk.Int, exitCoins sdk.Coins)
AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, shareInAmount sdk.Int, exitCoins sdk.Coins) error

// AfterSwap is called after SwapExactAmountIn and SwapExactAmountOut
AfterSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins)
AfterSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) error
}

var _ GammHooks = MultiGammHooks{}
Expand All @@ -26,26 +32,54 @@ func NewMultiGammHooks(hooks ...GammHooks) MultiGammHooks {
return hooks
}

func (h MultiGammHooks) AfterPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) {
func (h MultiGammHooks) AfterPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) error {
for i := range h {
h[i].AfterPoolCreated(ctx, sender, poolId)
wrappedHookFn := func(ctx sdk.Context) error {
return h[i].AfterPoolCreated(ctx, sender, poolId)
}
err := osmoutils.ApplyFuncIfNoError(ctx, wrappedHookFn)
if err != nil {
ctx.Logger().Error(fmt.Sprintf("error in gamm hook %v", err))
}
}
return nil
}

func (h MultiGammHooks) AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, enterCoins sdk.Coins, shareOutAmount sdk.Int) {
func (h MultiGammHooks) AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, enterCoins sdk.Coins, shareOutAmount sdk.Int) error {
for i := range h {
h[i].AfterJoinPool(ctx, sender, poolId, enterCoins, shareOutAmount)
wrappedHookFn := func(ctx sdk.Context) error {
return h[i].AfterJoinPool(ctx, sender, poolId, enterCoins, shareOutAmount)
}
err := osmoutils.ApplyFuncIfNoError(ctx, wrappedHookFn)
if err != nil {
ctx.Logger().Error(fmt.Sprintf("error in gamm hook %v", err))
}
}
return nil
}

func (h MultiGammHooks) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, shareInAmount sdk.Int, exitCoins sdk.Coins) {
func (h MultiGammHooks) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, shareInAmount sdk.Int, exitCoins sdk.Coins) error {
for i := range h {
h[i].AfterExitPool(ctx, sender, poolId, shareInAmount, exitCoins)
wrappedHookFn := func(ctx sdk.Context) error {
return h[i].AfterExitPool(ctx, sender, poolId, shareInAmount, exitCoins)
}
err := osmoutils.ApplyFuncIfNoError(ctx, wrappedHookFn)
if err != nil {
ctx.Logger().Error(fmt.Sprintf("error in gamm hook %v", err))
}
}
return nil
}

func (h MultiGammHooks) AfterSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) {
func (h MultiGammHooks) AfterSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) error {
for i := range h {
h[i].AfterSwap(ctx, sender, poolId, input, output)
wrappedHookFn := func(ctx sdk.Context) error {
return h[i].AfterSwap(ctx, sender, poolId, input, output)
}
err := osmoutils.ApplyFuncIfNoError(ctx, wrappedHookFn)
if err != nil {
ctx.Logger().Error(fmt.Sprintf("error in gamm hook %v", err))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we can do this approach if panic(err) here for now

}
}
return nil
}
16 changes: 8 additions & 8 deletions x/pool-incentives/keeper/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,23 @@ var (
func (k Keeper) Hooks() Hooks { return Hooks{k} }

// AfterPoolCreated creates a gauge for each pool’s lockable duration.
func (h Hooks) AfterPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) {
err := h.k.CreatePoolGauges(ctx, poolId)
if err != nil {
panic(err)
}
func (h Hooks) AfterPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) error {
return h.k.CreatePoolGauges(ctx, poolId)
}

// AfterJoinPool hook is a noop.
func (h Hooks) AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, enterCoins sdk.Coins, shareOutAmount sdk.Int) {
func (h Hooks) AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, enterCoins sdk.Coins, shareOutAmount sdk.Int) error {
return nil
}

// AfterExitPool hook is a noop.
func (h Hooks) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, shareInAmount sdk.Int, exitCoins sdk.Coins) {
func (h Hooks) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, shareInAmount sdk.Int, exitCoins sdk.Coins) error {
return nil
}

// AfterSwap hook is a noop.
func (h Hooks) AfterSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) {
func (h Hooks) AfterSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) error {
return nil
}

// Distribute coins after minter module allocate assets to pool-incentives module.
Expand Down
17 changes: 8 additions & 9 deletions x/twap/listeners.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,21 @@ func (k Keeper) GammHooks() types.GammHooks {
}

// AfterPoolCreated is called after CreatePool
func (hook *gammhook) AfterPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) {
err := hook.k.afterCreatePool(ctx, poolId)
// Will halt pool creation
if err != nil {
panic(err)
}
func (hook *gammhook) AfterPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) error {
return hook.k.afterCreatePool(ctx, poolId)
}

func (hook *gammhook) AfterSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) {
func (hook *gammhook) AfterSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) error {
hook.k.trackChangedPool(ctx, poolId)
return nil
}

func (hook *gammhook) AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, enterCoins sdk.Coins, shareOutAmount sdk.Int) {
func (hook *gammhook) AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, enterCoins sdk.Coins, shareOutAmount sdk.Int) error {
hook.k.trackChangedPool(ctx, poolId)
return nil
}

func (hook *gammhook) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, shareInAmount sdk.Int, exitCoins sdk.Coins) {
func (hook *gammhook) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, shareInAmount sdk.Int, exitCoins sdk.Coins) error {
hook.k.trackChangedPool(ctx, poolId)
return nil
}