Implement best pool finder (#352)

* implement best pool finder

* resolve unit tests

---------

Co-authored-by: Cosmic Vagabond <[email protected]>

x/accountedpool/types/expected_keepers.go

@@ -17,8 +17,6 @@ type AccountKeeper interface {
 //
 //go:generate mockery --srcpkg . --name AmmKeeper --structname AmmKeeper --filename amm_keeper.go --with-expecter
 type AmmKeeper interface {
-	// Get pool Ids that contains the denom in pool assets
-	GetAllPoolIdsWithDenom(sdk.Context, string) []uint64
 	// GetPool returns a pool from its index
 	GetPool(sdk.Context, uint64) (ammtypes.Pool, bool)
 	// Get all pools

x/amm/keeper/calc_in_route_by_denom.go

@@ -12,34 +12,34 @@ func (k Keeper) CalcInRouteByDenom(ctx sdk.Context, denomIn string, denomOut str
 	var route []*types.SwapAmountInRoute
 
 	// Check for a direct pool between the denoms
-	if poolId, found := k.GetPoolIdWithAllDenoms(ctx, []string{denomIn, denomOut}); found {
+	if pool, found := k.GetBestPoolWithDenoms(ctx, []string{denomIn, denomOut}); found {
 		// If the pool exists, return the route
 		route = append(route, &types.SwapAmountInRoute{
-			PoolId:        poolId,
+			PoolId:        pool.PoolId,
 			TokenOutDenom: denomOut,
 		})
 		return route, nil
 	}
 
 	// Find pool for initial denom to base currency
-	poolId, found := k.GetPoolIdWithAllDenoms(ctx, []string{denomIn, baseCurrency})
+	pool, found := k.GetBestPoolWithDenoms(ctx, []string{denomIn, baseCurrency})
 	if !found {
 		return nil, fmt.Errorf("no available pool for %s to base currency", denomIn)
 	}
 	// If the pool exists, append the route
 	route = append(route, &types.SwapAmountInRoute{
-		PoolId:        poolId,
+		PoolId:        pool.PoolId,
 		TokenOutDenom: baseCurrency,
 	})
 
 	// Find pool for base currency to target denom
-	poolId, found = k.GetPoolIdWithAllDenoms(ctx, []string{baseCurrency, denomOut})
+	pool, found = k.GetBestPoolWithDenoms(ctx, []string{baseCurrency, denomOut})
 	if !found {
 		return nil, fmt.Errorf("no available pool for base currency to %s", denomOut)
 	}
 	// If the pool exists, append the route
 	route = append(route, &types.SwapAmountInRoute{
-		PoolId:        poolId,
+		PoolId:        pool.PoolId,
 		TokenOutDenom: denomOut,
 	})
 

x/amm/keeper/calc_in_route_by_denom_test.go

@@ -3,15 +3,18 @@ package keeper_test
 import (
 	"testing"
 
-	keepertest "github.com/elys-network/elys/testutil/keeper"
+	tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	simapp "github.com/elys-network/elys/app"
 	"github.com/stretchr/testify/require"
 )
 
 func TestCalcInRouteByDenom(t *testing.T) {
-	k, ctx, _, _ := keepertest.AmmKeeper(t)
+	app := simapp.InitElysTestApp(initChain)
+	ctx := app.BaseApp.NewContext(initChain, tmproto.Header{})
+	k := app.AmmKeeper
 
 	// Setup mock pools and assets
-	SetupMockPools(k, ctx)
+	SetupMockPools(&k, ctx)
 
 	// Test direct pool route
 	route, err := k.CalcInRouteByDenom(ctx, "denom1", "denom2", "baseCurrency")

x/amm/keeper/calc_out_route_by_denom.go

@@ -18,34 +18,34 @@ func (k Keeper) CalcOutRouteByDenom(ctx sdk.Context, denomOut string, denomIn st
 	}
 
 	// Check for a direct pool between the denoms
-	if poolId, found := k.GetPoolIdWithAllDenoms(ctx, []string{denomOut, denomIn}); found {
+	if pool, found := k.GetBestPoolWithDenoms(ctx, []string{denomOut, denomIn}); found {
 		// If the pool exists, return the route
 		route = append(route, &types.SwapAmountOutRoute{
-			PoolId:       poolId,
+			PoolId:       pool.PoolId,
 			TokenInDenom: denomIn,
 		})
 		return route, nil
 	}
 
 	// Find pool for initial denom to base currency
-	poolId, found := k.GetPoolIdWithAllDenoms(ctx, []string{denomOut, baseCurrency})
+	pool, found := k.GetBestPoolWithDenoms(ctx, []string{denomOut, baseCurrency})
 	if !found {
 		return nil, fmt.Errorf("no available pool for %s to base currency", denomOut)
 	}
 	// If the pool exists, append the route
 	route = append(route, &types.SwapAmountOutRoute{
-		PoolId:       poolId,
+		PoolId:       pool.PoolId,
 		TokenInDenom: baseCurrency,
 	})
 
 	// Find pool for base currency to target denom
-	poolId, found = k.GetPoolIdWithAllDenoms(ctx, []string{baseCurrency, denomIn})
+	pool, found = k.GetBestPoolWithDenoms(ctx, []string{baseCurrency, denomIn})
 	if !found {
 		return nil, fmt.Errorf("no available pool for base currency to %s", denomIn)
 	}
 	// If the pool exists, append the route
 	route = append(route, &types.SwapAmountOutRoute{
-		PoolId:       poolId,
+		PoolId:       pool.PoolId,
 		TokenInDenom: denomIn,
 	})
 

x/amm/keeper/calc_out_route_by_denom_test.go

@@ -3,15 +3,18 @@ package keeper_test
 import (
 	"testing"
 
-	keepertest "github.com/elys-network/elys/testutil/keeper"
+	tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	simapp "github.com/elys-network/elys/app"
 	"github.com/stretchr/testify/require"
 )
 
 func TestCalcOutRouteByDenom(t *testing.T) {
-	k, ctx, _, _ := keepertest.AmmKeeper(t)
+	app := simapp.InitElysTestApp(initChain)
+	ctx := app.BaseApp.NewContext(initChain, tmproto.Header{})
+	k := app.AmmKeeper
 
 	// Setup mock pools and assets
-	SetupMockPools(k, ctx)
+	SetupMockPools(&k, ctx)
 
 	// Test direct pool route
 	route, err := k.CalcOutRouteByDenom(ctx, "denom2", "denom1", "baseCurrency")

x/amm/keeper/pool.go

@@ -79,27 +79,13 @@ func (k Keeper) PoolExists(ctx sdk.Context, poolId uint64) bool {
 	return b != nil
 }
 
-// Get pool Ids that contains the denom in pool assets
-func (k Keeper) GetAllPoolIdsWithDenom(ctx sdk.Context, denom string) (list []uint64) {
-	pools := k.GetAllPool(ctx)
-
-	for _, p := range pools {
-		for _, asset := range p.PoolAssets {
-			if denom == asset.Token.Denom {
-				list = append(list, p.PoolId)
-				break
-			}
-		}
-	}
-
-	return list
-}
-
-// GetPoolIdWithAllDenoms returns the first pool id that contains all specified denominations
-func (k Keeper) GetPoolIdWithAllDenoms(ctx sdk.Context, denoms []string) (poolId uint64, found bool) {
+// GetBestPoolWithDenoms returns the first pool id that contains all specified denominations
+func (k Keeper) GetBestPoolWithDenoms(ctx sdk.Context, denoms []string) (pool types.Pool, found bool) {
 	// Get all pools
 	pools := k.GetAllPool(ctx)
 
+	maxTvl := sdk.NewDec(-1)
+	bestPool := types.Pool{}
 	for _, p := range pools {
 		// If the number of assets in the pool is less than the number of denoms, skip
 		if len(p.PoolAssets) < len(denoms) {
@@ -125,13 +111,19 @@ func (k Keeper) GetPoolIdWithAllDenoms(ctx sdk.Context, denoms []string) (poolId
 			}
 		}
 
+		poolTvl, err := p.TVL(ctx, k.oracleKeeper)
+		if err != nil {
+			poolTvl = sdk.ZeroDec()
+		}
+
 		// If all denoms are found in this pool, return the pool id
-		if allDenomsFound {
-			return p.PoolId, true
+		if allDenomsFound && maxTvl.LT(poolTvl) {
+			maxTvl = poolTvl
+			bestPool = p
 		}
 	}
 
-	return 0, false
+	return bestPool, !maxTvl.IsNegative()
 }
 
 // IterateLiquidty iterates over all LiquidityPools and performs a

x/amm/keeper/pool_test.go

@@ -4,7 +4,9 @@ import (
 	"strconv"
 	"testing"
 
+	tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	simapp "github.com/elys-network/elys/app"
 	keepertest "github.com/elys-network/elys/testutil/keeper"
 	"github.com/elys-network/elys/testutil/nullify"
 	"github.com/elys-network/elys/x/amm/keeper"
@@ -73,9 +75,11 @@ func TestPoolGetAll(t *testing.T) {
 	)
 }
 
-func TestGetPoolIdWithAllDenoms(t *testing.T) {
-	keeper, ctx, _, _ := keepertest.AmmKeeper(t)
-	items := createNPool(keeper, ctx, 10)
+func TestGetBestPoolWithDenoms(t *testing.T) {
+	app := simapp.InitElysTestApp(initChain)
+	ctx := app.BaseApp.NewContext(initChain, tmproto.Header{})
+	keeper := app.AmmKeeper
+	items := createNPool(&keeper, ctx, 10)
 
 	// Add assets to some pools for testing
 	for i, item := range items {
@@ -93,11 +97,11 @@ func TestGetPoolIdWithAllDenoms(t *testing.T) {
 	}
 
 	// Test case where pool is found
-	poolId, found := keeper.GetPoolIdWithAllDenoms(ctx, []string{"denom2", "usdc"})
+	pool, found := keeper.GetBestPoolWithDenoms(ctx, []string{"denom2", "usdc"})
 	require.True(t, found)
-	require.Equal(t, uint64(2), poolId)
+	require.Equal(t, uint64(2), pool.PoolId)
 
 	// Test case where pool is not found
-	_, found = keeper.GetPoolIdWithAllDenoms(ctx, []string{"nonexistent", "usdc"})
+	_, found = keeper.GetBestPoolWithDenoms(ctx, []string{"nonexistent", "usdc"})
 	require.False(t, found)
 }

x/incentive/keeper/estimate_price.go

@@ -7,7 +7,7 @@ import (
 // Estimate the price : eg, 1 Eden -> x usdc
 func (k Keeper) EstimatePrice(ctx sdk.Context, tokenIn sdk.Coin, baseCurrency string) sdk.Int {
 	// Find a pool that can convert tokenIn to usdc
-	pool, found := k.FindPool(ctx, tokenIn.Denom, baseCurrency)
+	pool, found := k.amm.GetBestPoolWithDenoms(ctx, []string{tokenIn.Denom, baseCurrency})
 	if !found {
 		return sdk.ZeroInt()
 	}

x/incentive/keeper/keeper_fees.go

@@ -7,31 +7,6 @@ import (
 	ptypes "github.com/elys-network/elys/x/parameter/types"
 )
 
-// FindPool function gets a pool that can convert in_denom token to out_denom token
-// TODO:
-// Later on: add a logic to choose best pool
-func (k Keeper) FindPool(ctx sdk.Context, inDenom string, outDenom string) (ammtypes.Pool, bool) {
-	// Get pool ids that can convert tokenIn
-	poolIds := k.amm.GetAllPoolIdsWithDenom(ctx, inDenom)
-
-	for _, pId := range poolIds {
-		// Get a pool with poolId
-		pool, found := k.amm.GetPool(ctx, pId)
-		if !found {
-			continue
-		}
-
-		// Loop pool assets to find out pair
-		for _, asset := range pool.PoolAssets {
-			if asset.Token.Denom == outDenom {
-				return pool, true
-			}
-		}
-	}
-
-	return ammtypes.Pool{}, false
-}
-
 // Move gas fees collected to dex revenue wallet
 // Convert it into USDC
 func (k Keeper) CollectGasFeesToIncentiveModule(ctx sdk.Context, baseCurrency string) sdk.Coins {
@@ -59,7 +34,7 @@ func (k Keeper) CollectGasFeesToIncentiveModule(ctx sdk.Context, baseCurrency st
 		}
 
 		// Find a pool that can convert tokenIn to usdc
-		pool, found := k.FindPool(ctx, tokenIn.Denom, baseCurrency)
+		pool, found := k.amm.GetBestPoolWithDenoms(ctx, []string{tokenIn.Denom, baseCurrency})
 		if !found {
 			continue
 		}

x/incentive/types/expected_keepers.go

@@ -91,8 +91,7 @@ type AmmKeeper interface {
 		swapFeeOut sdk.Dec,
 		weightBalanceBonus sdk.Dec,
 	) (sdk.Int, error)
-	// Get pool Ids that contains the denom in pool assets
-	GetAllPoolIdsWithDenom(sdk.Context, string) []uint64
+	GetBestPoolWithDenoms(ctx sdk.Context, denoms []string) (pool ammtypes.Pool, found bool)
 	// GetPool returns a pool from its index
 	GetPool(sdk.Context, uint64) (ammtypes.Pool, bool)
 	// Get all pools

x/leveragelp/types/expected_keepers.go

@@ -17,8 +17,6 @@ type AccountKeeper interface {
 
 // AmmKeeper defines the expected interface needed to swap tokens
 type AmmKeeper interface {
-	// Get pool Ids that contains the denom in pool assets
-	GetAllPoolIdsWithDenom(sdk.Context, string) []uint64
 	// GetPool returns a pool from its index
 	GetPool(sdk.Context, uint64) (ammtypes.Pool, bool)
 	// Get all pools

x/perpetual/keeper/get_first_valid_pool.go → x/perpetual/keeper/get_best_pool.go

@@ -8,11 +8,11 @@ import (
 	"github.com/elys-network/elys/x/perpetual/types"
 )
 
-func (k Keeper) GetFirstValidPool(ctx sdk.Context, collateralAsset string, tradingAsset string) (uint64, error) {
+func (k Keeper) GetBestPool(ctx sdk.Context, collateralAsset string, tradingAsset string) (uint64, error) {
 	denoms := []string{collateralAsset, tradingAsset}
-	poolId, found := k.amm.GetPoolIdWithAllDenoms(ctx, denoms)
+	pool, found := k.amm.GetBestPoolWithDenoms(ctx, denoms)
 	if !found {
 		return 0, errorsmod.Wrap(types.ErrPoolDoesNotExist, fmt.Sprintf("%s", denoms))
 	}
-	return poolId, nil
+	return pool.PoolId, nil
 }