[x/gamm] Re-implement ExitSwapExternAmountOut generically

[ValarDragon]

No description provided.

[p0mvn]

@ValarDragon is this still planned for v8? If yes, could you write up the description for me please, and I can get started on this tomorrow

[p0mvn]

Just adding some notes for myself:

There is an explanation in code about what needs to be done:

osmosis/x/gamm/keeper/pool_service.go

Lines 337 to 353 in a68928e

	func (k Keeper) ExitSwapExternAmountOut(
	ctx sdk.Context,
	sender sdk.AccAddress,
	poolId uint64,
	tokenOut sdk.Coin,
	shareInMaxAmount sdk.Int,
	) (shareInAmount sdk.Int, err error) {
	// Basically what we have to do is:
	// estimate how many LP shares this would take to do.
	// We do so by calculating how much a swap of half of tokenOut to TokenIn would be.
	// Then we calculate how many LP shares that'd be worth.
	// We should have code for that once we implement JoinPoolNoSwap.
	// Then check if the number of shares is LTE to shareInMaxAmount.
	// if so, use the needed number of shares, do exit pool, and the swap.
	panic("To implement later")
	}

This PR adds helper functions to implement ExitSwapExternOutGenerically

[ValarDragon]

So what way ExitSwapExternAmountOut wants to do is:
ExitPool exactly S shares, and swap it all to asset coin such that you have user-specified-amount of that coin. The problem is then, how do we solve for how many shares we need to do this. To find this generically, the idea is we have to work backwards. So we would calculate how many LP shares you would get if you started with exact amount out of coins. How you do this is:

reversedNumShares := pool.calcJoinSwapExactAmountIn(user_specified_amount_in)
coins := k.ExitPool(reversedNumShares)
k.SwapCoinsToOneAsset(coins, asset)
require out_amount.Equal(specified_amount)

I suspect this will be off for non-50/50 pools, and will have rounding errors (cref #1196). I'm not sure if it maybe makes sense to not implement this generically, and just make a second interface AMMs have to implement for this feature, thats then runtime type-check'd.

So I'd suggest we just make a second interface thats optional, of the form

type ExitSwapExactAmountOutExtension interface {
  PoolI
  ExitSwapExactAmountOut(args)
}

And then we implement that method on balancer, but not on other pools

[ValarDragon]

Do you want to take a stab at defining the interface, and then changing the logic in amm.go to work around that interface? And we can just add the balancer implementation of the interface afterwards / in a second PR? (It should mostly be looking at the v7 code, and integrating that into amm.go)

[p0mvn]

I will work on that, thanks for the detailed explanation

[alexanderbez]

I like the 2nd interface idea.

[p0mvn]

Do you want to take a stab at defining the interface, and then changing the logic in amm.go to work around that interface? And we can just add the balancer implementation of the interface afterwards / in a second PR? (It should mostly be looking at the v7 code, and integrating that into amm.go)

Just an update, I'm working on this but I might do both in a single PR. I found that it is easier for me to understand the context while referencing the old implementation. Hope that's okay