XRPLF / rippled

Decentralized cryptocurrency blockchain daemon implementing the XRP Ledger protocol in C++
https://xrpl.org
ISC License
4.48k stars 1.45k forks source link

Proposal: All Offers Expire #3219

Open mDuo13 opened 4 years ago

mDuo13 commented 4 years ago

One of the long-term challenges for the XRP Ledger is managing the unbounded growth of ledger data size. History is one thing—it's inevitable that full historical data will grow indefinitely—but the size of the current state also has a tendency to get larger under normal usage as it accumulates ledger objects that no one wants or needs anymore—in a word, cruft.

One way we can get rid of some cruft is to make it so that all Offers in the decentralized exchange eventually expire, after which point they can be removed. Right now Offers only expire if their creator explicitly sets an Expiration time on them. A simple change that would improve the situation is to change the OfferCreate transactor so that if you omit the Expiration time, it gets set to a sensible default value such as 90 days. This alone could dramatically reduce the "incidental" introduction of cruft in the ledger.

There are lots of issued currencies in the XRP Ledger that are no longer used and as long as the XRP Ledger continues to exist it's likely that more issued currencies may pop up and die off. It's pointless to keep offers for these currencies around when they're no longer in use. There are also lots of dormant accounts that may never be used again; and even though it may be too hard to know which accounts can be reasonably deleted, there should be no problem with deleting those accounts' offers even in active markets. To my understanding, it's pretty normal for an exchange to have a maximum lifetime for any single offer. Most of the time the market is expected to move often enough that you don't want an offer to hang around that long.

We could also look at setting explicit upper bounds for Expiration so you can't, say, set it to expire at MAXINT to make an effectively immortal offer. I think 1 year seems like a reasonable "longest offer term" value. If you can't be bothered to update your offers annually I don't know what you're doing. I guess a possible use case for immortal offers would be if you issued a currency and then intentionally blackholed your account?

intelliot commented 4 years ago

I would be fine with setting the upper bound for Expiration to 90 days. For example, Robinhood has only two options for "Time in Force". Orders expire either "Today" (at after-hours close) or in 90 days ("Good till canceled" is the term used -- even though they actually expire in 90 days unless cancelled before that).

One concern is that "old" offers are difficult to find (so that they can be removed). One approach for this could be to have a new transaction type that can only delete expired offers from the books. Similar to finishing an escrow that has passed its FinishAfter time, anyone (a "good samaritan"?) could help reduce ledger cruft by clearing out expired offers in this way.

MarkusTeufelberger commented 4 years ago

This proposal spams the transaction database vs. the ledger state. I'm not sure that this is a good trade-off, but it might be worth it after all - RAM is more expensive than disk at the moment.

I share Elliot's concern about the actual cancelling mechanism, though I find the "good samaritan" approach of Escrows already rather kludgy - and with offers there's nearly no incentive for anyone other than server operators to actually remove outdated offers.

I also have concerns about "magic values". What makes 90 days, a year or a decade (or 10 minutes) special or worthy enough to write into the core of the protocol, force cancelling an offer after that time?

If you are concerned about ledger state growth, a "swap" mechanism that moves rarely used portions of the state trie on slower storage might be a more general solution that doesn't need such invasive measures.

mDuo13 commented 4 years ago

That's a good point that this hits the transaction database harder, since it means there will probably be more metadata for Offers that have been canceled. You mentioned that disk is cheaper than RAM "at the moment"—but realistically, disk will always be cheaper than RAM; if it weren't, people wouldn't use disk. But more importantly, the transaction database doesn't suffer from the same scaling problem as the ledger state. Full history is not necessary to process any future transactions; full state is. So it's absolutely achievable (even today!) to archive transactions beyond a certain age, whereas the tech to find and archive portions of the state trie that aren't in use is quite a bit more complicated and may not work at all for some possible ledger states.

I'm sympathetic to the issue of magic numbers, but at some point when designing a system you just have to make choices so you aren't paralyzed by indecision. I could see the maximum offer expiration being set on a votable parameter like the fee and reserve settings, but I honestly don't think it's important enough. If people later decide they want more flexibility in max offer expiration, a follow-up amendment can change it.

As for how the expired offers get deleted from the books: worst case scenario, no one does anything and they sit around untouched, which is exactly the current status quo. So while we can debate possible improvements, I don't think that should be a showstopper for implementing global offer expiration, because it's still an improvement in terms of keeping the ledger size in check.

On that subject, though, here's roughly how I would make the cleanup transaction work.

I have a feeling this transaction should have a higher than normal cost or have some other restrictions. It's likely a very intensive transaction to process in both RAM and CPU. Worse, running the transaction multiple times on the same book in quick succession provides little to no benefit but costs as much or more work as doing it on a very "cluttered" book where it removes a lot of expired offers.

¹ I considered making a pseudotransaction that servers could automatically propose, but I couldn't figure out an elegant mechanism for how they'd know which markets to submit cleanup transactions for. The nice thing of making these kinds of cleanup functions "normal transactions" is that someone can step up and handle the chore of doing it without burdening "ordinary" servers in the network with keeping track of when and how to do it, so it could be done entirely by one entity who's very good at it. The downside, of course, is the Tragedy of the Commons. Theoretically the community could donate to someone who does this sort of work to compensate them for the costs of inspecting the ledger for things that need cleanup, or something like that. Basically at some point the ledger may need a formal waste management utility or something but its structure and management don't need to be written into the XRPL protocol.

² There are likely some weird edge cases around declaring offers unfunded when the same person has placed offers at different depths of the same book. For example, a trader might have a small "front" offer at a cheap rate and a larger "backstop" offer at a more profitable rate. I'd like them to have the flexibility to keep the backstop offer on the books in case they cancel or replace their cheaper front offer. To achieve that, I think I'd like the CleanUpBook transaction to consider an offer unfunded only if the owner has none of the offered currency, which should be evaluated independently for each offer.

RareData commented 4 years ago

I would prefer an approach using familiar concepts:

  1. Change the OfferCreate transactor so that if you omit the Expiration time, it gets set to a sensible default value (90 days), as proposed by @mDuo13
  2. Change the OfferCancel transactor to include a mandatory Owner field. Offers and Escrows already share the OfferSequence field. Allow expired offers to be cancelled by anyone, just like escrows.

On another note, why are Offers worthy of a dedicated cleanup transaction, but not PaymentChannels, Checks and Escrows?

mDuo13 commented 4 years ago

I like the idea of OfferCancel having an Owner field so you can cancel other people's (expired/unfunded) Offers¹. It could be optional, and if omitted the transaction sender is the owner (just how the transaction works today).

¹ I also like the idea of having a transaction I can use to cancel other people's (non-expired, funded) offers. But that's only for me, not the rest of y'all. 😜

ximinez commented 3 years ago

Another idea: Add a flag to the OfferCreate and/or OfferCancel transactions that tells the transactor to do the proposed CleanUpBook action. If the flag is set, the base fee would be increased to pay for the extra work. The users working with that book would probably be the most incentivized to keep that book clean and usable. In combination with the other OfferCreate flags and non-crossing values, this could clean up abandoned books, and we wouldn't need a new transaction type.

yxxyun commented 3 years ago

If you want to save RAM, how about use AMM Liquidity Pool to instead of order book.

scottschurr commented 3 years ago

I saw this discussion and got curious about how many offers actually hang around for over 90 days. So I took a sample. I grabbed ledger 64177830 which is pretty recent. And I grabbed ledger 62119762, which is just a hair over 90 days old. Then I extracted the indexes of the offers in both ledgers and compared them to see how many offers over 90 days old were in the recent ledger. Here were the results:

Ledger        Offers          Offers over 90 days old        Percentage of old offers
62119762       37176
64177830       30969                            23765                             77%

So, assuming I did all the steps right, it turns out that 77% of the offers in ledger 64177830 are over 90 days old. That's substantial. Those offers may have changed in the last 90 days, but at a minimum the offers have been present in the ledger that long.

I also have to agree with @RareData that if we're going to take on the problem of stale ledger data we should make sure we prioritize the data types to get the most space savings. Offers may not be the biggest offenders.

mDuo13 commented 3 years ago

Thanks to work by NixerFFM, we can now actually get the counts of objects in the ledger and their space: https://xrpldata.com/api/v1/ledgerdata

Based on this, a single recent ledger by data size consists of accounts (47%), trust lines (31%), owner directories (16%), and everything else is margin. Offers are 1% with another 0.7% in order book DirectoryNodes.

So, while we probably could save a few megabytes by doing this, it's clearly not the place to get the most savings. In the long term, I still think setting limits on Offers will be necessary, and it might be easier to put into practice than some of the other places because there is precedent in the exchange industry and it's easy enough to re-create them, whereas it would certainly be more controversial to purge unused accounts or even trust lines.

intelliot commented 2 years ago

Given the relatively small amount of open offers, and the fact that an offer already ties up an owner reserve (this serves as an incentive for offer creators to cancel unwanted offers), I think there is minimal benefit to having all offers expire. There's also a good argument to be made that it's harmful since it reduces liquidity on the DEX (and more liquidity is more important than reducing ledger size, at least for now).

I vote for closing this issue. "not a bug / won't fix"

Of course, we can re-open or revisit if the size of open offers becomes a problem in the future.

RareData commented 2 years ago

One of the most used XRPL DEX interfaces, XRP Toolkit, enforces a max expiration time of 1 year. This is an issue that can be solved off chain.

On Thu, 15 Jul 2021, 21:29 Elliot Lee, @.***> wrote:

Given the relatively small amount of open offers, and the fact that an offer already ties up an owner reserve (this serves as an incentive for offer creators to cancel unwanted offers), I think there is minimal benefit to having all offers expire. There's also a good argument to be made that it's harmful since it reduces liquidity on the DEX (and more liquidity is more important than reducing ledger size, at least for now).

I vote for closing this issue. "not a bug / won't fix"

Of course, we can re-open or revisit if the size of open offers becomes a problem in the future.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ripple/rippled/issues/3219#issuecomment-880956476, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKSEFCT2DQ4YXSBN2P4GTVTTX4ZJZANCNFSM4KE62L6A .