-
Notifications
You must be signed in to change notification settings - Fork 118
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
Conversation
@m21, @marv-engine, @dexX7, @zathras-crypto please let me know if there are any objections to merging. |
…ble is invalid. Subtracting more coins than are for sale simply cancels everything
Note 1:
vs.
I moved 21 to 25, so there are a few slots for Bitcoin related transactions for the future. Note 2:
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:
... 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 ( If not as transaction type, then as sub action. Note 4: |
I don't want to make lots of new transaction types - we use action bytes in It's worth pointing out that there will never be multiple orders at the If we don't want to overload subtract with cancel-all-for-currency-pair, On Wed, Oct 22, 2014 at 2:49 PM, dexX7 notifications@github.com wrote:
|
Well, it's more explicit and less ambigious. :) With Case 1+2: add/subtract doesn't change:
Case 3: cancel orders of currency pair at a given price doesn't change either:
Case 4: cancel all orders of currency pair at any price:
Case 5: cancel every open order of any currency pair at any price:
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? |
Re: less fields and an implicit commands: |
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. |
There was a problem hiding this comment.
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.
Last night I added comments/questions inline in JR's PR. On Wednesday, October 22, 2014, dexX7 notifications@github.com wrote:
Marv Schneider |
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):
20 GC on the market SUBTRACT 20 GC = 0 GC 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:
20 GC on the market SUBTRACT 10 GC = 10 GC And if I wanted to increase my position to "50 GoldCoins @ 0.5 MSC/GC each", then I would send:
20 GC on the market ADD 30 GC = 50 GC |
Updated last post (@ those who only read emails). |
Also ping @m21: please clarify and correct. Especially the question about precision is relevant when it comes to "prices". |
Re: msg fields required for an ADD or SUBTRACT sub-action: If the original sell order involved 2 indivisible currencies where: 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? |
@marv-engine: it looks like all you can do is In case you are wondering what happens after a fill which leads to rounded numbers: #278 (comment) and the following comments. |
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. |
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 Edit- clarified table columns |
I second Dexx's suggestion here: |
Adding more @marv-engine: Bitcoin varint would be an option, another one is a base128 encoding: http://en.wikipedia.org/wiki/Variable-length_quantity#Examples 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? |
The trading engine does "selection by price level" -- that's the one and only way selection is done, so it can't be ditched. The main question now seems to be: "how to SUBTRACT any random amount from a previously placed order". No strong opinion on this one. |
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? |
The algorithm is nearly same for "any" type of matching; assume a new order coming in:
|
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? |
Internally? At each price level there is only 1 order right now. |
JR - can we put SUBTRACT on hold for a couple days, TBD. @dexX7 , anyone else -- any other flavors? thanks |
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:
I'm not sure, if Am I right that |
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. |
@marv-engine Please we need the test plan describing the sunny day trades, some corner cases, rainy day trades which are supposed to fail, etc. We do not need test cases for ADD, CANCEL, SUBTRACT this week, but having calculations we can verify via the client is crucial. |
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! |
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. |
Per our meeting, this is ready to merge
Hi JR, liquidity bonus is not supported per latest e-mail from David and Craig. Thanks,
|
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. |
Although see issue #284 - I may add something like "liquidity bonus not active in version 1" |
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..