search

XRPLF / xrpl.js

A JavaScript/TypeScript API for interacting with the XRP Ledger in Node.js and the browser
https://xrpl.org/
1.21k stars 512 forks source link

Orders from getOrderbook are not always sorted correctly #766

Closed inmyth closed 5 years ago

inmyth commented 7 years ago

I noticed that orders from the function getOrderbook return bids and asks sorted by makerExchangeRate from the lowest to the highest. But some of the orders are sometimes inserted at the wrong sequence, especially if the rate is 0.1. For example this is the order book from following configuration

const orderbook = {
  "base": { // the first currency in pair
    "currency": 'XRP'
  },
  "counter": {
    "currency": 'JPY',
    "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
  }
};

In the result below, all orders whose rate is exactly 0.1 come first. The ones who rates are smaller (0.027) come later. 

{
  "bids": [
    {
      "specification": {
        "direction": "buy",
        "quantity": {
          "currency": "XRP",
          "value": "10900"
        },
        "totalPrice": {
          "currency": "JPY",
          "value": "109000",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        }
      },
      "properties": {
        "maker": "raiY4zsVco3smDCqwjkotnnMELJt1r7yKV",
        "sequence": 89,
        "makerExchangeRate": "0.1"
      }
    },
    {
      "specification": {
        "direction": "buy",
        "quantity": {
          "currency": "XRP",
          "value": "2000"
        },
        "totalPrice": {
          "currency": "JPY",
          "value": "20000",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        }
      },
      "properties": {
        "maker": "rnQxjewXW36KsDvdJB92hPrWREdmkuWgjB",
        "sequence": 151,
        "makerExchangeRate": "0.1"
      }
    },
    {
      "specification": {
        "direction": "buy",
        "quantity": {
          "currency": "XRP",
          "value": "1000"
        },
        "totalPrice": {
          "currency": "JPY",
          "value": "10000",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        }
      },
      "properties": {
        "maker": "rN4agFYQwcLnAk5X6yYLBapcyrw1FstnWc",
        "sequence": 52,
        "makerExchangeRate": "0.1"
      }
    },
    {
      "specification": {
        "direction": "buy",
        "quantity": {
          "currency": "XRP",
          "value": "300"
        },
        "totalPrice": {
          "currency": "JPY",
          "value": "3000",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        }
      },
      "properties": {
        "maker": "r43NZaZCcDrGpYAPEfH7STPLC7xrkszi8Q",
        "sequence": 264,
        "makerExchangeRate": "0.1"
      }
    },
    {
      "specification": {
        "direction": "buy",
        "quantity": {
          "currency": "XRP",
          "value": "100"
        },
        "totalPrice": {
          "currency": "JPY",
          "value": "1000",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        }
      },
      "properties": {
        "maker": "rJj72ernMRfLD4kEg6iWnSBEiiJ8jxvFY3",
        "sequence": 7,
        "makerExchangeRate": "0.1"
      }
    },
    {
      "specification": {
        "direction": "buy",
        "quantity": {
          "currency": "XRP",
          "value": "50000"
        },
        "totalPrice": {
          "currency": "JPY",
          "value": "500000",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        }
      },
      "properties": {
        "maker": "rJs7cu6c2iAKDKhHbS55qMvVgDYaeUTkwa",
        "sequence": 284,
        "makerExchangeRate": "0.1"
      },
      "state": {
        "fundedAmount": {
          "currency": "JPY",
          "value": "0",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        },
        "priceOfFundedAmount": {
          "currency": "XRP",
          "value": "0"
        }
      }
    },
    {
      "specification": {
        "direction": "buy",
        "quantity": {
          "currency": "XRP",
          "value": "3000"
        },
        "totalPrice": {
          "currency": "JPY",
          "value": "30000",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        }
      },
      "properties": {
        "maker": "rupaVQe981xcacfrqTaTBijWvD1yT6NQY",
        "sequence": 5,
        "makerExchangeRate": "0.1"
      }
    },
    {
      "specification": {
        "direction": "buy",
        "quantity": {
          "currency": "XRP",
          "value": "130.555075"
        },
        "totalPrice": {
          "currency": "JPY",
          "value": "4811.566173",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        }
      },
      "properties": {
        "maker": "rp6XNsJ4at9ArRHLXWnWP8ovRLwhXBUmnt",
        "sequence": 321471,
        "makerExchangeRate": "0.02713359232854512"
      }
    },
    {
      "specification": {
        "direction": "buy",
        "quantity": {
          "currency": "XRP",
          "value": "2352.151857"
        },
        "totalPrice": {
          "currency": "JPY",
          "value": "86679.14805356543",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        }
      },
      "properties": {
        "maker": "rDwttW5K5hwftAZKxrYbswvtQXWehEnE4T",
        "sequence": 7046,
        "makerExchangeRate": "0.02713630566334699"
      }
    },
    {
      "specification": {
        "direction": "buy",
        "quantity": {
          "currency": "XRP",
          "value": "130.527293"
        },
        "totalPrice": {
          "currency": "JPY",
          "value": "4767.105473",
          "counterparty": "rB3gZey7VWHYRqJHLoHDEJXJ2pEPNieKiS"
        }
      },
      "properties": {
        "maker": "rp6XNsJ4at9ArRHLXWnWP8ovRLwhXBUmnt",
        "sequence": 321469,
        "makerExchangeRate": "0.02738082757750638"
      }
    },
--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/45428178-orders-from-getorderbook-are-not-always-sorted-correctly?utm_campaign=plugin&utm_content=tracker%2F323756&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F323756&utm_medium=issues&utm_source=github).
tuloski commented 7 years ago

See this: https://www.xrpchat.com/topic/8223-rippleapi-orderbooks-order-is-by-string-quality/ The quality is order by "string"...pretty weird eh?

intelliot commented 6 years ago

Would you like to contribute a PR? 🙂

inmyth commented 6 years ago

Sure. How much are you paying ?

antondalgren commented 6 years ago

I'm having the same issue, but only with asks Sending this request:

settings:  { counter: { currency: 'XRP' },
  base:
   { currency: 'USD',
     counterparty: 'rvYAfWj5gh67oV6fW32ZzP3Aw4Eubs59B' } }

Gives the following list makerExchangeRates for asks: 

10
10
10
10
1000
1000
10000
10000
10000
10000
10000
10000
10000
1000000000
100000000
100000000
100000000
100000000
100000000
10000000
10000000
10000000
10000000
10000000
1000000
1000000
1000000
100000
100000
100000
100000
100000
10.04618320836534
10.09666649468678
10.14740351739651
10.18329938900204
10.20408163265306
102.1
10.25535842477695
10.30689288922306
10.3586862940016
10.40028751386826
10.45255027174958
10.50507563234252
10.52631578947368
10.52631578947368
10.85
10.92896174863388
109.9999
11.11111111111111
11.11111111111111
11116.25052426
11.49425287356321
11.7
117.6470588235294
117.6470588235294
12.1402878243718
123000
123456
12.5
12.5
125
12.87001287001287
131.7523056653491
13888.88888888889
13895.31354148
139
139.8601398601998
14.28571428571428
14.35
1491.999993
15
1500
1500
15.625
162.3376623376623
166.3893510815307
16.66666666666666
166666.6666666666
17.2
179.8561151079136
18181.81818181818
1833.07975730024
18.43
18.51851851851851
1864.99999822
1923.076923076923
194.5525291828793
195.6947162426614
19.6078431372549
19.799995
198
19.8019801980198
19.84126984126984
19.99960000799984
1999.996000008
19999.96000007835
20
2000
2000
2000
20000000
200000
20.40816326530612
210.0840336134454
21.73913043472494
2.214212596831522
2.21423473943882
2.218954615177066
2.222487927088651
2.22286438304399
2.241096374234072
2.247191011235955
2.25231071955238
2.272727272727272
2.319782158003158
2331.25001952
2.352941176470588
2.354991404281374
2.355702466531407
2.380952380952381
238.0952380952381
24306
24.9
24.93765586034912
24.9999
250
2.564102564102564
2.770083102493074
2.777777777777777
2.777777777777778
27.77777777777777
27855.15320334262
2.8
285
28.57142857142857
2914.06249042
2.952465308532624
2.994011976047904
3
300
30.3030303030303
30.6
3.225806451612903
33.22259136212625
34.48275862068966
35.08771929824561
35.7040845472722
35.71428571428571
3642.57813958
3.703703703703704
370.3703703703703
3.738317757009345
38000
3.846153846153846
38.8
3.981034352345426
3.981085810124061
3.988831272437176
40
40000
40000
4.166666666666666
4.166666666666667
4.301075268817204
4.347826086956522
4.444444444444444
4.4583
4.545454545454545
4553.2227574
45872.45873321869
469.4835680751174
4.739246649352618
4.74
4.761904761904761
4.761904761904762
476190.4761904547
49.95004995
4.999
4.99990000199996
4999.975000124
5
5000
5000
50000000
5000000
51.999
5.263157894736842
5.263157894736842
5.555555555555556
5.603732954731092
5.617977528089888
5691.52838196
5.807200929152149
5.847953216374269
5.882214536128561
5.882352941176471
5.882352941176471
5.882352941176471
6
60000000000
6.014026689094571
6.024096385542169
6.233
6.25
6.329113924050633
6.430868167202572
6.618133686300463
6.666666666666667
6.666666666666667
6.666666666666667
6.666666666666667
67
6.790246780392246
6.896551724137931
7.092198581560284
7114.41067992
7.142857142857143
7.168458781362007
7.692307692307692
7.692307692307692
8.035714314413265
8.116883116883117
8.198871835235478
8.281688722460072
8.333333333333333
8.333333333333333
8.365342129903273
8.449840580237709
8.555463538693376
8.662409687124244
8.771172195328005
8.84
8.88130031117856
8893.0002217
8.998284027236006
9.000009000009
9.009009009009009
9.09090909090909
9.090909090909091
90.90909090909091
909.0909090909091
9090.909090909091
9090909.090909038
90909090.9090909
9.119452978732524
9.433962264150943
9.433962264150943
9.799995
98234.16935682522
9.830097093343506
9.879494568614488
99000
9.900990099009901
9.945972482875771
99.8
9.995952239100301
9999.00009999

It seems like its an ordering issue on the base API of Ripple and not just this library. It should be done on the main API to ensure the real-time data that we receive using this library.

tuloski commented 6 years ago

@antondalgren It's a problem of ripple-lib (RippleAPI), because the server returns the book in the correct quality order.

antondalgren commented 6 years ago

@tuloski Then it should be an easy fix

inmyth commented 6 years ago

I have read the code review and the issue which stems from some unnecessary sorting. Personally ripple-lib looks like Homer Simpson's super car which immediately sent his brother into bankruptcy. This is the second time I see something like it (the first one being maxFee param that caused massive XRP bleed and had to be resolved by placing 2XRP hard cap). So I think ripple-lib should not be a front layer on the ledger's API. People should use their own websocket client to connect and communicate while ripple-lib should be turned into a set of helper tools (signing, interpreting low level json response, etc).

intelliot commented 6 years ago

@inmyth Good point. We should recommend using request with raw rippled API requests, and provide helpers for generating the request and parsing the response.