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.

Sign up for GitHub

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

Jump to bottom

Problems to be fixed after manual tests on test-/stagenet #137

Closed
2 tasks done
da-kami opened this issue Jan 12, 2021 · 1 comment
Closed
2 tasks done

Problems to be fixed after manual tests on test-/stagenet #137

da-kami opened this issue Jan 12, 2021 · 1 comment
Assignees
@da-kami

Comments

@da-kami
Copy link
Member

da-kami commented Jan 12, 2021 •
edited
Loading

The following issues have to be fixed by adding e2e tests for edge cases:

  • Bob does not record Alice's lock proof (message 2) by saving a separate state after receiving Message2. So, when restarted Bob waits for the lock proof again, which never arrives. The time-window of this failure is closed once XMR lock transactions has the necessary confirmations and Bob transitions to XmrLocked.
  • Fix Alice redeem scenario #149 - After EncSigLearned Alice may fail when trying to redeem. There is a bug where we always publish the redeem tx on her side in that state without taking timelocks in account (Bob already moved and cancelled, Alice redeem always fails, she never transitions and never refunds / punishes).

Once those two issues above are done we should add more e2e tests for remaining edge cases (stop application after each state and see if the application behaves as expected after resuming).

Details to failure scenarios can be found in #135

Note: Scenarios involving the cancel path should not be actioned because for the MVP we will focus on manual recovery.
@da-kami da-kami self-assigned this Jan 12, 2021
@da-kami
Copy link
Member Author

da-kami commented Jan 12, 2021

I will start with working on

  • After EncSigLearned Alice may fail when trying to redeem. There is a bug where we always publish the redeem tx on her side in that state without taking timelocks in account (Bob already moved and cancelled, Alice redeem always fails, she never transitions and never refunds / punishes).

@D4nte D4nte mentioned this issue Jan 12, 2021
Closed
This was referenced Jan 19, 2021
Merged
Merged
bors bot added a commit that referenced this issue Jan 21, 2021
@bors @da-kami
Merge #149
d0db713 
149: Fix Alice redeem scenario r=da-kami a=da-kami

Follow up of #144, partial fix of #137

 Fix Alice redeem scenario

- Properly check the timelocks before trying to redeem
- Distinguish different failure scenarios and reactions to it.
    - if we fail to construct the redeem transaction: wait for cancel.
    - if we fail to publish the redeem transaction: wait for cancel but let the user know that restarting the application will result in retrying to publish the tx.
    - if we succeed to publish the tx but then fail when waiting for finality, print error to the user (secreat already leaked, the user has to check manually if the tx was included)



Co-authored-by: Daniel Karzel <daniel@comit.network>
bors bot added a commit that referenced this issue Jan 21, 2021
@bors @da-kami
Merge #149
4e85819 
149: Fix Alice redeem scenario r=da-kami a=da-kami

Follow up of #144, partial fix of #137

 Fix Alice redeem scenario

- Properly check the timelocks before trying to redeem
- Distinguish different failure scenarios and reactions to it.
    - if we fail to construct the redeem transaction: wait for cancel.
    - if we fail to publish the redeem transaction: wait for cancel but let the user know that restarting the application will result in retrying to publish the tx.
    - if we succeed to publish the tx but then fail when waiting for finality, print error to the user (secreat already leaked, the user has to check manually if the tx was included)



Co-authored-by: Daniel Karzel <daniel@comit.network>
@da-kami da-kami closed this as completed Feb 5, 2021
abraham-nixon added a commit to abraham-nixon/xmr-btc-swap that referenced this issue Feb 15, 2022
@abraham-nixon @da-kami
Merge #149
69df234 
149: Fix Alice redeem scenario r=da-kami a=da-kami

Follow up of #144, partial fix of comit-network/xmr-btc-swap#137

 Fix Alice redeem scenario

- Properly check the timelocks before trying to redeem
- Distinguish different failure scenarios and reactions to it.
    - if we fail to construct the redeem transaction: wait for cancel.
    - if we fail to publish the redeem transaction: wait for cancel but let the user know that restarting the application will result in retrying to publish the tx.
    - if we succeed to publish the tx but then fail when waiting for finality, print error to the user (secreat already leaked, the user has to check manually if the tx was included)



Co-authored-by: Daniel Karzel <daniel@comit.network>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@da-kami da-kami

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@da-kami