Skip to content

Commit

Permalink
fix: price impact for wbtc
Browse files Browse the repository at this point in the history
  • Loading branch information
p0mvn committed Mar 17, 2024
1 parent 7e2377d commit 634d159
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 13 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

# Changelog

## v0.11.0

Fix excessive price impact bug.

## v0.10.0

* [#108](https://github.com/osmosis-labs/sqs/pull/107) Add code id (omitempty) to /quote route response
Expand Down
4 changes: 2 additions & 2 deletions router/usecase/optimized_routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ func (s *RouterTestSuite) TestGetOptimalQuote_Mainnet() {

amountIn: osmomath.NewInt(1000_000_000),

expectedRoutesCount: 3,
expectedRoutesCount: 2,
},
"uosmo for uion": {
tokenInDenom: UOSMO,
Expand All @@ -603,7 +603,7 @@ func (s *RouterTestSuite) TestGetOptimalQuote_Mainnet() {

amountIn: osmomath.NewInt(100_000_000),

expectedRoutesCount: 2,
expectedRoutesCount: 1,
},
// This test validates that with a greater max routes value, SQS is able to find
// the path from umee to stOsmo
Expand Down
3 changes: 2 additions & 1 deletion router/usecase/quote.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ func (q *quoteImpl) PrepareResult(ctx context.Context, scalingFactor osmomath.De
// Update the spread factor pro-rated by the amount in
totalFeeAcrossRoutes.AddMut(routeTotalFee.MulMut(routeAmountInFraction))

newPools, routeSpotPriceInBaseOutQuote, effectiveSpotPriceInBaseOutQuote, err := curRoute.PrepareResultPools(ctx, q.AmountIn)
amountInFraction := q.AmountIn.Amount.ToLegacyDec().MulMut(routeAmountInFraction).TruncateInt()
newPools, routeSpotPriceInBaseOutQuote, effectiveSpotPriceInBaseOutQuote, err := curRoute.PrepareResultPools(ctx, sdk.NewCoin(q.AmountIn.Denom, amountInFraction))
if err != nil {
return nil, osmomath.Dec{}, err
}
Expand Down
49 changes: 45 additions & 4 deletions router/usecase/router_usecase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -512,9 +512,9 @@ func (s *RouterTestSuite) TestGetOptimalQuote_Cache_Overwrites() {
"cache is not set, computes routes": {
amountIn: defaultAmountInCache,

// For the default amount in, we expect pool 1400 to be returned.
// For the default amount in, we expect pool 1265 to be returned.
// See test description above for details.
expectedRoutePoolID: poolID1400Concentrated,
expectedRoutePoolID: poolID1265Concentrated,
},
"cache is set to balancer - overwrites computed": {
amountIn: defaultAmountInCache,
Expand All @@ -535,8 +535,8 @@ func (s *RouterTestSuite) TestGetOptimalQuote_Cache_Overwrites() {
// test execution.
cacheExpiryDuration: time.Nanosecond,

// We expect pool 1400 because the cache with balancer pool expires.
expectedRoutePoolID: poolID1400Concentrated,
// We expect pool 1265 because the cache with balancer pool expires.
expectedRoutePoolID: poolID1265Concentrated,
},
}

Expand Down Expand Up @@ -665,6 +665,47 @@ func (s *RouterTestSuite) TestGetCandidateRoutes_Chain_FindUnsupportedRoutes() {
s.Require().Zero(zeroPriceCounterNoMinLiq, "There are tokens with no routes even when min osmo liquidity is set to zero")
}

// We use this test as a way to ensure that we multiply the amount in by the route fraction.
// We caught a bug in production where for WBTC -> USDC swap the price impact was excessively large.
// The reason ended up being using a total amount for estimating the execution price.
// We keep this test to ensure that we don't regress on this.
// In the future, we should have stricter unit tests for this.
func (s *RouterTestSuite) TestPriceImpactRoute_Fractions() {
viper.SetConfigFile("../../config.json")
err := viper.ReadInConfig()
s.Require().NoError(err)

// Unmarshal the config into your Config struct
var config domain.Config
err = viper.Unmarshal(&config)
s.Require().NoError(err)

// Set up mainnet mock state.
router, mainnetState := s.SetupMainnetRouter(*config.Router)
mainnetUsecase := s.SetupRouterAndPoolsUsecase(router, mainnetState, cache.New(), cache.New())

tokenMetadata, err := mainnetUsecase.Tokens.GetFullTokenMetadata(context.Background())

chainWBTC, err := mainnetUsecase.Tokens.GetChainDenom(context.Background(), "wbtc")
s.Require().NoError(err)

wbtcMetadata, ok := tokenMetadata[chainWBTC]
s.Require().True(ok)

// Get quote.
quote, err := mainnetUsecase.Router.GetOptimalQuote(context.Background(), sdk.NewCoin(chainWBTC, osmomath.NewInt(1_00_000_000)), USDC)
s.Require().NoError(err)

// Prepare quote result.
_, _, err = quote.PrepareResult(context.Background(), osmomath.NewDec(int64(wbtcMetadata.Precision)))

priceImpact := quote.GetPriceImpact()

// 0.07 is chosen arbitrarily with extra buffer because we update test mainnet state frequently and
// would like to avoid flakiness.
s.Require().True(priceImpact.LT(osmomath.MustNewDecFromStr("0.07")))
}

// validates that for the given coinIn and tokenOutDenom, there is one route with one pool ID equal to the expectedPoolID.
// This helper is useful in specific tests that rely on this configuration.
func (s *RouterTestSuite) validatePoolIDInRoute(routerUseCase mvc.RouterUsecase, coinIn sdk.Coin, tokenOutDenom string, expectedPoolID uint64) {
Expand Down
2 changes: 1 addition & 1 deletion router/usecase/routertesting/parsing/pools.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion router/usecase/routertesting/parsing/taker_fees.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion router/usecase/routertesting/parsing/tokens.json

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions scripts/quote.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ url=$1

# This script compares single hop quotes by running them against SQS and chain directly.

chain_amount_out=$(osmosisd q poolmanager estimate-swap-exact-amount-in 1248 10000000000ibc/D79E7D83AB399BFFF93433E54FAA480C191248FC556924A2A8351AE2638B3877 --swap-route-pool-ids 1248 --swap-route-denoms uosmo --node $url:26657)
chain_amount_out=$(osmosisd q poolmanager estimate-swap-exact-amount-in 1436 100000000factory/osmo1z0qrq605sjgcqpylfl4aa6s90x738j7m58wyatt0tdzflg2ha26q67k743/wbtc --swap-route-pool-ids 1436 --swap-route-denoms ibc/498A0751C798A0D9A389AA3691123DADA57DAA4FE165D5C75894505B876BA6E4 --node $url:26657)

sqs_custom_res=$(curl "$url/router/custom-quote?tokenIn=10000000000ibc/D79E7D83AB399BFFF93433E54FAA480C191248FC556924A2A8351AE2638B3877&tokenOutDenom=uosmo&poolIDs=1248")
sqs_custom_res=$(curl "$url:9092/router/custom-direct-quote?tokenIn=100000000factory/osmo1z0qrq605sjgcqpylfl4aa6s90x738j7m58wyatt0tdzflg2ha26q67k743/wbtc&tokenOutDenom=ibc/498A0751C798A0D9A389AA3691123DADA57DAA4FE165D5C75894505B876BA6E4&poolID=1436")
sqs_custom_amount_out=$(echo $sqs_custom_res | jq .amount_out)

sqs_optimal_res=$(curl "$url/router/quote?tokenIn=10000000000ibc/D79E7D83AB399BFFF93433E54FAA480C191248FC556924A2A8351AE2638B3877&tokenOutDenom=uosmo")
sqs_optimal_res=$(curl "$url:9092/router/quote?tokenIn=100000000factory/osmo1z0qrq605sjgcqpylfl4aa6s90x738j7m58wyatt0tdzflg2ha26q67k743/wbtc&tokenOutDenom=ibc/498A0751C798A0D9A389AA3691123DADA57DAA4FE165D5C75894505B876BA6E4")
sqs_optimal_amount_out=$(echo $sqs_optimal_res | jq .amount_out)

echo "chain_amount_out: $chain_amount_out"
Expand Down

0 comments on commit 634d159

Please sign in to comment.