Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protorev: Post Handler + Proposal Handlers + Profit Sharing #3806

Merged
merged 18 commits into from
Jan 11, 2023

Conversation

davidterpay
Copy link
Contributor

@davidterpay davidterpay commented Dec 20, 2022

What is the purpose of the change

This PR implements the post handler, the proposal handlers that are responsible for setting the admin account, admin functionality, stateful changes depending on proposals, and developer profit sharing. This is PR 5/6 (#3594).

Brief Changelog

  • Implement the proposal handlers for x/protorev
  • Create admin functionality to set hot routes, the developer account, and number of pools to iterate per tx
  • Profit splitting after every successful trade execution for the developer account
  • Implement the post handler

Testing and Verifying

This change added tests and can be verified as follows:

  • Added unit tests for gas refunds
  • Added unit tests for mock swap tx
  • Added unit tests for extracting the correct pools from the tx
  • Added unit tests for proposals
  • Added unit tests for new setters and getters
  • Added unit tests for updating developer fees
  • Added unit tests for enabling or disabling the module (and affects in epoch and post handler)

Documentation and Release Note

The postHandler extracts pools that were swapped in a transaction and determines if there is a cyclic arbitrage opportunity. If so, the handler will find an optimal route and execute it - rebalancing the pool and returning arbitrage profits to the module account. Additionally, the gas meter gets reset at the end of the post handler in order to refund users for computation done in the handler.

  1. Extract all pools that were traded on in the transaction (ExtractSwappedPools) as well as the direction of the trade.
  2. Create cyclic arbitrage routes for each of the swaps above (BuildRoutes)
  3. For each feasible route, determine if there is a cyclic arbitrage opportunity (IterateRoutes)
    1. Determine the optimal amount to swap in and its respective profits via binary search over range of potential input amounts (FindMaxProfitForRoute)
    2. Compare profits of each route, keep the best route and input amount with the highest profit
  4. If the best route and input amount has a profit > 0, execute the trade (ExecuteTrade) and rebalance the pools on-behalf of the chain through the gammkeeper (MultiHopSwapExactAmountIn)
  5. Keep the profits in the module’s account for subsequent distribution.

Keeper Updates

State Object Description Key Values Store
ProtoRevEnabled Tracks whether the protorev module is enabled []byte{7} []byte{bool} KV
AdminAccount Tracks the admin account for protorev []byte{8} []byte{sdk.AccAddress} KV
DeveloperAccount Tracks the developer account for protorev []byte{9} []byte{sdk.AccAddress} KV
DaysSinceGenesis Tracks the number of days since the module was initialized. Used to track profits that can be withdrawn by the developer account []byte{10} []byte{uint} KV
DeveloperFees Tracks the profits that the developer account can withdraw []byte{11} + []byte{tokenDenom} []byte{sdk.Coin} KV
MaxRoutesPerTx Tracks the maximum number of routes that can be traversed per tx []byte{12} []byte{uint64} KV
MaxRoutesPerBlock Tracks the maximum number of routes that can be traversed per block []byte{13} []byte{uint64} KV
RouteCountForBlock Tracks the number of routes that have been traversed in the current block []byte{14} []byte{uint64} KV
LatestBlockHeight Tracks the latest recorded block height []byte{15} []byte{uint64} KV
RouteWeights Tracks the weights of the different routes []byte{16} []byte{uint64} KV

ProtoRevEnabled
x/protorev can be enabled or disabled through governance. As a proposal is a stateful change, we store whether the module is currently enabled or disabled in the module.

AdminAccount
The admin account is set through governance and has permissions to set hot routes and the developer account. On genesis, the admin account is nil.

DeveloperAccount
The developer account is set through a NewMsgSetDeveloperAccount tx. This is the account that will be able to withdraw a portion of the profits from x/protorev as specified by the Osmosis ↔ Skip proposal. Only the admin account has permission to make this message.

DaysSinceGenesis
x/protorev will distribute 20% of profits to skip in the year 1, 10% of profits in year 2, and 5% thereafter. To track how much profit can be distributed to the developer account at any given moment, we store the amount of days since genesis.

DeveloperFees
DeveloperFees tracks the total amount of profit that can be withdrawn by the developer account. These fees are sent to the developer account, if set, every week through the epoch hook. If unset, the funds are held in the module account.

MaxRoutesPerTx
MaxRoutesPerTx tracks the maximum number of routes that can be traversed in a given transaction. This is configurable (but bounded) by the admin account. We limit the number of routes per transaction so that all x/protorev execution is not limited to the top of the block.

MaxRoutesPerBlock
MaxRoutesPerBlock tracks the maximum number of routes that can be traversed in a given block. This is configurable (but bounded) by the admin account. We limit the number of routes per block so that the execution time of the x/protorev posthandler is reasonably bounded to ensure that block time remains as is.

RouteCountForBlock
RouteCountForBlock tracks the number of routes that have been traversed in the current block. Used to ensure that the module is not slowing down block speed.

LatestBlockHeight
LatestBlockHeight tracks the latest recorded block height. Used to track the number of routes that have been traversed in the current block.

RouteWeights
RouteWeights contains the weights of all of the different route types. Routes are broken up into different types based on the pool that is sandwiched in between the arbitrage route. This distinction is made and necessary because the execution time ranges fairly between the different route types.

Proposal Handlers

x/protorev implements two different governance proposals.

SetProtoRevAdminAccountProposal
As the landscape of pools on Osmosis evolves, an admin account will be able to add and remove routes for x/protorev to check for cyclic arbitrage opportunities. Largely, the purpose of maintaining hot routes is to reduce the amount of computation that would otherwise be required to determine optimal paths at runtime. In addition, introducing a means of altering hot can allow external researchers to conduct meaningful analysis into the markets on-chain.

SetProtoRevEnabledProposal
This proposal type allows the chain to turn the module on or off. This is meant to be used as a fail safe in the case stakers and the chain decide to turn the module off. This might be used to halt the execution of trades in the case that the x/gamm module has significant upgrades that might produce unexpected behavior from the module.

Profit Sharing

Profits accumulated by the module will be partially distributed to the developers that built the module in accordance with the governance proposal that was passed: year 1 is 20% of profits, year 2 is 10%, and subsequent years is 5%.

In order to track how much profit the developers can withdraw at any given moment, the module tracks the number of days since genesis. This gets incremented in the epoch hook after every day. When a trade gets executed by the module, the module will determine how much of the profit from the trade the developers can receive by using daysSinceGenesis in a simple calculation.

If the developer account is not set (which it is not on genesis), all funds are held in the module account. Once the admin account is set through a successful governance proposal, the developer address can be set and will start to automatically receive a share of profits every week through the epoch hook. The distribution of funds from the module account is done through SendDeveloperFeesToDeveloperAccount. Once the funds are distributed, the amount of profit developers can withdraw gets reset to 0 and profits will start to be accumulated and distributed on a week to week basis.

@github-actions github-actions bot added C:app-wiring Changes to the app folder C:CLI labels Dec 20, 2022
@davidterpay davidterpay added the V:state/breaking State machine breaking PR label Dec 20, 2022
@davidterpay davidterpay requested a review from bpiv400 December 20, 2022 18:58
@davidterpay davidterpay mentioned this pull request Dec 20, 2022
8 tasks
@davidterpay davidterpay changed the title Protorev pr5 Protorev: Post Handler + Governance Proposal Handlers + Developer Profit Sharing Dec 20, 2022
@davidterpay davidterpay changed the title Protorev: Post Handler + Governance Proposal Handlers + Developer Profit Sharing Protorev: Post Handler + Proposal Handlers + Profit Sharing Dec 20, 2022
@NotJeremyLiu NotJeremyLiu marked this pull request as ready for review December 22, 2022 03:43
x/protorev/genesis.go Outdated Show resolved Hide resolved
Copy link
Contributor

@stackman27 stackman27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the late review, got caught up with some other work.

left few comments! still reviewing files past posthandler 🌮

x/protorev/client/cli/tx.go Show resolved Hide resolved
x/protorev/keeper/developer_fees_test.go Outdated Show resolved Hide resolved
x/protorev/keeper/developer_fees_test.go Outdated Show resolved Hide resolved
x/protorev/keeper/developer_fees_test.go Outdated Show resolved Hide resolved
x/protorev/keeper/developer_fees_test.go Outdated Show resolved Hide resolved
x/protorev/client/cli/tx.go Show resolved Hide resolved
x/protorev/genesis.go Outdated Show resolved Hide resolved
x/protorev/keeper/posthandler.go Show resolved Hide resolved
x/protorev/keeper/posthandler.go Outdated Show resolved Hide resolved
x/protorev/keeper/posthandler.go Outdated Show resolved Hide resolved
@davidterpay davidterpay force-pushed the protorev-pr5 branch 3 times, most recently from d68f658 to 5836130 Compare December 29, 2022 20:46
Copy link

@bpiv400 bpiv400 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few minor comments around naming and cleaning up the readme in light of the new keeper data.

x/protorev/genesis.go Show resolved Hide resolved
x/protorev/keeper/protorev.go Show resolved Hide resolved
x/protorev/keeper/protorev.go Show resolved Hide resolved
x/protorev/keeper/protorev.go Show resolved Hide resolved
x/protorev/keeper/posthandler.go Outdated Show resolved Hide resolved
@davidterpay davidterpay force-pushed the protorev-pr5 branch 2 times, most recently from caf39f6 to 47c767b Compare January 4, 2023 00:04
bpiv400
bpiv400 previously requested changes Jan 5, 2023
x/protorev/keeper/routes.go Outdated Show resolved Hide resolved
@github-actions github-actions bot added C:docs Improvements or additions to documentation C:simulator Edits simulator or simulations C:x/concentrated-liquidity C:x/gamm Changes, features and bugs related to the gamm module. C:x/pool-incentives labels Jan 9, 2023
@github-actions github-actions bot removed C:docs Improvements or additions to documentation C:x/gamm Changes, features and bugs related to the gamm module. C:simulator Edits simulator or simulations T:CI C:x/pool-incentives C:x/txfees labels Jan 9, 2023
Copy link
Contributor

@stackman27 stackman27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left few comments, still catching up with changes that were made. But its looking solid :))

