handle errors initializing trade after deposits requested

offer remains valid until trade initialized
delete maker and taker trades on error after deposits requested
schedule trade deletion if unfunded after timeout or startup
DepositResponse supports error message to confirm failure
show deposit tx ids in trade details window
This commit is contained in:
woodser 2023-01-12 07:38:09 -05:00
parent 646380bc7a
commit 308f6e8077
10 changed files with 111 additions and 33 deletions

View File

@ -22,14 +22,11 @@ import bisq.core.locale.Res;
import bisq.core.monetary.Altcoin;
import bisq.core.monetary.Volume;
import bisq.core.offer.OpenOffer;
import bisq.core.trade.Tradable;
import bisq.core.trade.Trade;
import bisq.core.util.FormattingUtils;
import bisq.core.util.coin.CoinFormatter;
import org.bitcoinj.core.Coin;
import org.bitcoinj.core.Monetary;
import org.bitcoinj.core.TransactionConfidence;
import org.bitcoinj.utils.Fiat;
import javax.inject.Inject;

View File

@ -635,7 +635,7 @@ public abstract class Trade implements Tradable, Model {
isInitialized = true;
// start listening to trade wallet
if (isDepositPublished()) {
if (isDepositRequested()) {
updateSyncing();
// allow state notifications to process before returning
@ -1462,9 +1462,14 @@ public abstract class Trade implements Tradable, Model {
if (isDepositUnlocked()) getWallet().rescanSpent();
// get txs with outputs
List<MoneroTxWallet> txs = getWallet().getTxs(new MoneroTxQuery()
.setHashes(Arrays.asList(processModel.getMaker().getDepositTxHash(), processModel.getTaker().getDepositTxHash()))
.setIncludeOutputs(true));
List<MoneroTxWallet> txs;
try {
txs = getWallet().getTxs(new MoneroTxQuery()
.setHashes(Arrays.asList(processModel.getMaker().getDepositTxHash(), processModel.getTaker().getDepositTxHash()))
.setIncludeOutputs(true));
} catch (Exception e) {
return;
}
// check deposit txs
if (!isDepositUnlocked()) {

View File

@ -63,6 +63,7 @@ import bisq.network.p2p.P2PService;
import bisq.network.p2p.network.TorNetworkNode;
import com.google.common.collect.ImmutableList;
import bisq.common.ClockWatcher;
import bisq.common.UserThread;
import bisq.common.crypto.KeyRing;
import bisq.common.handlers.ErrorMessageHandler;
import bisq.common.handlers.FaultHandler;
@ -382,6 +383,7 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
if (isShutDown) return;
initTradeAndProtocol(trade, getTradeProtocol(trade));
requestPersistence();
scheduleDeletionIfUnfunded(trade);
}
private void initTradeAndProtocol(Trade trade, TradeProtocol tradeProtocol) {
@ -1063,24 +1065,50 @@ public class TradeManager implements PersistedDataHost, DecryptedDirectMessageLi
synchronized(tradableList) {
if (!tradableList.contains(trade)) return;
// unreserve key images
// unreserve taker key images
if (trade instanceof TakerTrade && trade.getSelf().getReserveTxKeyImages() != null) {
xmrWalletService.thawOutputs(trade.getSelf().getReserveTxKeyImages());
xmrWalletService.saveMainWallet();
trade.getSelf().setReserveTxKeyImages(null);
}
// stop if trade wallet possibly funded
if (xmrWalletService.multisigWalletExists(trade.getId()) && trade.isDepositRequested()) {
log.warn("Refusing to delete {} {} because trade wallet could be funded", trade.getClass().getSimpleName(), trade.getId());
// remove trade if wallet deleted
if (!xmrWalletService.multisigWalletExists(trade.getId())) {
removeTrade(trade);
return;
}
// delete trade wallet if exists
if (xmrWalletService.multisigWalletExists(trade.getId())) trade.deleteWallet();
// remove trade and wallet unless timeout after deposit requested
boolean isTimeoutError = TradeProtocol.isTimeoutError(trade.getErrorMessage());
if (!trade.isDepositRequested() || !isTimeoutError) {
removeTrade(trade);
if (xmrWalletService.multisigWalletExists(trade.getId())) trade.deleteWallet();
} else {
scheduleDeletionIfUnfunded(trade);
}
}
}
// remove trade
removeTrade(trade);
private void scheduleDeletionIfUnfunded(Trade trade) {
if (trade.isDepositRequested() && !trade.isDepositPublished()) {
log.warn("Scheduling to delete trade if unfunded for {} {}", trade.getClass().getSimpleName(), trade.getId());
UserThread.runAfter(() -> {
if (isShutDown) return;
// get trade's deposit txs from daemon
MoneroTx makerDepositTx = xmrWalletService.getDaemon().getTx(trade.getMaker().getDepositTxHash());
MoneroTx takerDepositTx = xmrWalletService.getDaemon().getTx(trade.getTaker().getDepositTxHash());
// delete multisig trade wallet if neither deposit tx published
if ((makerDepositTx != null && makerDepositTx.isRelayed()) || (takerDepositTx != null && takerDepositTx.isRelayed())) {
log.warn("Refusing to delete {} {} after protocol timeout because its wallet might be funded", trade.getClass().getSimpleName(), trade.getId());
} else {
log.warn("Deleting {} {} after protocol timeout", trade.getClass().getSimpleName(), trade.getId());
removeTrade(trade);
failedTradesManager.removeTrade(trade);
if (xmrWalletService.multisigWalletExists(trade.getId())) trade.deleteWallet();
}
}, 60);
}
}

View File

@ -21,8 +21,11 @@ import bisq.core.proto.CoreProtoResolver;
import bisq.network.p2p.DirectMessage;
import bisq.network.p2p.NodeAddress;
import bisq.common.crypto.PubKeyRing;
import java.util.Optional;
import bisq.common.crypto.PubKeyRing;
import bisq.common.proto.ProtoUtil;
import lombok.EqualsAndHashCode;
import lombok.Value;
@ -32,17 +35,20 @@ public final class DepositResponse extends TradeMessage implements DirectMessage
private final NodeAddress senderNodeAddress;
private final PubKeyRing pubKeyRing;
private final long currentDate;
private final String errorMessage;
public DepositResponse(String tradeId,
NodeAddress senderNodeAddress,
PubKeyRing pubKeyRing,
String uid,
String messageVersion,
long currentDate) {
long currentDate,
String errorMessage) {
super(messageVersion, tradeId, uid);
this.senderNodeAddress = senderNodeAddress;
this.pubKeyRing = pubKeyRing;
this.currentDate = currentDate;
this.errorMessage = errorMessage;
}
@ -58,6 +64,7 @@ public final class DepositResponse extends TradeMessage implements DirectMessage
.setPubKeyRing(pubKeyRing.toProtoMessage())
.setUid(uid);
builder.setCurrentDate(currentDate);
Optional.ofNullable(errorMessage).ifPresent(e -> builder.setErrorMessage(errorMessage));
return getNetworkEnvelopeBuilder().setDepositResponse(builder).build();
}
@ -70,7 +77,8 @@ public final class DepositResponse extends TradeMessage implements DirectMessage
PubKeyRing.fromProto(proto.getPubKeyRing()),
proto.getUid(),
messageVersion,
proto.getCurrentDate());
proto.getCurrentDate(),
ProtoUtil.stringOrNullFromProto(proto.getErrorMessage()));
}
@Override
@ -79,6 +87,7 @@ public final class DepositResponse extends TradeMessage implements DirectMessage
"\n senderNodeAddress=" + senderNodeAddress +
",\n pubKeyRing=" + pubKeyRing +
",\n currentDate=" + currentDate +
",\n errorMessage=" + errorMessage +
"\n} " + super.toString();
}
}

