Skip to content

Commit

Permalink
Arbitrage tx mempool filter (#741)
Browse files Browse the repository at this point in the history
* Add Arb checktx filter, to raise gas

* Add config params for min fee for arbitrage tx

* Small improvements, add changelog

* Fix bug in swap_msg logic
  • Loading branch information
ValarDragon authored Jan 12, 2022
1 parent bc65a83 commit 13531e5
Show file tree
Hide file tree
Showing 10 changed files with 224 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Features

- [#741](https://github.com/osmosis-labs/osmosis/pull/741) Allow node operators to set a second min gas price for arbitrage txs.
- [#623](https://github.com/osmosis-labs/osmosis/pull/623) Use gosec for staticly linting for common non-determinism issues in SDK applications.

## Minor improvements & Bug Fixes
Expand Down
29 changes: 28 additions & 1 deletion app/ante.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package app

import (
"fmt"

servertypes "github.com/cosmos/cosmos-sdk/server/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

Expand All @@ -18,19 +21,24 @@ import (
// Link to default ante handler used by cosmos sdk:
// https://github.com/cosmos/cosmos-sdk/blob/v0.43.0/x/auth/ante/ante.go#L41
func NewAnteHandler(
appOpts servertypes.AppOptions,
ak ante.AccountKeeper, bankKeeper authtypes.BankKeeper,
txFeesKeeper *txfeeskeeper.Keeper, spotPriceCalculator txfeestypes.SpotPriceCalculator,
sigGasConsumer ante.SignatureVerificationGasConsumer,
signModeHandler signing.SignModeHandler,
channelKeeper channelkeeper.Keeper,
) sdk.AnteHandler {
mempoolFeeDecorator := txfeeskeeper.NewMempoolFeeDecorator(*txFeesKeeper)
// Optional if anyone else is using this repo. Primarily of impact for Osmosis.
// TODO: Abstract this better
mempoolFeeDecorator.SetArbMinGasFee(parseArbGasFromConfig(appOpts))
return sdk.ChainAnteDecorators(
ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first
ante.NewRejectExtensionOptionsDecorator(),
NewMempoolMaxGasPerTxDecorator(),
// Use Mempool Fee Decorator from our txfees module instead of default one from auth
// https://github.com/cosmos/cosmos-sdk/blob/master/x/auth/middleware/fee.go#L34
txfeeskeeper.NewMempoolFeeDecorator(*txFeesKeeper),
mempoolFeeDecorator,
ante.NewValidateBasicDecorator(),
ante.TxTimeoutHeightDecorator{},
ante.NewValidateMemoDecorator(ak),
Expand All @@ -45,6 +53,25 @@ func NewAnteHandler(
)
}

// TODO: Abstract this function better. We likely need a parse `osmosis-mempool` config section.
func parseArbGasFromConfig(appOpts servertypes.AppOptions) sdk.Dec {
arbMinFeeInterface := appOpts.Get("osmosis-mempool.arbitrage-min-gas-fee")
arbMinFee := txfeeskeeper.DefaultArbMinGasFee
if arbMinFeeInterface != nil {
arbMinFeeStr, ok := arbMinFeeInterface.(string)
if !ok {
panic("invalidly configured osmosis-mempool.arbitrage-min-gas-fee")
}
var err error
// pre-pend 0 to allow the config to start with a decimal, e.g. ".01"
arbMinFee, err = sdk.NewDecFromStr("0" + arbMinFeeStr)
if err != nil {
panic(fmt.Errorf("invalidly configured osmosis-mempool.arbitrage-min-gas-fee, err= %v", err))
}
}
return arbMinFee
}

// NewMempoolMaxGasPerTxDecorator will check if the transaction's gas
// is greater than the local validator's max_gas_wanted_per_tx.
// TODO: max_gas_per_tx is hardcoded here, should move to being defined in app.toml.
Expand Down
1 change: 1 addition & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ func NewOsmosisApp(
app.SetBeginBlocker(app.BeginBlocker)
app.SetAnteHandler(
NewAnteHandler(
appOpts,
app.AccountKeeper, app.BankKeeper,
app.TxFeesKeeper, app.GAMMKeeper,
ante.DefaultSigVerificationGasConsumer,
Expand Down
23 changes: 22 additions & 1 deletion cmd/osmosisd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,14 @@ func NewRootCmd() (*cobra.Command, params.EncodingConfig) {
// return "", nil if no custom configuration is required for the application.
func initAppConfig() (string, interface{}) {

type OsmosisMempoolConfig struct {
ArbitrageMinGasPrice string `mapstructure:"arbitrage-min-gas-fee"`
}

type CustomAppConfig struct {
serverconfig.Config

OsmosisMempoolConfig OsmosisMempoolConfig `mapstructure:"osmosis-mempool"`
}

// Optionally allow the chain developer to overwrite the SDK's default
Expand All @@ -94,7 +100,22 @@ func initAppConfig() (string, interface{}) {
srvCfg.StateSync.SnapshotKeepRecent = 2
srvCfg.MinGasPrices = "0uosmo"

return serverconfig.DefaultConfigTemplate, CustomAppConfig{Config: *srvCfg}
memCfg := OsmosisMempoolConfig{ArbitrageMinGasPrice: "0.01"}

OsmosisAppCfg := CustomAppConfig{Config: *srvCfg, OsmosisMempoolConfig: memCfg}

OsmosisAppTemplate := serverconfig.DefaultConfigTemplate + `
###############################################################################
### Osmosis Mempool Configuration ###
###############################################################################
[osmosis-mempool]
# This is the minimum gas fee any arbitrage tx should have, denominated in uosmo per gas
# Default value of ".005" then means that a tx with 1 million gas costs (.005 uosmo/gas) * 1_000_000 gas = .005 osmo
arbitrage-min-gas-fee = ".005"
`

return OsmosisAppTemplate, OsmosisAppCfg
}

func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) {
Expand Down
42 changes: 42 additions & 0 deletions x/gamm/types/msg_lp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package types

type LiquidityChangeType int

const (
AddLiquidity LiquidityChangeType = iota
RemoveLiquidity
)

// LiquidityChangeMsg defines a simple interface for determining if an LP msg
// is removing or adding liquidity.
type LiquidityChangeMsg interface {
LiquidityChangeType() LiquidityChangeType
}

var _ LiquidityChangeMsg = MsgExitPool{}
var _ LiquidityChangeMsg = MsgExitSwapShareAmountIn{}
var _ LiquidityChangeMsg = MsgExitSwapExternAmountOut{}

var _ LiquidityChangeMsg = MsgJoinPool{}
var _ LiquidityChangeMsg = MsgJoinSwapExternAmountIn{}
var _ LiquidityChangeMsg = MsgJoinSwapShareAmountOut{}

func (msg MsgExitPool) LiquidityChangeType() LiquidityChangeType {
return RemoveLiquidity
}
func (msg MsgExitSwapShareAmountIn) LiquidityChangeType() LiquidityChangeType {
return RemoveLiquidity
}
func (msg MsgExitSwapExternAmountOut) LiquidityChangeType() LiquidityChangeType {
return RemoveLiquidity
}

func (msg MsgJoinPool) LiquidityChangeType() LiquidityChangeType {
return AddLiquidity
}
func (msg MsgJoinSwapExternAmountIn) LiquidityChangeType() LiquidityChangeType {
return AddLiquidity
}
func (msg MsgJoinSwapShareAmountOut) LiquidityChangeType() LiquidityChangeType {
return AddLiquidity
}
42 changes: 42 additions & 0 deletions x/gamm/types/msg_swap.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package types

// SwapMsg defines a simple interface for getting the token denoms on a swap message route.
type SwapMsgRoute interface {
TokenInDenom() string
TokenOutDenom() string
TokenDenomsOnPath() []string
}

var _ SwapMsgRoute = MsgSwapExactAmountOut{}
var _ SwapMsgRoute = MsgSwapExactAmountIn{}

func (msg MsgSwapExactAmountOut) TokenInDenom() string {
return msg.Routes[0].GetTokenInDenom()
}
func (msg MsgSwapExactAmountOut) TokenOutDenom() string {
return msg.TokenOut.Denom
}
func (msg MsgSwapExactAmountOut) TokenDenomsOnPath() []string {
denoms := make([]string, 0, len(msg.Routes)+1)
for i := 0; i < len(msg.Routes); i++ {
denoms = append(denoms, msg.Routes[i].TokenInDenom)
}
denoms = append(denoms, msg.TokenOutDenom())
return denoms
}

func (msg MsgSwapExactAmountIn) TokenInDenom() string {
return msg.TokenIn.Denom
}
func (msg MsgSwapExactAmountIn) TokenOutDenom() string {
lastRouteIndex := len(msg.Routes) - 1
return msg.Routes[lastRouteIndex].GetTokenOutDenom()
}
func (msg MsgSwapExactAmountIn) TokenDenomsOnPath() []string {
denoms := make([]string, 0, len(msg.Routes)+1)
denoms = append(denoms, msg.TokenInDenom())
for i := 0; i < len(msg.Routes); i++ {
denoms = append(denoms, msg.Routes[i].TokenOutDenom)
}
return denoms
}
8 changes: 8 additions & 0 deletions x/txfees/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ Currently the only supported metadata & spot price calculator is using a GAMM po
* The simple alternative is only check fee equivalency at a txs entry into the mempool, which allows someone to manipulate price down to have many txs enter the chain at low cost.
* Another alternative is to use TWAP instead of Spot Price once it is available on-chain
* The former concern isn't very worrisome as long as some nodes have 0 min tx fees.
* A separate min-gas-fee can be set on every node for arbitrage txs. Methods of detecting an arb tx atm
* does start token of a swap = final token of swap (definitionally correct)
* does it have multiple swap messages, with different tx ins. If so, we assume its an arb.
* This has false positives, but is intended to avoid the obvious solution of splitting an arb into multiple messages.
* We record all denoms seen across all swaps, and see if any duplicates. (TODO)
* Contains both JoinPool and ExitPool messages in one tx.
* Has some false positives.
* These false positives seem like they primarily will get hit during batching of many distinct operations, not really in one atomic action.

## New SDK messages

Expand Down
28 changes: 24 additions & 4 deletions x/txfees/keeper/feedecorator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,37 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

"github.com/osmosis-labs/osmosis/x/txfees/keeper/txfee_filters"
"github.com/osmosis-labs/osmosis/x/txfees/types"
)

// DefaultArbMinGasFee if its not set in a config somewhere.
// currently 0 uosmo/gas to preserve functionality with old node software
// TODO: Bump after next minor version. (in 6.2+)
var DefaultArbMinGasFee sdk.Dec = sdk.ZeroDec()

// MempoolFeeDecorator will check if the transaction's fee is at least as large
// as the local validator's minimum gasFee (defined in validator config).
// If fee is too low, decorator returns error and tx is rejected from mempool.
// Note this only applies when ctx.CheckTx = true
// If fee is high enough or not CheckTx, then call next AnteHandler
// CONTRACT: Tx must implement FeeTx to use MempoolFeeDecorator
type MempoolFeeDecorator struct {
TxFeesKeeper Keeper
TxFeesKeeper Keeper
ConfigArbMinGasFee sdk.Dec
}

func NewMempoolFeeDecorator(txFeesKeeper Keeper) MempoolFeeDecorator {
return MempoolFeeDecorator{
TxFeesKeeper: txFeesKeeper,
TxFeesKeeper: txFeesKeeper,
ConfigArbMinGasFee: DefaultArbMinGasFee.Clone(),
}
}

func (mfd *MempoolFeeDecorator) SetArbMinGasFee(dec sdk.Dec) {
mfd.ConfigArbMinGasFee = dec
}

func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
// The SDK currently requires all txs to be FeeTx's in CheckTx, within its mempool fee decorator.
// See: https://github.com/cosmos/cosmos-sdk/blob/f726a2398a26bdaf71d78dbf56a82621e84fd098/x/auth/middleware/fee.go#L34-L37
Expand Down Expand Up @@ -59,8 +71,7 @@ func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b
// So we ensure that the provided fees meet a minimum threshold for the validator,
// converting every non-osmo specified asset into an osmo-equivalent amount, to determine sufficiency.
if (ctx.IsCheckTx() || ctx.IsReCheckTx()) && !simulate {
minGasPrices := ctx.MinGasPrices()
minBaseGasPrice := minGasPrices.AmountOf(baseDenom)
minBaseGasPrice := mfd.GetMinBaseGasPriceForTx(ctx, baseDenom, tx)
if !(minBaseGasPrice.IsZero()) {
if len(feeCoins) != 1 {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "no fee attached")
Expand Down Expand Up @@ -96,3 +107,12 @@ func (k Keeper) IsSufficientFee(ctx sdk.Context, minBaseGasPrice sdk.Dec, gasReq

return nil
}

func (mfd MempoolFeeDecorator) GetMinBaseGasPriceForTx(ctx sdk.Context, baseDenom string, tx sdk.Tx) sdk.Dec {
cfgMinGasPrice := ctx.MinGasPrices().AmountOf(baseDenom)

if txfee_filters.IsArbTxLoose(tx) {
return sdk.MaxDec(cfgMinGasPrice, mfd.ConfigArbMinGasFee)
}
return cfgMinGasPrice
}
5 changes: 5 additions & 0 deletions x/txfees/keeper/txfee_filters/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# TXfee filters

See https://github.com/osmosis-labs/osmosis/issues/738

Want to move towards that, right now this is a stepping stone for that. We currently define a filter for recognizing if a tx is an arb transaction, and if so raising its gas price accordingly.
51 changes: 51 additions & 0 deletions x/txfees/keeper/txfee_filters/arb_tx.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package txfee_filters

import (
sdk "github.com/cosmos/cosmos-sdk/types"
gammtypes "github.com/osmosis-labs/osmosis/x/gamm/types"
)

// We check if a tx is an arbitrage for the mempool right now by seeing:
// 1) does start token of a msg = final token of msg (definitionally correct)
// 2) does it have multiple swap messages, with different tx ins. If so, we assume its an arb.
// - This has false positives, but is intended to avoid the obvious solution of splitting
// an arb into multiple messages.
// 3) We record all denoms seen across all swaps, and see if any duplicates. (TODO)
// 4) Contains both JoinPool and ExitPool messages in one tx.
// - Has some false positives, but they seem relatively contrived.
// TODO: Move the first component to a future router module
func IsArbTxLoose(tx sdk.Tx) bool {
msgs := tx.GetMsgs()

swapInDenom := ""
lpTypesSeen := make(map[gammtypes.LiquidityChangeType]bool, 2)

for _, m := range msgs {
// (4) Check that the tx doesn't have both JoinPool & ExitPool msgs
lpMsg, isLpMsg := m.(gammtypes.LiquidityChangeMsg)
if isLpMsg {
lpTypesSeen[lpMsg.LiquidityChangeType()] = true
if len(lpTypesSeen) > 1 {
return true
}
}

swapMsg, isSwapMsg := m.(gammtypes.SwapMsgRoute)
if !isSwapMsg {
continue
}

// (1) Check that swap denom in != swap denom out
if swapMsg.TokenInDenom() == swapMsg.TokenOutDenom() {
return true
}

// (2)
if swapInDenom != "" && swapMsg.TokenInDenom() != swapInDenom {
return true
}
swapInDenom = swapMsg.TokenInDenom()
}

return false
}

0 comments on commit 13531e5

Please sign in to comment.