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

fix(watchtower): fix watchtower taker-side restart bug #1908

Merged
merged 61 commits into from
Oct 27, 2023
Merged

Conversation

caglaryucekaya
Copy link

@caglaryucekaya caglaryucekaya commented Jul 11, 2023

This PR fixes #1887, which causes the swaps to appear failed on the taker side if the watcher already completed it and the taker is restarted. The issue is fixed by checking if the watcher has spent the maker payment or refunded the taker payment before kick-starting an unfinished saved swap. If the watcher has not completed the swap, taker continues the swap itself. If the watcher has completed the swap, the saved swap is also completed by using new events MakerPaymentSpentByWatcher or TakerPaymentRefundedByWatcher.

@caglaryucekaya caglaryucekaya added the in progress Changes will be made from the author label Jul 11, 2023
@caglaryucekaya caglaryucekaya added under review and removed in progress Changes will be made from the author labels Jul 20, 2023
@smk762
Copy link

smk762 commented Aug 28, 2023

@kivqa @endrilickollari to test this one, set up a UTXO (e.g. doc/marty) swap to yourself or each other with legacy desktop and/or CLI. After taker has sent payment, but before maker has validated / confirmed it, kill the maker and taker. Leave them dead for a few hours / overnight; long enough for the refund process to initiate - the watchtower should complete the refund / trade.

Once done, review your swap json files for the swap uuid. Confirm they contain the new events listed in #1908 (comment)

@cipig
Copy link
Member

cipig commented Aug 28, 2023

started swap, killed taker after takerpayment sent and waited for watcher to send makerpayment to taker in this tx https://kmdexplorer.io/tx/a2513faf3d479b8e14096be9b0ba8977d669a7a9ca722be0e2bbe3833fa51a4e, then restarted taker

swap is shown as successful but also as "recoverable":
image

when i click "recover funds" i get this error:

{
    "error": "rpc:211] dispatcher_legacy:141] lp_swap:1290] saved_swap:115] taker_swap:2054] Maker payment is spent, swap is not recoverable",
    "is_valid": false
}

@smk762 do you know where ADEX Desktop gets the "is_recoverable" info from?
this is the swap json on taker:

