server/db: Update lot sizes when necessary.

[JoeGruffins]

server/db: Update lot sizes when necessary.

No longer panic when lot size changes. If lot sizes changed, update them
in the db. Then, flush all persisted orders belonging to that market.

depends on #1202
closes #1174

[JoeGruffins]

Something wrong with the second commit still. Is config cached? Also not sure the best placement, or if this is racing for something.

[chappjc]

Thanks for this @JoeGruffins! I'm gonna review this first thing.
The test failure just looks like TArchivist needs an update.

[chappjc]

DB setup looks good. I think we need to do the actual book purge in a different spot though, before (*Market).Run and probably even before NewMarket, where book orders get loaded and coins locked and checked and other stuff.

[chappjc]

This would be the place where we might address hot/live market config changes that require market suspend/resume without shutdown. In this PR we just want to deal with dex startup, not resume, detecting lot size changes.

There are a few ways to approach that, but each would use storage.FlushBook before NewMarket to avoid ever loading incompatible orders and locking coins etc. only to immediately undo it with Market.PurgeBook

1. In the DEX manager (server/dex.NewDEX), call storage.FlushBook before even constructing the Markets. i.e.

// server/dex/dex.go, after db.Open, before NewMarket

// Before creating markets, purge books for markets where lot size changed.
for _, staleMarket := range storage.MarketsNeedOrderPurge() {
	base, quote := assetsIDsForMarket(staleMarket) // find it in cfg.Markets, ignoring case
	storage.FlushBook(base, quote)                 // TODO: log and check err
}

OR

2. Modify NewMarket to do the storage.FlushBook rather than storage.BookOrders. The need to do this could be communicated via the storage.MarketsNeedOrderPurge method or more simply as a flag to NewMarket in the market.Config struct. i.e.

// server/market/market.go in NewMarket

// Load existing book orders from the DB, or purge them if instructed.
base, quote := mktInfo.Base, mktInfo.Quote
var bookOrders []*order.LimitOrder
if cfg.FlushBook {
	storage.FlushBook(base, quote) // TODO: check error and log counts of unbooked orders
} else {
	var err error
	bookOrders, err = storage.BookOrders(base, quote)
	if err != nil {
		return nil, err
	}
	log.Infof("Loaded %d stored book orders.", len(bookOrders))
}

OR

3. Simplest, just clear the book in NewArchiver (also before calling NewMarket). For that, NewArchiver itself would call FlushBook. i.e.

// server/driver/db/pg.go in NewArchiver

// Ensure all tables required by the current market configuration are ready.
purgeMarkets, err := prepareTables(ctx, db, cfg.MarketCfg)
if err != nil {
	return nil, err
}
for _, staleMarket := range purgeMarkets {
	mkt := mktMap[staleMarket]
	if mkt == nil { // shouldn't happen
		return nil, fmt.Errorf("unrecognized market %v", staleMarket)
	}
	unbookedSells, unbookedBuys, err := archiver.FlushBook(mkt.Base, mkt.Quote)
	if err != nil {
		return nil, fmt.Errorf("failed to flush book for market %v: %w", staleMarket, err)
	}
	log.Infof("Flushed %d sell orders and %d buy orders from market %v with a changed lot size.",
		len(unbookedSells), len(unbookedBuys))
}

With this last option, there'd be no need for the marketsNeedOrderPurge field or the call-once MarketsNeedOrderPurge method.

To me, the question is a philosophical: should the decision to purge booked orders with a newly-incompatible lot size be the job of DEX manager logic (option 1), the Market constructor (option 2), or the Archiver constructor (option 3). I'm leaning toward #3 since it's the Archiver that knows when lot size changed and booked orders are no longer valid.

I think any of these approaches would accommodate a --allowlotsizechange flag, if we wanted to make it harder for an operator to mess up and accidentally purge a book.

[JoeGruffins]

Thank you for the great comments.

In this PR we just want to deal with dex startup

Oh! The issue does say that. So, suspend persisted, shutdown completely, change markets.json manually, and startup.

[chappjc]

There sure are a lot of ifs ands or buts in this.

You got the DB update of lot_size down just right.

Just needs a little tweak to the purge call if the operator forgets to purge before shutdown. 👍

[chappjc]

That reminds me I need to carefully review Swapper functionality w.r.t. lot size (shouldn't be any, but I don't recall), otherwise it's going to be a burden to change lot size with active swaps.

[JoeGruffins]

Went with your third option, which doesn't require any changes outside of the database code.

[JoeGruffins]

Actually testing with active swaps, completed swaps, and booked orders, doesn't seem to cause any errors.

[chappjc]

account discovery pr #1201 is merged. Also the btc-fee PR is fixed up and rebased.

Would this PR be messy based on master instead of btc-fee? We can keep it on btc-fee assuming that gets in pretty soon, but this PR looks good to merge now.

EDIT: it wasn't too bad on master. With your branch checked out I did git rebase -i --onto master HEAD~2 updatelotsizes, squashed in "Add doc." and resolved one minor conflict in system_online_test.go with prepareTables having a new output, but that was it.