Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to TX21 per issue #270 #276

Merged
merged 5 commits into from
Oct 31, 2014
Merged

Conversation

dacoinminster
Copy link
Contributor

Per issue #270 I have modified the spec to use ADD and SUBTRACT instead of Add/Update/Cancel. Logic for merging orders at the same price is described..

@dacoinminster
Copy link
Contributor Author

@m21, @marv-engine, @dexX7, @zathras-crypto please let me know if there are any objections to merging.

@dexX7
Copy link
Member

dexX7 commented Oct 22, 2014

Note 1:
Instead of action fields, I suggest we use the full range of transaction types.

Tx 21, sub action 1 (ADD)
Tx 21, sub action 2 (SUBTRACT)

vs.

Tx 25 (ADD)
Tx 26 (SUBTRACT)

I moved 21 to 25, so there are a few slots for Bitcoin related transactions for the future.

Note 2:
From the other thread:

Q: May be nice to give users some way to SUBTRACT (CANCEL) ALL at that effective price - perhaps requiring huge amounts, perhaps given 0, perhaps a new ActionType.
R: The UI can always cancel-all at an effective price with a huge subtract, so I don't think we need a different command for this.

I do see a many benefits of different explicit actions instead of overloading and letting the UI handle it. This goes hand in hand with:

Note 3:

If the UI wishes to cancel ALL open orders for a given currency pair, use action=2 (SUBTRACT) amount for sale=0 and amount desired=0.

... which I don't like and especially not, if you introduce more action fields at the same time which basically waste space. :)

There are furthermore fine nuances: one might want to cancel orders 1. of a specific currency pair and price, 2. all orders of a specific currency pair or 3. every open order. So I suggest to add a transaction type for each of those actions (ADD, SUBTRACT, CANCEL, CANCEL-ALL, CANCEL-EVERYTHING).

If not as transaction type, then as sub action.

Note 4:
I don't like the sub action route, because this requires to zero fields and to add the action field behind. My first idea was to swap the order and move the sub action to the beginning, but fields filled with zero are difficult to distinguish from padding with zeros. But as mentioned: I consider this route as workaround where I see no need for a workaround.

@dacoinminster
Copy link
Contributor Author

I don't want to make lots of new transaction types - we use action bytes in
other places, and I think it makes sense to do it here too.

It's worth pointing out that there will never be multiple orders at the
same price. Those are merged, remember? The main reason to allow the user
to subtract more than is for sale is when the subtract collides with the
order getting partially filled. Since we have to handle that case, and
since orders are merged, there's no added value having a
cancel-all-at-price command.

If we don't want to overload subtract with cancel-all-for-currency-pair,
I'm fine with a new action type for that, and a new action for
cancel-everything-all-currencies is fine too. Then we'd have ADD, SUBTRACT,
CANCEL-ALL, and CANCEL-EVERYTHING.

On Wed, Oct 22, 2014 at 2:49 PM, dexX7 [email protected] wrote:

Note 1:
Instead of action fields, I suggest we use the full range of transaction
types.

Tx 21, sub action 1 (ADD)
Tx 21, sub action 2 (SUBTRACT)

vs.

Tx 25 (ADD)
Tx 26 (SUBTRACT)

I moved 21 to 25, so there are a few slots for Bitcoin related
transactions for the future.

Note 2:
From the other thread:

Q: May be nice to give users some way to SUBTRACT (CANCEL) ALL at that
effective price - perhaps requiring huge amounts, perhaps given 0, perhaps
a new ActionType.
R: The UI can always cancel-all at an effective price with a huge
subtract, so I don't think we need a different command for this.

I do see a many benefits of different explicit actions instead of
overloading and letting the UI handle it. This goes hand in hand with:

Note 3:

If the UI wishes to cancel ALL open orders for a given currency pair, use
action=2 (SUBTRACT) amount for sale=0 and amount desired=0.

... which I don't like and especially not, if you introduce more action
fields at the same time which basically waste space. :)

There are furthermore fine nuances: one might want to cancel orders 1. of
a specific currency pair and price, 2. all orders of a specific currency
pair or 3. every open order. So I suggest to add a transaction type for
each of those actions (ADD, SUBTRACT, CANCEL, CANCEL-ALL,
CANCEL-EVERYTHING).

If not as transaction type, then as sub action.

