client/{core,cmd/dexc}: load unfunded orders for recovery #967

chappjc · 2021-02-05T23:01:05Z

This is part 2 of a resumeTrades issue resolution following #965

This addresses the issue of orders that lose their funding coins one way or another (e.g. spend outside of dexc) can never be loaded. Prior to this PR, that meant two things:

On login, the user will be forever spammed by an "Order coin error" /
"Source coins retrieval error".
Matches for such orders can never be recovered, even if they do not
require funding coins. This made recovery a manual task if there were
matches in progress or requiring refund, etc.

Unfunded orders that require funding (epoch/booked or with matches needing to send swap txns) are explicitly blocked from negotiating new matches, and any offending matches that require funding coins are blocked with swapErr. However, notably, the trackedTrade is added to the dc.trades map, unlike before this change. In addition to allowing unaffected matches to proceed, match status resolution can be performed for the order and its matches, potentially revoking it, and revoke_order/revoke_matches requests from the server will be recognized.

This allows recovery or completion of other unaffected matches belonging to the trade. The order and the unfunded matches stay active (but halted via swapErr) until they are either revoked or the user possibly resolves a wallet issue that made the funding coins inaccessible and restarts dexc to try loading the funding coins again. If the coins are not spent, they probably are using the wrong wallet that does not control the referenced coins.

Unfunded standing limit orders that are either epoch or booked are also canceled to prevent further matches that will be likely to fail.

[INF] CORE: Loaded 1 active orders.
[INF] CORE: Loaded 0 active match orders
[ERR] CORE: notify: |ERROR| (order) Order coin error - Source coins retrieval error for order f9b17e7e (btc): funding coin not found: 9504587e6f92acb8515d0c12b8d8160367e78d8e5e8b947abdd81c532399c1e9:0 - Order: f9b17e7eb7b7185bb315bc1886a9309aac9821f928d475db2275d5762bc37c28
309aac9821f928d475db2275d5762bc37c28
[WRN] CORE: Check the status of your BTC wallet and the coins logged above! Resolve the wallet issue if possible and restart the DEX client.
[WRN] CORE: Unfunded order f9b17e7eb7b7185bb315bc1886a9309aac9821f928d475db2275d5762bc37c28 will be canceled on connect, but 0 active matches need funding coins!",
[TRC] CORE: notify: |DATA| (order) Order loaded - Order: f9b17e7eb7b7185bb315bc1886a9309aac9821f928d475db2275d5762bc37c28
[TRC] CORE: notify: |DATA| (balance) balance updated
[INF] CORE: loaded 1 incomplete orders
[DBG] CORE: Authenticated connection to 127.0.0.1:17273, acct e6e066dc1e41430b6dffd456914dbe58e8fc522743da03c6657e3831c7d0424c, 1 active orders, 0 active matches, score 0
[TRC] CORE: Status reconciliation not required for order f9b17e7eb7b7185bb315bc1886a9309aac9821f928d475db2275d5762bc37c28, status "booked", server-reported status "booked"
[WRN] CORE: Canceling unfunded standing limit order f9b17e7eb7b7185bb315bc1886a9309aac9821f928d475db2275d5762bc37c28
[INF] CORE: Cancel order 7daaefb750e1ea48db4d3dea5b76158a8da4cea11804bf3c9d651239cd8e6d95 targeting order f9b17e7eb7b7185bb315bc1886a9309aac9821f928d475db2275d5762bc37c28 at 127.0.0.1:17273 has been placed
[DBG] CORE: notify: |POKE| (order) Cancelling order - A cancel order has been submitted for order f9b17e7e - Order: f9b17e7eb7b7185bb315bc1886a9309aac9821f928d475db2275d5762bc37c28

chappjc · 2021-02-10T15:48:33Z

Note that there are two better changes that can be made eventually/alternatively:

allow orders to change their funding coins, but there still needs to be handling for the case when insufficient funding coins are available. This also has implications server-side where funding coins are tracked, requiring a new client-originating request to inform server of the change. However, for the sake of active matches, the funding coins may be switched without informing the server.
client can query the server earlier in resumption (in resumeTrades, before they are even added to the trades map) so it can automatically revoke these. Presently order status and match status resolution don't happen until the orders are put in the trades map

See also 172dbb7#r46977835

chappjc · 2021-02-11T00:08:10Z

Major change to this PR. The dexc/Core option is removed, and unfunded orders are now loaded into the trades map so that unaffected matches can be processed as usual, and the order and affected matches can be revoked via status resolution or server requests.

client/core/core.go

chappjc · 2021-02-11T00:11:58Z

client/core/core.go

