core: properly handle client->server request errors during trade #459
Conversation
itswisdomagain
force-pushed
the
more-auto-refund
branch
from
ec45deb to
fa65bbf
Compare
|
I'm thinking #432 should go in first so I can add test case(s) for client->server request error scenarios and validate that the recovery steps implemented here work as expected. |
client/core/trade.go
Outdated
| wallet := t.wallets.toWallet | ||
| if !wallet.unlocked() { | ||
| log.Errorf("cannot redeem order %s, match %s, because %s wallet is not unlocked", | ||
| t.ID(), match.id, unbip(wallet.AssetID)) | ||
| return false | ||
| } | ||
|
|
||
| walletLocked := !t.wallets.toWallet.unlocked() |
There was a problem hiding this comment.
I didn't mean to get rid of the first check. I just meant that there's no need to keep checking after that. The walletLocked variable would always be false, since you just verified it a few lines up.
There was a problem hiding this comment.
Oh. Thought you meant all checks. The checks are not too relevant anyways since core tries to keep the wallets unlocked while there are active trades. But a user may manually lock the wallet, so restoring the checks just to be safe.
itswisdomagain
force-pushed
the
more-auto-refund
branch
from
ca41198 to
e9f6179
Compare
Currently, init request errors prevent swap details from being saved for a even though the swap tx has been broadcasted. This made it impossible to re-attempt sending the init request after some time or to refund the swap if it becomes necessary to do so. This is corrected by saving relevant swap details to database even if sending the init request fails. Also, if sending the init req fails for a match, the request is re-tried in the next call to trade.tick(). Finally, if sending the init request does not complete successfully before the swap's locktime expires, the swap is refunded.
|
Note the new conflict for you. |
itswisdomagain
force-pushed
the
more-auto-refund
branch
from
e9f6179 to
8f352e7
Compare
|
This should be up next for me:
|
There was a problem hiding this comment.
OK with me now. Not sure if @buck54321 's review comment was addressed though.
One potential cause for trade disruption as documented in #361 is when notifying the DEX after taking a required action results in an error such as when sending
initrequest after broadcasting swap tx or when sendingredeemrequest after redeeming the counter-party swap.Currently, the client does not update the match status to indicate that the required action has been executed and also does not re-attempt sending the request (unless dexc is restarted).
Also, as mentioned in #302 (comment), for
initerrors, the client is unable to perform auto-refund when the swap's locktime expires.This corrects the behaviour for failed client->server requests by
trade.tick()(aka every 1/3rdbcastTimeoutuntil the match is revoked or the client's swap is refunded, whichever happens first)initfailures up till expiration of the client's swap locktime.Worth noting that with this PR, all auto-refund cases should be covered. If there's a potential refund scenario not yet considered, please mention.