Trading issue #612

offerm · 2018-10-28T17:11:04Z

Guys,

Assume that I'm an exchange that just installed XUD.

See what I did.
Can you please help me to explain what happened?

At startup I'm connected to LNDs and to 3 peers and have 11 peer orders. all good.

admins-MacBook-Pro-2:~ admin$ xucli getinfo
{
  "version": "1.0.0-alpha.1",
  "nodePubKey": "034606c6a0616397cb5dee9f84cad5186d38bc65712b672ff4fcf1eb46e6e768ea",
  "urisList": [],
  "numPeers": 3,
  "numPairs": 1,
  "orders": {
    "peer": 11,
    "own": 0
  },
  "lndbtc": {
    "error": "",
    "channels": {
      "active": 1,
      "inactive": 0,
      "pending": 0
    },
    "chainsList": [
      "bitcoin"
    ],
    "blockheight": 20386,
    "urisList": [],
    "version": "0.5.0-beta commit=v0.4.2-beta-1247-ge22ae9985b696924d5934004c269c930d3ccba43",
    "alias": "BTC@admins-MacBook-Pro-2.local"
  },
  "lndltc": {
    "error": "",
    "channels": {
      "active": 1,
      "inactive": 0,
      "pending": 0
    },
    "chainsList": [
      "litecoin"
    ],
    "blockheight": 25540,
    "urisList": [],
    "version": "0.5.0-beta commit=v0.4.2-beta-1247-ge22ae9985b696924d5934004c269c930d3ccba43",
    "alias": "LTC@admins-MacBook-Pro-2.local"
  }
}

Looking at the orders I see BUY orders and SELL orders.
The sell orders are not sorted by price

admins-MacBook-Pro-2:~ admin$ xucli getorders
{
  "ordersMap": [
    [
      "LTC/BTC",
      {
        "buyOrdersList": [
          {
            "price": 0.9,
            "quantity": 0.001,
            "pairId": "LTC/BTC",
            "id": "926e2741-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065069,
            "side": 0,
            "isOwnOrder": false
          },
          {
            "price": 0.89,
            "quantity": 0.0015,
            "pairId": "LTC/BTC",
            "id": "926e7561-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065070,
            "side": 0,
            "isOwnOrder": false
          },
          {
            "price": 0.88,
            "quantity": 0.002,
            "pairId": "LTC/BTC",
            "id": "926e9c72-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065070,
            "side": 0,
            "isOwnOrder": false
          },
          {
            "price": 0.87,
            "quantity": 0.0025,
            "pairId": "LTC/BTC",
            "id": "926ec382-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065070,
            "side": 0,
            "isOwnOrder": false
          },
          {
            "price": 0.86,
            "quantity": 0.003,
            "pairId": "LTC/BTC",
            "id": "926f11a1-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065071,
            "side": 0,
            "isOwnOrder": false
          }
        ],
        "sellOrdersList": [
          {
            "price": 1.12,
            "quantity": 0.002,
            "pairId": "LTC/BTC",
            "id": "9269ba71-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065071,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 1.11,
            "quantity": 0.0015,
            "pairId": "LTC/BTC",
            "id": "926ceec1-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065071,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 1.1,
            "quantity": 0.001,
            "pairId": "LTC/BTC",
            "id": "926db211-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065071,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 1.14,
            "quantity": 0.003,
            "pairId": "LTC/BTC",
            "id": "9262dca1-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065072,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 1.13,
            "quantity": 0.0025,
            "pairId": "LTC/BTC",
            "id": "92677081-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065072,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 0.96,
            "quantity": 0.004,
            "pairId": "LTC/BTC",
            "id": "dba6efa1-d9ec-11e8-8fcc-87b38397d593",
            "peerPubKey": "03fd337659e99e628d0487e4f87acf93e353db06f754dccc402f2de1b857a319d0",
            "localId": "",
            "createdAt": 1540746070972,
            "side": 1,
            "isOwnOrder": false
          }
        ]
      }
    ]
  ]
}

trying to sell 0.003 at market. Expecting to take the first 2 BUY orders and part of the 3rd order
buy got nothing back.

admins-MacBook-Pro-2:~ admin$ xucli sell 0.003 LTC/BTC market
{
  "internalMatchesList": [],
  "swapResultsList": []
}

looking at the orders now I see that ALL the BUY orders are missing.

admins-MacBook-Pro-2:~ admin$ xucli getorders
{
  "ordersMap": [
    [
      "LTC/BTC",
      {
        "buyOrdersList": [],
        "sellOrdersList": [
          {
            "price": 1.12,
            "quantity": 0.002,
            "pairId": "LTC/BTC",
            "id": "9269ba71-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065071,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 1.11,
            "quantity": 0.0015,
            "pairId": "LTC/BTC",
            "id": "926ceec1-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065071,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 1.1,
            "quantity": 0.001,
            "pairId": "LTC/BTC",
            "id": "926db211-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065071,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 1.14,
            "quantity": 0.003,
            "pairId": "LTC/BTC",
            "id": "9262dca1-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065072,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 1.13,
            "quantity": 0.0025,
            "pairId": "LTC/BTC",
            "id": "92677081-dad2-11e8-80b4-af8455b00957",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540746065072,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 0.96,
            "quantity": 0.004,
            "pairId": "LTC/BTC",
            "id": "dba6efa1-d9ec-11e8-8fcc-87b38397d593",
            "peerPubKey": "03fd337659e99e628d0487e4f87acf93e353db06f754dccc402f2de1b857a319d0",
            "localId": "",
            "createdAt": 1540746070972,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 0,
            "quantity": 0.003,
            "pairId": "LTC/BTC",
            "id": "664c9651-dad3-11e8-9e01-71efc822fdb7",
            "peerPubKey": "",
            "localId": "664c9650-dad3-11e8-9e01-71efc822fdb7",
            "createdAt": 1540746212405,
            "side": 1,
            "isOwnOrder": true
          }
        ]
      }
    ]
  ]
}
admins-MacBook-Pro-2:~ admin$

The text was updated successfully, but these errors were encountered:

sangaman · 2018-10-28T17:27:53Z

I'll try to take a look at all of this tonight (or tomorrow at the latest), but GetOrders being unsorted is expected as that's how they're stored within xud. But there's an open PR #600 to improve how the results are displayed including sorting them.

kilrau · 2018-10-29T12:39:03Z

Stumpled upon this independently and was just about to open an issue too, let's leave sorted/non sorted out here for now. It's about order execution not being correct:

kilrau@ubuntu:~$ xucli getorders LTC/BTC
{
  "ordersMap": [
    [
      "LTC/BTC",
      {
        "buyOrdersList": [],
        "sellOrdersList": [
          {
            "price": 1.14,
            "quantity": 0.003,
            "pairId": "LTC/BTC",
            "id": "0c25e001-dad8-11e8-ba91-19129e5f0bc9",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540812880977,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 1.13,
            "quantity": 0.001,
            "pairId": "LTC/BTC",
            "id": "0c2a73e1-dad8-11e8-ba91-19129e5f0bc9",
            "peerPubKey": "02b66438730d1fcdf4a4ae5d3d73e847a272f160fee2938e132b52cab0a0d9cfc6",
            "localId": "",
            "createdAt": 1540812880977,
            "side": 1,
            "isOwnOrder": false
          },
          {
            "price": 1.13,
            "quantity": 0.0005,
            "pairId": "LTC/BTC",
            "id": "bfbad831-db75-11e8-afc4-1728eb314f38",
            "peerPubKey": "",
            "localId": "bfbad830-db75-11e8-afc4-1728eb314f38",
            "createdAt": 1540815940915,
            "side": 1,
            "isOwnOrder": true
          }
        ]
      }
    ]
  ]
}
kilrau@ubuntu:~$ xucli buy 0.001 LTC/BTC 1.14
Error: 2 UNKNOWN: discardRemaining must be true on recursive calls where maxTime could exceed
kilrau@ubuntu:~$ xucli getorders LTC/BTC
{
  "ordersMap": [
    [
      "LTC/BTC",
      {
        "buyOrdersList": [],
        "sellOrdersList": []
      }
    ]
  ]
}

moshababo · 2018-10-29T14:25:23Z

@offerm the current behavior is that if xud finds non-internal match, and a swap is not supported (Swaps.verifyExecution), which includes the a case where an LND client is not synced, then the match is being discarded, and the already-removed maker peer order is not being returned to the repository. That's the behavior we settled on in #541.
It means that if one of your LND's is not connected/synced, all your matched orders will be eaten out. You can verify this by listening to subscribeRemovedOrders.

We can reconsider this behavior, improve the logging, and add swap failures output (#609).

moshababo · 2018-10-29T14:34:48Z

@kilrau your sell orders disappearance is similar to @offerm's, but the order placing response shows a bug. I'll fix it now.

sangaman · 2018-10-29T20:11:03Z

We can reconsider this behavior, improve the logging, and add swap failures output

I think we should keep the behavior, but more logging and returning swap failures on the grpc level might be helpful. I'm pretty sold that returning the swap failures is a good idea - we already have an issue for it - for logging we should already log the swap failures, but maybe we can also log something everytime we do swapFailures.push if that doesn't seem excessive, personally I'm on the fence.

offerm · 2018-10-29T20:36:35Z

There are several issues here:
lets start:
@moshe, why the market sell order got into the book with price 0?

sangaman · 2018-10-29T20:42:41Z

why the market sell order got into the book with price 0?

That's due to a bug in the placeOrder logic that reiterates with discardRemaining set to false after a swap fails. That means it wasn't discarding the remainder of the order is it should. It's fixed in #614.

moshababo · 2018-10-29T21:31:21Z

@offerm the same bug created many problems. The fix was merged, let's test again.

offerm · 2018-10-30T08:09:10Z

Using the latest master:

started with 10 orders (5 buy, 5 sell)
executed:

xucli buy 0.0005 LTC/BTC market --stream

Side note:
The error message for CLI is back

(node:89714) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.

Impact:

nothing to the user - just the promopt
the first 3 sell orders disappeared.
log shows 9 swap attempts. price 1.1 - 2 times, price 1.11 - 3 times, price 1.12 - 4 times, price 1.13 - 1 times.
log ends with

10/30/2018, 9:51:22 AM [ORDERBOOK] info: addOwnOrder max time exceeded. order ({"pairId":"LTC/BTC","price":1.7976931348623157e+308,"quantity":0.0005,"isBuy":true,"localId":"91e722d0-dc18-11e8-b90d-aba0d836b671","id":"91e722d1-dc18-11e8-b90d-aba0d836b671","createdAt":1540885871997}) won't be matched

so I guess this is why price 1.13 was done only 1 time and why these 2 orders are still there.

IMHO we have to reduce the complexity or things and not implement mechanisms for which we are not sure about the need, The retry is a good example. why are we doing retry when we get a permanent error? at that time we could swap with another peer.

Also, keep in mind the subject of this issue - it is the user and how he can understand what is going on.
my expectation is to see something lite this:

xucli buy 0.0005 LTC/BTC market --stream

market order accepted
matched with peer order : .......
attempting swap: .....
swap failed: .....
peer order removed from book: .....
attempting swap with: .....
swap failed: .....
peer order removed from book: .....
attempting swap with: .....
swap failed: .....
no more matches found
quitting.

the .... stands for information that should be presented to the user in a user readable format.

moshababo · 2018-10-30T13:20:26Z

(node:89714) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.

I opened #618 .

--

We have 3 kind of results when placing an order:

internal matches
swap results
remaining order

The output that you suggest also includes:

swap failures (Add failed swaps to placeOrder (buy/sell) response #609)
match event (currently not published/logged)
order removal event (currently not logged, published on a different endpoint)
swap beginning event (currently not published/logged)

Out stream method output is identical to the non-stream, but not aggregated. I'm not sure if we want to publish all these events - we can improve the logging instead. If others will endorse your format (@kilrau @sangaman @michael1011 ), we also need to decide on whether to have it in the non-stream version as well, which will make it more like the full scenario story, and not just the information that the client needs for processing.

--

No output on your market order:
That's the current situation if you have no internal matches & no successful swaps. market order leaves no remaining order.

--

Retry mechanism:
Please open a separate issue with a suggestion for a different logic. Mind that we can have different types of errors.

--

I wasn't sure if there were more problems in what you described. Let me know if so.

sangaman · 2018-10-30T14:56:58Z

xucli buy 0.0005 LTC/BTC market --stream

market order accepted
matched with peer order : .......
attempting swap: .....
swap failed: .....
peer order removed from book: .....
attempting swap with: .....
swap failed: .....
peer order removed from book: .....
attempting swap with: .....
swap failed: .....
no more matches found
quitting.

I like this general format for the output of the cli, I'll create an issue for that specifically.

sangaman · 2018-10-30T15:03:06Z

I'm not sure what the issue is with the retry logic? Retrying what exactly? Do you mean how we handle trying a different order when one order fails?

kilrau · 2018-10-30T15:28:37Z

xucli buy 0.0005 LTC/BTC market --stream

market order accepted
matched with peer order : .......
attempting swap: .....
swap failed: .....
peer order removed from book: .....
attempting swap with: .....
swap failed: .....
peer order removed from book: .....
attempting swap with: .....
swap failed: .....
no more matches found
quitting.

I like this general format for the output of the cli, I'll create an issue for that specifically.

Good, I like the format too - supportive! We might even save some events and just state starting swap directly after match, no need for a particular event here.

kilrau · 2018-10-30T15:42:33Z

EDIT: couldn't find your issue, so just added it @sangaman : #620

moshababo · 2018-11-03T11:02:55Z

Can we close this? since we have #609 / #620

offerm · 2018-11-03T13:26:39Z

@moshababo do we have a case for the retries?

moshababo · 2018-11-05T15:41:54Z

@offerm you mean the swaps retries? if so, what's the question about it?

kilrau · 2018-11-05T16:57:28Z

I don't want to make this issue more messy, here as a separate bug list which this issue should close: #638

offerm · 2018-11-05T20:22:04Z

Opened #639 for the retries.

Closing this one

moshababo mentioned this issue Oct 29, 2018

fix: addOwnOrder iteration call #614

Merged

kilrau added this to the 1.0.0-alpha.2 milestone Oct 29, 2018

kilrau assigned moshababo Oct 29, 2018

kilrau added P1 top priority bug Something isn't working labels Oct 29, 2018

kilrau mentioned this issue Oct 30, 2018

Improved buy/sell output for xucli #620

Closed

kilrau modified the milestones: 1.0.0-alpha.2, 1.0.0-alpha.3 Nov 1, 2018

kilrau assigned offerm Nov 5, 2018

kilrau mentioned this issue Nov 5, 2018

Trading Test Cases (WIP) #638

Closed

offerm closed this as completed Nov 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trading issue #612

Trading issue #612

offerm commented Oct 28, 2018

sangaman commented Oct 28, 2018

kilrau commented Oct 29, 2018 •

edited

Loading

moshababo commented Oct 29, 2018

moshababo commented Oct 29, 2018

sangaman commented Oct 29, 2018

offerm commented Oct 29, 2018

sangaman commented Oct 29, 2018

moshababo commented Oct 29, 2018

offerm commented Oct 30, 2018

moshababo commented Oct 30, 2018 •

edited

Loading

sangaman commented Oct 30, 2018

sangaman commented Oct 30, 2018

kilrau commented Oct 30, 2018

kilrau commented Oct 30, 2018

moshababo commented Nov 3, 2018

offerm commented Nov 3, 2018

moshababo commented Nov 5, 2018

kilrau commented Nov 5, 2018 •

edited

Loading

offerm commented Nov 5, 2018

Trading issue #612

Trading issue #612

Comments

offerm commented Oct 28, 2018

sangaman commented Oct 28, 2018

kilrau commented Oct 29, 2018 • edited Loading

moshababo commented Oct 29, 2018

moshababo commented Oct 29, 2018

sangaman commented Oct 29, 2018

offerm commented Oct 29, 2018

sangaman commented Oct 29, 2018

moshababo commented Oct 29, 2018

offerm commented Oct 30, 2018

moshababo commented Oct 30, 2018 • edited Loading

sangaman commented Oct 30, 2018

sangaman commented Oct 30, 2018

kilrau commented Oct 30, 2018

kilrau commented Oct 30, 2018

moshababo commented Nov 3, 2018

offerm commented Nov 3, 2018

moshababo commented Nov 5, 2018

kilrau commented Nov 5, 2018 • edited Loading

offerm commented Nov 5, 2018

kilrau commented Oct 29, 2018 •

edited

Loading

moshababo commented Oct 30, 2018 •

edited

Loading

kilrau commented Nov 5, 2018 •

edited

Loading