ProtoRev: Route building #3683

davidterpay · 2022-12-10T01:05:48Z

What is the purpose of the change

This PR implements the logic of building cyclic arbitrage routes given a swap. There are two primary methods for route generation: Highest Liquidity Pools and Hot Routes.

Brief Changelog

Build routes that start and end with osmo (Highest Liquidity Pools)
Build routes that start and end with atom (Highest Liquidity Pools)
Build routes that start and end with osmo or atom from the set of hot routes (Hot Routes)

Testing and Verifying

This change added tests and can be verified as follows:

Added unit tests for atom routes
Added unit tests for osmo routes
Added unit tests for routes built with tokenPairArbRoutes
Added unit tests for BuildRoutes (which is a wrapper to the above three)

Documentation and Release Note

Highest Liquidity Pools

There are two types of trades that must be considered:

The assets in the trade are neither Atom nor Osmo
- In this case, x/protorev will sandwich the pool with either Osmo or Atom on the other end. For example, say that a swap occurs on the Akash ↔ Juno pool. There are four possible routes that can be taken (in order of trades made starting on the left and ending on the right).
  - (Osmo → Akash), (Akash → Juno), (Juno → Osmo)
  - (Osmo → Juno), (Juno → Akash), (Akash → Osmo)
  - (Atom → Akash), (Akash → Juno), Juno → Atom)
  - (Atom → Juno), (Juno → Akash), (Akash → Atom)
- Capturing cyclic arbitrage opportunities happen in the opposite direction of the trade. Using the example above, we can cut down the routes to only two routes if we know that the user traded Akash → Juno. The routes in that case would be
  - (Osmo → Juno), (Juno → Akash), (Akash → Osmo)
  - (Atom → Juno), (Juno → Akash), (Akash → Atom)
The assets in the trade include either Atom or Osmo or both

If the Osmo ↔ Atom pool is swapped against, then the highest liquidity pool route building method does not produce any routes. Otherwise, if only one of OSMO or ATOM was swapped against in a single pool, then x/protorev will look for the opposite asset pools from what was traded in the pool. For example, say a swap has been executed on the Osmo ↔ TokenXYZ pool, tendering OSMO and receiving TokenXYZ, the route generated would be:
- (Atom → TokenXYZ), (TokenXYZ → Osmo), (Osmo → Atom)

In both cases, the route that is built will always sandwich the swap that was made. However, we allow for more flexibility in route generation as the highest liquidity method may not be optimal via Hot Routes.

Hot Routes

Populated through the admin account (which will be implemented in a later PR), the module’s keeper holds a KV store that associates token pairs (for example, osmo/juno) to the routes that result in a high percentage of arbitrage profit on Osmosis (as determined by external analysis).

The purpose of storing Hot Routes is a recognition that the Highest Liquidity Pool method may not present the best arbitrage routes. As such, hot routes can be configured through governance to store additional routes that may be more effective at capturing arbitrage opportunities. Each hot route will store a placeholder (the value 0) for where the current swapped pool will fit into the trade. TokenPairArbRoutes are always going to be sorted in the direction of the trade i.e. if we see a swap of Atom -> Osmo we can immediately find a route that goes in the opposite direction by replacing the placeholder (0) with this pool id.

stackman27

Left few questions that i have about Routes. Also realized that this module doesnot have a README.md yet, i think it'll be nice to add one 🌮

x/protorev/keeper/routes.go

stackman27 · 2022-12-12T08:04:27Z

x/protorev/keeper/routes.go

+
+		var newTrade TradeInfo
+		for _, trade := range route.Trades {
+			// 0 is a placeholder for swaps that should be entered into the hot route


A little confused with Hot Routes, How does the gov update where the hot routes are? Is there an example of what hot route looks like vs Highest liquidity pool route?

AFAIU there would be an admin key for just setting hot routes, and governance can choose who is the hot route admin

stackman27 · 2022-12-13T01:39:07Z

x/protorev/keeper/routes.go

+// BuildOsmoRoute builds a cyclic arbitrage route that starts and ends with osmo given the tokenIn, tokenOut and poolId that were used in the swap
+func (k Keeper) BuildOsmoRoute(ctx sdk.Context, tokenIn, tokenOut string, poolId uint64) ([]TradeInfo, error) {
+	// Creating the first trade in the arb
+	entryPoolId, err := k.GetOsmoPool(ctx, tokenOut)


Does this get the highest liquidity osmo pool?

yes, it gets the highest liquidity osmo pool that has osmo <-> tokenOut as pool assets

stackman27 · 2022-12-13T01:46:59Z

x/protorev/keeper/routes_test.go

+			inputDenom:  "bitcoin",
+			outputDenom: "ethereum",
+			poolID:      19,
+			expected: [][]TestRoute{


Asking to learn: In what case does making trade in "opposite direction" reduce arbitrage? Is there an example available?

From what I understand, cyclic arbitrage is always going to be favorable in the opposite direction because the pool reserves will essentially let you extract more (relative to the standardized price against other pools and assets) if you decide to swap against the original direction of the trade. You can then take that marginal amount and trade it on pools where prices are not fully in sync.

Not sure if there are exact cases where trading in the opposite direction would reduce arbitrage. @NotJeremyLiu could probably answer that better.

@davidterpay is correct.

@stackman27 if a regular user swaps OSMO -> JUNO on a single pool (pool 5 for this example), they are selling OSMO for JUNO, increasing the price of JUNO and decreasing the price of OSMO (relative prices) on pool 5.

If you assume other OSMO/JUNO pools or routes are in sync w.r.t prices, the pool the user just swapped on now has cheap OSMO and expensive JUNO. The goal for the cyclic arbitrage is to find JUNO on a different pool that's now "cheap" compared to the JUNO on pool 5. They want to buy JUNO elsewhere, then sell JUNO on Pool 5 where it's expensive.

So when looking at pool 5, an arbitrager will always go the opposite direction as the original swap to capture the mispricing opportunity (swapping JUNO -> OSMO on pool 5).

Hope that makes sense, but ask more questions if not!

stackman27 · 2022-12-13T01:49:00Z

x/protorev/keeper/routes_test.go

+			hasRoute:      true,
+		},
+		{
+			description:   "Route exists for swap in Terra and swap out Atom (no mapping pool)",


Shouldn't this description be "Route doesnot exist for swap in Terra and swap out Atom because the pool doesnot exist"?

yea you are correct. i'll switch it up to what you suggested.

ValarDragon

Logic LGTM, nice job!

I only consider de-duplicating BuildOsmoRoute & BuildAtomRoute as blocking

ValarDragon · 2022-12-13T04:26:03Z

x/protorev/keeper/routes.go

+func (k Keeper) BuildRoutes(ctx sdk.Context, tokenIn, tokenOut string, poolId uint64) [][]TradeInfo {
+	routes := make([][]TradeInfo, 0)
+
+	// Check hot routes if enabled


not sure where the if enabled part comes from

thats a typo, will fix

ValarDragon · 2022-12-13T04:26:54Z

x/protorev/keeper/routes.go

+}
+
+// BuildRoutes builds all of the possible arbitrage routes given the given tokenIn, tokenOut and poolId that were used in the swap
+func (k Keeper) BuildRoutes(ctx sdk.Context, tokenIn, tokenOut string, poolId uint64) [][]TradeInfo {


Should we explicitly make a type Route, thats []TradeInfo ?

Wrapped the trades into a Route struct. Updated in the latest commit.

ValarDragon · 2022-12-13T05:44:10Z

x/protorev/keeper/routes.go

+	// Iterate through all of the routes and build hot routes
+	routes := make([][]TradeInfo, 0)
+	for _, route := range tokenPairArbRoutes.ArbRoutes {
+		newRoute := make([]TradeInfo, 0)


Suggested change

newRoute := make([]TradeInfo, 0)

newRoute := make([]TradeInfo, 0, len(route.Trades))

ValarDragon · 2022-12-13T05:45:43Z

x/protorev/keeper/routes.go

+		var newTrade TradeInfo
+		for _, trade := range route.Trades {
+			// 0 is a placeholder for swaps that should be entered into the hot route
+			if trade.Pool == 0 {
+				pool, err := k.GetAndCheckPool(ctx, poolId)
+				if err != nil {
+					return [][]TradeInfo{}, err
+				}
+
+				newTrade = TradeInfo{
+					InputDenom:  tokenOut,
+					OutputDenom: tokenIn,
+					SwapFee:     pool.GetSwapFee(ctx),
+					Pool:        pool,
+				}
+			} else {
+				pool, err := k.GetAndCheckPool(ctx, trade.Pool)
+				if err != nil {
+					return [][]TradeInfo{}, err
+				}
+
+				newTrade = TradeInfo{
+					InputDenom:  trade.TokenIn,
+					OutputDenom: trade.TokenOut,
+					SwapFee:     pool.GetSwapFee(ctx),
+					Pool:        pool,
+				}
+			}
+
+			newRoute = append(newRoute, newTrade)
+		}


Annoying code style comment:

Perhaps extract this into a method BuildTradeInfoRoute

Added new BuildTradeInfoHotRoute and CheckValidHotRoute helper functions in latest commit.

ValarDragon · 2022-12-13T05:48:28Z

x/protorev/keeper/routes.go

+
+		var newTrade TradeInfo
+		for _, trade := range route.Trades {
+			// 0 is a placeholder for swaps that should be entered into the hot route


AFAIU there would be an admin key for just setting hot routes, and governance can choose who is the hot route admin

ValarDragon · 2022-12-13T05:49:32Z

x/protorev/keeper/routes.go

+		// Only append if the arbitrage denom is valid
+		// In denom and out denom must be the same
+		// In denom must be uosmo or atom
+		// There must be exactly three hops in the route


Suggested change

// There must be exactly three hops in the route

// There must be exactly three hops in the route

// Note: The hot route itself may comprise of more hops

ValarDragon · 2022-12-13T05:55:56Z

x/protorev/keeper/routes.go

+}
+
+// BuildAtomRoute builds a cyclic arbitrage route that starts and ends with atom given the tokenIn, tokenOut and poolId that were used in the swap
+func (k Keeper) BuildAtomRoute(ctx sdk.Context, tokenIn, tokenOut string, poolId uint64) ([]TradeInfo, error) {


Can we deduplicate the BuildOsmoRoute BuildAtomRoute logic by making another method that takes in as args

string for types.AtomDenomination vs osmoDenom

Function for k.GetAtomPool vs k.GetOsmoPool

Added a new BuildTradeInfoRoute helper function which abstracts the getter and swap so code is not duplicated.

…ades in route struct

davidterpay added 2 commits December 9, 2022 18:59

implementing route building logic for atom, osmo and hot routes

00ce908

route building unit tests

65508f7

davidterpay added the V:state/breaking State machine breaking PR label Dec 10, 2022

lint fixes

ff7f583

davidterpay marked this pull request as ready for review December 10, 2022 01:40

davidterpay requested review from bpiv400, NotJeremyLiu, stackman27, alexanderbez and ValarDragon December 10, 2022 01:41

stackman27 reviewed Dec 13, 2022

View reviewed changes

ValarDragon approved these changes Dec 13, 2022

View reviewed changes

davidterpay added 2 commits December 13, 2022 12:02

de-duplicating route building logic, fixing comments, and wrapping tr…

e96bf77

…ades in route struct

creating trade array in the beginning instead of appending

c10d9e9

bpiv400 requested a review from ValarDragon December 13, 2022 19:09

ValarDragon approved these changes Dec 13, 2022

View reviewed changes

ValarDragon merged commit 67ab2f5 into main Dec 13, 2022

ValarDragon deleted the protorev-route-building branch December 13, 2022 20:40

davidterpay mentioned this pull request Dec 13, 2022

x/protorev PR Breakdown #3594

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ProtoRev: Route building #3683

ProtoRev: Route building #3683

davidterpay commented Dec 10, 2022 •

edited

Loading

stackman27 left a comment

stackman27 Dec 12, 2022

ValarDragon Dec 13, 2022

stackman27 Dec 13, 2022

davidterpay Dec 13, 2022

stackman27 Dec 13, 2022

davidterpay Dec 13, 2022

NotJeremyLiu Dec 13, 2022

stackman27 Dec 13, 2022

davidterpay Dec 13, 2022 •

edited

Loading

ValarDragon left a comment

ValarDragon Dec 13, 2022

davidterpay Dec 13, 2022

ValarDragon Dec 13, 2022

davidterpay Dec 13, 2022

ValarDragon Dec 13, 2022

ValarDragon Dec 13, 2022

davidterpay Dec 13, 2022

ValarDragon Dec 13, 2022

ValarDragon Dec 13, 2022

ValarDragon Dec 13, 2022

davidterpay Dec 13, 2022

davidterpay Dec 13, 2022

	newRoute := make([]TradeInfo, 0)
	newRoute := make([]TradeInfo, 0, len(route.Trades))

	// There must be exactly three hops in the route
	// There must be exactly three hops in the route
	// Note: The hot route itself may comprise of more hops

ProtoRev: Route building #3683

ProtoRev: Route building #3683

Conversation

davidterpay commented Dec 10, 2022 • edited Loading

What is the purpose of the change

Brief Changelog

Testing and Verifying

Documentation and Release Note

Highest Liquidity Pools

Hot Routes

stackman27 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidterpay Dec 13, 2022 • edited Loading

Choose a reason for hiding this comment

ValarDragon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidterpay commented Dec 10, 2022 •

edited

Loading

davidterpay Dec 13, 2022 •

edited

Loading