auth,book,market,ws: Unbook MIA users orders after a timeout

[chappjc]

When a user disconnects with booked orders, this gives the user a certain time to reconnect before all of their orders across all markets get unbooked (revoked). This time is set to broadcast timeout.

dex/ws and server/comms

Create a Done() <-chan struct{} method so consumers like server/auth can actually know when the connection died. Previously, a ping timeout or a clean client-initiated disconnect could never make the AuthManager aware of the connection status change.

server/auth

Make AuthManager aware of underlying wsLink drops with the (*wsLink).Done method. When successful 'connect' request registers the user connection in addClient, it begins a goroutine waiting on the channel from Done, which then triggers removeClient.

removeClient creates an unbook timer for the user. If the user does not return by the time the timer fires, it unbooks their orders. If the user does return, addClient stops the timer.

Care is taken in addClient and removeClient to deal with reused links and new links for already-connected users. Reused links simply update the clientInfo map entries. New links for already-connected users also disconnect the old link but prevent removing the clientInfo entries in the new link.

Create UserUnbooker func(account.AccountID) and MiaUserTimeout time.Duration config fields and corresponding AuthManager fields. The unbook function is called from the unbook timer created by removeClient.

Add (*AuthManager).ExpectUsers(users map[account.AccountID]struct{}, within time.Duration) to be used after AuthManager construction and before Run to specify a list of users with booked orders who are expected to connect within a certain time or have their orders unbooked. Note that this list is not specified to the constructor since we are using the (*Market).Book method as a convenience (and authoritative source) for booked orders and their owners, and Market requires the AuthManager on construction. Neither subsystem is running at this point, so alternatively NewMarket could be provided a pointer to an uninitialized market that is subsequently set-up.

dcrdex startup with 2 booked orders, dexc not running and thus not reconnecting:

[INF] MKT: Loaded 2 stored book orders.
[DBG] MKT: Locking 1 base asset (42) coins.
[TRC] MKT:  - order d19d4ab05928f0a93f163eea28c1f7f9738001b076aa53052f9eef1e4d137664: [84396bc15e7158e9a5504be3cdf75d7d1990356f65e857a09cf2dd8ba0b3b4b400000001]
[DBG] MKT: Locking 1 quote asset (0) coins.
[TRC] MKT:  - order 78dbde722f0a87571c57e048310600ad34106ee121943e18367af2e4167d61f3: [863e8175747ad0015563be2ec0f3bad89673f33e853b1bc9be58477e0362e25700000000]
[DBG] AUTH: Expecting 1 users with booked orders to connect within 40s     # set with --miatimeout=20s
...
[TRC] AUTH: Unbooking all orders for MIA user 366a30a2743028d3df90858b6d7bf19639f8e3689577f76da54b9d9ac4f3a247
[INF] MKT: Unbooked 2 orders (1 buys, 1 sells) from market dcr_btc from user 366a30a2743028d3df90858b6d7bf19639f8e3689577f76da54b9d9ac4f3a247.
... <user comes back, too late>
[DBG] COMM: New websocket client 127.0.0.1
[TRC] COMM[WS]: Starting websocket messaging with peer 127.0.0.1
[TRC] AUTH: User 366a30a2743028d3df90858b6d7bf19639f8e3689577f76da54b9d9ac4f3a247 cancellation rate is now 100.00% (6 cancels : 0 successes). Violation = false
[DBG] AUTH: User 366a30a2743028d3df90858b6d7bf19639f8e3689577f76da54b9d9ac4f3a247 score = 0: 0 (violations) + 0 (0 preimage misses) - 0 (0 successes)
[TRC] AUTH: 2 results for 2 requested order statuses, acct = 366a30a2743028d3df90858b6d7bf19639f8e3689577f76da54b9d9ac4f3a247

dexc started up after the timeout:

[INF] CORE: loaded 2 incomplete orders
[DBG] CORE: Authenticated connection to 127.0.0.1:7232, 0 active orders, 0 active matches, score 0
[DBG] CORE: Requesting statuses for 2 orders from DEX 127.0.0.1:7232
[WRN] CORE: Order d19d4ab05928f0a93f163eea28c1f7f9738001b076aa53052f9eef1e4d137664 updated from recorded status "booked" to new status "revoked" reported by DEX 127.0.0.1:7232
[WRN] CORE: Order 78dbde722f0a87571c57e048310600ad34106ee121943e18367af2e4167d61f3 updated from recorded status "booked" to new status "revoked" reported by DEX 127.0.0.1:7232
[DBG] CORE: notify: |POKE| (dex_auth) Orders reconciled with DEX - Statuses updated for 2 orders.
... <tick>
[INF] CORE: Retiring inactive order 78dbde722f0a87571c57e048310600ad34106ee121943e18367af2e4167d61f3 in status revoked
[DBG] CORE[btc]: returning coins [57e262037e4758bec91b3b853ef37396d8baf3c02ebe635501d07a7475813e86:0]
[INF] CORE: Retiring inactive order d19d4ab05928f0a93f163eea28c1f7f9738001b076aa53052f9eef1e4d137664 in status revoked
[DBG] CORE[dcr]: returning coins [b4b4b3a08bddf29ca057e8656f3590197d5df7cde34b50a5e958715ec16b3984:1]

server/market

Add (*Market).UnbookUserOrders(user account.AccountID) for unbooking all of a users orders. This uses the server/book changes described below to unbook the orders from the in-memory book, unlocks the funding coins, revokes the orders, and notifies orderbook subscribers.

server/book

Add (*Book).RemoveUserOrders and (*Book).UserOrderTotals.
Add (*OrderPQ).RemoveUserOrders(user account.AccountID) (removed []*order.LimitOrder) and (*OrderPQ).UserOrderTotals(user account.AccountID) (amt, count uint64).
Add a userOrders map[account.AccountID]map[order.OrderID]*order.LimitOrder to OrderPQ to facilitate the new methods.

server/coinlock

Add (*AssetCoinLocker).UnlockOrdersCoins(oids []order.OrderID) for batch unlocking of order coins.

server/dex

Create the userUnbookFun dispatcher for the AuthManager.UserUnbooker. This dispatcher calls the new (*Market).UnbookUserOrders method for each market.

[JoeGruffins]

Noticed while reviewing this should be proof.IsRevoked():

dcrdex/client/core/trade.go

Line 594 in 33c531a

proof.RefundCoin, proof.Script, proof.IsRevoked)

Not totally related but I can get an err log on the server by disconnecting during an epoch:

[ERR] AUTH: Send requested for unknown user f7d86c5fd3a442451e6390011b2744d766fadc2520f12307ff4983419c84fd8f

I don't know if it should be error level. Not important though.

Also not related. The placement here seems off. Should be 2 (1 preimage misses) I think:

[DBG] AUTH: User f7d86c5fd3a442451e6390011b2744d766fadc2520f12307ff4983419c84fd8f score = 1: 0 (violations) + 1 (2 preimage misses) - 1 (1 successes)

Warning after completed trade that seems out of place. It only happens if I make a match, restart the client, and continue as normal:

[INF] CORE: Retiring inactive order 1258dae19b956c2f32ab22355656a3aec3ea60ee61fbdeda16682dadd2cbe47e in status executed
[INF] CORE: Retiring inactive order cc672c57012abaaf5fbe1937de4f609a92f81fa1eb190b19fe2c0d4e19fb3392 in status executed
[WRN] CORE: Unable to return btc funding coins: cannot return zero coins

I can't find any problems with this pr however.

[chappjc]

@JoeGruffins Good catches. The proof.IsRevoked function thing is resolved in #706, so letting that ride here. Just fixed the logging and comment issues.

I've noticed the [WRN] CORE: Unable to return btc funding coins: cannot return zero coins issue lately too. We need to look into that. Seems like it's trying to unlock nothing or it's already unlocked and we can ignore the error from lockunspent. The WRN is likely to rattle users.

[buck54321]

Looks good on a first pass. I see you're preparing to solve #712 too.

[chappjc]

Back to draft while implementing revoke_order

[chappjc]

revoke_order is implemented and tested. revoke_match is now a notification instead of a request.

[buck54321]

A couple items you may choose to follow up on, but otherwise looking good and testing well.