View File

@ -80,6 +80,7 @@ import javax.annotation.Nullable;
public abstract class TradeProtocol implements DecryptedDirectMessageListener, DecryptedMailboxListener {
public static final int TRADE_TIMEOUT = 60;
private static final String TIMEOUT_REACHED = "Timeout reached.";
protected final ProcessModel processModel;
protected final Trade trade;
@ -331,8 +332,7 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
.from(sender))
.setup(tasks(
// TODO (woodser): validate request
ProcessSignContractResponse.class,
RemoveOffer.class)
ProcessSignContractResponse.class)
.using(new TradeTaskRunner(trade,
() -> {
startTimeout(TRADE_TIMEOUT);
@ -364,7 +364,8 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
.from(sender)) // TODO (woodser): ensure this asserts sender == response.getSenderNodeAddress()
.setup(tasks(
// TODO (woodser): validate request
ProcessDepositResponse.class)
ProcessDepositResponse.class,
RemoveOffer.class)
.using(new TradeTaskRunner(trade,
() -> {
stopTimeout();
@ -548,8 +549,7 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
String err = "Received AckMessage with error state for " + ackMessage.getSourceMsgClassName() +
" from "+ peer + " with tradeId " + trade.getId() + " and errorMessage=" + ackMessage.getErrorMessage();
log.warn(err);
stopTimeout();
if (errorMessageHandler != null) errorMessageHandler.handleErrorMessage(err);
handleError(err);
}
}
@ -608,7 +608,7 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
synchronized (timeoutTimerLock) {
stopTimeout();
timeoutTimer = UserThread.runAfter(() -> {
handleError("Timeout reached. Protocol did not complete in " + timeoutSec + " sec. TradeID=" + trade.getId() + ", state=" + trade.stateProperty().get());
handleError(TIMEOUT_REACHED + " Protocol did not complete in " + timeoutSec + " sec. TradeID=" + trade.getId() + ", state=" + trade.stateProperty().get());
}, timeoutSec);
}
}
@ -622,6 +622,9 @@ public abstract class TradeProtocol implements DecryptedDirectMessageListener, D
}
}
public static boolean isTimeoutError(String errorMessage) {
return errorMessage != null && errorMessage.contains(TIMEOUT_REACHED);
}
///////////////////////////////////////////////////////////////////////////////////////////
// Task runner

View File

@ -23,7 +23,6 @@ import bisq.common.crypto.PubKeyRing;
import bisq.common.crypto.Sig;
import bisq.common.taskrunner.TaskRunner;
import bisq.core.offer.Offer;
import bisq.core.offer.OfferDirection;
import bisq.core.trade.HavenoUtils;
import bisq.core.trade.Trade;
import bisq.core.trade.messages.DepositRequest;
@ -47,6 +46,8 @@ import monero.daemon.model.MoneroSubmitTxResult;
@Slf4j
public class ArbitratorProcessDepositRequest extends TradeTask {
private boolean depositTxsRelayed = false;
@SuppressWarnings({"unused"})
public ArbitratorProcessDepositRequest(TaskRunner taskHandler, Trade trade) {
super(taskHandler, trade);
@ -54,6 +55,7 @@ public class ArbitratorProcessDepositRequest extends TradeTask {
@Override
protected void run() {
MoneroDaemon daemon = trade.getXmrWalletService().getDaemon();
try {
runInterceptHook();
@ -108,12 +110,12 @@ public class ArbitratorProcessDepositRequest extends TradeTask {
if (processModel.getMaker().getDepositTxHex() != null && processModel.getTaker().getDepositTxHex() != null) {
// relay txs
MoneroDaemon daemon = trade.getXmrWalletService().getDaemon();
MoneroSubmitTxResult makerResult = daemon.submitTxHex(processModel.getMaker().getDepositTxHex(), true);
MoneroSubmitTxResult takerResult = daemon.submitTxHex(processModel.getTaker().getDepositTxHex(), true);
if (!makerResult.isGood()) throw new RuntimeException("Error submitting maker deposit tx: " + JsonUtils.serialize(makerResult));
if (!takerResult.isGood()) throw new RuntimeException("Error submitting taker deposit tx: " + JsonUtils.serialize(takerResult));
daemon.relayTxsByHash(Arrays.asList(processModel.getMaker().getDepositTxHash(), processModel.getTaker().getDepositTxHash()));
depositTxsRelayed = true;
// update trade state
log.info("Arbitrator submitted deposit txs for trade " + trade.getId());
@ -126,7 +128,8 @@ public class ArbitratorProcessDepositRequest extends TradeTask {
processModel.getPubKeyRing(),
UUID.randomUUID().toString(),
Version.getP2PMessageVersion(),
new Date().getTime());
new Date().getTime(),
null);
// send deposit response to maker and taker
sendDepositResponse(trade.getMaker().getNodeAddress(), trade.getMaker().getPubKeyRing(), response);
@ -139,7 +142,30 @@ public class ArbitratorProcessDepositRequest extends TradeTask {
// TODO (woodser): request persistence?
complete();
} catch (Throwable t) {
failed(t);
// handle error before deposits relayed
if (!depositTxsRelayed) {
try {
daemon.flushTxPool(processModel.getMaker().getDepositTxHash(), processModel.getTaker().getDepositTxHash());
} catch (Exception e) {
e.printStackTrace();
}
// create deposit response with error
DepositResponse response = new DepositResponse(
trade.getOffer().getId(),
processModel.getMyNodeAddress(),
processModel.getPubKeyRing(),
UUID.randomUUID().toString(),
Version.getP2PMessageVersion(),
new Date().getTime(),
t.getMessage());
// send deposit response to maker and taker
sendDepositResponse(trade.getMaker().getNodeAddress(), trade.getMaker().getPubKeyRing(), response);
sendDepositResponse(trade.getTaker().getNodeAddress(), trade.getTaker().getPubKeyRing(), response);
}
failed(t);
}
}

View File

@ -20,6 +20,7 @@ package bisq.core.trade.protocol.tasks;
import bisq.common.taskrunner.TaskRunner;
import bisq.core.trade.Trade;
import bisq.core.trade.messages.DepositResponse;
import lombok.extern.slf4j.Slf4j;
@Slf4j
@ -34,6 +35,14 @@ public class ProcessDepositResponse extends TradeTask {
protected void run() {
try {
runInterceptHook();
// throw if error
DepositResponse message = (DepositResponse) processModel.getTradeMessage();
if (message.getErrorMessage() != null) {
throw new RuntimeException(message.getErrorMessage());
}
// set success state
trade.setStateIfValidTransitionTo(Trade.State.ARBITRATOR_PUBLISHED_DEPOSIT_TXS);
processModel.getTradeManager().requestPersistence();
complete();

View File

@ -2036,7 +2036,7 @@ The above error message will be copied to the clipboard when you click either of
It will make debugging easier if you include the haveno.log file by pressing "Open log file", saving a copy, and attaching it to your bug report.
popup.error.tryRestart=Please try to restart your application and check your network connection to see if you can resolve the issue.
popup.error.takeOfferRequestFailed=An error occurred when someone tried to take one of your offers:\n{0}
popup.error.takeOfferRequestFailed=An error occurred while taking an offer:\n{0}
error.spvFileCorrupted=An error occurred when reading the SPV chain file.\nIt might be that the SPV chain file is corrupted.\n\nError message: {0}\n\nDo you want to delete it and start a resync?
error.deleteAddressEntryListFailed=Could not delete AddressEntryList file.\nError: {0}

View File

@ -269,12 +269,12 @@ public class TradeDetailsWindow extends Overlay<TradeDetailsWindow> {
Res.get(contract.getPaymentMethodId()));
}
if (trade.getMakerDepositTx() != null)
if (trade.getMaker().getDepositTxHash() != null)
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.makerDepositTransactionId"),
trade.getMakerDepositTx().getHash());
if (trade.getTakerDepositTx() != null)
trade.getMaker().getDepositTxHash());
if (trade.getTaker().getDepositTxHash() != null)
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.takerDepositTransactionId"),
trade.getTakerDepositTx().getHash());
trade.getTaker().getDepositTxHash());
if (trade.getPayoutTxId() != null && !trade.getPayoutTxId().isBlank())
addLabelTxIdTextField(gridPane, ++rowIndex, Res.get("shared.payoutTxId"),

View File

@ -344,6 +344,7 @@ message DepositResponse {
PubKeyRing pub_key_ring = 3;
string uid = 4;
int64 current_date = 5;
string error_message = 6;
}
message DepositsConfirmedMessage {