kelp icon indicating copy to clipboard operation
kelp copied to clipboard

[13] using a fixed price feed will result in large rounding errors for BTC

Open ddombrowsky opened this issue 5 years ago • 21 comments

When doing a simple bot to trade around the buy/sell spread of a BTC market, the "buy" side is pretty much useless, because it only uses 7 decimal points to represent the price. e.g.

1/0.0000069 = 144927 XLM per BTC 1/0.0000070 = 142857 XLM per BTC

That's almost 1.5% spread.

I can't find any way in the configuration to work around this limitation.

Expected behavior

The bot should place buy orders in a BTC market with a normal spread, not one every 2070 XLM.

Frequency

This always happens. Either it's a limitation of the configuration, or I'm just not seeing the right way to configure this.

A sample config using the buysell strategy is:

DATA_TYPE_B="fixed"
DATA_FEED_B_URL="1.0"
DATA_TYPE_A = "sdex"
DATA_FEED_A_URL="BTC:GBVOL67TMUQBGL4TZYNMY3ZQ5WGQYFPFD5VJRWXR72VA33VFNL225PL5/XLM:"

Your Environment

  cli version: master:v1.8.1
  gui version: v0.0.1-alpha
  git branch: master
  git hash: 90e31022e46f5585a81f6890e6a1d4747455673f
  build date: 20200217T212830Z
  env: release
  GOOS: linux
  GOARCH: amd64

Context

I am unable to run a simple bot on a BTC market because of this bug.

ddombrowsky avatar Apr 18 '20 01:04 ddombrowsky

maybe if I could change PricePrecision in :

2020/04/17 22:32:06 orderConstraints for trading pair BTC/XLM: OrderConstraints[PricePrecision: 7, VolumePrecision: 7, MinBaseVolume: 0.0000001, MinQuoteVolume: <nil>]

ddombrowsky avatar Apr 18 '20 02:04 ddombrowsky

This appears to be hardcoded:

plugins/sdex.go

 30 var sdexOrderConstraints = model.MakeOrderConstraints(7, 7, 0.0000001)

Welp, that was fun while it lasted.

ddombrowsky avatar Apr 18 '20 02:04 ddombrowsky

@ddombrowsky the Stellar DEX only allows 7 decimals of precision for all assets. This cannot be changed.

You are already pricing your market as BTC/XLM, which is ~= 143000 so you should not be seeing the issues mentioned since the number is large. If you set your market to XLM/BTC you would see a price ~= 0.0000069 and you would see the precision issues mentioned.

Can you share your log file so I can get a better understanding of the issues you are facing.

nikhilsaraf avatar Apr 18 '20 07:04 nikhilsaraf

@nikhilsaraf

@ddombrowsky the Stellar DEX only allows 7 decimals of precision for all assets. This cannot be changed.

While this is true, prices are specified in numerator / denominator values, so it is very possible to place a "buy" order for BTC between the values of 144927 XLM per BTC and 142857 XLM per BTC.

You are already pricing your market as BTC/XLM, which is ~= 143000 so you should not be seeing the issues mentioned since the number is large. If you set your market to XLM/BTC you would see a price ~= 0.0000069 and you would see the precision issues mentioned.

If you flip the pricing, you'll then see the same issue on the sell side. I've tried both.

Can you share your log file so I can get a better understanding of the issues you are facing.

Here's an example using the "balanced" method:

2020/04/18 10:28:25 buy  | modify | old level=1 | new level = 1 | triggers=[amount] | targetPriceQuote=142857.14285714 | targetAmtBase=0.00000020 | curPriceQuote=142857.14285714 | lowPriceQuote=129870.12987013 | highPriceQuote=158730.15873016 | curAmtBase=0.00000010 | minAmtBase=0.00000018 | maxAmtBase=0.00000022
2020/04/18 10:28:25 buy  | create | new level=2 | priceQuote=142857.14285714 | amtBase=0.00000020
2020/04/18 10:28:25 buy  | create | new level=3 | priceQuote=142857.14285714 | amtBase=0.00000020
2020/04/18 10:28:25 sell, done creating preceding offers (numLevelsConsumed=0, hitCapacityLimit=false, numOps=0, newTopOfferPrice=<nil>)
2020/04/18 10:28:25 sell | modify | unmodified original level = 1 | newLevel number = 1
2020/04/18 10:28:25 sell | modify | unmodified original level = 2 | newLevel number = 2
2020/04/18 10:28:25 sell | modify | unmodified original level = 3 | newLevel number = 3

You'll notice that the priceQuote for all 3 buy orders are the same.

I don't know enough about go to point to a quick fix. I'm hoping this isn't a system limitation.

ddombrowsky avatar Apr 18 '20 14:04 ddombrowsky

Unfortunately, it's not as simple as editing that one line in plugins/sdex.go. It blows up:

2020/04/18 10:48:03 error in selling sub-strategy: unable to create preceding offers: unable to create new preceding offer: error: cannot create or modify offer, invalid price: -92233.72036855

ddombrowsky avatar Apr 18 '20 14:04 ddombrowsky

If I look at one of the raw offers:

{
  "id": "125589649803780097",
  "paging_token": "125589649803780097",
  "transaction_successful": true,
  "source_account": "GAWYHDRCFT2YOFFDYAAPSLJB6ULIZBSNNPJKW6KIEU3EQ7KJZPUBVT24",
  "type": "manage_sell_offer",
  "type_i": 3,
  "created_at": "2020-04-18T16:58:02Z",
  "transaction_hash": "4d143c226e44d06c303644551ef5c70f0fedbba4dbc724cd56a50f65dcad3519",
  "amount": "0.0287770",
  "price": "0.0000070",
  "price_r": {
    "n": 7,
    "d": 1000000
  },
  "buying_asset_type": "credit_alphanum4",
  "buying_asset_code": "BTC",
  "buying_asset_issuer": "GBVOL67TMUQBGL4TZYNMY3ZQ5WGQYFPFD5VJRWXR72VA33VFNL225PL5",
  "selling_asset_type": "native",
  "offer_id": "0"
}

That 7/1000000 isn't going to work.

ddombrowsky avatar Apr 18 '20 17:04 ddombrowsky

@ddombrowsky if the bot computes prices for the two buy levels as 0.00000701234 and 0.00000701345 then both of them will get rounded down to 7 decimal places (because of the SDEX limitation), making them 0.0000070. This would result in the same price for both offers when inverted = 1/0.0000070 = 142857.1428571. In order to fix this, you will need to increase the spread in your config to at least 1.42857% on either side [1].

Since you are seeing a price of 0.0000070 it could be one of two things:

  1. you are getting 7 decimal places of precision but you are being rounded as described above. The only solution to this is increasing your spread as described above.
  2. your price is actually something like 0.000007234 but you are getting only 6 decimal places of precision, which would mean it's a bug in kelp since this should show up as 0.0000072. If so I will put out a fix ASAP.

In order to figure out which situation we are in here it would help to see your full log file. Can you upload it here?


[1] Note that every trading exchange has a minimum spread that is possible depending on the precision of price that is allowed. Since SDEX has 7 decimals of precision, the smallest amount you can represent is 0.0000001. At a price of 0.0000070, this means the minimum allowed spread is 0.0000001/0.0000070 = 1.42857%. As the price of the underlying asset increases, the minimal allowed spread decreases. This may not be the issue you are seeing but it's an important factor to consider when configuring your bot and I thought I should mention it here. This means that your levels need to be at least 1.42857% apart from each other on either side, so they produce different prices after rounding.

nikhilsaraf avatar Apr 20 '20 12:04 nikhilsaraf

@nikhilsaraf

  1. you are getting 7 decimal places of precision but you are being rounded as described above. The only solution to this is increasing your spread as described above.

This is what I'm seeing. It's using 7 decimals of precision, which means BTC assets priced in XLM cannot be priced accurately using the kelp system.

This means that your levels need to be at least 1.42857% apart from each other on either side, so they produce different prices after rounding.

Again, this is not true. It is very possible to place orders in a BTC/XLM market between 1/0.0000070 and 1/0.0000069. This is not a limitation of the protocol.

I'll attached the full output of the kelp binary, using the balanced strategy, run overnight.

If you can confirm that this is a system limitation, I might dig into the code to see if there is a fix that doesn't overhaul the guts of the trading bot. Otherwise, it looks like this bot isn't useful for any assets trading higher than 100,000 XLM per unit.

(Consequentially, it means anything trading above 1,000,000 XLM will barely work at all, such as BTC/NGNT)

ddombrowsky avatar Apr 20 '20 12:04 ddombrowsky

@ddombrowsky The code to add support for more than 7 decimals in Kelp is relatively straightforward (see experimental commit of modifications needed).

However, I believe there is a restriction in the go sdk which gives an error "more than 7 significant digits". This error is currently preventing us from using more than 7 decimals in kelp. stellar-core will honor more than 7 decimals internally as it uses fractions to represent price. We would need to have this patched in the go sdk first after which it would work in kelp with the experimental commit. See a test that proves the limitations of the amount.ParseInt64 function.

I will try to update the precision issue for prices via the go sdk but that may take some time as I'm tied up with releasing the GUI for Kelp.

For now you could try running the branch in the first link as a starting point (there are some hacks in it right now as you can see with the arbitrary decimal values of 11 and 13 and excessive logging). You could create a fork of the stellar/go repo (see the glide.yaml file for the fork currently being used) to correct the issue in the sdk at amount.go#ParseInt64. If you can get the sdk to allow more than 7 decimals then it should work with the experimental commit proposed (first link).

Note that Kelp is currently using an older version of the go sdk so it's possible that the latest go sdk does not have this issue and then it's just a matter of upgrading to the new sdk.

nikhilsaraf avatar Apr 20 '20 14:04 nikhilsaraf

@nikhilsaraf

This error is currently preventing us from using more than 7 decimals in kelp.

Can you instead specify the price using the fractional notation? IIRC the SDK allows you to specify one or the other, when posting an order.

ddombrowsky avatar Apr 20 '20 15:04 ddombrowsky

The old Go SDK allows you to specify the price in fractions, which is what we are doing internally via the new Go SDK. However from what I recall, the check for 7 decimals of precision still applies in the old Go SDK. This check is not correct imo and should be removed.

I haven't looked into an alternative code path for this check yet, which would mitigate the issue. The old Go SDK is deprecated and Kelp has partially migrated to the new SDK where this precision issue would not be a problem anyway. However, the migration is blocked on other work slated for Q2/Q3 and until we complete the migration we will still need to go through the old SDK to sign and submit transactions.

I will be leaving this issue open to be fixed soon once I've gotten through the release of rc1 of the GUI.

nikhilsaraf avatar Apr 20 '20 16:04 nikhilsaraf

as a work-around, I was able to hack in a switch from "sell" to "buy" when the price is too low to be significant.

https://github.com/ddombrowsky/kelp/commits/405-flip-buy-sell

@nikhilsaraf If this problem is still present in the "new" go SDK, then I can re-write this to be a non-hack fix.

ddombrowsky avatar Apr 21 '20 02:04 ddombrowsky

Created two issues in Go SDK that should fix the behaviour described here: https://github.com/stellar/go/issues/2514 https://github.com/stellar/go/issues/2515.

bartekn avatar Apr 27 '20 17:04 bartekn

Leaving this issue open until the related issues in stellar/go are resolved so I remember to allow more than 7 decimals. Once fixed we should be able to remove the associated compatibility code from Kelp.

nikhilsaraf avatar Apr 27 '20 18:04 nikhilsaraf

@ddombrowsky you can check out the new pendulum strategy, which is an improvement on the balanced strategy

nikhilsaraf avatar Jun 01 '20 19:06 nikhilsaraf

@nikhilsaraf does this mean I can remove my code that flipped the buy/sell values if the precision wasn't enough? (see my commits on https://github.com/ddombrowsky/kelp/commits/415-balanced-amounts )

ddombrowsky avatar Jun 19 '20 15:06 ddombrowsky

BTW I tried out stock 1.9.0, and I still get the same problem. All the "buy" orders are for the same amount:

2020/07/08 16:12:11 buy  | create | new level=1 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=2 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=3 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=4 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=5 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=6 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=7 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=8 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=9 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=10 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=11 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=12 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=13 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=14 | priceQuote=117647.05882353 | amtBase=0.00000040
2020/07/08 16:12:11 buy  | create | new level=15 | priceQuote=117647.05882353 | amtBase=0.00000040

ddombrowsky avatar Jul 08 '20 20:07 ddombrowsky

note to self: consider using https://github.com/shopspring/decimal library

nikhilsaraf avatar Mar 09 '21 18:03 nikhilsaraf

looks like both issues are still open in the stellar go SDK: stellar/go#2514 stellar/go#2515 . As such, code on the master branch will still not work with markets that require more than a few decimal points of precision (such as BTC/USD or BTC/anything really).

I've updated my fork to merge in changes from current master. As before, this does the following:

  • Uses the PassiveOffer flag temporarily when a sale price is less than 0.001
  • When the price is too small, it flips the order from a ManageSellOffer to a ManageBuyOffer, which flips the fraction and effectively works around the precision problem.
  • It disables some tracking code because that is all written specifically for ManagedSellOffer objects. This could be fixed, but would require a bit more refactoring.

Code is here: https://github.com/ddombrowsky/kelp/tree/405-flip-buy-sell

If anyone besides me find this useful or interesting, let me know and I will clean it up so I can submit it as a PR. (@nikhilsaraf @bartekn )

ddombrowsky avatar Jun 23 '21 21:06 ddombrowsky

FYI: The rounding bug is STILL present in the latest version of kelp and the SDK.

I have updated my fork to the latest v1.12.0-dd, cli version: master-dd:v1.12.0-14-g3f14c71c

https://github.com/ddombrowsky/kelp/tree/master-dd

ddombrowsky avatar Nov 26 '21 17:11 ddombrowsky