-			coins, err := wallets.fromWallet.FundingCoins(byteIDs)
-			if err != nil {
-				notifyErr(SubjectOrderCoinError, "Source coins retrieval error for %s %s: %v", unbip(wallets.fromAsset.ID), tracker.token(), err)
-				continue


Removal of these continues are the crux of this change, allowing this trackedTrade to enter the trades map. The order is canceled automatically if it is a booked standing limit order to prevent new matches. Any matches that do not require funding coins will continue swap negotiation as usual, and matches needing funding coins are blocked with swapErr.

chappjc · 2021-02-11T00:13:24Z

client/core/core.go

@@ -3695,7 +3763,7 @@ func generateDEXMaps(host string, cfg *msgjson.ConfigResult) (map[uint32]*dex.As
 }

 // runMatches runs the sorted matches returned from parseMatches.
-func (c *Core) runMatches(dc *dexConnection, tradeMatches map[order.OrderID]*serverMatches) (assetMap, error) {
+func (c *Core) runMatches(tradeMatches map[order.OrderID]*serverMatches) (assetMap, error) {


The dexConnection input became unused when refreshUser was removed.

client/core/trade.go

client/core/core.go

This addresses the issue of orders that lose their funding coins one way or another (e.g. spend outside of dexc) can never be loaded. Prior to this PR, that meant two things: - On login, the user will be forever spammed by an "Order coin error" / "Source coins retrieval error". - Matches for such orders can never be recovered, even if they do not require funding coins. This made recovery a manual task if there were matches in progress or requiring refund, etc. Unfunded orders that require funding (epoch/booked or with matches needing to send swap txns) are explicitly blocked from negotiating new matches, and any offending matches that require funding coins are blocked with swapErr. However, notably, the trackedTrade is added to the dc.trades map, unlike before this change. In addition to allowing unaffected matches to proceed, match status resolution can be performed for the order and its matches, potentially revoking it, and revoke_order/revoke_matches requests from the server will be recognized. This allows recovery or completion of other unaffected matches belonging to the trade. The order and the unfunded matches stay active (but halted via swapErr) until they are either revoked or the user possibly resolves a wallet issue that made the funding coins inaccessible and restarts dexc to try loading the funding coins again. If the coins are not spent, they probably are using the wrong wallet that does not control the referenced coins. Unfunded standing limit orders that are either epoch or booked are also canceled to prevent further matches that will be likely to fail.

chappjc · 2021-02-25T14:58:14Z

Thanks, @JoeGruffins, I had forgotten about this PR since the backport was already merged. I'm still catching up on review.

chappjc · 2021-02-25T20:54:13Z

client/core/core.go

+	if status := tracker.metaData.Status; status != order.OrderStatusEpoch && status != order.OrderStatusBooked {
+		return fmt.Errorf("order %v not cancellable in status %v", oid, status)


We removed this from the release-0.1 backport (already merged), but it's still on this master PR. What do you say to leaving @buck54321?

chappjc referenced this pull request Feb 10, 2021

client/core: resumeTrades auditInfo search must retry

172dbb7

chappjc marked this pull request as draft February 10, 2021 16:38

This comment has been minimized.

Sign in to view

chappjc force-pushed the revokeunfunded branch from bce82ff to 5972945 Compare February 10, 2021 23:24

chappjc changed the title ~~client/{core,cmd/dexc}: revokeunfunded option~~ client/{core,cmd/dexc}: load unfunded orders for recovery Feb 11, 2021

chappjc commented Feb 11, 2021

View reviewed changes

chappjc marked this pull request as ready for review February 11, 2021 16:24

chappjc force-pushed the revokeunfunded branch 2 times, most recently from 8b3caa6 to 3079fb7 Compare February 11, 2021 16:49

buck54321 requested changes Feb 12, 2021

View reviewed changes

client/core/trade.go Outdated Show resolved Hide resolved

client/core/core.go Outdated Show resolved Hide resolved

client/core/core.go Show resolved Hide resolved

chappjc mentioned this pull request Feb 13, 2021

[0.1.5] client/core: store swap/redeem immediately after bcast #973

Merged

chappjc added this to the 0.1.5 milestone Feb 13, 2021

chappjc force-pushed the revokeunfunded branch from 1c9f0e0 to ef208a6 Compare February 13, 2021 20:52

chappjc mentioned this pull request Feb 20, 2021

[0.1.5] client/{core,cmd/dexc}: load unfunded orders for recovery #981

Merged

chappjc removed this from the 0.1.5 milestone Feb 20, 2021

chappjc force-pushed the revokeunfunded branch from ef208a6 to bb575e4 Compare February 21, 2021 23:18

JoeGruffins approved these changes Feb 25, 2021

View reviewed changes

chappjc commented Feb 25, 2021

View reviewed changes

chappjc merged commit 122277e into decred:master Mar 1, 2021

chappjc deleted the revokeunfunded branch March 1, 2021 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

client/{core,cmd/dexc}: load unfunded orders for recovery #967

client/{core,cmd/dexc}: load unfunded orders for recovery #967

chappjc commented Feb 5, 2021 •

edited

Loading

chappjc commented Feb 10, 2021 •

edited

Loading

This comment has been minimized.

chappjc commented Feb 11, 2021

chappjc Feb 11, 2021 •

edited

Loading

chappjc Feb 11, 2021

chappjc commented Feb 25, 2021

chappjc Feb 25, 2021

		if status := tracker.metaData.Status; status != order.OrderStatusEpoch && status != order.OrderStatusBooked {
		return fmt.Errorf("order %v not cancellable in status %v", oid, status)

client/{core,cmd/dexc}: load unfunded orders for recovery #967

client/{core,cmd/dexc}: load unfunded orders for recovery #967

Conversation

chappjc commented Feb 5, 2021 • edited Loading

chappjc commented Feb 10, 2021 • edited Loading

This comment has been minimized.

chappjc commented Feb 11, 2021

chappjc Feb 11, 2021 • edited Loading

Choose a reason for hiding this comment

chappjc Feb 11, 2021

Choose a reason for hiding this comment

chappjc commented Feb 25, 2021

chappjc Feb 25, 2021

Choose a reason for hiding this comment

chappjc commented Feb 5, 2021 •

edited

Loading

chappjc commented Feb 10, 2021 •

edited

Loading

chappjc Feb 11, 2021 •

edited

Loading