dcrdex icon indicating copy to clipboard operation
dcrdex copied to clipboard

workaround for pre-synced dcrwallet deadlock

Open chappjc opened this issue 2 years ago • 2 comments

https://github.com/decred/dcrwallet/issues/2199#issuecomment-1414287955

Raises questions of if and where we need to be more proactive about blocking certain things before a wallet reports synced=true.

In lieu of more broad effects from Core changes, perhaps some obvious checks in the dcr ExchangeWallet:

diff --git a/client/asset/dcr/dcr.go b/client/asset/dcr/dcr.go
index 3f4f81f37..ef8e203a5 100644
--- a/client/asset/dcr/dcr.go
+++ b/client/asset/dcr/dcr.go
@@ -1785,6 +1785,10 @@ func (dcr *ExchangeWallet) SingleLotRedeemFees(req *asset.PreRedeemForm) (uint64
 // dex.Bytes should be appended to the redeem scripts collection for coins with
 // no redeem script.
 func (dcr *ExchangeWallet) FundOrder(ord *asset.Order) (asset.Coins, []dex.Bytes, error) {
+       if synced, _, _ := dcr.SyncStatus(); !synced {
+               return nil, nil, errors.New("cannot fund an order until wallet is synced")
+       }
+
        cfg := dcr.config()
 
        // Consumer checks dex asset version, so maybe this is not our job:
@@ -2422,6 +2426,9 @@ func (dcr *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) {
 // because they're not auto-unlocked by the wallet and therefore inaccurately
 // included as part of the locked balance despite being spent.
 func (dcr *ExchangeWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin, uint64, error) {
+       if synced, _, _ := dcr.SyncStatus(); !synced {
+               return nil, nil, 0, errors.New("cannot swap until wallet is synced")
+       }
        if swaps.FeeRate == 0 {
                return nil, nil, 0, fmt.Errorf("cannot send swap with with zero fee rate")
        }
@@ -3317,6 +3324,9 @@ func (dcr *ExchangeWallet) fatalFindRedemptionsError(err error, contractOutpoint
 // was created. The client should store this information for persistence across
 // sessions.
 func (dcr *ExchangeWallet) Refund(coinID, contract dex.Bytes, feeRate uint64) (dex.Bytes, error) {
+       if synced, _, _ := dcr.SyncStatus(); !synced {
+               return nil, errors.New("cannot refund until wallet is synced")
+       }
        // Caller should provide a non-zero fee rate, so we could just do
        // dcr.feeRateWithFallback(feeRate), but be permissive for now.
        if feeRate == 0 {
@@ -3439,6 +3449,9 @@ func (dcr *ExchangeWallet) refundTx(coinID, contract dex.Bytes, val uint64, refu
 // DepositAddress returns an address for depositing funds into the exchange
 // wallet.
 func (dcr *ExchangeWallet) DepositAddress() (string, error) {
+       if synced, _, _ := dcr.SyncStatus(); !synced {
+               return "", errors.New("cannot get new addresses until wallet is synced")
+       }
        addr, err := dcr.wallet.ExternalAddress(dcr.ctx, dcr.depositAccount())
        if err != nil {
                return "", err

Thoughts?

Startup trade resumption does some of this stuff right away on login. How to handle that? Don't, and let it retry? Just wait for the upstream deadlock fix?

chappjc avatar Feb 02 '23 20:02 chappjc

Anything to do with confirming funds or signing txn should not be done with a clear picture of the current state of the chain. Some utxo that should have been there may not be there any more. Maybe refunds and redeems should just try anyway, but definitely funding logic should fail and try again later, imo.

JoeGruffins avatar Feb 24 '23 04:02 JoeGruffins

Agree with that. This issue is still open to make sure we've figured it out properly, but for completeness I should note that we added a (*xcWallet).synchronized method that core can use: https://github.com/decred/dcrdex/commit/c15b89228ba541dcaade6817f5d6f642efb80f14#diff-d84cc9f5973845d42ec47b494f0fa69252f4b9c5befdda4ac48e2eab6377bde9R336 So far I only applied it to block the posting (funding) of new bond transactions because I had observed a double spend on startup while the wallet was far behind. But we can block the other funding locations like you suggest.

chappjc avatar Feb 24 '23 14:02 chappjc