ExchangeUnion / xud

Exchange Union Daemon 🔁 ⚡️
https://exchangeunion.com
GNU Affero General Public License v3.0
115 stars 44 forks source link

Add disableMatching mode #145

Closed moshababo closed 6 years ago

moshababo commented 6 years ago

xud should have only 2 matching modes: do no matching, or all matching. represent this by adding a boolean config property, disableMatching, which its default value will be false. It should be a top-level config (not under a specific module). Should be passed, at least, to the order book module.

DONE

TODO I'd avoid the double negation disableMatching = false, also unified gRPC as per https://github.com/ExchangeUnion/xud/issues/123#issuecomment-419176541 with following events:

1) ownOrder added (placeOrder) 2) ownOrder removed (cancelOrder) 3) ownOrder removed (due to internal match) 4) ownOrder removed (due to a successful swap that was instantiated remotely) 5) peerOrder added 6) peerOrder removed (due to non-internal match) 7) peerOrder removed (due to peer order invalidation) 8) ownOrder incomingSwap (incoming swap that was instantiated remotely) 9) peerOrder initiateSwap (due to non-internal match)

DONE (https://github.com/ExchangeUnion/xud/issues/123) matching = true;

TODO (this issue) matching = false;

Reminder: matching scenarios

kilrau commented 6 years ago

Related: https://github.com/ExchangeUnion/xud/issues/146

itsbalamurali commented 6 years ago

@moshababo I've updated the issue description since #146 is closed and internal matching is implemented as part of #196

kilrau commented 6 years ago

This task is rather about implementing the non-matching mode which needs some fleshing out on implementation side. Once we found a way for https://github.com/ExchangeUnion/xud/issues/215, we can flesh out here @moshababo @sangaman , right?

moshababo commented 6 years ago

Yes, we can do it here and on the weekly chat as well.

kilrau commented 6 years ago

https://github.com/ExchangeUnion/xud/issues/215 about to be merged, let's flesh out here @moshababo

kilrau commented 6 years ago

I'd avoid the double negation disableMatching = false, also unified gRPC as per https://github.com/ExchangeUnion/xud/issues/123#issuecomment-419176541 with following events:

1) ownOrder added (placeOrder) 2) ownOrder removed (cancelOrder) 3) ownOrder removed (due to internal match) 4) ownOrder removed (due to a successful swap that was instantiated remotely) 5) peerOrder added 6) peerOrder removed (due to non-internal match) 7) peerOrder removed (due to peer order invalidation) 8) ownOrder incomingSwap (incoming swap that was instantiated remotely) 9) peerOrder initiateSwap (due to non-internal match)

DONE (https://github.com/ExchangeUnion/xud/issues/123) matching = true;

TODO (this issue) matching = false;

Reminder: matching scenarios

kilrau commented 6 years ago

A first shot on non-matching. What do you think? @moshababo @sangaman @offerm @itsbalamurali @michael1011

moshababo commented 6 years ago

@kilrau following what you wrote for the no-matching mode:

  • placeOrder (sync, returns success/error) event (1) (propagates ownOrder to peers)

Yes.

  • cancelOrder (sync, returns success/error) event (2) (sends order invalidation packet for ownOrder to peers)

Yes.

  • swap (bidirectional, sync, returns success/error) event (8), (9)

We need to differentiate between a taker swap and a maker swap.

  • subscribeOrders (streaming) events (5), (7)

If we alert on incoming maker swaps via subscribeSwaps, then yes, this will be applied only to peer orders. I think it's another point for a differentiation between subscribePeerOrders and subscribeOwnOrders. For no-matching mode, the latter will be disabled. for matching mode, subscribeSwaps will be disabled.

So on matching mode we notify the client for peerOrders & ownOrders events, and on no-matching mode we notify on peerOrders & swaps events.

kilrau commented 6 years ago

We need to differentiate between a taker swap and a maker swap.

I don't think that's right. According to the new swap protocol a swap only gets initiated by the taker. In your description you differentiate between a swap initiated by remote peer or by me. That's it.

we can expose it via subscribeSwaps, which is probably better

you are right, I got (8) wrong, put it under subscribeOrders' like you suggested. I believe we usedsubscribeOrdersforxud` in matching mode, so we should stick with this naming. If not, let's stick with whatever we used in matching mode.

We can have a synchronous version as executeSwapSync, which will return the swap resolution. if async, results will be given via subscribeSwaps by using swapId or something.

Ok, agree - fixed.

sangaman commented 6 years ago

Still definitely a priority, but it won't be ready for alpha.1

kilrau commented 6 years ago

Can be closed? @moshababo