{
   "error_events" : [
      "StartFailed",
      "NegotiateFailed",
      "TakerFeeSendFailed",
      "MakerPaymentValidateFailed",
      "MakerPaymentWaitConfirmFailed",
      "TakerPaymentTransactionFailed",
      "TakerPaymentWaitConfirmFailed",
      "TakerPaymentDataSendFailed",
      "TakerPaymentWaitForSpendFailed",
      "MakerPaymentSpendFailed",
      "TakerPaymentWaitRefundStarted",
      "TakerPaymentRefundStarted",
      "TakerPaymentRefunded",
      "TakerPaymentRefundedByWatcher",
      "TakerPaymentRefundFailed",
      "TakerPaymentRefundFinished"
   ],
   "events" : [
      {
         "event" : {
            "data" : {
               "fee_to_send_taker_fee" : {
                  "amount" : "0.01",
                  "coin" : "BKC",
                  "paid_from_trading_vol" : false
               },
               "lock_duration" : 7800,
               "maker" : "1bb83b58ec130e28e0a6d5d2acf2eb01b0d3f1670e021d47d31db8a858219da8",
               "maker_amount" : "0.0002242635176757669346718091484148480034385278502795202464610220754081201372130419989391613335048222965",
               "maker_coin" : "KMD",
               "maker_coin_htlc_pubkey" : "023c5ba1d7ef6fa015eb33defb3aba2a961898a51bbb7ff30344d07ba75ad3f289",
               "maker_coin_start_block" : 3565984,
               "maker_payment_confirmations" : 4,
               "maker_payment_requires_nota" : false,
               "maker_payment_spend_trade_fee" : {
                  "amount" : "0.00001",
                  "coin" : "KMD",
                  "paid_from_trading_vol" : true
               },
               "maker_payment_wait" : 1693238935,
               "my_persistent_pub" : "023c5ba1d7ef6fa015eb33defb3aba2a961898a51bbb7ff30344d07ba75ad3f289",
               "p2p_privkey" : null,
               "started_at" : 1693235815,
               "taker_amount" : "1744.271629717673521850899742930591259640102827763496143958868894601542416452442159383033419023136247",
               "taker_coin" : "BKC",
               "taker_coin_htlc_pubkey" : "023c5ba1d7ef6fa015eb33defb3aba2a961898a51bbb7ff30344d07ba75ad3f289",
               "taker_coin_start_block" : 803049,
               "taker_payment_confirmations" : 10,
               "taker_payment_lock" : 1693243615,
               "taker_payment_requires_nota" : false,
               "taker_payment_trade_fee" : {
                  "amount" : "0.01",
                  "coin" : "BKC",
                  "paid_from_trading_vol" : false
               },
               "uuid" : "959ffdd6-e44a-4bd0-a3bd-63638b2633fb"
            },
            "type" : "Started"
         },
         "timestamp" : 1693235815735
      },
      {
         "event" : {
            "data" : {
               "maker_coin_htlc_pubkey" : "031bb83b58ec130e28e0a6d5d2acf2eb01b0d3f1670e021d47d31db8a858219da8",
               "maker_coin_swap_contract_addr" : null,
               "maker_payment_locktime" : 1693251415,
               "maker_pubkey" : "000000000000000000000000000000000000000000000000000000000000000000",
               "secret_hash" : "c1eae4df923e04a343030902472e70664a525e6d",
               "taker_coin_htlc_pubkey" : "031bb83b58ec130e28e0a6d5d2acf2eb01b0d3f1670e021d47d31db8a858219da8",
               "taker_coin_swap_contract_addr" : null
            },
            "type" : "Negotiated"
         },
         "timestamp" : 1693235817736
      },
      {
         "event" : {
            "data" : {
               "tx_hash" : "41786ada882ae6a4813d652b61891b1edc8811092e2f84f72f88e8260fda84fa",
               "tx_hex" : "010000000111392e35fad1a0e0b455384b646b9bb0eb27e956ab80c5399ec843c9a0c38659020000006b483045022100d29d609751f023bf722049e4b7f19a46a44cf7e8f6dcb9f1a3c7a06867ece9f6022013624858e7edbc95e7547af1fe27970433c73078b30cdced601e870b4abca6c10121023c5ba1d7ef6fa015eb33defb3aba2a961898a51bbb7ff30344d07ba75ad3f289ffffffff0290df0a0c000000001976a914ca1e04745e8ca0c60d8c5881531d51bec470743f88ac3b16119e280000001976a91484c74592ed8ac05340906784d277ca4d4e0af08e88ac69baec64"
            },
            "type" : "TakerFeeSent"
         },
         "timestamp" : 1693235817945
      },
      {
         "event" : {
            "data" : null,
            "type" : "TakerPaymentInstructionsReceived"
         },
         "timestamp" : 1693235818946
      },
      {
         "event" : {
            "data" : {
               "tx_hash" : "fde016f09eb34cbeb80eace3fa5e8d8598ecbdb798434836edc895ec8ab931d3",
               "tx_hex" : "0400008085202f8901a2d6167b681be5c74843205934b797acf08a1220049a4784b221e3c44e604593010000006b483045022100a2b353b98e311d5f9b47d06cda8e57d981061800196d0155392f489267d939e90220375498361c352e89c45aeb6a88898a0b959ec6d5aa46b7ff50852c5e848a14dd0121031bb83b58ec130e28e0a6d5d2acf2eb01b0d3f1670e021d47d31db8a858219da8ffffffff039a5700000000000017a91434dc893ec34afc65e2fd80f32861c96ed862da86870000000000000000166a14c1eae4df923e04a343030902472e70664a525e6d9a554508000000001976a914c3f710deb7320b0efa6edb14e3ebeeb9155fa90d88ac6cb2ec64000000000000000000000000000000"
            },
            "type" : "MakerPaymentReceived"
         },
         "timestamp" : 1693235818947
      },
      {
         "event" : {
            "type" : "MakerPaymentWaitConfirmStarted"
         },
         "timestamp" : 1693235818947
      },
      {
         "event" : {
            "type" : "MakerPaymentValidatedAndConfirmed"
         },
         "timestamp" : 1693236044367
      },
      {
         "event" : {
            "data" : {
               "tx_hash" : "78f952ba2fee441a0b7bb7480c928ed76b85ba48c2b094280e6ab5e55ef27377",
               "tx_hex" : "0100000001fa84da0f26e8882ff7842f2e091188dc1e1b89612b653d81a4e62a88da6a7841010000006a47304402206b47af35808015c7dc5637e5b595791cf3ce8efd98dab220cb7202403eefd2520220009cad2f585b7c415fd44456055f75cd6aa8e543902cc328c297b1a23a63ad1e0121023c5ba1d7ef6fa015eb33defb3aba2a961898a51bbb7ff30344d07ba75ad3f289ffffffff035b49ab9c2800000017a91478e5eaa6d834d91fb0772351a16764d0cd069b1d870000000000000000166a14c1eae4df923e04a343030902472e70664a525e6da08a5601000000001976a91484c74592ed8ac05340906784d277ca4d4e0af08e88ac4cbbec64"
            },
            "type" : "TakerPaymentSent"
         },
         "timestamp" : 1693236044762
      },
      {
         "event" : {
...
            ],
            "type" : "WatcherMessageSent"
         },
         "timestamp" : 1693236044764
      },
      {
         "event" : {
            "data" : {
               "secret" : "17456a9b1b3817b612737c929a731e241a28147fc0530e876f9f79e34abd86f7",
               "transaction" : {
                  "tx_hash" : "f51443a662c9335d09af53d29d4ff763704dcbc6eb5224cbd4b836d1d33e675f",
                  "tx_hex" : "01000000017773f25ee5b56a0e2894b0c248ba856bd78e920c48b77b0b1a44ee2fba52f97800000000d747304402206b2683f63bc2a93dd48e99432cf5a7479982a351010c391f7a243f5c72ecdfd70220370d82fb4ffbc2d639fd0ec603c9a69850edd4117c43efec981728b8762bc39e012017456a9b1b3817b612737c929a731e241a28147fc0530e876f9f79e34abd86f7004c6b6304dfd8ec64b17521023c5ba1d7ef6fa015eb33defb3aba2a961898a51bbb7ff30344d07ba75ad3f289ac6782012088a914c1eae4df923e04a343030902472e70664a525e6d8821031bb83b58ec130e28e0a6d5d2acf2eb01b0d3f1670e021d47d31db8a858219da8ac68ffffffff011b079c9c280000001976a914c3f710deb7320b0efa6edb14e3ebeeb9155fa90d88acdfd8ec64"
               }
            },
            "type" : "TakerPaymentSpent"
         },
         "timestamp" : 1693245236842
      },
      {
         "event" : {
            "data" : {
               "tx_hash" : "a2513faf3d479b8e14096be9b0ba8977d669a7a9ca722be0e2bbe3833fa51a4e",
               "tx_hex" : "0400008085202f8901d331b98aec95c8ed36484398b7bdec98858d5efae3ac0eb8be4cb39ef016e0fd00000000d7473044022049a5831a2ef057c5a119b19a694dc653e37a1a8b0122125132aecd5dd47312ff02206672c43113830247623f508ee49ba2db15732c22d8f2fec3c8acfc1f19b84654012017456a9b1b3817b612737c929a731e241a28147fc0530e876f9f79e34abd86f7004c6b630457f7ec64b17521031bb83b58ec130e28e0a6d5d2acf2eb01b0d3f1670e021d47d31db8a858219da8ac6782012088a914c1eae4df923e04a343030902472e70664a525e6d8821023c5ba1d7ef6fa015eb33defb3aba2a961898a51bbb7ff30344d07ba75ad3f289ac68ffffffff01b2530000000000001976a91484c74592ed8ac05340906784d277ca4d4e0af08e88ac57f7ec64000000000000000000000000000000"
            },
            "type" : "MakerPaymentSpentByWatcher"
         },
         "timestamp" : 1693245236928
      },
      {
         "event" : {
            "type" : "Finished"
         },
         "timestamp" : 1693245236928
      }
   ],
   "gui" : "Komodo Wallet 0.6.0-beta",
   "maker_amount" : "0.0002242635176757669346718091484148480034385278502795202464610220754081201372130419989391613335048222965",
   "maker_coin" : "KMD",
   "maker_coin_usd_price" : null,
   "mm_version" : "1.0.7-beta_d1d09b4",
   "my_order_uuid" : "959ffdd6-e44a-4bd0-a3bd-63638b2633fb",
   "success_events" : [
      "Started",
      "Negotiated",
      "TakerFeeSent",
      "TakerPaymentInstructionsReceived",
      "MakerPaymentReceived",
      "MakerPaymentWaitConfirmStarted",
      "MakerPaymentValidatedAndConfirmed",
      "TakerPaymentSent",
      "WatcherMessageSent",
      "TakerPaymentSpent",
      "MakerPaymentSpent",
      "MakerPaymentSpentByWatcher",
      "Finished"
   ],
   "taker_amount" : "1744.271629717673521850899742930591259640102827763496143958868894601542416452442159383033419023136247",
   "taker_coin" : "BKC",
   "taker_coin_usd_price" : null,
   "type" : "Taker",
   "uuid" : "959ffdd6-e44a-4bd0-a3bd-63638b2633fb"
}