Note 4:
I don't like the sub action route, because this requires to zero fields
and to add the action field behind. My first idea was to swap the order
and move the sub action to the beginning, but fields filled with zero are
difficult to distinguish from padding with zeros. But as mentioned: I
consider this route as workaround where I see no need for a workaround.


Reply to this email directly or view it on GitHub
#276 (comment).

@dexX7
Copy link
Member

dexX7 commented Oct 22, 2014

there's no added value having a cancel-all-at-price command

Well, it's more explicit and less ambigious. :)

With CANCEL-ALL and CANCEL-EVERYTHING it's possible to move the action byte at the beginning and we can remove some fields in some cases. This does not depend on an explicit CANCEL:

Case 1+2: add/subtract doesn't change:

[version: 0] [type: 21] [action] [currency] [amount] [currency] [amount]

Case 3: cancel orders of currency pair at a given price doesn't change either:

[version: 0] [type: 21] [action] [currency] [amount] [currency] [amount]

Case 4: cancel all orders of currency pair at any price:

[version: 0] [type: 21] [action: cancel-all] [currency] [currency]

Case 5: cancel every open order of any currency pair at any price:

[version: 0] [type: 21] [action: cancel-everything]

Note how 2 or 4 fields can be removed in case 4 and 5.

And since orders are selected by price, just to tackle the topic quickly: any reason for or against providing price instead of a ratio of two amounts?

@dexX7
Copy link
Member

dexX7 commented Oct 22, 2014

Re: less fields and an implicit commands: SUBTRACT could be expressed by ADD of a negative amount - and given that the number of coins fields is signed and not unsigned .... Do you think this would be a good idea? I prefer more commands, if they are basically for free. :)

The new sell order's unit price is computed from two of the fields in the transaction message: the "Amount desired" divided by the "Amount for sale". An existing order's original unit price is used to match against new orders. The unit price does not change except by using the Change action. See below.

An address cannot create a new sell order while that address has an active sell order with the same currencies in the same roles (for sale, desired). An active sell order is one that has not been canceled or fully accepted. The currency id for sale must be different from the currency id desired. Both currency id's must refer to existing currencies.
The new sell order's unit price is computed from two of the fields in the transaction message: the "Amount desired" divided by the "Amount for sale". An existing order's original unit price is used to match against new orders. The unit price does not change. The currency id for sale must be different from the currency id desired. Both currency id's must refer to existing currencies.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two currencies have to be in the same ecosystem.

@marv-engine
Copy link

Last night I added comments/questions inline in JR's PR.

On Wednesday, October 22, 2014, dexX7 [email protected] wrote:

Re: less fields and an implicit commands: SUBTRACT could be expressed by
ADD of a negative amount - and given that the number of coins fields is
signed and not unsigned .... Do you think this would be a good idea? I
prefer more commands, if they are basically for free. :)


Reply to this email directly or view it on GitHub
#276 (comment).

Marv Schneider
VP, User Experience/Product Usability
Engine, Inc.
[email protected]

@dexX7
Copy link
Member

dexX7 commented Oct 24, 2014

I have a difficult time to understand this in practise, too and while I have some understanding of Master Core, I haven't dived into the meta DEx logic yet. The high level idea is that "amount/amount" returns "price" (why not price in the first place?).

Note: the traditional DEx uses "for sale/desired" while the meta DEx uses "desired/for sale". (!)

But maybe we can crack it:

Let's say I have "20 GoldCoin for sale @ 0.5 MSC/GC each", then this would equal "amount desired = 10 MSC" and "amount for sale = 20 GoldCoins", thus "10 MSC/20 GC = 0.5 MSC/GC".

If I wanted to cancel this order, I would send (with the current template which I believe can be improved based on my previous comments):

Version: 0
Type: 21
Currency for sale: GoldCoin
Amount for sale: 20 GC
Currency desired: Mastercoin
Amount desirerd: 10 MSC
Action: Subtract

20 GC on the market SUBTRACT 20 GC = 0 GC
10 MSC on the market SUBTRACT 10 MSC = 0 MSC
Result: order canceled

If I wanted to update my order from "20 GoldCoins at @ 0.5 MSC each" to "10 GoldCoins at @ 0.5 MSC/GC each", then I would send:

Version: 0
Type: 21
Currency for sale: GoldCoin
Amount for sale: 10 GC
Currency desired: Mastercoin
Amount desirerd: 5 MSC
Action: Subtract 

20 GC on the market SUBTRACT 10 GC = 10 GC
10 MSC on the market SUBTRACT 5 MSC = 5 MSC
Result: 10 GC @ 0.5 MSC/GC

And if I wanted to increase my position to "50 GoldCoins @ 0.5 MSC/GC each", then I would send:

Version: 0
Type: 21
Currency for sale: GoldCoin
Amount for sale: 30 GC
Currency desired: Mastercoin
Amount desirerd: 15 MSC
Action: Add

20 GC on the market ADD 30 GC = 50 GC
10 MSC on the market ADD 15 MSC = 25 MSC
Result: 50 GC @ 0.5 MSC/GC

@dexX7
Copy link
Member

dexX7 commented Oct 24, 2014

Updated last post (@ those who only read emails).

@dexX7
Copy link
Member

dexX7 commented Oct 24, 2014

Also ping @m21: please clarify and correct. Especially the question about precision is relevant when it comes to "prices".

@marv-engine
Copy link

Re: msg fields required for an ADD or SUBTRACT sub-action:

If the original sell order involved 2 indivisible currencies where:
Amount for sale = 17
Amount desired = 13

What values do I use to add 1 token or subtract 2 tokens to the original sell order? Where does the spec define the general way to specify the same unit price for any addition or subtraction to the number for sale?

@dexX7
Copy link
Member

dexX7 commented Oct 27, 2014

@marv-engine: it looks like all you can do is ADD 17*n A 13*n B or SUB 17*n A 13*n B where n is a whole number above zero and A and B are currencies. Any other "ratio" than 17 / 13 results in a different price level.

In case you are wondering what happens after a fill which leads to rounded numbers: #278 (comment) and the following comments.

@marv-engine
Copy link

So, we're no closer to a general, workable solution. We've entertained a variety of ideas to identify the sell order to be affected. If we use:

currency for sale (4 bytes), amount for sale (8), currency desired (4), amount desired (8)

that's 24 bytes, which is several bytes more than would be statistically necessary to uniquely identify the target tx if we used the prefix of the tx hash ( .0625 ^ 24 = 1.2621774e-29). The field of candidate tx's is narrowed by the target address and the list of active instances of the corresponding tx type.

(We could use half the bytes if we packed 2 tx hash hex digits per byte rather than 1 hex value in ascii.)

Or, we could use the Bitcoin var-int approach where the first byte indicates the length of the integer that immediately follows. For the vast majority of cases, that would whittle off some of the 24 bytes.

The point is we need to choose something that clearly meets all the needs.

@marv-engine
Copy link

here's the probability of a collision between the prefixes of 2 tx hashes if we pack 2 hex digits per byte in the message field (use all 8 bits rather than just 4):

N bytes: (1/256) ^ N
2: 0.00001525878
3: 5.96046448e-8
4: 2.3283064e-10
6: 3.5527137e-15
8: 5.4210109e-20

Edit- clarified table columns

@m21
Copy link

m21 commented Oct 27, 2014

I second Dexx's suggestion here:
#278 (comment)
(For an expanded CANCEL command).

@dexX7
Copy link
Member

dexX7 commented Oct 27, 2014

Adding more CANCEL operations aside, if SUB is removed or only usable with constraints, then we are sort of back at the very beginning.

@marv-engine: Bitcoin varint would be an option, another one is a base128 encoding:

http://en.wikipedia.org/wiki/Variable-length_quantity#Examples
http://en.wikipedia.org/wiki/LEB128

But I suggest encoding specifics should be discussed later.


For me it's very unclear where we stand at the moment regarding to the commands and if a "selection by price level" is viable or if the whole merged price levels approach should be ditched in favor of a selection of individual orders , whether that is by txhash or some other mechanism?

@m21
Copy link

m21 commented Oct 27, 2014

The trading engine does "selection by price level" -- that's the one and only way selection is done, so it can't be ditched.
Thus:
ADD is automatically done by price level.
CANCEL by price level is trivial as well.

The main question now seems to be: "how to SUBTRACT any random amount from a previously placed order". No strong opinion on this one.

@dexX7
Copy link
Member

dexX7 commented Oct 27, 2014

The trading engine does "selection by price level" ...

The order matching engine matches orders based on price level, I assume, but this has no direct relation to how orders are added or removed from the system?

@m21
Copy link

m21 commented Oct 27, 2014

The algorithm is nearly same for "any" type of matching; assume a new order coming in:

  1. For a given currency (pair) scan the orderbook looking for a trade (at price given or better)
  2. If found - trade; exchanging tokens
  3. If anything remains in this new order - scan the orderbook looking for an exact price level match, then insert.

@dexX7
Copy link
Member

dexX7 commented Oct 27, 2014

Well, yeah, I guess my point was something like: it doesn't really matter how the backend works (to some degree), because this is rather a question about the interface.

But something I almost forgot: targeting individual orders by hash or something else only works, if orders have an identity. Is this currently the case, @m21? Is it possible to distinguish and identify individual orders?

@m21
Copy link

m21 commented Oct 27, 2014

Internally? At each price level there is only 1 order right now.
Perhaps you mean something else?

@dacoinminster
Copy link
Contributor Author

Note: in #278, @m21 said he liked getting rid of subtract and just having various flavors of cancel. I told him I'd happily amend this pull request to whatever his code does, since he's writing the logic.

@dacoinminster dacoinminster mentioned this pull request Oct 27, 2014
@m21
Copy link

m21 commented Oct 28, 2014

JR - can we put SUBTRACT on hold for a couple days, TBD.
But yes if you can perhaps document the various flavors of CANCEL (pretending SUBTRACT will not exist).
The flavors could be:
CANCEL-ALL-FROM-THE-ADDRESS
CANCEL-ALL-FOR-CURRENCY-PAIR
CANCEL-ALL-AT-OR-ABOVE-PRICE

@dexX7 , anyone else -- any other flavors? thanks

@dexX7
Copy link
Member

dexX7 commented Oct 28, 2014

Hm. No, I think that's all. There could be a timeout and an order is only active until block n, but that's just some bonus.

So to summarize:

Add:
[version] [type] [action] [property] [number] [property] [number]

Cancel-Everything:
[version] [type] [action] [ecosystem]

Cancel-All-Of-Currency-Pair:
[version] [type] [action] [property] [property]

Cancel-All-Of-Currency-Pair-At-Or-Above:
[version] [type] [action] [property] [number] [property] [number]

I'm not sure, if [property] [number] [property] [number] or [property] [property] [number] [number] is a better choice.

Am I right that Cancel-Everything requires the [ecosystem] field?

@marv-engine
Copy link

As I said yesterday at the all-hands mtg, it seems there are details that have not been vetted or approved. We need to have those in place in order to write test reqs, not to mention the code to be tested.

@m21
Copy link

m21 commented Oct 29, 2014

@marv-engine
Trade & ADD are solid.
CANCEL is being defined.

Please we need the test plan describing the sunny day trades, some corner cases, rainy day trades which are supposed to fail, etc.
We need trading examples and computations we can run through the client to make sure they match your expectations.

We do not need test cases for ADD, CANCEL, SUBTRACT this week, but having calculations we can verify via the client is crucial.

@dacoinminster
Copy link
Contributor Author

Pull request has been updated - SUBTRACT is gone - the three types of CANCEL have been added. @m21 if you want any further changes, please let me know, otherwise I'd like to merge this. Thanks!

@dexX7
Copy link
Member

dexX7 commented Oct 30, 2014

One last note to consider regarding the order of fields: it might also be an option to assign a new transaction type number to each action and remove the action field - sort of similar to how it's done with crowdsales and the manual cancel which is expressed explicitly by tx 53.

dacoinminster pushed a commit that referenced this pull request Oct 31, 2014
Per our meeting, this is ready to merge
@dacoinminster dacoinminster merged commit 0b734c0 into OmniLayer:master Oct 31, 2014
@m21
Copy link

m21 commented Oct 31, 2014

Hi JR,

liquidity bonus is not supported per latest e-mail from David and Craig.
Should say so in the spec?

Thanks,
Michael
On Oct 31, 2014 11:50 AM, "dacoinminster" [email protected] wrote:

Merged #276 #276.


Reply to this email directly or view it on GitHub
#276 (comment).

@dacoinminster
Copy link
Contributor Author

Well, it won't get merged into the "implemented" branch of the spec. I usually try to keep the spec matching the ultimate goal, even if we only do part of it.

@dacoinminster
Copy link
Contributor Author

Although see issue #284 - I may add something like "liquidity bonus not active in version 1"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants