Offers do not affect asset balances.

[istrau2]

What I mean by this is that when I create an offer with a selling asset of ABCD, my account's asset balance for ABCD is not diminished (I am referring to the asset balance returned by Horizon for the account, I assume this is the same balance that exists in the db).

This results in an individual being able to create multiple offers that sum to a greater balance than is in their account.

I tested this on the test network as follows:

1. Create an account and fund with 5000 XLM (actually, get 10000 XLM from friendbot and burn 5000 of them).
2. Create two offers:
   a. Bid 4000 XLM for 50 BTC (price of 80 XLM/BTC)
   b. Bid 4050 XLM for 50 BTC (price of 81 XLM/BTC)
   As mentioned above, because the offers do not affect the account balance, offers can be created with a higher total value that is in the account.
3. I accepted the offers from another account (with 100 BTC). What happened is that only 4960 XLM worth of BTC was executed and an ask offer created for the rest of the BTC.

I think this behavior is buggy for several reasons:

1. It is VERY not intuitive for a user to see offers for 100 BTC, accept the offer, and have it only partially execute because some of the offers were vapor offers. Which leads me to point number 2)…
2. This allows users to manipulate markets. A user can create fake buy or sell walls (and even pop them themselves later) without the funds necessary by simply repeatedly creating offers with a value that is less than their account balance.

I think that an effect of manageOffer should be an adjustment to the account's asset balance.

[MonsieurNicolas]

yes, this is by design. It allows for people to sell an asset in exchange for any combination of other assets.

For example, if I want to sell my 100 Bitstamp BTCs for some ETH (and I have accounts with Bitfinex, Poloniex and Kraken), I would create an account with 100 BTC.Bitstamp and the following offers:

What I sell	What I buy
100 BTC.Bitstamp	ETH.Bitfinex
100 BTC.Bitstamp	ETH.Poloniex
100 BTC.Bitstamp	ETH.Kraken

Then, even though I have 300 BTC.Bitstamp up for sale, the total amount will be limited by the account to 100 and will get some combination of ETH.Bitfinex/Poloniex/Kraken.

[istrau2]

@MonsieurNicolas Yes, that advantage was explained to me by @jedmccaleb.

Still, I disagree with the design decision because, in order to gain the advantage you describe you basically sacrifice the integrity of the order book. I mean look at all those exchanges you mention (and any other existing exchange), do any of them allow you to create multiple offers with the same funds?

If stellar ever really takes hold and starts getting used for trade at a decent volume, you can bet that people will start exploiting this to manipulate orderbooks (especially given how cheap it is to create an order). The orderbooks will become meaningless.

[MonsieurNicolas]

I don't think those are exclusive: at the end of the day the protocol has to enable a multitude of scenario.

I don't think people should care about individual offers as end users of the order book. I think what you are advocating for is for new views for the order book (enabled at a higher level, like Horizon) to give a sense of liquidity for a particular asset (that does the proper aggregation in the background), potentially using various path configurations.

[istrau2]

@MonsieurNicolas @jedmccaleb Actually, I've been thinking about this but forgot to post.

I think I kind of put my finger on the real problem here IMO. I think the real problem is that a user can enter in multiple orders less than their account balance for the same market summing to greater than their account's balance.

Actually, in the scenario that @MonsieurNicolas described above, the orderbook is actually correct in the sense that all the depth in the orderbook is real (if you put in an offer it can actually execute against all those orders). The only issue is if multiple offers accept the same funds in different markets in the same ledger. In that case there will be race condition of sorts and a user who is attempting to accept the offer might experience unexpected effects. However, that is a rare scenario and possibly acceptable IMO.

What is not acceptable is that a user can create actual fake offers by entering multiple orders in the same market. For example if I have 1000 XLM I can create bids for as much BTC asset (i.e. a single issuer) as I want by repeatedly entering offers for 999 XLM. I think this is a serious flaw in the protocol which can be used to manipulate orderbooks. I can't think of any advantage to allowing users to do this.

[jedmccaleb]

It isn't that this is desired behavior. It is just that it is expensive to prevent. I think if someone really cared they could do the processing themselves to clean up the orderbook. Maybe someone will do this as a service. Basically it seems better to offload this processing outside of the validators.

[istrau2]

@jedmccaleb Why is it so expensive (I believe you, just for curiosity's sake)? Cleaning up the orderbook as you suggest is quite an intimidating task. Are you suggesting to keep a second db that is kept in sync (with the exception of the exclusion of orders that violate this constraint) or are you suggesting that someone modify their version of stellar-core to disallow this kind of thing (wouldn't this interfere with consensus?)

[MonsieurNicolas]

note that there is already a second much more feature rich database managed at the Horizon layer, this one can be forked & modified without any impact on consensus (as it's just derived data from what is in core).

[fiatjaf]

What is not acceptable is that a user can create actual fake offers by entering multiple orders in the same market. For example if I have 1000 XLM I can create bids for as much BTC asset (i.e. a single issuer) as I want by repeatedly entering offers for 999 XLM. I think this is a serious flaw in the protocol which can be used to manipulate orderbooks. I can't think of any advantage to allowing users to do this.

Besides psychological effects it may have on other parties in the market, I don't see how doing that can cause a problem to anyone. On stock exchanges everywhere it is common for parties to create and remove and move orders to "manipulate the book". This is not fruitful to anyone, I think, and the fact that Stellar allows anyone to create infinite orders makes it meaningless. It's a win-win scenario.

[mfontanini]

I'm curious of what happens if I posted 2 sell orders for XLM -> some anchor's BTC and I only have funds to fulfill one of them? Assuming the first order is fulfilled I imagine the second one is still going to be there even though it can't be fulfilled due to a lack of funds on my side, is that true? What happens if someone creates a buy order that matches my second one? Will my order be removed at that point or is it still going to be left as an orphan that can never be fulfilled in the orderbook?

Also in this scenario, what's the difference between my second order being able to be filled partially/completely? If it can b, will the order be actually partially filled and then removed from the orderbook?

[jedmccaleb]

mfontanini: It will leave the 2nd order there until some other order interacts with it. At that point the order will be cleaned up.
If there is just say 1/2 the lumens left over for the second order, then these will be sold.

[istrau2]

Is there anything preventing changing the protocol temporarily (to disallow offers using the same asset) until such a time that a higher level process is developed that is capable of reliably trimming the orderbook?

[efritze]

Bumping this issue.

Apps that permit market orders, or swaps, might give users a "fake" amount of total available assets they can effectively trade.

This will also affect path payments, which to my understanding also depend on market orders.

Users will be confused and disappointed when their orders are not completely fulfilled.

Orders change all the time, so maybe the apps themselves should do extra checks. But the SDEX shouldn't make this harder than necessary.

[efritze]

I am not fully aware of how core works internally. However, I'd like to suggest an extra data field for trustlines.

I believe each trustline has an asset_code, and asset_issuer, and a balance. I'm suggesting a new field called reserved_balance. Every time you place an offer, the reserved_balance for the asset you are selling increases. Your effective asset balance will thus be: (balance - reserved_balance). You cannot send more than (balance - reserved_balance) to another account, and you cannot place an extra offer for more than (balance - reserved_balance). The native asset will also have to consider number of sub-entries.

As mentioned before, I am not fully aware of how core works internally. So I have no idea if the above proposal will be very expensive on the ledger.

[ghost]

I started a project for effective Stellar orderbook calculation. It's already useable (albeit not battle-tested) but plan on adding more functionality. Perhaps adding detection of more forms of orderbook manipulation. You can follow progress here: https://github.com/lenondupe/dedupe

[nukec]

Hi. This is a huge issue at the moment, as it's very easy to manipulate market with massive spam orders. If I have 500,000 lumens and I put my sell order 5 times in a row, it counts five orders, one with amount I have and four orders with amount I don't have, which is totally illogical. This, you get it, and it's expensive, but how can we have legit order book if we can spam orders with coins we don't have?

[ghost]

It's a real problem but the issue is not with the protocol as the behaviour is by design and with good reason. This ticket is invalid in my opinion therefore and should be closed. The real problem is that horizon has an endpoint that provides invalid orderbooks. Given the Stellar design, the orderbooks must be calculated with consideration of the balances of the market participants. This is not very hard to do but requires a little bit of processing and perhaps some smart caching. Simply returning all the offers in the database as horizon does currently does not hold any useful information for a trader and is certainly useless as an orderbook on an exchange. So the endpoint must either be removed entirely so as not to mislead or fixed to deliver proper data.

[istrau2]

I agree, ideally this should be fixed at a higher level.

[MonsieurNicolas]

It seems that people have started building around this limitation to avoid being stuck on this (which I appreciate).
Yet, I would like to continue discussing a potential change on that front.
I don't have strong feelings on this shadowing problem, but at the end of the day if the complexity pushed upwards in the stack is so high, it's a signal that we can't ignore :)

Also, talking to others, I realized that the use case of allowing to trade over multiple markets is, with the current set of functionality actually not practical as most issuers require KYC and in order for this to work a temporary account needs to be created (that acts as the "limit" for the overlapping offers).

[ghost]

@MonsieurNicolas Yes it would be very welcome if it were addressed down the stack at core or horizon level. Perhaps the most convincing argument would be for effective path finding and the ability to provide accurate quotes for path payments. I assume the find payment paths endpoint on horizon does not take these ineffective orders into account either.

[nullstyle]

I assume the find path payments endpoint on horizon does not take these ineffective orders into account either.

fyi: you assume correctly

[MonsieurNicolas]

Here is a strawman solution that we can discuss.

I think that the core change on this side is actually fairly small:
we need to keep track of the amount of "committed assets" (for XLM and all other assets) which represents the sum of amounts in offers for each asset.
With that in place, we can then enforce a policy that looks a lot like the "base reserve" for XLM that exists today, but that takes into account the amount secured as "committed assets".

Implementation wise:
in the database, we can simply add a new column for each account entry and trust line entry - we have to update those rows anyways when offers get created or modified, and when performing payments. I think the overhead is minimal to keep those sums in sync.

Then, there is the question of XDR: and here we do not strictly need to include this data in the ledger entries as the data can easily be reconstructed from the outstanding offers, so my inclination is to not bother with a change of the ledger entries at this time (we can always add those fields later in extensions). By not changing the XDR now, we avoid churn in the SDKs etc.

The downside with not including this in the XDR is that we make apply bucket more complicated (as it needs to compute those values as it applies buckets), but this complexity seems quite marginal and the overhead is only occurred when catching up.

Finally, there is a question on how to transition to this new protocol.

When this gets turned on, what do we do with existing offers?

I can see a couple possibilities:

1. delete all offers from accounts if they can't commit to the outstanding offers
2. keep all offers - some accounts are "over limit" (in a similar way than what happens when raising the base reserve), and the only way to get those unstuck is to either send them the corresponding assets or that somebody touches the offending offer(s) (that gets deleted). In this context, being stuck by running out of XLM above reserve makes it that the account can't even delete those (potentially) bogus offers.

I much prefer option 1 as it doesn't create a situation where we end up with a bunch of accounts that are stuck (with the only way to get them "unstuck" is to do something like #62 ).
The challenge with that option is that right now ledger entries only get modified by transactions - this option requires us to figure out a way to express changes happening "out of band", a concept that seems useful.

Also, another issue that may arise from this change is that it may increase the chance of running into stuck accounts situation ( again, #62 ), as it makes it easier to commit to XLM with "ManageOffer", in a way similar to what can happen today with payment.

[ghost]

I can't think of any objections to this proposal although I do like the idea of being able to post multiple offers in different markets that together exceed the amount on offer. But as far as I can tell this ability is very atypical for exchanges. What you describe seems a cheap and simple way to solve the problem.

[istrau2]

@MonsieurNicolas I think solving this in the core protocol is probably the most expedient solution. One thing I didn't understand from your proposal is: Are you suggesting that the offers will be rejected by the network if the "commited amount" resulting from the new offer is greater than the balance (or balance minus reserve in the case of XLM) amount in that asset?

Also, I also like option 1) better for the transition. Seems simpler (and simpler is usually better).

[istrau2]

@MonsieurNicolas @jedmccaleb What is the status on this? Has a decision been made on whether SDF is going to work on solving this on the protocol level? This is becoming a bigger and bigger issue (the orderbooks are full of vapor offers) and we (Stellarport) need to do something about it. I would like to get a clearer idea on status/eta regarding this before we decide what to do. Thanks.

[MonsieurNicolas]

yeah pretty sure we'll have this figured out by the Summer @istrau2

[MonsieurNicolas]

We've posted https://github.com/stellar/stellar-protocol/blob/master/core/cap-0003.md ; we're looking for feedback. Some gotcha in there around what happens with certain pre-upgrade (smart contract-y) setups that will be broken.

[MonsieurNicolas]

On the topic of using INT64_MAX as the limit for native assets, is that the right thing @jonjove ?

I think the goal is to guarantee that offers in the books are "immediately executable in full" and I don't think this is good enough:
to illustrate, imagine creating an offer (or offers) to sell a token for XLM at a price of 2^31-1 (or something very high like that), then the actual amount of X that can be traded is much smaller than the number of X that this account has ; in this case the amount can only be up to about 48 X as 48*price is the total number of XLM in circulation. If we were using INT64_MAX as the maximum, then the offer would be adjusted to a much higher number (minimum of 2^32 and amount of X that the account has), and this is very far from reality. The fact that this offer can, practically speaking, only be taken partially seems to be a different issue.

Maybe this is a statement about how we adjust offers in the order book, but I think it has to be covered properly as part of CAP003. If we want to exempt native assets from being IEIF, it's a different question.

[johansten]

Don't like.

As long as all offers are individually backed they are all liquid as long as no-one takes an offer. That's a good thing. When an offer gets taken, first then do you have a potential liquidity problem, and that is what needs to be taken care of.

This will cripple the DEX.

[johansten]

@MonsieurNicolas

How would a new type of order work for you, one which as a unit is IEIF, but which contains multiple actual orders? Orders that are linked, and can be removed when the first one is taken?

[MonsieurNicolas]

As long as all offers are individually backed they are all liquid as long as no-one takes an offer. That's a good thing

Yes and no: current implementation does not guarantee at all that individual offers are backed. As a consequence even submitting a single transaction can lead to unexpected results right now (pathpayment is known to be a pain to work with).

If you read the CAP0003 document you will see that right now we have many offers that are executable only partially or not at all in the order book.

How would a new type of order work for you, one which as a unit is IEIF, but which contains multiple actual orders? Orders that are linked, and can be removed when the first one is taken?

Yes, this is definitively something that we can do in the future on top of the IEIF semantics and that we considered as part of this solution, this is not the last change to the order book :) In fact things like linked offers allows to do a lot more than just that.

First step with CAP0003 is to ensure that we can easily reason about the state of the order book, stop pushing unnecessary complexity to the higher layers, and convert the existing APIs to be consistent with the IEIF semantics. As there is no concept of "linked offers" right now, the APIs are basically equivalent to dealing with "batches of 1 offer".

[jedmccaleb]

Addressed here: https://github.com/stellar/stellar-protocol/blob/master/core/cap-0003.md