@shamardy
Copy link
Collaborator

swap is shown as successful but also as "recoverable":
do you know where ADEX Desktop gets the "is_recoverable" info from?

I guess my_swap_status is used for this, so this is probably a bug in the PR. I will look into it.

@shamardy
Copy link
Collaborator

I guess my_swap_status is used for this, so this is probably a bug in the PR. I will look into it.

Should be fixed after the latest commit :)

@cipig
Copy link
Member

cipig commented Aug 29, 2023

Should be fixed after the latest commit :)

tested and it is fixed, recover button is gone in Desktop

other question: does watcher cover maker refunds too (eg when maker needs to refund makerpayment because takerpayment was not sent), or only taker?

@shamardy
Copy link
Collaborator

does watcher cover maker refunds too (eg when maker needs to refund makerpayment because takerpayment was not sent), or only taker?

Currently, watchers only covers takers, this was the priority due to security considerations but it's planned to cover maker refund in the future.

@kivqa
Copy link

kivqa commented Aug 30, 2023

I checked last commit 1eb1d48 on Komodo web wallet and macos native. Following was observed for Taker:
Status becomes 'Started' after step 'Taker Payment Sent'

dex_taker_issue.mov

App logs attached:
komodo_wallet_log_30.08.2023_13-08-53.log.zip

@shamardy
Copy link
Collaborator

I checked last commit 1eb1d48 on Komodo web wallet and macos native. Following was observed for Taker:
Status becomes 'Started' after step 'Taker Payment Sent'

I think this related to swaps events handling in web wallet, please note that WatcherMessageSent is a new event introduced by this PR (It was introduced before but it's activated in this PR) and it looks like that it's not handled correctly by web wallet.

@kivqa
Copy link

kivqa commented Aug 30, 2023

So we need enhancements for web app and potentially mobile client right? Or this fix should be applicable only to legacy Desktop and CLI?

@smk762
Copy link

smk762 commented Aug 30, 2023

yeah apps will need to tweak how they parse the swap json - https://github.com/KomodoPlatform/komodo-wallet/pull/1298#issuecomment-1698919691

@cipig
Copy link
Member

cipig commented Sep 9, 2023

https://sdk.devbuilds.komodo.earth/bugfix-watchtower/mm2_1eb1d48-linux-x86-64.zip is no more
can we please merge dev branch into this, so that a new build is compiled?

@shamardy
Copy link
Collaborator

can we please merge dev branch into this, so that a new build is compiled?

Merged with dev, new build should be compiled soon.

smk762
smk762 previously approved these changes Sep 21, 2023
Copy link

@smk762 smk762 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working in CLI, though apps will need to update json parsing prior to using any release containing these changes.
cc:@CharlVS @naezith

* use ALICE_PASSPHRASE for eth_distributor in docker_tests_common.rs
@shamardy shamardy merged commit cfd2799 into dev Oct 27, 2023
29 of 30 checks passed
@shamardy shamardy deleted the bugfix-watchtower branch October 27, 2023 13:52
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants