feat: removeorder output changed to a more meaningful message (#1526)

[rsercano]

resolves #1526

[raladev]

resolves #1526

it does not resolve #1526, main thing that should be done in the issue is grpc response changing (to get more useful info about removeorder action) and only after that it worth to change json output to text.

[rsercano]

But I asked about that in discord to @kilrau and he mentioned how the output should be and what issue is all about, I guess there's a misunderstanding. @raladev

https://discord.com/channels/547402601885466658/709337789304668160/769118027119198278

[rsercano]

It's not a big deal to add removedAmount into GRPC response tho. Just confirm you want it this way and I'll add. @raladev @kilrau

[raladev]

to add removedAmount into GRPC response tho. Just confirm you want it this way and I'll add. @raladev @kilrau

discussed on the meeting today, lets try to implement following structure:

0. order is in orderbook with 20 qty

1. swap for 5 qty is started

2. removeorder my_id 13

   {
   removedQty:13;# qty that was successfully removed
   remainingQty:7;# remaining qty after partial removing (should not contain quantityOnHold because it will be removed after unholding)
   quantityOnHold:0; # qty in swap, but it will be removed if swap fails
   }

3. removeorder my_id (swap still in progress)

   {
   removedQty:2;# qty that was successfully removed
   remainingQty:0;# remaining qty after partial removing (should not contain quantityOnHold because it will be removed after unholding)
   quantityOnHold:5; # qty in swap, but it will be removed if swap fails
   }

4. swap is finshed

5. removeorder my_id -> Err no order with such id

[rsercano]

Understood, I'm not sure about remainingQty and quantityOnHold logic, because it maybe requires removeOrder call to be refactored completely, but I'll just double check.

[rsercano]

Refactored, could you please recheck @raladev

[raladev]

• [x] it would be good to change satoshi in text messages to user-readeble values (1 ETH, 2 BTC) if we can get access to currency Order 7bf55980-1788-11eb-96ca-8f4fabe5ff7a partially removed, remaining quantity: 0, on hold: 210000000
• [x] there is a problem with remaining qty calculation (it is 0 when should be 1.9)

  
  simnet > sell 3.9 ETH/BTC 0.01 1
  remaining 3.9 ETH entered the order book as 3509a9c0-178f-11eb-96ca-8f4fabe5ff7a
  no matches found
  simnet > removeorder 1 2 -j
  {
  "quantityOnHold": 0,
  "remainingQuantity": 0,
  "removedQuantity": 200000000
  }
  simnet > listorders

Trading pair: BTC/USDT ┌────────────────────────────────────────────────┬────────────────────────────────────────────────┐ │ Buy │ Sell │ ├───────────────┬─────────────┬──────────────────┼───────────────┬─────────────┬──────────────────┤ │ Quantity │ Price │ Alias │ Quantity │ Price │ Alias │ ├───────────────┼─────────────┼──────────────────┼───────────────┼─────────────┼──────────────────┤ │ 0.00518323 │ 12922.28 │ NestGrid │ 0.34963553 │ 13449.72 │ NestGrid │ └───────────────┴─────────────┴──────────────────┴───────────────┴─────────────┴──────────────────┘

Trading pair: ETH/BTC ┌────────────────────────────────────────────────┬────────────────────────────────────────────────┐ │ Buy │ Sell │ ├───────────────┬─────────────┬──────────────────┼───────────────┬─────────────┬──────────────────┤ │ Quantity │ Price │ Alias │ Quantity │ Price │ Alias │ ├───────────────┼─────────────┼──────────────────┼───────────────┼─────────────┼──────────────────┤ │ │ │ │ 1.9 │ 0.01 │ ClusterCourse │ └───────────────┴─────────────┴──────────────────┴───────────────┴─────────────┴──────────────────┘

- [ ] it would be good to change text output for `removeallorders` command, for onhold case, becaouse it is not full truth (all not holded qty was removed). `Order with id 1 was partially removed, xxx btc in hold because of active swap, it will be fully removed afterwards`

simnet > removeallorders Removed order with id 2 Order with id 1 has a hold for a pending swap and will be removed afterwards

[rsercano]

I fixed first two points, for the last point I need to refactor and add fields to grpc response of removeallorders, I rather to do on a separate PR, but if @sangaman if okay with it I can implement it here too.

[raladev]

I fixed first two points, for the last point I need to refactor and add fields to grpc response of removeallorders, I rather to do on a separate PR, but if @sangaman if okay with it I can implement it here too.

Im fine with moving third point to separate PR, it looks more correct

[rsercano]

Then let's focus on merging this first, since I'll depend on this one for it @raladev

[sangaman]

I'm a little confused by this feature. The "remaining quantity" and the "quantity on hold" should always be the same, right? And this quantity that's on hold will be removed unless it ends up being swapped, but perhaps that is not very clear. I'd rather we try to clarify the existing fields rather than duplicate data in the response.

Returning the quantity that was removed makes sense, but returning the pair id also seems a little unusual since presumably the caller should know the trading pair the order they're removing is for, although I suppose it doesn't hurt to have for informational purposes only.

[raladev]

I'm a little confused by this feature. The "remaining quantity" and the "quantity on hold" should always be the same, right?

remaining qty is a qty which remains in the order book after deletion, this field is not equal 0 only if u use partial removing and it does not include onhold amount that will dissapear in any case after the swap (it will be swapped or removed).

[sangaman]

remaining qty is a qty which remains in the order book after deletion, this field is not equal 0 only if u use partial removing and it does not include onhold amount that will dissapear in any case after the swap (it will be swapped or removed).

Ah yes, my mistake for misunderstanding. I'll review again with this in mind.