Allow buy offers

[tomerweller]

Currently, stellar only allows for sell offers. However, translating buy offers to sell offers does not preserve the intent of the trader.

Quoting @ScottHogge from the stellar's keybase team:

Lets say you want to buy exactly 10 Marbles for at most $1 each... if you re-frame this as an offer to sell $10 at a price of 1 Marble/$, you could end up getting 20 Marbles at $0.5 if the orderbook contains an offer to sell Marbles at $0.5 --while the desired effect would be just 10 Marbles at $0.5. This behavior is different than in any traditional market (stocks, options, futures, etc...) and is kinda problematic for the financial industry adopting SDEX usage when they have a duty to execute customer orders as directed.

[ScottHogge]

There was a discussion regarding this on keybase, about the best way to handle this given stellar's adoption of a Unidirectional Order Model, and whether switching to separate bid/ask books was the best long term solution. While I'm not sure on the answer to that, I believe the issue can likely be solved by the addition of two more fields to the ManageOffer operation, 'buying_quantity' and 'buying_price'. These need only be checked once when the order is first submitted to ensure that any matching with resting orders do not exceed either of those criteria, and then any remainder can still be booked in terms of a sell offer as described above. The issue only occurs when an order is first submitted if it crosses existing contra-side orders.

[johansten]

Don't you just need to flag if it's a buy or sell?

[skirsch]

Adding more fields does NOT preserve the intent. It makes it more complicated by adding more constraints and making it more complex.

The intent is a single constraint. I'm buying EUR at 1.1 USD or better, and selling EUR at 1.2 USD or better. That's the sole constraint on the transaction. Same quote currency for buy and sell orders. The way Stellar does this is:

1. horribly confusing
2. doesn't preserve the intent of the trader For example, I use Kelp for a buysell strategy and the prices of the buy and the sell are DIFFERENT, even though they are supposed to be EXACTLY the same. It is super frustrating.

I think Stellar is brilliantly designed and this is the single biggest design error. A close second is that is harder than heck for me to give out new USD/token.io dollars to someone directly.

[johansten]

@skirsch

What's the quote currency then? The protocol doesn't give precedence to any specific currency.

Adding a flag, or more fields, is one way we can keep a UDOM, and add something that works like a user would expect a buy order to work.

[skirsch]

@johansten The person who places the order decides the quote currency which ideally matches the qoute currency of the orderbook. otherwise, person placing the trade would provide the conversion factor (or simply express the order in the quote currency of the orderbook)? To keep compatibility, you leave the code as is, and create the new buy/sell interface as the newer API. Therefore, in the case of XLM/USD, USD is the quote currency and I can put in an order to buy 5 XLM @.24 and sell 5 XLM @.25 and be perfectly flat after both orders fill. I think most of the trades will be made where the trader is matching the quote currency of the orderbook. Make sense? So perhaps the best solution is you create a new set of APIs where you spec the two assets, you spec the quote currency, and that determines your orderbook you use. then you do buy and sells as normal. that is the way the world works today. how abou that?

[johansten]

Again, the protocol doesn't care about quote currencies.

It's a uni-directional order model. All orders are expressed as selling asset A for asset B. No asset is more important than the other. There's no preferred order book -- XLM/USD is just as valid as USD/XLM. (Edit: actually, there are no order books in Stellar, and it has to be that way for path finding to be efficient)

The problem is that with the order semantics, a sell order isn't simply the inverse of a buy order. A flag can fix that, making it work more akin to what we're used to in traditional trading.

[skirsch]

@johansten The problem is the math and rounding. 1/3 and .333333 are not the same thing. I need to be able to express the trade in the same units on the buy and sell side, not units and inverse units like I do now. That's the heart of the problem. Right?

[johansten]

@skirsch

Prices are not the issue, since they are already defined in fractional form.

1. All orders are expressed as selling X units of asset A for Y units of asset B.
2. A sell order will maximize Y, keeping X fixed.

If you look at the buy order it would take to match a sell order,

buying X units of asset A for Y units of asset B

and rewrite is as a sell order,

selling Y units of B for X units of A

that would keep Y fixed, and maximize X.

This is clearly not what you wanted. You wanted X units of A. Not >= X.

That is the problem.

[skirsch]

@johansten I agree. you're right. thank you.

If you look at the token.io orderbook (which is done in Kelp), you'll see .999 for quantities, e.g., 499.9999 instead of 500. Is that because he didn't use API call with price as a ratio of two numbers and instead just computed the ratio?

If you look at the Ripple API, it is done just like I would have expected: https://developers.ripple.com/rippleapi-reference.html#order What is the genesis of the Stellar design? was it basically what Ripple did at the time Stellar was forked from Ripple? If not, what was the thinking??

[skirsch]

@tomerweller @johansten OK there are basically two issues here. if you keep the current sell only system, then not only do you have the problem @johansten pointed out (you can't express the semantics you want), but you ALSO have the problem the quantity must also be expressed as a ratio. If I place offer as "sell A.B XLM for USD at price X/Y" it looks ok on XLM/USD orderbook, but on USD/XLM orderbook it looks like "buy 1/A.B USD at price Y/X" and this 1/A.B maybe not the same number that you would expect to see, like 0.9999999 instead of 1 or things like that.

So I propose there is a new API endpoint that basically looks like how Ripple (and everyone else in the world) does it: https://developers.ripple.com/rippleapi-reference.html#order

Why not just do that as the external interface? It's going to be less problematic to make this the main interface and translate the current API call to that API call. The problem would be converting the existing orderbooks. Or you could simply use the new approach for new orderbooks and do a backwards convert (rounding errors and all) with existing orderbooks.

The problem you are trying to solve is to allow me to express they buy/sell orders as I normally do and have it act correctly: no rounding errors on price or quantity, and proper semantics (in terms of what is "fixed" X or Y as @johansten wrote above

[MonsieurNicolas]

Ripple tracks buy and sell amounts separately, the price resolution we have is "only" 31 bits:31 bits.

@jonjove has some ideas on how to solve this without changing what is in the order book.

[johansten]

@MonsieurNicolas

As ScottHogge mentioned in his first comment here, it's only a problem in the order matching itself. Any eventual non-marketable part of an order will hit the be put on the order book, and look just as any other offer. No changes to the order book are needed.

[skirsch]

@MonsieurNicolas @johansten I thought we agreed the semantics aren't the same if you just have sell orders. you can't express a buy order of a certain quantity. i don't see how you fix that without changing the order book.

[johansten]

The semantics doesn't matter outside of the order matching. That's where the problem is, i.e., what's intended to be a buy order buys too much. Once any left over volume gets stored as an offer on the books, everything is fine.

[theaeolianmachine]

Hey everyone - the reason that CAP-0006 introduces a new operation is because of the principle that each operation should have a unique effect on the system, along with preserving backwards compatibility with existing operations.

Having a flag on a single operation actually creates more ambiguity around usage the API, since these are very distinct ideas. It's easier to mismanage a single operation with a flag by forgetting to set it - with two distinct operations that can also have their own separate documentation, we feel it's easier to be a consumer of the protocol.

The Rationale for CAP-0006 explains how @jonjove maintained backcompat as well by not changing the orderbook.

Because this is heading towards finalization by the end of this week, if you have any major concerns with this proposal, please let us know.

@johansten @skirsch

[johansten]

Can we not just make the old operation deprecated, and create a ManageOffer-v2, that has a flag? If you don't add a flag to the offers in the order book, I can only imagine the confusion when you have people ManageBuyOffer on sell offers, and vice versa, since there's no way of knowing what is what.

It's ManageOffer{decrecated} + ManageOffer{v2}(buy/sell), vs ManageOffer + ManageBuyOffer

I know which one looks cleaner.

Renaming the old op ManageSellOffer would be better, but still, there's no stopping people from removing/updating buy offers with it w/o modifications to the offers table.

[skirsch]

@theaeolianmachine I agree with @johansten ... the vast millions of people haven't experienced stellar yet and you want to expose them to a nice clean interface which is the ManageOffer{decrecated} (which they will NOT see) and ManageOffer{v2}(buy/sell).

Look at Ripple: https://developers.ripple.com/rippleapi-reference.html#order. Flag.

and in fact, I'd bet that 90% of order management system have a flag for buy/sell.

[MisterTicot]

Edit: Never mind I got it wrong. The price of the offer already provides a second condition that prevent big slippages. Doesn't prevent little slippage though but the additional complexity doesn't worth it.

Ideally manageOffer should accept both sellAmount(currently known as amount) and buyAmount; with the following behavior:

• If neither (sell)amount nor buyAmount are present, fails
• If only (sell)amount is present, keep current behavior
• If only buyAmount is present, behave like CAP-0006 manageBuyOffer
• If both are presents, behave like CAP-0006 if the cost can be kept under sellAmount, else fails.

I know this is a bit tricky, but IMHO the last point is kind of great importance. Here's why:

This is similar to the double requirement introduced in pathPayment, were we set both the final quantity of asset to send and the maximal cost we are willing to pay for it. Being able to set both conditions is important because pathPayment is basically a market order and the cost of this kind of order can raise unexpectedly; especially on a low refresh rate exchange & when dealing with low-volume orderbooks. We don't want ending up paying double price because of missing liquidity, right?

buyOffers are the basis on which market orders can be emulated properly: they have the same property of being able to make the cost raises unexpectedly. There's no possible solution to that problem at software-level, because it is not possible to predict the state of the orderbooks at the time the transaction will be validated. Hence, the protocol must provide this kind of double conditional that makes the transaction fails in case the prices are not the one user were presented with in the UI.

Just to insist, wrongly handled market orders can have dramatic consequences. When you see those candles with big tails on a SDEX exchange interface, this is a market order that slept badly, then you can tell someone got very (very) angry behind the screen. People trading low amount without much care only end up paying 5~20%(!) fees in price slippage and you barely notice on the graph; but when someone is trading big volumes you get those slippages that double/triple the price and you can tell a lot of money has been left on the table. Each of those long tail is one, you can see some every week, so you can imagine how many low-volume trader encounter this problem when you know there are hundreds times more and not always as much experienced. So: I'm really talking about a meaningful user experience issue that need to be dealt with.

Now we can't implement the last point with a new operation, neither can we with a flag. This is why I think extending manageOffer is only meaningful option.

[johansten]

A market order is just an order without a buying/selling limit, and can be emulated good enough with either buy/sell orders by simply selecting a limit that's high/low enough...

The proper way to do a market order is to make the limit optional. I.e, for a sell offer, "sell X units of asset A, for Y units of asset B", just skip Y. In the operation parameters, that would be Price. If there's no price you have a market order.

Re: Flags, I'd make sure it's explicit w/ no default, but that's on an API level... If you're crafting your own XDR I'd assume you know what you're doing.

[MisterTicot]

@johansten

This is great on liquids markets; Else you just happily sending your users in a trap.

[johansten]

@MisterTicot

If you don't know what you're doing, market orders are a trap.

[MisterTicot]

@johansten

Yes I know, but it doesn't have to be that way. Some interfaces are mitigating the risk by cancelling orders if the market moved too much between the time you passed the order and the time of execution; That is if the orders you would've expected to cross are gone.

This can be done by cancelling the order if it can't be filled entirely under a given price.

Maybe I didn't use a right terminology and it is not technically a market order; Either way this is what we need to let people buy assets in safety.

[johansten]

I think you're onto something with adding fill-and-kill/fill-or-kill modifiers to the offers.

I think what you are looking for is fill-and-kill limit orders w/ the buying/selling limit a bit higher/lower than market price: You'll get filled up to your limit price, but if you're only able to partially fill your amount, nothing gets left behind on the order book.

[nebolsin]

If the current goal is to minimize changes to protocol / core, it might be possible to use the existing ManageOfferOp operation with negative amount values to represent buy offers (I believe amount is defined as int64)

---

I agree with @johansten and @skirsch that the currently proposed structure with ManageOffer, ManageBuyOffer and CreatePassiveOffer starts getting complicated, especially when you think about ManageBuyOffer being applied to modify the existing offer from the order book (which is always sell).

Also it seems to drift away from the industry standards like FIX/FAST, SAIL or London Stock Exchange's Millennium protocols. While there're a lot of differences between those protocols, it looks like they all agree on the minimal set of fields required to submit an order:

• trading instrument,
• order type (market/limit/...)
• order side (buy/sell)
• quantity
• price (for limit orders)
• expiration time
• flags for immediate-or-cancel, fill-or-kill, passiveness, etc

I think deprecating ManageOffer/CreatePassiveOffer and replacing them with a carefully designed ManageOfferV2 operation will result in a much cleaner API and easier to understand semantics.

[MisterTicot]

We don't even need to deprecate manageOffer, as we can extend its scope and make the default setting match its current behavior.

[nebolsin]

Maybe I’ wrong, but I don’t think XDR definitions for individual operations are designed for extension. So while it’s possible to add new operation types, structure of the existing operations cannot be changed without breaking the backward compatibility in a bad way (like if you add a single field to ManageOfferOp, all existing XDR data from history archive won’t even parse)

[MisterTicot]

@nebolsin

Ok.

I think I was referring to the API in which we can still use the same operation (name) in a backward compatible way to make things dev-friendly.

I don't think that's a big deal if we have a different XDR object under the hood, or it is?

[theaeolianmachine]

Hey everyone,

This topic has been under discussion for sometime on and off, and ultimately, we've decided to go with the following solution:

• Rename ManageOffer -> ManageSellOffer
• Add ManageBuyOffer

We received a lot of really great feedback regarding the orderbook in general, but at this point, we don't want to fundamentally change too much without making it a major theme of development for the protocol and really giving it the space it needs towards designing a OrderBookV2.

For now, we believe that CAP-0006 fixes a fundamental flaw in the orderbook that won't take up a lot of developer resources, and it's more important to deliver this ahead of a major redesign.

@jonjove — can you go ahead and open a PR and link it here making these changes to the CAP-0006 draft so I can mark it as accepted?

Finally, sorry for the lack of updates here — we're in the final steps of starting our new process for CAPs and SEPs to increase transparency (see #247), and hope that once implemented we can have more public artifacts of where the current status of items are.

[MisterTicot]

That's a good news!

For the record, I made a research about what would lead us to a feature-complete DEX, here's the summary: better-dex

The 2 fundamental steps are:

• Make createPassiveOffer post-only (= fail instead of crossing offers)
• Implement pathDonation

pathDonation is to pathPayment what manageBuyOffer is to manageSellOffer, so I think that's the logical next step. Those pathX operation can be used as fill-or-kill orders for securing market orders against unpredictable price slippage.

Those two changes are straightforward & useful, so I'd advice to consider implementing them soon.

[nebolsin]

@theaeolianmachine While I understand the decision to go with "quick fix" approach, I would love to know why this specific ManageSell/ManageBuy implementation is better than two alternatives proposed in this discussion above:

1. Introduce new CreateOffer(kind: buy|sell) operation, and keep existing ManageOffer.
2. Use existing ManageOffer operation with negative amount to represent buy offers.

I'm particularly interested to hear the arguments against the option 2, because while being a bit "hacky" it is significantly less intrusive comparing to the current proposal. It doesn't require XDR changes, you can already build such transaction and it will currently fail with op_malformed, while instead it could be executed with buy offer semantics (as specified in current CAP-0006). /cc @MonsieurNicolas

[johansten]

+1 on thinking this through a bit more. "Quick fixes" live on forever on a blockchain.

[MonsieurNicolas]

@nebolsin sorry for the late reply.

Introduce new CreateOffer(kind: buy|sell) operation, and keep existing ManageOffer.

We're trying to limit the number of ways to do anything to one. It's very hard and slow to "deprecate" (as in remove) anything at the protocol layer.

Use existing ManageOffer operation with negative amount to represent buy offers.

While this would avoid creating a new operation, it changes the semantics of existing code and can lead to pretty catastrophic bugs in applications. Like you mentioned: code that would fail today would now do something that the developer didn't anticipate.

Stepping back a bit: XDR changes are a good thing.

What we want to do is to ensure that client code (SDKs) break if they encounter a scenario that they were not designed to handle. The safest way to do this is by implementing those safety mechanisms in XDR as we have high confidence that all clients will implement XDR properly.

Using your example of using negative numbers, we would be relying on clients checking that amount is positive before doing something with that transaction. I am pretty sure nobody checks that today. And here, what I mean by "doing something with that transaction": there are many ways to use the network, some people submit transactions, and some people "watch" transactions and perform some action based on the fact that those transactions got processed by the network - this particular problem impacts the later more, I think.

By making this an XDR change, existing code will just break (as in "throw") instead of performing something arbitrary.

Going back to the concerns raised by the first point "why not use CreateOffer(kind: buy|sell)", this can easily be done at the SDK layer. This comes with similar trade-offs than for example the decision in Horizon to expose schema-less JSON instead of XDR where we're trading safety for prototyping speed.

I imagine that some SDKs can be more "strict" vs others on that front: the "mini SDKs" will only expose the minimalist protocol (for the most part this is what we have right now), and you can layer on top of that extensions that expose simplified semantics. It's much harder to do the other way around: if the lower layer is ambiguous or not as strict, we end up with unsecure code at best.

This already happens when people specify a price using floating point numbers.

I would encourage SDK developers to experiment more on that front: think of the stellar-core protocol more as the "CLR runtime byte code" equivalent to the C# language instead of asking everybody to understand assembly language. I'd prefer to see each SDK embrace the semantics of their respective language: for example, I'd expect a C++ SDK to be ultra pedantic, while the perl one would probably be a lot more expressive.

[Synesso]

I like this approach of forcing errors rather than permitting (guaranteeing?) mis-interpretations of data. It is in line with my recently aired concerns about operation results defaulting to 0,0 (create account success) when a preceding operation failed the transaction.

On Thu, 7 Feb 2019 at 09:29, MonsieurNicolas notifications@github.com wrote:

@nebolsin https://github.com/nebolsin sorry for the late reply.

Introduce new CreateOffer(kind: buy|sell) operation, and keep existing ManageOffer.

We're trying to limit the number of ways to do anything to one. It's very hard and slow to "deprecate" (as in remove) anything at the protocol layer.

Use existing ManageOffer operation with negative amount to represent buy offers.

While this would avoid creating a new operation, it changes the semantics of existing code and can lead to pretty catastrophic bugs in applications. Like you mentioned: code that would fail today would now do something that the developer didn't anticipate.

Stepping back a bit: XDR changes are a good thing.

What we want to do is to ensure that client code (SDKs) break if they encounter a scenario that they were not designed to handle. The safest way to do this is by implementing those safety mechanisms in XDR as we have high confidence that all clients will implement XDR properly.

Using your example of using negative numbers, we would be relying on clients checking that amount is positive before doing something with that transaction. I am pretty sure nobody checks that today. And here, what I mean by "doing something with that transaction": there are many ways to use the network, some people submit transactions, and some people "watch" transactions and perform some action based on the fact that those transactions got processed by the network - this particular problem impacts the later more, I think.

By making this an XDR change, existing code will just break (as in "throw") instead of performing something arbitrary.

Going back to the concerns raised by the first point "why not use CreateOffer(kind: buy|sell)", this can easily be done at the SDK layer. This comes with similar trade-offs than for example the decision in Horizon to expose schema-less JSON instead of XDR where we're trading safety for prototyping speed.

I imagine that some SDKs can be more "strict" vs others on that front: the "mini SDKs" will only expose the minimalist protocol (for the most part this is what we have right now), and you can layer on top of that extensions that expose simplified semantics. It's much harder to do the other way around: if the lower layer is ambiguous or not as strict, we end up with unsecure code at best.

This already happens when people specify a price using floating point numbers.

I would encourage SDK developers to experiment more on that front: think of the stellar-core protocol more as the "CLR runtime byte code" equivalent to the C# language instead of asking everybody to understand assembly language. I'd prefer to see each SDK embrace the semantics of their respective language: for example, I'd expect a C++ SDK to be ultra pedantic, while the perl one would probably be a lot more expressive.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/stellar/stellar-protocol/issues/180#issuecomment-461229914, or mute the thread https://github.com/notifications/unsubscribe-auth/AABVYz0fFZWqF42Az7CaSegwE8cOju8vks5vK2VKgaJpZM4W7u7j .

[theaeolianmachine]

Closing this out with the acceptance of CAP-0006. We're really thankful for all the feedback we've received about the orderbook — in due time it will become a larger theme of our development efforts (especially around performance), and we'll continue to keep folks updated as we consider larger design efforts. Otherwise, the intent is for CAP-0006 to be implemented in protocol V11 to solve this problem.

[skirsch]

@theaeolianmachine : Your CAP-0006 above linked to SEP-0006 which is really confusing! typo perhaps? so the text is right, but the link on CAP-0006 points to SEP-0006!

[theaeolianmachine]

Most definitely a typo! Went ahead and fixed it.