Add shortcut to move failed trade to pending trades #3809
Conversation
If there is no deposit tx I think it does not make any sense to use mediation. Normal support needs to handle such cases. Mediators should not be used for such cases. |
|
A motivation why to not let it to mediation was also to avoid that invalid txs, spead around by making invalid payouts which will further "infect" new offeers/trades. We had cases where the payout was an invalid tx and the utxo was used as inout for the next offers/trades. Enabling mediation for trades where the deposit tx is not valid/not existing will re-open that risk again. |
There was a problem hiding this comment.
NACK, see comments...
I think if we allow that it must check the deposit tx, if it is null there is no reason why to open mediation. If it is another reason (e.g. we received an error from BitcoinJ that tx was invalid) we have to check for that as well to avoid that a invalid tx spread further by causing a invalid payout from mediator.
| @@ -74,4 +74,7 @@ private void applyList() { | |||
| list.sort((o1, o2) -> o2.getTrade().getDate().compareTo(o1.getTrade().getDate())); | |||
| } | |||
|
|
|||
| public void unfailTrade(Trade trade) { | |||
There was a problem hiding this comment.
I would find a name like "revertFailedTradeToPending" more clear.
Do you think it's worth continuing this effort to unfail (or revertToPending if you prefer) trades or should we scrap it? Have you seen trades with actual locked funds from the NTP end up in failed trades? Anything related to old trade protocol doesn't seem worth the risk, and I agree that users should not open disputes for failed trades with with deposit tx. |
|
I have not seen any 'valid" case so far. Only screwed up cases with update to 1.2. I would suggest to wait until we are shure that it is needed on some case, but even then we should rather fix the reason why it was moved to failed trades. |
|
I will close this PR since there might be more drawbacks than benefits from the sound of it. |
|
Request to re-open this PR. I am helping a user who needs this exact use case. His trade got moved to failed trades, but somehow the TX got confirmed later and he needs to move it back now and complete the trade. |
|
@wiz Tell the user to PM me n keybase, I will help him. We should not add hacks to the software for exceptional bugs which are often already fixed (if not we should find out what caused it and fix it). I just improved the handling for failed trades with a new PR. We get into weird "endless loops" situations when software tries to move to failed trades and a shortcut allows to move it back.... |
|
@chimp1984 I respectfully disagree because if the trade state == DEPOSIT_CONFIRMED then there cannot be an endless loop, right? Today I needed to use this patch to fix my own trade so I think it is needed, we can just add some sanity to checks to prevent the "endless loops" situation you describe |
|
@chimp1984 this issue of "how do I move a failed trade back to open trades" is now a FAQ in our support questions - we really need this PR. to be clear this PR is missing the critical part of re-associating the wallet private key with the trade ID. |
|
@chimp1984 @sqrrm @ripcurlx we need this patch now, more than ever. many many users who upgraded to v1.3.0 have funds locked up and are waiting for instructions how to move the failed trades back to pending trades |
Looking at #3764 and #3794 to see how to unlock the funds and get to arbitration. This PR adds a shortcut (ctrl+y, same as moving it from pending to failed) to move back the trade from failed trades to pending trades.
Testing
I have run this in regtest.