Protorev smarter logic and more testing (#4181)

* Add generalized tests

Added:
- 4 pool mainnet route test
- 2 pool mainnet route test
- Non-osmo/atom denom test

* Add stableswap doomsday test

- Currently panics due to solveCFMMBinarySearchMulti in stableswap calculation

* Add tests for pool point limits

- Tests that the per tx limit works properly
- Tests that the per block limit works properly

* Add pre-check before binary search

- Adds a pre-check before the binary search that tells us if we need to run the binary search at all
- Reduces overall computation/time by avoiding binary searching profit amounts over routes without any profit opportunity

* Add denom to make test pass

- Added "test/3" denom to test non-osmo/atom denoms in previous PR
- didn't notice it broke one test that expected us to only have 2 denoms, so adding "test/3" here so everything passes

* Implement minimum change to have smarter binary search bounds

- This implements a way for us to have a binary search bound that can increase in size to account for large trades above the default range
- I believe this is the minimum-amount-of-new-code approach, but may not be the optimal approach (what this method avoids is having to save the amount in or amount out of the original swap, and then backtracking/converting that amount to the base denom for an arb, and creating bounds off of that)

* Switch range increasing logic

- Doubling isn't the wanted behavior (only wanted if lower bound is 1), just increasing bound by the same range size is wanted (so adding MaxInputAmount achieves this)

* Add logic to extend search bounds when finding optimal amount in

- Increases max iterations to 17 to allow for situation when we need to increase the upper bound
- Add new ExtendedMaxInputAmount variable for us to use as this new max bound range
- Replace bound changing logic from iteratively changing the range to immediately giving our max range

* Move range extension into it's own helper function

* basic benchmark testing for posthandler and epoch hook

* Add SwapAmountOut Test

* Add extended range test

- Tests binary search range extension logic works properly

* Add panic catching test

- This currently fails, this is on purpose so we don't merge the PR and think we are good until this passes and the panics are handled properly

* dynamic step size

* pool points only incremented if profitable

* adding sanity checks for pool point calcs, nits

* Update doomsday testing accounting for refund system

- Makes pool 41 reserves to have an arb opportunity in the doomsday routes
- Changes tx limit and block limit specifically for doomsday testing

* Return nil for no profit opportunity

* adding E2E, removing atom as a base denom, more testing for post handler

* find max profit test fix

* nit

* backporting version tag to 14.x

* comment update for protorev admin account

---------

Co-authored-by: David Terpay <[email protected]>
(cherry picked from commit 23b13a1)

# Conflicts:
#	tests/e2e/configurer/chain/queries.go

proto/osmosis/protorev/v1beta1/genesis.proto

@@ -11,6 +11,4 @@ option go_package = "github.com/osmosis-labs/osmosis/v14/x/protorev/types";
 message GenesisState {
   // Module Parameters
   Params params = 1 [ (gogoproto.nullable) = false ];
-  // Hot routes that are configured on genesis
-  repeated TokenPairArbRoutes token_pairs = 2 [ (gogoproto.nullable) = false ];
 }

proto/osmosis/protorev/v1beta1/protorev.proto

@@ -18,6 +18,12 @@ message TokenPairArbRoutes {
   string token_in = 2;
   // Token denomination of the second asset
   string token_out = 3;
+  // The step size that will be used to find the optimal swap amount in the
+  // binary search
+  string step_size = 4 [
+    (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
+    (gogoproto.nullable) = true
+  ];
 }
 
 // Route is a hot route for a given pair of tokens

tests/e2e/configurer/chain/queries.go

@@ -22,10 +22,181 @@ import (
 	"github.com/osmosis-labs/osmosis/v14/tests/e2e/util"
 	epochstypes "github.com/osmosis-labs/osmosis/v14/x/epochs/types"
 	gammtypes "github.com/osmosis-labs/osmosis/v14/x/gamm/types"
+<<<<<<< HEAD
+=======
+	poolmanagertypes "github.com/osmosis-labs/osmosis/v14/x/poolmanager/types"
+	protorevtypes "github.com/osmosis-labs/osmosis/v14/x/protorev/types"
+>>>>>>> 23b13a1d (Protorev smarter logic and more testing (#4181))
 	superfluidtypes "github.com/osmosis-labs/osmosis/v14/x/superfluid/types"
 	twapqueryproto "github.com/osmosis-labs/osmosis/v14/x/twap/client/queryproto"
 )
 
+// QueryProtoRevNumberOfTrades gets the number of trades the protorev module has executed.
+func (n *NodeConfig) QueryProtoRevNumberOfTrades() (sdk.Int, error) {
+	path := "/osmosis/v14/protorev/number_of_trades"
+
+	bz, err := n.QueryGRPCGateway(path)
+	if err != nil {
+		return sdk.Int{}, err
+	}
+
+	// nolint: staticcheck
+	var response protorevtypes.QueryGetProtoRevNumberOfTradesResponse
+	err = util.Cdc.UnmarshalJSON(bz, &response)
+	require.NoError(n.t, err) // this error should not happen
+	return response.NumberOfTrades, nil
+}
+
+// QueryProtoRevProfits gets the profits the protorev module has made.
+func (n *NodeConfig) QueryProtoRevProfits() ([]*sdk.Coin, error) {
+	path := "/osmosis/v14/protorev/all_profits"
+
+	bz, err := n.QueryGRPCGateway(path)
+	if err != nil {
+		return []*sdk.Coin{}, err
+	}
+
+	// nolint: staticcheck
+	var response protorevtypes.QueryGetProtoRevAllProfitsResponse
+	err = util.Cdc.UnmarshalJSON(bz, &response)
+	require.NoError(n.t, err) // this error should not happen
+	return response.Profits, nil
+}
+
+// QueryProtoRevAllRouteStatistics gets all of the route statistics that the module has recorded.
+func (n *NodeConfig) QueryProtoRevAllRouteStatistics() ([]protorevtypes.RouteStatistics, error) {
+	path := "/osmosis/v14/protorev/all_route_statistics"
+
+	bz, err := n.QueryGRPCGateway(path)
+	if err != nil {
+		return []protorevtypes.RouteStatistics{}, err
+	}
+
+	// nolint: staticcheck
+	var response protorevtypes.QueryGetProtoRevAllRouteStatisticsResponse
+	err = util.Cdc.UnmarshalJSON(bz, &response)
+	require.NoError(n.t, err) // this error should not happen
+	return response.Statistics, nil
+}
+
+// QueryProtoRevTokenPairArbRoutes gets all of the token pair hot routes that the module is currently using.
+func (n *NodeConfig) QueryProtoRevTokenPairArbRoutes() ([]*protorevtypes.TokenPairArbRoutes, error) {
+	path := "/osmosis/v14/protorev/token_pair_arb_routes"
+
+	bz, err := n.QueryGRPCGateway(path)
+	if err != nil {
+		return []*protorevtypes.TokenPairArbRoutes{}, err
+	}
+
+	// nolint: staticcheck
+	var response protorevtypes.QueryGetProtoRevTokenPairArbRoutesResponse
+	err = util.Cdc.UnmarshalJSON(bz, &response)
+	require.NoError(n.t, err) // this error should not happen
+	return response.Routes, nil
+}
+
+// QueryProtoRevDeveloperAccount gets the developer account of the module.
+func (n *NodeConfig) QueryProtoRevDeveloperAccount() (sdk.AccAddress, error) {
+	path := "/osmosis/v14/protorev/developer_account"
+
+	bz, err := n.QueryGRPCGateway(path)
+	if err != nil {
+		return nil, err
+	}
+
+	// nolint: staticcheck
+	var response protorevtypes.QueryGetProtoRevDeveloperAccountResponse
+	err = util.Cdc.UnmarshalJSON(bz, &response)
+	require.NoError(n.t, err) // this error should not happen
+
+	account, err := sdk.AccAddressFromBech32(response.DeveloperAccount)
+	if err != nil {
+		return nil, err
+	}
+
+	return account, nil
+}
+
+// QueryProtoRevPoolWeights gets the pool point weights of the module.
+func (n *NodeConfig) QueryProtoRevPoolWeights() (*protorevtypes.PoolWeights, error) {
+	path := "/osmosis/v14/protorev/pool_weights"
+
+	bz, err := n.QueryGRPCGateway(path)
+	if err != nil {
+		return nil, err
+	}
+
+	// nolint: staticcheck
+	var response protorevtypes.QueryGetProtoRevPoolWeightsResponse
+	err = util.Cdc.UnmarshalJSON(bz, &response)
+	require.NoError(n.t, err) // this error should not happen
+	return response.PoolWeights, nil
+}
+
+// QueryProtoRevMaxPoolPointsPerTx gets the max pool points per tx of the module.
+func (n *NodeConfig) QueryProtoRevMaxPoolPointsPerTx() (uint64, error) {
+	path := "/osmosis/v14/protorev/max_pool_points_per_tx"
+
+	bz, err := n.QueryGRPCGateway(path)
+	if err != nil {
+		return 0, err
+	}
+
+	// nolint: staticcheck
+	var response protorevtypes.QueryGetProtoRevMaxPoolPointsPerTxResponse
+	err = util.Cdc.UnmarshalJSON(bz, &response)
+	require.NoError(n.t, err) // this error should not happen
+	return response.MaxPoolPointsPerTx, nil
+}
+
+// QueryProtoRevMaxPoolPointsPerBlock gets the max pool points per block of the module.
+func (n *NodeConfig) QueryProtoRevMaxPoolPointsPerBlock() (uint64, error) {
+	path := "/osmosis/v14/protorev/max_pool_points_per_block"
+
+	bz, err := n.QueryGRPCGateway(path)
+	if err != nil {
+		return 0, err
+	}
+
+	// nolint: staticcheck
+	var response protorevtypes.QueryGetProtoRevMaxPoolPointsPerBlockResponse
+	err = util.Cdc.UnmarshalJSON(bz, &response)
+	require.NoError(n.t, err) // this error should not happen
+	return response.MaxPoolPointsPerBlock, nil
+}
+
+// QueryProtoRevBaseDenoms gets the base denoms used to construct cyclic arbitrage routes.
+func (n *NodeConfig) QueryProtoRevBaseDenoms() ([]*protorevtypes.BaseDenom, error) {
+	path := "/osmosis/v14/protorev/base_denoms"
+
+	bz, err := n.QueryGRPCGateway(path)
+	if err != nil {
+		return []*protorevtypes.BaseDenom{}, err
+	}
+
+	// nolint: staticcheck
+	var response protorevtypes.QueryGetProtoRevBaseDenomsResponse
+	err = util.Cdc.UnmarshalJSON(bz, &response)
+	require.NoError(n.t, err) // this error should not happen
+	return response.BaseDenoms, nil
+}
+
+// QueryProtoRevEnabled queries if the protorev module is enabled.
+func (n *NodeConfig) QueryProtoRevEnabled() (bool, error) {
+	path := "/osmosis/v14/protorev/enabled"
+
+	bz, err := n.QueryGRPCGateway(path)
+	if err != nil {
+		return false, err
+	}
+
+	// nolint: staticcheck
+	var response protorevtypes.QueryGetProtoRevEnabledResponse
+	err = util.Cdc.UnmarshalJSON(bz, &response)
+	require.NoError(n.t, err) // this error should not happen
+	return response.Enabled, nil
+}
+
 func (n *NodeConfig) QueryGRPCGateway(path string, parameters ...string) ([]byte, error) {
 	if len(parameters)%2 != 0 {
 		return nil, fmt.Errorf("invalid number of parameters, must follow the format of key + value")
@@ -111,6 +282,18 @@ func (n *NodeConfig) QuerySupplyOf(denom string) (sdk.Int, error) {
 	return supplyResp.Amount.Amount, nil
 }
 
+func (n *NodeConfig) QuerySupply() (sdk.Coins, error) {
+	path := "cosmos/bank/v1beta1/supply"
+	bz, err := n.QueryGRPCGateway(path)
+	require.NoError(n.t, err)
+
+	var supplyResp banktypes.QueryTotalSupplyResponse
+	if err := util.Cdc.UnmarshalJSON(bz, &supplyResp); err != nil {
+		return nil, err
+	}
+	return supplyResp.Supply, nil
+}
+
 func (n *NodeConfig) QueryContractsFromId(codeId int) ([]string, error) {
 	path := fmt.Sprintf("/cosmwasm/wasm/v1/code/%d/contracts", codeId)
 	bz, err := n.QueryGRPCGateway(path)

tests/e2e/containers/config.go

@@ -27,7 +27,7 @@ const (
 	previousVersionOsmoTag        = "v14.x-4d4583fa-1676370337"
 	// Pre-upgrade repo/tag for osmosis initialization (this should be one version below upgradeVersion)
 	previousVersionInitRepository = "osmolabs/osmosis-e2e-init-chain"
-	previousVersionInitTag        = "v14.x-4d4583fa-1676370337-manual"
+	previousVersionInitTag        = "v14.x-937d601e-1676550460-manual"
 	// Hermes repo/version for relayer
 	relayerRepository = "osmolabs/hermes"
 	relayerTag        = "0.13.0"

tests/e2e/e2e_test.go

@@ -34,6 +34,134 @@ import (
 
 // Reusable Checks
 
+// TestProtoRev is a test that ensures that the protorev module is working as expected. In particular, this tests and ensures that:
+// 1. The protorev module is correctly configured on init
+// 2. The protorev module can correctly back run a swap
+// 3. the protorev module correctly tracks statistics
+func (s *IntegrationTestSuite) TestProtoRev() {
+	const (
+		poolFile1 = "protorevPool1.json"
+		poolFile2 = "protorevPool2.json"
+		poolFile3 = "protorevPool3.json"
+
+		walletName = "swap-that-creates-an-arb"
+
+		denomIn      = initialization.LuncIBCDenom
+		denomOut     = initialization.UstIBCDenom
+		amount       = "10000"
+		minAmountOut = "1"
+
+		epochIdentifier = "week"
+	)
+
+	chainA := s.configurer.GetChainConfig(0)
+	chainANode, err := chainA.GetDefaultNode()
+	s.NoError(err)
+
+	// --------------- Module init checks ---------------- //
+	// The module should be enabled by default.
+	enabled, err := chainANode.QueryProtoRevEnabled()
+	s.T().Logf("checking that the protorev module is enabled: %t", enabled)
+	s.Require().NoError(err)
+	s.Require().True(enabled)
+
+	// The module should have no new hot routes by default.
+	hotRoutes, err := chainANode.QueryProtoRevTokenPairArbRoutes()
+	s.T().Logf("checking that the protorev module has no new hot routes: %v", hotRoutes)
+	s.Require().NoError(err)
+	s.Require().Len(hotRoutes, 0)
+
+	// The module should have no trades by default.
+	numTrades, err := chainANode.QueryProtoRevNumberOfTrades()
+	s.T().Logf("checking that the protorev module has no trades on init: %s", err)
+	s.Require().Error(err)
+
+	// The module should have pool weights by default.
+	poolWeights, err := chainANode.QueryProtoRevPoolWeights()
+	s.T().Logf("checking that the protorev module has pool weights on init: %s", poolWeights)
+	s.Require().NoError(err)
+	s.Require().NotNil(poolWeights)
+
+	// The module should have max pool points per tx by default.
+	maxPoolPointsPerTx, err := chainANode.QueryProtoRevMaxPoolPointsPerTx()
+	s.T().Logf("checking that the protorev module has max pool points per tx on init: %d", maxPoolPointsPerTx)
+	s.Require().NoError(err)
+
+	// The module should have max pool points per block by default.
+	maxPoolPointsPerBlock, err := chainANode.QueryProtoRevMaxPoolPointsPerBlock()
+	s.T().Logf("checking that the protorev module has max pool points per block on init: %d", maxPoolPointsPerBlock)
+	s.Require().NoError(err)
+
+	// The module should have only osmosis as a supported base denom by default.
+	supportedBaseDenoms, err := chainANode.QueryProtoRevBaseDenoms()
+	s.T().Logf("checking that the protorev module has only osmosis as a supported base denom on init: %v", supportedBaseDenoms)
+	s.Require().NoError(err)
+	s.Require().Len(supportedBaseDenoms, 1)
+	s.Require().Equal(supportedBaseDenoms[0].Denom, "uosmo")
+
+	// The module should have no developer account by default.
+	_, err = chainANode.QueryProtoRevDeveloperAccount()
+	s.T().Logf("checking that the protorev module has no developer account on init: %s", err)
+	s.Require().Error(err)
+
+	// --------------- Set up for a calculated backrun ---------------- //
+	// Create all of the pools that will be used in the test.
+	chainANode.CreateBalancerPool(poolFile1, initialization.ValidatorWalletName)
+	swapPoolId := chainANode.CreateBalancerPool(poolFile2, initialization.ValidatorWalletName)
+	chainANode.CreateBalancerPool(poolFile3, initialization.ValidatorWalletName)
+
+	// Wait for the creation to be propogated to the other nodes + for the protorev module to
+	// correctly update the highest liquidity pools.
+	s.T().Logf("waiting for the protorev module to update the highest liquidity pools (wait %.f sec) after the week epoch duration", initialization.EpochWeekDuration.Seconds())
+	chainA.WaitForNumEpochs(1, epochIdentifier)
+
+	// Create a wallet to use for the swap txs.
+	swapWalletAddr := chainANode.CreateWallet(walletName)
+	coinIn := fmt.Sprintf("%s%s", amount, denomIn)
+	chainANode.BankSend(coinIn, chainA.NodeConfigs[0].PublicAddress, swapWalletAddr)
+
+	// Check supplies before swap.
+	supplyBefore, err := chainANode.QuerySupply()
+	s.Require().NoError(err)
+	s.Require().NotNil(supplyBefore)
+
+	// Performing the swap that creates a cyclic arbitrage opportunity.
+	s.T().Logf("performing a swap that creates a cyclic arbitrage opportunity")
+	chainANode.SwapExactAmountIn(coinIn, minAmountOut, fmt.Sprintf("%d", swapPoolId), denomOut, swapWalletAddr)
+
+	// --------------- Module checks after a calculated backrun ---------------- //
+	// Check that the supplies have not changed.
+	s.T().Logf("checking that the supplies have not changed")
+	supplyAfter, err := chainANode.QuerySupply()
+	s.Require().NoError(err)
+	s.Require().NotNil(supplyAfter)
+	s.Require().Equal(supplyBefore, supplyAfter)
+
+	// Check that the number of trades executed by the protorev module is 1.
+	numTrades, err = chainANode.QueryProtoRevNumberOfTrades()
+	s.T().Logf("checking that the protorev module has executed 1 trade")
+	s.Require().NoError(err)
+	s.Require().NotNil(numTrades)
+	s.Require().Equal(uint64(1), numTrades.Uint64())
+
+	// Check that the profits of the protorev module are not nil.
+	profits, err := chainANode.QueryProtoRevProfits()
+	s.T().Logf("checking that the protorev module has non-nil profits: %s", profits)
+	s.Require().NoError(err)
+	s.Require().NotNil(profits)
+	s.Require().Len(profits, 1)
+
+	// Check that the route statistics of the protorev module are not nil.
+	routeStats, err := chainANode.QueryProtoRevAllRouteStatistics()
+	s.T().Logf("checking that the protorev module has non-nil route statistics: %x", routeStats)
+	s.Require().NoError(err)
+	s.Require().NotNil(routeStats)
+	s.Require().Len(routeStats, 1)
+	s.Require().Equal(sdk.OneInt(), routeStats[0].NumberOfTrades)
+	s.Require().Equal([]uint64{swapPoolId - 1, swapPoolId, swapPoolId + 1}, routeStats[0].Route)
+	s.Require().Equal(profits, routeStats[0].Profits)
+}
+
 // CheckBalance Checks the balance of an address
 func (s *IntegrationTestSuite) CheckBalance(node *chain.NodeConfig, addr, denom string, amount int64) {
 	// check the balance of the contract