-
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
TX21: need to prevent sellers from getting no indivisible coins from a matching order #170
Comments
Maybe I fail to grasp the problem, but since indivisible units are not further divisible and an amount between < 1 and > 0 doesn't exist in the first place? Or is your intention to specifiy exactly this? If there are 5 IndivisibleCoins up for sale for a total amount of 10.0 MSC, someone has a balance of 6.35 MSC and wants to buy as many as possible, then he ends up with a final balance 0.35 MSC + 3 IndivisibleCoins. |
I've worked through a couple examples, and I just can't find a way to make this bug actually happen without an impossible transaction (i.e. trying to buy or sell 0.4 indivisible coins, which the protocol does not allow). So lets imagine I place an offer to buy 10 indivisible coins on the dEX for 1000 divisible USDCoins each. Can anybody hypothesize a transaction which exploits rounding error to steal my USDCoins? I couldn't figure out a way to do it. |
Here's a case: The offers match at the older (A1's) unit price (.3). A2 wants only 2 MSC so it has to give only 2 * .3 = .6 Indivs in return. .6 will be rounded down to 0. |
In the example you give, wouldn't A1 get 1 indiv and A2 get 3 MSC? I don't think the fact that A2 wants 2 MSC comes into play, since it is the later order and a better price was available. |
Here is a problem I noticed a few minutes ago: Divisible tokens are actually
What is the unit price here? The calculation |
@marv-engine time 0: A1 offers 3 MSC for 1 Indiv ==> unit price = 1/3 = .33 If the Indiv coins round to 0 the match is not valid. Get the next available match. |
@dexX7 if the unit price is 0 tx should be invalid. |
For Indiv coins of tx 21 let's drop the roundown. Indiv coins must be a whole number A1 offers 3 MSC for 1 Indiv ==> unit price = .3 2 MSC * .3 (A1 offer) = .6 Indivs. not a whole number Invalid match A2 trades with B1. |
@dacoinminster interesting point. A1 offers 3 msc for 1 Indiv = .3 A2 ask A1 how much would you pay for 1 Indiv ? A1 replies 3 msc A1 gets 1 Indiv gives 3 msc (A1 is happy, A2 is happier. The important thing is both are happy :) |
@dacoinminster @Bitoy There are two ways that could be used to determine the number of coins exchanged when a non-exact match is found and seller is offering more coins than buyer desires:
My understanding is that the "full buy/partial sell" is what we want. The objective is to fulfill the new (buyer's) order for a desired number of coins, as opposed to selling all the coins the existing (seller) order is offering. Per the referenced algorithm, the PR currently says:
|
@dexX7 I'm not sure if the protocol's internal representation of the number of divis & indiv tokens matters in determining the unit price, or do we just treat them as a regular double precision float and integer for the calculation. I have to think that one through. It might be yet another edge case (YAEC). I do agree with:
Here's a way to handle the divis for indiv case: For the situation where the coins for sale and desired are both indivs: At what point does the truncated fraction not matter? The user could limit his exposure if he's allowed to specify the minimum purchase amount. Specifying a minimum purchase amount would also let sellers offer volume discounts. Without it, a buyer could choose to buy 1 satoshi of a divis coin or 1 indiv coin, even if the seller wants to sell billions. |
Marv: the property "divisible" and "indivisible" acts as amount modifier: 1 Indivisible Unit = 1 Base Unit |
@marv-engine indiv for indiv |
There's no way to fix the indiv for indiv integer problem in general. So, one of the parties will be subject to receiving up to the equivalent of .99999999 coins less than they should in each match. The question is how can the parties limit their loss? I see two alternatives:
The min purchase amount alternative does support a seller offering a volume discount (which could also be accomplished if a single order allowed tiered unit pricing). We need to make a decision so the devs can start working on implementation. |
I still don't see an actual example of this problem here. It still seems to me that this problem can't actually happen with the transactions which we can express. For indiv vs indiv: UserA: wants to sell 1 indivA to get 2 indivB Where's the fractional problem here? The problem I see here is what precision to store unit price. I suggest double-precision floats should be sufficient. |
Time 0: A1 offers 20 Indiv1 coins for 10 Indiv2 coins (unit price .5) Process:
There's no way the number of both coins transferred can be whole numbers. |
I think the result of this example would be: A1 gets 10 indiv1 coins, A2 gets 20 indiv2 coins and has 2 Indiv2 coins left for sale. HOWEVER, the unit price of 1.4 would no longer make sense with only 2 coins for sale, and I think the problem WOULD then happen in subsequent transactions. |
Solution: after each sale, update the unit price to the next best price which can be represented in a MSC TX21. So for Marv's example above, after the sale was complete, the A2 would have 2 Indiv2 coins left for sale in exchange for 3 indiv1 coins, adjusting the unit price to a more favorable 1.5 |
Sorry - hit close by accident. This is definitely still open :) |
OK, adjusting Marv's example, I can make this happen in one tx: Time 0: A1 offers 20 Indiv1 coins for 11 Indiv2 coins (unit price .55) This results in a partial fill. A1 gets 6 indiv2, but A2 should get 3.3 indiv1! Do we all agree that this is a good example to work from? |
A human being looking at the example above would probably suggest A1 gets 6 indiv2, A2 gets 10 indiv1, for unit price of 0.6, which is better than the asking price on both sides, but good luck describing a general-purpose algorithm to do that!! edit: fixed my wrong math :) |
@dacoinminster Coins are transferred from the seller (A1) to the buyer (A2) until he gets the amount he desires, so A2 will get 17 Indiv1 coins and has to give 8.5 Indiv2 coins to A1. My example is clear to me. It looks like yours is backwards. The transfer amounts are a function of the 8 Indiv1 coins desired, not the 6 Indiv2 coins for sale. |
The spec says throw away the coins desired, and keep unit price instead, right? |
In fact, it does. |
But the unit price implies the amount desired (which decreases when this existing order is partially used to fulfill a buy order). |
Clarified: |
No one ever gets more than they desire. They might pay less than they're willing to, though. |
Yes, I think this is a good idea. What happens in the case of fractional amounts? Using the old example:
39 * 0,50847458 = 19.83050862 I think rounding up is the correct solution, because the user still gets what he wanted for the price he wanted - in total - and no orders are created which are "higher than market price", e.g. reversed unit price of 0.48717, if rounded down to 19. |
Agreed round up if there is a fraction. X= Roundup (remaining No. Of coins desired * original price ) Change = no. Of coins for sale - X Btw precision of unit price must be defined in the spec. Suggest up to 8 decimal places. |
Floating point numbers are somewhat special and instead of using a fixed number of rounded decimal places, I suggest to use a standardized percision, e.g. "double percision" as defined in IEEE 794, see: http://en.wikipedia.org/wiki/Floating_point#IEEE_754:_floating_point_in_modern_computers This is C/.NET data type "double" and "float" in Python as far as I know. Might also be worth to specify this spec wide. I assume the differences related to the Exodus balance are based on this. |
I uploaded an order match simulator: |
thanks dexX, I'm checking this out now |
Remember, the Amount desired is needed only to determine the unit price (exchange ratio) for Action=New or Update. After that, the remaining number of coins desired is determined by calculating the remaining number of coins for sale multiplied by the unit price.
This fractional additional amount has to be given or these two orders would not match. I haven't figured out if this happens only for a match that completes one of the orders. |
Actually I think storing both amount offered as well as desired amount might be safer whereby the unit price is calculated on the fly. The desired amount seems to be the most important property and the other values are more flexible, as long as the limits are not overstepped. I apply those assumptions (with loose words):
I'm not sure, if this is the absolut minimum, but this seems to be sufficient for these examples: Calculating the traded amounts: a1_available = order_old.amount_for_sale
a1_unit_price = order_old.get_unit_price()
a1_desired = round_up(order_old.amount_desired)
a2_desired = round_up(order_new.amount_desired)
# traded amounts
amount_to_a2 = min(a2_desired, a1_available)
amount_to_a1 = round_up(amount_to_a2 * a1_unit_price) Updating amounts after a trade: # amount received is the amount gained from a trade
updated_amount_desired = amount_desired - amount_received
updated_amount_for_sale = updated_amount_desired / get_unit_price() And the example: New Order [ID 0] created: 20 Indiv2 offered, 10 Indiv1 desired @ 0.5 (2.0)
New Order [ID 1] created: 30 Indiv1 offered, 59 Indiv2 desired @ 1.96666667 (0.50847458)
New Order [ID 2] created: 40 Indiv2 offered, 20 Indiv1 desired @ 0.5 (2.0)
Executed [ID 1], [ID 0]: 20 Indiv2 traded for 10 Indiv1 @ 0.5 (2.0)
Updated Order [ID 0]: 20 => 0 Indiv2 offered, 10 => 0 Indiv1 desired
[ID 0] filled completely: 10/10 Indiv1
Updated Order [ID 1]: 30 => 20 Indiv1 offered, 59 => 39 Indiv2 desired @ 1.95 (0.51282051)
[ID 1] partially filled: 20/59 Indiv2
Executed [ID 2], [ID 1]: 20 Indiv1 traded for 39 Indiv2 @ 1.95 (0.51282051)
Updated Order [ID 1]: 20 => 0 Indiv1 offered, 39 => 0 Indiv2 desired
[ID 1] filled completely: 59/59 Indiv2
Updated Order [ID 2]: 40 => 0 Indiv2 offered, 20 => 0 Indiv1 desired
[ID 2] filled completely: 20/20 Indiv1 |
The unit price is not changed as the result of a match, so it doesn't change on the fly. This step isn't correct:
The unit price remains at 1.96666667 (0.50847458), what it was when the order was placed. The only way the unit price changes is with an Update action. This would be much easier to keep straight if tx21 accepted the unit price rather than the amount desired. |
How would that work or what do you suggest otherwise? 39.333 is not a whole number. |
Actually I see no reason why this is not legit. A2 wants 59 for 30 @ 0.50847 and gets 20 for 10 at @ 0.5 which is in A2's favor and within A1' allowed range. Edit: green is allowed for both (fullscreen: http://i.imgur.com/qSTrATC.png): |
I think rounding up in the calculation (to 40 Indiv2 if a buyer wants to purchase 20 Indiv1) will address it the same way as in other cases - it just requires the resulting calculated unit price to be <= the buyer's unit price. The seller (ID 1) will get a fractional amount more than he had asked for. If the unit price is recalculated after a partial purchase: When larger quantities are involved, the unit price can have more wild swings. The seller is at the mercy of future buyers, whose actions would end up changing the seller's unit price in unpredictable ways. |
A2 wants 59, offers 30 -- this is an unit price of 1.966 ("the "Amount desired" divided by the "Amount for sale") and A4 would get 2 for ~4 and complete, if I understand it correctly. Why would A2 get such a high amount? I assume A2 was listed before A4. |
Guys, unit price doesn't move once it is set. This is another place to reduce complexity. If nothing else, it avoids having orders move around their position in the order book (which is ordered by unit price). I see a lot of discussion here, but nothing which indicates we can't use the round-up method to avoid fractional losses of indiv properties. The same logic will apply to divisible properties too, avoiding losses of fractions beyond the eighth decimal place. Marv, I think once you have the appropriate clarifications in the spec, we can close this. |
Why would you move the unit price? I think this is not really up for discussion. The rounding-up method seems to work fine and I posted the exact ruleset I applied with this method a few posts ago. For my part I'd like to know if my understanding is correct. The only edge case which may need further clarification is related to this: #170 (comment) |
@marv-engine please check if this correct. A1 offers 20 Indiv2 and desires 10 Indiv1 at >= 0.5 Indiv1/Indiv2. A3 offer 6 indiv1 for 3 indiv2 A1in total gets 15 indiv1 (9 + 6 : 5 more than the original 10 he wants) Total: 20 indiv2, 18 indiv1 |
I've been thinking about this a lot and I'm starting to appreciate the approach where the unit price (exchange ratio) is calculated based on the remaining number of coins desired and the remaining number of coins for sale, as opposed to unit price remaining fixed. When a new order is posted, that unit price (reciprocal of amount desired / amount for sale) is used to find matching existing orders with a unit price <= the new order's unit price. If the new order is not fulfilled (either there are no existing orders that match or the matching orders don't offer enough coins), then the new order is added to the list of existing orders and its unit price is recalculated based on its remaining amount desired and remaining amount for sale. If an existing order is partially consumed to fulfill a new order, the existing order's unit price is recalculated based on its remaining amount desired and remaining amount for sale. If the order involves divisible coins on both sides, then the new unit price will be the same as the previous one because coins were exchanged at exactly the unit price. If the order involves an indivisible coin on either or both sides, then the new unit price will change if a fractional portion of the coin was rounded up to the next whole number. Note that the new unit price could be dramatically lower than the previous unit price, depending on the new number of coins remaining.
This will have the effect of making the remaining existing order more attractive to new orders - because the unit price is now lower. So these remaining smaller orders will match and be used sooner than they would have at their previous, higher, unit price. When an existing order is fully consumed, the average unit price for all the matches it was part of will be the original Amount desired / Amount for sale, because the seller will have received the original number of coins desired in exchange for the original number of coins for sale. One issue to be addressed by all implementations - the unit price or its reciprocal could be a very large or very small number, so it must be stored in a datatype that can hold values that range from:
Did I miss anything or make any mistakes? |
For case 2 Whatever option we take, no one (seller, buyer, or next buyer) will loose out. What do we want? pro seller, neutral or pro buyer? |
Let's calculate the unit price up to 8 decimals. If it rounds to 0 the tx is invalid. |
I use MSC/BTC now, because it's more vivid for me and the terms buy and sell order are used loosely: Case 1: The order is executed and the buyer receives 1 MSC, but at a price of 0.05, because he buys into the existing 200 MSC sell wall. He get what he wants and after updating: 199 MSC are up for sale at a price of 0.05 BTC/MSC (total: 9.95 BTC). Case 2: Edit: the new order should not be more expensive, because this would lower the chances of a fill, so in doubt: make it cheaper. Storing "amount for sale" and "amount desired" instead of "unit price" may solve the problem of fractions. Edit 2: *actually he wants to have 10 BTC instead of selling 200 MSC, but since this is no conflict here, I used this point of view for the sake of an easier example. |
@Bitoy says:
For the next sale, the price will be lower than it was, but that means the sale can happen sooner than it would have (favoring the seller if he wants to sell ASAP) and, in total, the seller still gets the number of coins desired for the number of coins he's selling. It all balances out because the earlier sale was at an effective unit price that was higher than the seller's original terms.
But the remaining coins may not sell as fast as they would with a lower unit price, so that's not helpful to the seller. @dexX7 says:
I recommend storing unit price because it can be included in a table index, making it faster to find existing orders that match the new order. |
…nversation in issue OmniLayer#170
This needs to be addressed as part of the discussion for PR #165 but that thread was getting long so I broke this out.
This issue is about the possibility that a seller can end up getting 0 indivisible coins from someone's matching order. This is due to the fact that the number of indivisible coins affected by an exchange has to be rounded down to a whole integer value.
On average, a seller will not receive the equivalent of .5 indivisible coins when another order matches with the seller's order that desires an indivisible currency. At worst, the seller won't receive the equivalent of .99999999 coins in a tx21 coin swap. At best, he won't lose out on any equivalent fractional parts.
A seller who posts an order desiring an indivisible coin has no control over the number of his coins that will match a future tx21 order. So, it's possible that the calculated number of indiv coins he will get could be less than 1 and would therefore be rounded down to 0. That's the worst case. The same rounding will occur regardless of the number of indiv coins that were calculated. So, the seller is open to the risk of many of these transactions that give away his coins with the potential that he will get 0 coins in return, or possibly a non-zero amount that's less than he desires. He has no control over this.
We should protect the seller, perhaps by letting him specify the minimum number of desired coins he's willing to accept in a tx21 transaction. That number would be used in the matching algorithm until the seller has fewer than that number left for sale.
Maybe there's another way to address this risk.
Note that participating in a crowdsale has the same truncation issue, but it happens only when a participant explicitly sends coins. In that case, the user isn't subject to the risk caused by future actions of others.
The text was updated successfully, but these errors were encountered: