KomodoPlatform
Swap recovery mechanism can be overzealous sometimes
Describe the bug
Recently, we saw a user report where kdf tries takerpayment refund eventhough both taker payment and maker payment are already spent. This is because it saw MakerPaymentSpendConfirmFailed (confirmations were later than expected for makepaymentspent txn). But, TakerPaymentRefundFailed because the inputs were already spent. user saw a "recover" button in the app but the button can't do anything and the UI has no way to resolve this
@shamardy said the PR: https://github.com/KomodoPlatform/komodo-defi-framework/pull/2280 aims to solve this issue and the PR description includes:
Also recover_funds has been adapted to return more structured error types for information about retrials and such. Also removing some pre-checks that makes us fail early based on local data (e.g. is_swap_finished), as we should still query the rpc to make sure our recovery tx isn't lost or something.
but, it was also mentioned that the linked PR is a draft for now and the same recovery logic must be implemented for the TPU implementation instead
Will be targeting the fix for this in 2.7.0-beta release as next release is already planned and since this doesn't lead to any lost funds it's not urgent.
When work begins on this update, can we please also review the case below where a swap fails, and is marked "recoverable": true although there is not actually anything to recover as taker, because TakerPaymentTransactionFailed.
The swap below from v1 my_recent_swaps response shows in GUI as recoverable, but then throws error on attempt, which is harsh UX.
{
"result": {
"swaps": [
{
"type": "Taker",
"uuid": "b98062ae-d6c2-4429-ac66-d6bbbeb5516a",
"my_order_uuid": "b98062ae-d6c2-4429-ac66-d6bbbeb5516a",
"events": [
{
"timestamp": 1751964875701,
"event": {
"type": "Started",
"data": {
"taker_coin": "MATIC",
"maker_coin": "KMD",
...
"uuid": "b98062ae-d6c2-4429-ac66-d6bbbeb5516a"
}
}
},
{
"timestamp": 1751964877701,
"event": {
"type": "Negotiated",
"data": {
...
}
}
},
{
"timestamp": 1751964886090,
"event": {
"type": "TakerFeeSent",
"data": {
...
}
}
},
{
"timestamp": 1751964889090,
"event": {
"type": "TakerPaymentInstructionsReceived",
"data": null
}
},
{
"timestamp": 1751964889091,
"event": {
"type": "MakerPaymentReceived",
"data": {
"tx_hex":
...
}
}
},
{
"timestamp": 1751964889099,
"event": {
"type": "MakerPaymentWaitConfirmStarted"
}
},
{
"timestamp": 1751967389707,
"event": {
"type": "MakerPaymentValidatedAndConfirmed"
}
},
{
"timestamp": 1752571350857,
"event": {
"type": "TakerPaymentTransactionFailed",
"data": {
"error": "taker_swap:1710] Timeout 1752571350 > 1751977353"
}
}
},
{
"timestamp": 1752571350857,
"event": {
"type": "Finished"
}
}
],
"success_events": [
...
],
"error_events": [
...
],
"my_info": {
...
},
"recoverable": true,
"is_finished": true,
"is_success": false
}
],
"from_uuid": null,
"skipped": 0,
"limit": 10,
"total": 1,
"page_number": 1,
"total_pages": 1,
"found_records": 1
}
}
problem could be that you can't know for sure if the tx was sent or not in a case like
"type": "TakerPaymentTransactionFailed",
"data": {
"error": "taker_swap:1710] Timeout 1752571350 > 1751977353"
it is a timeout, but what if electrum/RPC actually sent the tx and the timeout was about sending the response about the operation to the client? so i guess kdf should check that when you call recover_swap would be nice though if kdf would mark the swap as unrecoverable when you call recover_swap, it checks and finds out that the tx was indeed not sent... so that users don't try again and again to recover... this requests about potential txes from weeks or months ago are also taking long to process, so would be nice to reduce them
or can we say for sure that "Timeout 1752571350 > 1751977353" means that the tx was indeed not sent?
