Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

track and handle potential trade disruptions #361

Closed
itswisdomagain opened this issue May 13, 2020 · 5 comments
Closed

track and handle potential trade disruptions #361

itswisdomagain opened this issue May 13, 2020 · 5 comments

Comments

@itswisdomagain
Copy link
Member

itswisdomagain commented May 13, 2020

Every matched order goes through a series of states in the below-listed order (each state follows from the previous):

NewlyMatched:	DEX has sent match notifications, but the maker has not yet acted
MakerSwapCast:	Maker has initiated their swap and notified the DEX
TakerSwapCast:	Taker has initiated their swap and notified the DEX
MakerRedeemed:	Maker has redeemed Taker's swap and notified the DEX
TakerRedeemed:	Taker has redeemed Maker's swap and notified the DEX

A match/trade is considered disrupted or failed if it stays in a pre-final state longer than expected. Failed trades are revoked by the DEX.

Potential disruptions recovery

Below is a list of all potential trade disruptions (or failure states) and how each should be handled.

Disruption Recovery for Affected Party Implementation
No (or invalid) Swap notification from Maker Not necessary. Taker not affected. N/A
No (or invalid) Swap notification from Taker Maker issues refund. Implemented. See #401
No redeem notification from Maker Find redemption or issue refund if redemption not found after locktime. Implemented. See #513 and #401
No redeem notification from Taker Not necessary. Maker not affected. N/A

Potential causes of trade disruptions; and prevention

Where possible, attempts should be made to prevent avoidable disruptions. The following table seeks to identify potential "legitimate" causes of disruptions and document steps an actor may take to forestall the disruption.

Potential Cause of Disruption Prevention Implementation
Failure to act because of a missed DEX request, possibly due to connection issues.
E.g. Taker fails to create counter-swap and notify the DEX because Taker did not receive an audit request from the DEX relaying details of the Maker's init swap.
Reconnect to DEX. If trade has not been revoked, recover and act. Implemented. See #704 and #716
Error sending request ack to DEX. A party's action, even if valid, would be rejected if it was not preceded by a required acknowledgement. N/A Acks requirement removed in #699. See #716
Error notifying the DEX after taking a required action.
E.g. error sending init request after broadcasting initialization contract
Persistent re-tries within the BroadcastTimeout window. Implemented. See #459
Unable to initiate or redeem swap contract because wallet is locked. Keep wallets unlocked when orders are active. Notify user if wallet cannot be unlocked Partially implemented. See #214, #267 and #298. Remaining #676
Counter party's swap failed audit, either the contract could not be retrieved from the blockchain or it did not check out. This should practically not happen but if it did, the party that received the audit request would typically not take the next required action e.g. sending their own swap contract or redeeming the audited contract; and would be penalized by the DEX. Since the DEX finds and validates contracts before relaying the details to counter-parties, a recipient of the DEX-originating audit request should be able to find and validate the received contract details. An invalid contract would imply a bad-acting DEX and the client may want to stop acting on other trades with the DEX. Assume network latency if a contract is not found on the blockchain, retry for some time.
Assume DEX is malicious for failed audits. Blacklist the DEX?
Incomplete
Network latency retry implemented
Taker's failure to issue own counter swap because of missing swapConfth confirmation on Maker's swap. Related: Maker's failure to redeem Taker's counter swap because of missing swapConfth confirmation on Taker's swap. Regular, repeated checks for swapConfth confirmation on other party's swap tx in order to take appropriate next steps timely. Implemented
See repeated call to trade.tick() which checks confirmations here and here.
Wallet error in attempt to create and broadcast swaps or redemptions. Try again? Not implemented.
See comment here. Related #676
Taker unable to redeem because of invalid secret received from Maker through the DEX-relayed redemption request. Begin FindRedemption after the match is revoked. Implemented. See #513
@buck54321
Copy link
Member

Unable to initiate or redeem swap contract because wallet is locked. is at least partially resolved by #316.

@itswisdomagain
Copy link
Member Author

Unable to initiate or redeem swap contract because wallet is locked. is at least partially resolved by #316.

Yeah, but only partially. Those issues remain if the wallets become locked after a trade is placed e.g. when dexc is restarted. Are there cases where unlocked wallets get locked while dexc is running? I suppose so...

@itswisdomagain
Copy link
Member Author

I updated the links in the issue description, were previously pointing to outdated master.

@itswisdomagain
Copy link
Member Author

Looks like most tricky spots have been covered. Just these 2 now:

  • When the contract sent in a DEX-originating audit request fails validation. Would imply a bad-acting DEX and the client may want to stop acting on other trades with the DEX. Do we want to add a blacklist feature so the client doesn't trade with the DEX going forward and if there are other matches pending swap, the client refuses to send their swap?
  • reconnecting to wallet must sync wallet lock state with Core's state #676

@ukane-philemon
Copy link
Contributor

@chappjc, @itswisdomagain any other issue here?

@chappjc chappjc closed this as completed Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants