[CL][Swap Router Module]: refactor simulation hack

[p0mvn]

Background

In #3073, we ported swap messages from gamm to swap-router.

Additionally, we ported pre-existing simulation

Pre-existing setup for simulation requires access to several gamm keeper APIs.

Current simulation utility functions do not provide support to wire additional keepers. As a result, extra keeper methods were defined on swaprouter keeper to route the calls to gamm keeper internally.

osmosis/x/swaprouter/types/routes.go

Lines 29 to 38 in ba30351

	// SimulationExtension defines the swap simulation extension.
	// TODO: refactor simulator setup logic to avoid having to define these
	// extra methods just for the simulation.
	type SimulationExtension interface {
	SwapI
	GetPoolAndPoke(ctx sdk.Context, poolId uint64) (gammtypes.TraditionalAmmInterface, error)
	GetNextPoolId(ctx sdk.Context) uint64
	}

We should refactor simulator API to provide the ability for wiring additional keepers to avoid this issue.

Suggested Design

• Remove GetPoolId from swaprouter
• Remove GetNextPoolId from `swaprouter
• Remove SimulatorExtension
• Make gammKeeper in swaprouter keeper to be of type SwapI
• Refactor simulator logic to support wiring additional keepers
• wire up gamm keeper in swaprouter simulator

Acceptance Criteria

• suggested design implemented
• simulator `swaprouter tests work
• tests pass

[p0mvn]

@rusakh said that he's on it. Can you self-assign please?

[ValarDragon]

I don't understand, the simulator already supports this? This shouldn't require any scaffolding or simulator changes?

All you'd have to do is add the other keepers to the AppModule in the case of simulator being ran, and then link the keeper in the call to getting the actions?

[p0mvn]

I think you are right and that is possible without additional changes, just not very intuitive.

@rusakh suggested this method:

We are thinking of creating a wrapper around swaprouters' keeper which will export all keepers, something like this:

type SimulationTest struct {
    swaprouter.Keeper
    GammKeeper gammkeeper.Keeper
}

and then to use this struct in simulations

func RandomSwapExactAmountIn(k swaprouter.Keeper SimulationTest, sim *simtypes.SimCtx, ctx sdk.Context) (*types.MsgSwapExactAmountIn, error) { (edited) 

and other functions as well

[pysel]

I don't understand, the simulator already supports this? This shouldn't require any scaffolding or simulator changes?

All you'd have to do is add the other keepers to the AppModule in the case of simulator being ran, and then link the keeper in the call to getting the actions?

But if we simply include new keepers in AppModule we will not be able to access them in simulations because AppModule does not export them, right? I thought we were trying to make all the logic of other keepers and swaprouter keeper available in one place (which I called SimulationKeeper or SimulationTest in Roman's comment) to be used in simulations. The reason for that is that with this approach we do not need to multiple these entities (SimulationExtensions) for each new keeper included.

I might be wrong and might not understand something correctly, but I think we would actually need to implement SimulationKeeper abstract. If I am wrong, please, I would really appreciate any corrections.

[p0mvn]

Can we supply the gamm keeper to swaprouter AppModule here?

osmosis/app/modules.go

Line 150 in 70bf005

swaproutermodule.NewAppModule(*app.SwapRouterKeeper),