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

Arbitrage tx mempool filter #741

Merged
merged 4 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}