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

Xud crash for taker after some sucessful/unsuccessful swaps #1851

Closed
raladev opened this issue Aug 31, 2020 · 3 comments · Fixed by #1854
Closed

Xud crash for taker after some sucessful/unsuccessful swaps #1851

raladev opened this issue Aug 31, 2020 · 3 comments · Fixed by #1854
Assignees
Labels
bug Something isn't working P2 mid priority

Comments

@raladev
Copy link
Contributor

raladev commented Aug 31, 2020

Taker logs:
testnet_connext_taker.log
testnet_utils_taker.log
testnet_xud_crash_taker.log

Screenshot from 2020-08-31 19-22-10
!
Screenshot from 2020-08-31 19-34-04

@raladev raladev added bug Something isn't working P2 mid priority labels Aug 31, 2020
@kilrau
Copy link
Contributor

kilrau commented Aug 31, 2020

Maker logs: https://paste.ubuntu.com/p/t2cpndGT6Q/

@kilrau
Copy link
Contributor

kilrau commented Aug 31, 2020

Maker:

testnet > getbalance

Balance:
┌──────────┬───────────────┬────────────────────────────┬───────────────────────────────┐
│ Currency │ Total Balance │ Channel Balance (Tradable) │ Wallet Balance (Not Tradable) │
├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤
│ BTC      │ 0.73413619    │ 0.10685344                 │ 0.62728275                    │
├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤
│ ETH      │ 32.397688     │ 2.89773                    │ 29.499958                     │
├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤
│ LTC      │ 210.88649808  │ 0                          │ 210.88649808                  │
└──────────┴───────────────┴────────────────────────────┴───────────────────────────────┘
testnet > tradehistory
Trades:
┌─────────────────┬────────────────┬───────┬─────────────┬────────────────────────────┬───────────┬────────────────────────┐
│ Execution       │ Price          │ Role  │ Order Id    │ Order Id    (Counterparty) │ Swap Hash │ Executed At            │
├─────────────────┼────────────────┼───────┼─────────────┼────────────────────────────┼───────────┼────────────────────────┤
│ Sell 0.0001 ETH │ 0.03813472 BTC │ Maker │ cfb4f140... │ N/A (MediaSolution)        │ 40b91f... │ 8/31/2020, 4:06:52 PM  │
├─────────────────┼────────────────┼───────┼─────────────┼────────────────────────────┼───────────┼────────────────────────┤

@kilrau
Copy link
Contributor

kilrau commented Aug 31, 2020

As just discussed with @sangaman , we would want to catch this error and make xud not crash for this error, just print an error message and keep going. As of now, we can't come up with a path that could be "dangerous" for continuing after a deal successfully completed. It's too late anyways 🤷‍♂️

@kilrau kilrau assigned sangaman and unassigned sangaman and kilrau Aug 31, 2020
sangaman added a commit that referenced this issue Aug 31, 2020
This prevents trying to settle an invoice more than once in case Connext
sends us duplicate transfer received events. Previously, ConnextClient
would emit the `htlcAccepted` event multiple times, resulting in one
successful settleInvoice call and at least one failed call, which would
then attempt to fail a completed deal and crash xud.

Closes #1851.
sangaman added a commit that referenced this issue Aug 31, 2020
This prevents trying to settle an invoice more than once in case Connext
sends us duplicate transfer received events. Previously, ConnextClient
could emit the `htlcAccepted` event multiple times, resulting in one
successful settleInvoice call and at least one failed call, which would
then attempt to fail a completed deal and crash xud.

Closes #1851.
raladev pushed a commit that referenced this issue Sep 1, 2020
* fix(connext): prevent duplicate htlcAccepted evts

This prevents trying to settle an invoice more than once in case Connext
sends us duplicate transfer received events. Previously, ConnextClient
could emit the `htlcAccepted` event multiple times, resulting in one
successful settleInvoice call and at least one failed call, which would
then attempt to fail a completed deal and crash xud.

Closes #1851.

* refactor(swaps): relax faildeal complete check

This relaxes the assertion in `failDeal` to ensure we don't "fail" a
deal that has already been completed. Previously such a case would crash
xud, now it only prints an error message and does nothing. Although this
shouldn't happen and indicates a flaw with xud code should it arise, it
is not a critical error and does not risk funds or indicate that xud is
inoperable.
rsercano pushed a commit that referenced this issue Sep 11, 2020
* fix(connext): prevent duplicate htlcAccepted evts

This prevents trying to settle an invoice more than once in case Connext
sends us duplicate transfer received events. Previously, ConnextClient
could emit the `htlcAccepted` event multiple times, resulting in one
successful settleInvoice call and at least one failed call, which would
then attempt to fail a completed deal and crash xud.

Closes #1851.

* refactor(swaps): relax faildeal complete check

This relaxes the assertion in `failDeal` to ensure we don't "fail" a
deal that has already been completed. Previously such a case would crash
xud, now it only prints an error message and does nothing. Although this
shouldn't happen and indicates a flaw with xud code should it arise, it
is not a critical error and does not risk funds or indicate that xud is
inoperable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P2 mid priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants