Hotfix: lightning swaps maker payment instructions#1824
Merged
Conversation
caglaryucekaya
approved these changes
May 13, 2023
caglaryucekaya
left a comment
There was a problem hiding this comment.
Sorry for the bug. I understand how the payment instructions work, this wasn't intentional but was a mistake. With this fix, only one more line needs to be changed for the watchers to work properly. While testing for this problem I discovered another one-line bug, I can open a PR for both bugfixes after you merge this one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the ETH/ERC20/UTXO swaps with watcher rewards PR #1750 , the coin used for
maker_payment_instructionswas changed frommaker_cointotaker_coinhttps://github.com/KomodoPlatform/atomicDEX-API/blob/a1fc8f7c92819f68c0dcdf7b1c650d86cdf945ad/mm2src/mm2_main/src/lp_swap/taker_swap.rs#L946-L956
This caused lightning swaps to fail. It wasn't catched by CI since lightning swaps unit tests are ignored because the
tBTCaddresses are required to be refilled periodically by coins which makes the test unstable.https://github.com/KomodoPlatform/atomicDEX-API/blob/a1fc8f7c92819f68c0dcdf7b1c650d86cdf945ad/mm2src/mm2_main/tests/mm2_tests/lightning_tests.rs#L685-L689
This problem was also missed in the PR review.
I try to fix this issue in this PR but I guess it will mess up some of the watchers operations if only the coin was changed back as done here. @caglaryucekaya please comment with what needs to be changed for watchers operations to work as they were after this PR #1750.
Please note that in the other side of the swap (the maker swap), the instructions are validated using the
maker_coin, meaning the fix in this PR is the right way to do this.https://github.com/KomodoPlatform/atomicDEX-API/blob/a1fc8f7c92819f68c0dcdf7b1c650d86cdf945ad/mm2src/mm2_main/src/lp_swap/maker_swap.rs#L723-L724
Taker swap should also mirror maker swap in operations.
https://github.com/KomodoPlatform/atomicDEX-API/blob/a1fc8f7c92819f68c0dcdf7b1c650d86cdf945ad/mm2src/mm2_main/src/lp_swap/maker_swap.rs#L455-L456
The instructions are supposed to tell the other side how to send their coin payment so it's supposed to use the other side's coin to generate the instructions.