-
Notifications
You must be signed in to change notification settings - Fork 94
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
failed swaps are marked as successful on maker and not refunded #2175
Comments
I think there is something in the code: here in spend_taker_payment() we create a tx spending the maker payment tx. So I believe in fact we do not wait for spend tx confirmation so if it is reverted for any reason we won't know about that and mark MakerSwap as finished |
Wow, this is bad, It was changed by me in the past here #1724 and has not been catched at all for a long time. Will fix it straight away. |
following swap failed on maker on takerpaymentspend
|
i have to reopen this issue because i found the same problem, now on taker:
|
@cipig I will start with this comment #2175 (comment) as this error |
c.c. @dimxy #2175 (comment) |
taker coin was MOVR, but that doesn't really matter... the problem is the same that was fixed in #2176, but this time the fix is needed on taker side, the PR fixed it for maker |
I understand, Eth transaction types were added in the last release and I wanted to know why |
Were you trying/testing |
It didn't occur here. I tried to swap MOVR for DAI-PLG20 as taker.. makerpayment (DAI-PLG20) was sent by maker, but taker failed to spend it (tx reverted because of too low gas limit) and taker still showed swap as successful... that is the problem... same as in the initial issue, where the error was on maker, now it's on taker, maker side was already fixed |
I understand again :), I am trying to debug the first one first then will move to this one #2175 (comment) . Although this #2175 (comment) might not be of importance to you as maker payment could be refunded, I want to know why |
the first one is not important, @dimxy is already at it... it comes from #2170... he will fix that problem i actually used that comment to confirm that the issue on maker was resolved :-) but 10 minutes later i detected that the same issue exists on taker too and reopened this issue |
It's not the same issue even so it appears to be :) We always made swaps successful on taker side once the spending transaction is broadcasted since it was assumed that it should never fail at this point. I will check why it was reverted, maybe it's a gas estimation issue. |
Yes, the reason why it was reverted is gas limit... i reproduced later... then i increased gas limit and tx was not reverted any more... but the problem is that taker marked the swap as successful even though he was not able to spend makerpayment.. i had to edit the swap json files on taker, remove the makerpaymentspend event (that was wrongfully marked successful), then i could call
but a normal user will not be able to do this tricks in case it happens to him |
Agreed, we should make sure that execution was successful after broadcasting the spend transaction. @laruh can you please take care of this in the next sprint as it might be needed in v2 swaps as well. Please open a PR that fixes this in legacy swaps independent of the v2 swaps PR. |
'Eth transaction type not supported' error occurs because of an issue with max_eth_tx_type coin setting. I tried to implement it that it could be obtained from the platform coin config (so no need to set it for each token) but it currently does not work if a token is initialised with "enable_eth_with_tokens". |
#2206 fixes the problem on taker side |
I found failed swaps that are shown as successful on maker and not refunded while taker see them as failed and refunds.
example:
722becac-4773-424f-84dc-43981a1c1d98
TakerPaymentSpend (USDT-PLG20) was reverted: https://polygonscan.com/tx/0x8074aa40823c937cde14e6f911c2165c71a9ce58855eaaedf31fb5c8f1f1da1c (idk why and it actually doesn't matter), but maker shows the swap as successful, the relevant part is
this tx was reverted though, so this event should actually be "failed"
the taker shows swap as failed and refunds his takerpayment... the entire swap JSON on taker:
main problem is that those swaps are hard to find... i found them by looking at reverted txes of our PLG20 swap contract: https://polygonscan.com/txs?a=0x9130b257d37a52e52f21054c4da3450c72f595ce&f=1 (the ones with
receiverSpend
), then checked if it's one of my addresses and if so searched all swap JSON files on that node for the txidrefunding is also not trivial, need to edit the swap JSON, remove all events after
MakerPaymentSent
and call revover_funds manually... for the above swap, it worked fine:The text was updated successfully, but these errors were encountered: