xmr-btc-swap
xmr-btc-swap copied to clipboard
comit-network
Fix edge case for enc sig and BTC redeem phase
I encountered a weird situation during testing on stagenet, where both a sudden surge of transactions and blocks came in within a short time frame (i.e. dozens of blocks within a few minutes). It seemed to have caused my swap program to crash, leaving me in the state XmrLocked, however I seemed to have actually sent Alice the encrypted signature and Alice was able to redeem the BTC.
Because I was stuck in XmrLocked state, I couldn't watch for the redemption transaction, meaning I could never redeem the XMR. Adding this to the XmrLocked state for Bob fixed the issue, and allowed me to redeem the XMR.
This seems like a hack. I don't want to merge this without before trying to really find the root cause of this issue. How did swap crash?
I'm not entirely sure and I no longer have the log on hand. I was running it, Alice submitted XMR lock tx proof, and the swap client was monitoring the transaction, then a sudden rush of blocks on stagenet came in (I'm talking like 20 blocks within ~5 minutes or so), and the swap client crashed shortly after with Alice having redeemed the BTC, but we were stuck in XmrLocked state in the database.
After further investigation, I believe it might be due to some failure during sending of the encrypted signature. Alice seems to receive it, but Bob does not get confirmation of that. He is then in XmrLocked state until the cancel timelock expires, which he then tries to broadcast the cancel tx.
Yes that might be. I believe we encountered this issue before. Because Alice will only sent the confirmation of receiving the end sig once, after that further requests b Bob will be ignored.
Thanks @pokkst, I'd really love to see this covered by a test if it's possible. Let me know if you can look into it
I've taken a look, and I'm not the most experienced with Rust, so do you guys think you can go over where exactly I should put the test, how to structure it, etc.? Assuming it's even possible to write a test for.
I'm gonna review this again tommorow. I think this is also a fix for https://github.com/comit-network/xmr-btc-swap/issues/1063
Is it possible to have a test covering this?
Perhaps for one of the cases yet. The other one I can't replicate unfortunatelym
Is it possible to have a test covering this?
I added an integration test. It passes on this branch and doesn't pass on master.
Ready to merge? @delta1
some CI failing, and just give me a few mins to test
Oh I see the linter is complaining.
Also I just realized we might want to distinguish between "Transaction was not found" and we failed to fetch the transaction.
@binarybaron pushed 2 commits to fix lint and add the new test to CI
but there was also an rpc_tests failure?
awesome thank you! I'm so glad we got found a fix for https://github.com/comit-network/xmr-btc-swap/issues/1063