x/protorev/keeper/developer_fees_test.go Outdated Show resolved Hide resolved
x/protorev/keeper/developer_fees_test.go Outdated Show resolved Hide resolved
x/protorev/keeper/developer_fees.go Outdated Show resolved Hide resolved
x/protorev/keeper/protorev.go Show resolved Hide resolved
x/protorev/keeper/protorev.go Show resolved Hide resolved
const MaxIterations = 14
const MaxIterations int = 14

// Max number of routes that can be arbitraged per tx (default of 6)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to understand this, does this mean 6 routes each containing 3 pool routes? So it'd look something like [[route1][route2][route3]], [[route1a][route2a][route3a]], [[route1b][route2b][route3b]] ... 6times in genesis

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes exactly. But this is also dependent on the route that is taken. Routes that include a stable swap pool inheritely are more expensive to backrun. Because of this we count routes that include a stable swap pool with more weight. Currently the distribution is 2:5 for balancer : stable weights. The basic idea here is that MaxIterableRoutesPerTx and MaxIterableRoutesPerBlock closely follow the actual ms that we expect the module to take and route weights are the approximate amount of time it takes to actually traverse the route (in ms).

Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ValarDragon ValarDragon merged commit 6d668bf into main Jan 11, 2023
@ValarDragon ValarDragon deleted the protorev-pr5 branch January 11, 2023 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:app-wiring Changes to the app folder C:CLI V:state/breaking State machine breaking PR
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants